Skip to content

Commit

Permalink
Feature/5247 transfer project ownership (hotosm#5250)
Browse files Browse the repository at this point in the history
* Fix transfer project ownership API

* Replace text field with  dropdown to select org manager

* Add logic to  enable/disable button

* Send email to org managers after project ownership transfer

* Add tests for transfer project ownership

* Set isLoading status when managers are being fetched
  • Loading branch information
Aadesh-Baral authored Jul 24, 2022
1 parent c3f0777 commit c1df224
Show file tree
Hide file tree
Showing 16 changed files with 276 additions and 84 deletions.
7 changes: 5 additions & 2 deletions backend/api/projects/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@
from backend.models.dtos.message_dto import MessageDTO
from backend.models.dtos.grid_dto import GridDTO
from backend.services.project_service import ProjectService, NotFound
from backend.services.project_admin_service import ProjectAdminService
from backend.services.project_admin_service import (
ProjectAdminService,
ProjectAdminServiceError,
)
from backend.services.grid.grid_service import GridService
from backend.services.messaging.message_service import MessageService
from backend.services.users.authentication_service import token_auth, tm
Expand Down Expand Up @@ -63,7 +66,7 @@ def post(self, project_id):
project_id, authenticated_user_id, username
)
return {"Success": "Project Transferred"}, 200
except ValueError as e:
except (ValueError, ProjectAdminServiceError) as e:
return {"Error": str(e).split("-")[1], "SubCode": str(e).split("-")[0]}, 403
except Exception as e:
error_msg = f"ProjectsActionsTransferAPI POST - unhandled error: {str(e)}"
Expand Down
61 changes: 61 additions & 0 deletions backend/services/messaging/message_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@
from backend.models.postgis.statuses import TeamRoles
from backend.services.messaging.smtp_service import SMTPService
from backend.services.messaging.template_service import (
get_template,
get_txt_template,
template_var_replacing,
clean_html,
)
from backend.services.organisation_service import OrganisationService
from backend.services.users.user_service import UserService, User


