From a64b9237f1a514a8acfb3d71b53e01fffba22cf1 Mon Sep 17 00:00:00 2001 From: Aadesh-Baral Date: Mon, 25 Jul 2022 15:04:43 +0545 Subject: [PATCH] Fix transfer project ownership to admins --- backend/api/users/resources.py | 20 ++++++-- backend/models/dtos/user_dto.py | 2 + backend/models/postgis/user.py | 14 ++++-- backend/services/project_admin_service.py | 12 +++-- docs_old/error_code.md | 2 +- .../src/components/projectEdit/actionsForm.js | 50 +++++++++++++------ .../src/components/projectEdit/messages.js | 4 ++ 7 files changed, 77 insertions(+), 27 deletions(-) diff --git a/backend/api/users/resources.py b/backend/api/users/resources.py index fd72867917..c59a53e8e2 100644 --- a/backend/api/users/resources.py +++ b/backend/api/users/resources.py @@ -1,3 +1,4 @@ +from distutils.util import strtobool from flask_restful import Resource, current_app, request from schematics.exceptions import DataError @@ -77,6 +78,16 @@ def get(self): name: page description: Page of results user requested type: integer + - in: query + name: pagination + description: Whether to return paginated results + type: boolean + default: true + - in: query + name: per_page + description: Number of results per page + type: integer + default: 20 - in: query name: username description: Full or part username @@ -99,9 +110,12 @@ def get(self): """ try: query = UserSearchQuery() - query.page = ( - int(request.args.get("page")) if request.args.get("page") else 1 - ) + query.pagination = strtobool(request.args.get("pagination", "True")) + if query.pagination: + query.page = ( + int(request.args.get("page")) if request.args.get("page") else 1 + ) + query.per_page = request.args.get("perPage", 20) query.username = request.args.get("username") query.mapping_level = request.args.get("level") query.role = request.args.get("role") diff --git a/backend/models/dtos/user_dto.py b/backend/models/dtos/user_dto.py index f0a6ff6db9..2308ceff8c 100644 --- a/backend/models/dtos/user_dto.py +++ b/backend/models/dtos/user_dto.py @@ -190,6 +190,8 @@ class UserSearchQuery(Model): serialized_name="mappingLevel", validators=[is_known_mapping_level] ) page = IntType() + pagination = BooleanType(default=True) + per_page = IntType(default=20, serialized_name="perPage") def __hash__(self): """Make object hashable so we can cache user searches""" diff --git a/backend/models/postgis/user.py b/backend/models/postgis/user.py index c28625aa31..fb75ae9d46 100644 --- a/backend/models/postgis/user.py +++ b/backend/models/postgis/user.py @@ -153,9 +153,13 @@ def get_all_users(query: UserSearchQuery) -> UserSearchDTO: roles = query.role.split(",") role_array = [UserRole[role].value for role in roles] base = base.filter(User.role.in_(role_array)) - - results = base.order_by(User.username).paginate(query.page, 20, True) - + if query.pagination: + results = base.order_by(User.username).paginate( + query.page, query.per_page, True + ) + else: + per_page = base.count() + results = base.order_by(User.username).paginate(per_page=per_page) dto = UserSearchDTO() for result in results.items: listed_user = ListedUser() @@ -166,8 +170,8 @@ def get_all_users(query: UserSearchQuery) -> UserSearchDTO: listed_user.role = UserRole(result.role).name dto.users.append(listed_user) - - dto.pagination = Pagination(results) + if query.pagination: + dto.pagination = Pagination(results) return dto @staticmethod diff --git a/backend/services/project_admin_service.py b/backend/services/project_admin_service.py index 7036f87ad0..8729851360 100644 --- a/backend/services/project_admin_service.py +++ b/backend/services/project_admin_service.py @@ -297,16 +297,20 @@ def transfer_project_to(project_id: int, transfering_user_id: int, username: str 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: + if not (is_admin or is_author or is_org_manager): raise ProjectAdminServiceError( "TransferPermissionError- User does not have permissions to transfer project" ) # Check permissions for the new owner - must be project's org manager - if not OrganisationService.is_user_an_org_manager( + is_new_owner_org_manager = OrganisationService.is_user_an_org_manager( project.organisation_id, new_owner.id - ): - error_message = "InvalidNewOwner- New owner must be project's org manager" + ) + is_new_owner_admin = UserService.is_user_an_admin(new_owner.id) + if not (is_new_owner_org_manager or is_new_owner_admin): + error_message = ( + "InvalidNewOwner- New owner must be project's org manager or TM admin" + ) if current_app: current_app.logger.debug(error_message) raise ValueError(error_message) diff --git a/docs_old/error_code.md b/docs_old/error_code.md index 0ba15e279c..5e998bb85c 100644 --- a/docs_old/error_code.md +++ b/docs_old/error_code.md @@ -19,7 +19,7 @@ In addition to descriptive error text, error messages also contains SubCodes. Wh | 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 project's org manager | +| 403 | InvalidNewOwner | New owner must be project's org manager or TM admin | | 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 | diff --git a/frontend/src/components/projectEdit/actionsForm.js b/frontend/src/components/projectEdit/actionsForm.js index 889273bd47..95fc5b69f1 100644 --- a/frontend/src/components/projectEdit/actionsForm.js +++ b/frontend/src/components/projectEdit/actionsForm.js @@ -1,4 +1,4 @@ -import React, { useState, useContext } from 'react'; +import React, { useState, useContext, useEffect } from 'react'; import { useSelector } from 'react-redux'; import Popup from 'reactjs-popup'; import Select from 'react-select'; @@ -17,7 +17,6 @@ import { useAsync } from '../../hooks/UseAsync'; import FileRejections from '../comments/fileRejections'; import DropzoneUploadStatus from '../comments/uploadStatus'; import { DROPZONE_SETTINGS } from '../../config'; -import { useFetch } from '../../hooks/UseFetch'; const ActionStatus = ({ status, action }) => { let successMessage = ''; @@ -366,19 +365,44 @@ const TransferProject = ({ projectId, orgId }: Object) => { const token = useSelector((state) => state.auth.get('token')); const { projectInfo, } = useContext(StateContext); const [username, setUsername] = useState(''); - const [, loadingOptions, organisation] = useFetch(`organisations/${orgId}/?omitManagerList=false`) - - const options = organisation.managers?.map(({ username }) => ({ - label: username, - value: username, - })); + const [managers, setManagers] = useState([]); + const [admins, setAdmins] = useState([]); + const [isFetchingOptions, setIsFetchingOptions] = useState(true); + + useEffect(() => { + fetchLocalJSONAPI(`organisations/${orgId}/?omitManagerList=false`, token).then((r) => + setManagers(r.managers.map((m) => m.username))).then(() => + setIsFetchingOptions(false)); + + fetchLocalJSONAPI(`users/?pagination=false`, token).then((t) => + setAdmins(t.users.map((u) => u.username))) + }, [token, orgId]); + + const optionsExtended = [ + { + label: projectInfo.organisationName, + options: managers?.map(manager => ({ + label: manager, + value: manager, + })) + }, + { + label: , + options: admins?.filter( + admin => !managers?.includes(admin) + ).map(adminName => ({ + label: adminName, + value: adminName, + })) + }, + ]; const handleSelect = (value) => { setUsername(value); }; const { username: loggedInUsername, role: loggedInUserRole } = useSelector((state) => state.auth.get('userDetails')); const hasAccess = ( - organisation.managers?.includes(loggedInUsername) || + managers?.includes(loggedInUsername) || loggedInUserRole === 'ADMIN' || loggedInUsername === projectInfo.author ); @@ -400,15 +424,13 @@ const TransferProject = ({ projectId, orgId }: Object) => {