Remove user permission ncheck on service function to update project a…

…s this is already checked in resource function
hotosm · Aug 30, 2023 · 5dcde8b · 5dcde8b
1 parent 77b3d5f
commit 5dcde8b
Show file tree

Hide file tree

Showing 3 changed files with 9 additions and 56 deletions.
diff --git a/backend/api/projects/resources.py b/backend/api/projects/resources.py
@@ -386,7 +386,7 @@ def patch(self, project_id):
             return {"Error": "Unable to update project", "SubCode": "InvalidData"}, 400
 
         try:
-            ProjectAdminService.update_project(project_dto, authenticated_user_id)
+            ProjectAdminService.update_project(project_dto)
             return {"Status": "Updated"}, 200
         except InvalidGeoJson as e:
             return {"Invalid GeoJson": str(e)}, 400

diff --git a/backend/services/project_admin_service.py b/backend/services/project_admin_service.py
@@ -114,7 +114,7 @@ def get_project_dto_for_admin(project_id: int) -> ProjectDTO:
         return project.as_dto_for_admin(project_id)
 
     @staticmethod
-    def update_project(project_dto: ProjectDTO, authenticated_user_id: int):
+    def update_project(project_dto: ProjectDTO):
         project_id = project_dto.project_id
 
         if project_dto.project_status == ProjectStatus.PUBLISHED.name:
@@ -125,18 +125,8 @@ def update_project(project_dto: ProjectDTO, authenticated_user_id: int):
         if project_dto.license_id:
             ProjectAdminService._validate_imagery_licence(project_dto.license_id)
 
-        # To be handled before reaching this function
-        if ProjectAdminService.is_user_action_permitted_on_project(  # FLAGGED: ALREADY CHECKED IN VIEW FUNCTION
-            authenticated_user_id, project_id
-        ):
-            project = ProjectAdminService._get_project_by_id(project_id)
-            project.update(project_dto)
-        else:
-            raise Forbidden(
-                sub_code="USER_NOT_PROJECT_MANAGER",
-                user_id=authenticated_user_id,
-                project_id=project_id,
-            )
+        project = ProjectAdminService._get_project_by_id(project_id)
+        project.update(project_dto)
 
         return project
 

diff --git a/tests/backend/integration/services/test_project_admin_service.py b/tests/backend/integration/services/test_project_admin_service.py
@@ -287,7 +287,7 @@ def test_update_published_project_with_incomplete_default_locale_raises_error(
         mock_user.return_value = stub_admin_user
         # Act / Assert
         with self.assertRaises(ProjectAdminServiceError):
-            ProjectAdminService.update_project(dto, mock_user.id)
+            ProjectAdminService.update_project(dto)
 
     @patch.object(User, "get_by_id")
     @patch.object(Project, "update")
@@ -310,7 +310,7 @@ def test_updating_a_private_project_with_no_allowed_users_raises_error(
 
         # Act
         try:
-            ProjectAdminService.update_project(dto, mock_user.id)
+            ProjectAdminService.update_project(dto)
         # Assert
         except ProjectAdminServiceError:
             self.fail("update_project raised an exception when setting it as private")
@@ -349,44 +349,7 @@ def test_update_project_with_non_existant_license_raises_error(
         mock_user.return_value = stub_admin_user
         # Act / Assert
         with self.assertRaises(ProjectAdminServiceError):
-            ProjectAdminService.update_project(dto, mock_user.id)
-
-    @patch.object(User, "get_by_id")
-    @patch.object(Project, "update")
-    @patch.object(Project, "get")
-    def test_updating_a_project_with_different_roles_raises_error(
-        self, mock_project, mock_project2, mock_user
-    ):
-        # Arrange
-        stub_project = Project()
-        stub_project.status = ProjectStatus.DRAFT.value
-
-        mock_project.return_value = stub_project
-
-        locales = []
-        info = ProjectInfoDTO()
-        info.locale = "en"
-        info.name = "Test"
-        locales.append(info)
-
-        dto = ProjectDTO()
-        dto.project_id = 1
-        dto.default_locale = "en"
-        dto.project_status = ProjectStatus.DRAFT.name
-        dto.project_priority = ProjectPriority.LOW.name
-        dto.difficulty = ProjectDifficulty.EASY.name
-        dto.mapping_types = ["ROADS"]
-        dto.mapping_editors = ["ID"]
-        dto.validation_editors = ["ID"]
-        dto.project_info_locales = locales
-
-        stub_user = User()
-        stub_user.username = "mapper"
-        stub_user.role = UserRole.MAPPER.value
-        mock_user.return_value = stub_user
-        # Act/Assert
-        with self.assertRaises(Forbidden):
-            ProjectAdminService.update_project(dto, mock_user.id)
+            ProjectAdminService.update_project(dto)
 
     def test_updating_a_project_with_valid_project_info(self):
         locales = []
@@ -398,7 +361,7 @@ def test_updating_a_project_with_valid_project_info(self):
         info.instructions = "Test instructions"
         locales.append(info)
 
-        test_project, test_user = create_canned_project()
+        test_project, _ = create_canned_project()
 
         dto = ProjectDTO()
         dto.project_id = test_project.id
@@ -411,7 +374,7 @@ def test_updating_a_project_with_valid_project_info(self):
         dto.validation_editors = ["ID"]
         dto.project_info_locales = locales
         # Act
-        updated_project = ProjectAdminService.update_project(dto, test_user.id)
+        updated_project = ProjectAdminService.update_project(dto)
         # Assert
         self.assertEqual(
             updated_project.difficulty, ProjectDifficulty[dto.difficulty.upper()].value