Expand Down Expand Up @@ -290,6 +292,55 @@ def send_message_after_comment(

MessageService._push_messages(messages)

@staticmethod
def send_project_transfer_message(
project_id: int,
transferred_to: str,
transferred_by: str,
):
"""Will send a message to the manager of the organization after a project is transferred"""
app = (
create_app()
) # Because message-all run on background thread it needs it's own app context

with app.app_context():
project = Project.get(project_id)
project_name = project.get_project_title(project.default_locale)

message = Message()
message.message_type = MessageType.PROJECT_ACTIVITY_NOTIFICATION.value
message.subject = (
f"Project {project_name} was transferred to {transferred_to}"
)
message.message = (
f"Project {project_name} associated with your "
+ f"organisation {project.organisation.name} was transferred to {transferred_to} by {transferred_by}."
)
values = {
"PROJECT_ORG_NAME": project.organisation.name,
"PROJECT_ORG_ID": project.organisation_id,
"PROJECT_NAME": project_name,
"PROJECT_ID": project_id,
"TRANSFERRED_TO": transferred_to,
"TRANSFERRED_BY": transferred_by,
}
html_template = get_template("project_transfer_alert_en.html", values)

managers = OrganisationService.get_organisation_by_id_as_dto(
project.organisation_id, User.get_by_username(transferred_by).id, False
).managers
for manager in managers:
manager = UserService.get_user_by_username(manager.username)
message.to_user_id = manager.id
message.save()
if manager.email_address and manager.is_email_verified:
SMTPService._send_message(
manager.email_address,
message.subject,
html_template,
message.message,
)

@staticmethod
def get_user_link(username: str):
base_url = current_app.config["APP_BASE_URL"]
Expand Down Expand Up @@ -733,3 +784,13 @@ def get_user_settings_link(section=None, base_url=None) -> str:
base_url = current_app.config["APP_BASE_URL"]

return f'<a href="{base_url}/settings#{section}">User Settings</a>'

@staticmethod
def get_organisation_link(
organisation_id: int, organisation_name: str, base_url=None
) -> str:
"""Helper method to generate a link to a user profile"""
if not base_url:
base_url = current_app.config["APP_BASE_URL"]

return f'<a href="{base_url}/organisations/{organisation_id}">{organisation_name}</a>'
16 changes: 10 additions & 6 deletions backend/services/messaging/templates/been_some_time.html
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
</p>
<div>
<!-- card starts here -->
<table> <!-- Noncompliant -->
<table aria-label="" > <!-- Noncompliant -->
<thead>
<th>
</th>
Expand All @@ -40,6 +40,7 @@
<a style="text-decoration: none;" href="{{ values['APP_BASE_URL'] }}/projects/{{ project.id }}" >
<div class="card-content" style="margin-bottom: 3.7rem; justify-content: left;">
<table
aria-label=""
style="border-collapse: separate; mso-table-lspace: 0pt; mso-table-rspace: 0pt; width: 100%; margin-bottom:2.285rem;"> <!-- Noncompliant -->
<th style="width:60%;">
<img style="text-decoration: none; font-weight: normal;" src={{ project.org_logo }}
Expand Down Expand Up @@ -135,11 +136,14 @@
</div>
</div>
</div>
<table style="
margin-top: 4px;
align-items: center;
width: 100%;
"> <!-- Noncompliant -->
<table
style="
margin-top: 4px;
align-items: center;
width: 100%;
"
aria-label=""
> <!-- Noncompliant -->
<th style="
color: #2c3038;
font-weight: 500;
Expand Down
4 changes: 2 additions & 2 deletions backend/services/messaging/templates/profile-progress.html
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
>
These 3 easy steps get you on your way to a great profile:
</p>
<table> <!-- Noncompliant -->
<table aria-label=""> <!-- Noncompliant -->
<thead>
<th>
</th>
Expand Down Expand Up @@ -100,7 +100,7 @@
</tr>
</table> <!-- Noncompliant -->
<!-- button component starts -->
<table width="100%" cellspacing="0" cellpadding="0" style="margin-top: 2.285rem; margin-bottom: 3.428rem;">
<table width="100%" cellspacing="0" cellpadding="0" aria-label="" style="margin-top: 2.285rem; margin-bottom: 3.428rem;">
<thead>
<thead>
<th>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{% extends "base.html" %}
{% block content %}
<br>
<div style="padding: 0 2.357rem">
<p>
Project
<a style="color: #d73f3f" href="{{values['APP_BASE_URL']}}/projects/{{values['PROJECT_ID']}}">{{values['PROJECT_NAME']}}</a>
associated with your organisation
<a style="color: #d73f3f" href="{{values['APP_BASE_URL']}}/organisations/{{values['PROJECT_ORG_ID']}}">{{values['PROJECT_ORG_NAME']}}</a>
has been transferred to
<a style="color: #d73f3f" href="{{values['APP_BASE_URL']}}/users/{{values['TRANSFERRED_TO']}}">{{values['TRANSFERRED_TO']}}</a>
by
<a style="color: #d73f3f" href="{{values['APP_BASE_URL']}}/users/{{values['TRANSFERRED_BY']}}">{{values['TRANSFERRED_BY']}}</a>.
</p>
<br><br>
Please ignore this email if you have received it by mistake.<br>
Many thanks,<br>
{{values['ORG_NAME']}}<br>
</div>
{% endblock %}
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
mapping skills and it shows. You&#39;ve accomplished a lot to get to
this moment:
</p>
<table style="text-align: center;"> <!-- Noncompliant -->
<table aria-label="" style="text-align: center;"> <!-- Noncompliant -->
<th style="width:150px;">
<h3 style="
font-size: 48px;
Expand Down Expand Up @@ -85,7 +85,7 @@
Now, it’s time to celebrate!
</p>

<table width="100%" cellspacing="0" cellpadding="0" style="margin-top: 2.285rem; margin-bottom: 3.428rem;"> <!-- Noncompliant -->
<table width="100%" cellspacing="0" cellpadding="0" aria-label="" style="margin-top: 2.285rem; margin-bottom: 3.428rem;"> <!-- Noncompliant -->
<thead>
<th>
</th>
Expand Down
2 changes: 1 addition & 1 deletion backend/services/messaging/templates/welcome.html
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@

<img src="https://raw.githubusercontent.com/hotosm/tasking-manager/59c92150108884f7b6bee9b6108d4ef0d6a33933/backend/services/messaging/templates/assets/learning_steps.png" alt="Learning_steps" height="530px" width="567px" style="margin-left: 33px">
<!-- table component starts -->
<table width="100%" cellspacing="0" cellpadding="0" style="margin-top: 2.285rem; margin-bottom: 3.428rem;"> <!-- Noncompliant -->
<table width="100%" cellspacing="0" cellpadding="0" aria-label="" style="margin-top: 2.285rem; margin-bottom: 3.428rem;"> <!-- Noncompliant -->
<thead>
<th>
</th>
Expand Down
43 changes: 31 additions & 12 deletions backend/services/project_admin_service.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import json

import threading
import geojson
from flask import current_app

Expand All @@ -12,9 +12,11 @@
from backend.models.postgis.project import Project, Task, ProjectStatus
from backend.models.postgis.statuses import TaskCreationMode, TeamRoles
from backend.models.postgis.task import TaskHistory, TaskStatus, TaskAction
from backend.models.postgis.user import User
from backend.models.postgis.utils import NotFound, InvalidData, InvalidGeoJson
from backend.services.grid.grid_service import GridService
from backend.services.license_service import LicenseService
from backend.services.messaging.message_service import MessageService
from backend.services.users.user_service import UserService
from backend.services.organisation_service import OrganisationService
from backend.services.team_service import TeamService
Expand Down Expand Up @@ -282,23 +284,40 @@ def get_projects_for_admin(
def transfer_project_to(project_id: int, transfering_user_id: int, username: str):
"""Transfers project from old owner (transfering_user_id) to new owner (username)"""
project = Project.get(project_id)
new_owner = UserService.get_user_by_username(username)
# No operation is required if the new owner is same as old owner
if username == project.author.username:
return

# Check permissions for the user (transferring_user_id) who initiatied the action
if not ProjectAdminService.is_user_action_permitted_on_project(
transfering_user_id, project_id
):
raise ValueError("UserNotPermitted- User action not permitted")
new_owner = UserService.get_user_by_username(username)
is_admin = UserService.is_user_an_admin(transfering_user_id)
is_author = UserService.is_user_the_project_author(
transfering_user_id, project.author_id
)
is_org_manager = OrganisationService.is_user_an_org_manager(
project.organisation_id, transfering_user_id
)
if not is_admin and not is_author and not is_org_manager:
raise ProjectAdminServiceError(
"TransferPermissionError- User does not have permissions to transfer project"
)

# Check permissions for the new owner - must be an admin or project's org manager or a PM team member
if not ProjectAdminService.is_user_action_permitted_on_project(
new_owner.id, project_id
# Check permissions for the new owner - must be project's org manager
if not OrganisationService.is_user_an_org_manager(
project.organisation_id, new_owner.id
):
raise ValueError(
"InvalidNewOwner- New owner must be an admin or project's org manager or a PM team member"
)
error_message = "InvalidNewOwner- New owner must be project's org manager"
if current_app:
current_app.logger.debug(error_message)
raise ValueError(error_message)
else:
transferred_by = User.get_by_id(transfering_user_id).username
project.author_id = new_owner.id
project.save()
threading.Thread(
target=MessageService.send_project_transfer_message,
args=(project_id, username, transferred_by),
).start()

@staticmethod
def is_user_action_permitted_on_project(
Expand Down
10 changes: 5 additions & 5 deletions docs_old/error_code.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@ When the TM API returns error messages, it does so in JSON format. For example,
### Error Codes
In addition to descriptive error text, error messages also contains SubCodes. While the **text for an error message may change, the SubCode will stay the same**.

| Code | Subcode | Text |
| Code | Subcode | Text |
| ---- | ------------------------ | ---------------------------------------------------------------------------------- |
| 403 | AlreadyFeatured | Project is already featured |
| 403 | CannotValidateMappedTask | Tasks cannot be validated by the same user who marked task as mapped or badimagery |
| 500 | InternalServerError | Internal Server Error |
| 400 | InvalidData | Error validating request |
| 400 | InvalidDateRange | Date range can not be bigger than 1 year |
| 400 | InvalidMultipolygon | Area of Interest: Invalid MultiPolygon |
| 403 | InvalidNewOwner | New owner must be an admin or project's org manager or a PM team member |
| 403 | InvalidNewOwner | New owner must be project's org manager |
| 400 | InvalidStartDate | Start date must be earlier than end date |
| 403 | InvalidTaskState | Task in invalid state for mapping |
| 403 | InvalidUnlockState | Can only set status to MAPPED, BADIMAGERY, READY after mapping |
Expand All @@ -44,8 +44,7 @@ In addition to descriptive error text, error messages also contains SubCodes. Wh
| 403 | UserNotAllowed | Mapping not allowed because: User not on allowed list |
| 403 | UserNotPermitted | User action not permitted |
| 403 | UserPermissionError | User is not a manager of the project |
| 403 | PrivateProject | User not permitted: Private Project
|
| 403 | PrivateProject | User not permitted: Private Project |
| 403 | ProjectNotFetched | Unable to fetch project |
| 403 | NotPermittedToCreate | User is not permitted to create project |
| 400 | MustBeFeatureCollection | GeoJson must be FeatureCollection |
Expand Down Expand Up @@ -85,4 +84,5 @@ In addition to descriptive error text, error messages also contains SubCodes. Wh
| 403 | OrgHasProjects | Organization has some projects |
| 403 | MustHaveAdmin | Must have at least one admin |
| 403 | LoginToFilterManager | Filter by manager\_user\_id is not allowed to unauthenticated requests |
| 400 | SelfIntersectingAOI | Invalid geometry. Polygon is self-intersecting |
| 400 | SelfIntersectingAOI | Invalid geometry. Polygon is self-intersecting |
| 400 | TransferPermissionError | Project ownership transfer is only allowed to TM Admin, Organization admin and project author|
Loading

0 comments on commit c1df224

Please sign in to comment.