Hide private teams on organisation detail view for users without manage permission

backend/api/organisations/resources.py

@@ -17,6 +17,7 @@
 
 
 class OrganisationsBySlugRestAPI(Resource):
+    @token_auth.login_required(optional=True)
     def get(self, slug):
         """
         Retrieves an organisation
@@ -209,6 +210,7 @@ def delete(self, organisation_id):
             current_app.logger.critical(error_msg)
             return {"Error": error_msg, "SubCode": "InternalServerError"}, 500
 
+    @token_auth.login_required(optional=True)
     def get(self, organisation_id):
         """
         Retrieves an organisation

backend/services/organisation_service.py

@@ -21,6 +21,7 @@
 from backend.models.postgis.organisation import Organisation
 from backend.models.postgis.project import Project, ProjectInfo
 from backend.models.postgis.task import Task
+from backend.models.postgis.team import TeamVisibility
 from backend.models.postgis.statuses import ProjectStatus, TaskStatus
 from backend.models.postgis.utils import NotFound
 from backend.services.users.user_service import UserService
@@ -72,7 +73,14 @@ def get_organisation_dto(org, user_id: int, abbreviated: bool):
         if abbreviated:
             return organisation_dto
 
-        organisation_dto.teams = [team.as_dto_inside_org() for team in org.teams]
+        if organisation_dto.is_manager:
+            organisation_dto.teams = [team.as_dto_inside_org() for team in org.teams]
+        else:
+            organisation_dto.teams = [
+                team.as_dto_inside_org()
+                for team in org.teams
+                if team.visibility == TeamVisibility.PUBLIC.value
+            ]
 
         return organisation_dto
 
@@ -189,7 +197,9 @@ def get_projects_by_organisation_id(organisation_id: int) -> Organisation:
         return projects
 
     @staticmethod
-    def get_organisation_stats(organisation_id: int, year: int) -> OrganizationStatsDTO:
+    def get_organisation_stats(
+        organisation_id: int, year: int = None
+    ) -> OrganizationStatsDTO:
         projects = db.session.query(
             Project.id, Project.status, Project.last_updated, Project.created
         ).filter(Project.organisation_id == organisation_id)

tests/backend/integration/services/test_organisation_service.py

@@ -2,19 +2,24 @@
 from schematics.exceptions import UndefinedValueError
 
 from tests.backend.base import BaseTestCase
-from backend.models.postgis.statuses import ProjectStatus
+from backend.models.postgis.statuses import ProjectStatus, TeamVisibility
 from tests.backend.helpers.test_helpers import (
     add_manager_to_organisation,
     create_canned_organisation,
     TEST_ORGANISATION_ID,
     TEST_USER_ID,
     create_canned_project,
     create_canned_user,
+    return_canned_team,
 )
 from backend.services.organisation_service import OrganisationService, NotFound
 
 
 class TestOrgansitaionService(BaseTestCase):
+    def setUp(self):
+        super().setUp()
+        self.test_org = create_canned_organisation()
+
     def test_is_user_an_org_manager_raises_error_if_organistion_not_found(self):
         # Assert/Act
         with self.assertRaises(NotFound):
@@ -25,12 +30,10 @@ def test_is_user_an_org_manager_raises_error_if_organistion_not_found(self):
     def test_is_user_an_org_manager_returns_false_if_user_not_manager_of_organisation(
         self,
     ):
-        # Arrange
-        test_org = create_canned_organisation()
         test_user = create_canned_user()
         # Act
         is_org_manager = OrganisationService.is_user_an_org_manager(
-            test_org.id, test_user.id
+            self.test_org.id, test_user.id
         )
         # Assert
         self.assertFalse(is_org_manager)
@@ -39,20 +42,17 @@ def test_is_user_an_org_manager_returns_true_if_user_is_manager_of_organisation(
         self,
     ):
         # Arrange
-        test_org = create_canned_organisation()
         test_user = create_canned_user()
-        test_org.managers = [test_user]
+        self.test_org.managers = [test_user]
         # Act
         is_org_manager = OrganisationService.is_user_an_org_manager(
-            test_org.id, test_user.id
+            self.test_org.id, test_user.id
         )
         # Assert
         self.assertTrue(is_org_manager)
 
     def test_get_organisations_as_dto(self):
         # Test returns stats when omit stats enabled
-        # Arrange
-        test_org = create_canned_organisation()
         # Act
         orgs_dto = OrganisationService.get_organisations_as_dto(
             manager_user_id=None,
@@ -62,8 +62,8 @@ def test_get_organisations_as_dto(self):
         )
         # Assert
         self.assertEqual(len(orgs_dto.organisations), 1)
-        self.assertEqual(orgs_dto.organisations[0].organisation_id, test_org.id)
-        self.assertEqual(orgs_dto.organisations[0].name, test_org.name)
+        self.assertEqual(orgs_dto.organisations[0].organisation_id, self.test_org.id)
+        self.assertEqual(orgs_dto.organisations[0].name, self.test_org.name)
         # Since omitManagers is set to true
         with self.assertRaises(UndefinedValueError):
             orgs_dto.organisations[0].managers
@@ -72,7 +72,7 @@ def test_get_organisations_as_dto(self):
         # Test returns stats when omit_stats_disabled
         # Arrange
         test_project, test_author = create_canned_project()
-        test_project.organisation = test_org
+        test_project.organisation = self.test_org
         test_project.save()
         # Act
         orgs_dto = OrganisationService.get_organisations_as_dto(
@@ -88,7 +88,7 @@ def test_get_organisations_as_dto(self):
 
         # Test returns managers when omit managers disabled
         # Arrange
-        add_manager_to_organisation(test_org, test_author)
+        add_manager_to_organisation(self.test_org, test_author)
         # Act
         orgs_dto = OrganisationService.get_organisations_as_dto(
             manager_user_id=None,
@@ -105,13 +105,12 @@ def test_get_organisations_as_dto(self):
     def test_get_organisation_stats(self):
         # Test returns all time stats if year is None
         # Arrange
-        test_org = create_canned_organisation()
         test_project, _ = create_canned_project()
-        test_project.organisation = test_org
+        test_project.organisation = self.test_org
         test_project.status = ProjectStatus.PUBLISHED.value
         test_project.save()
         # Act
-        org_stats = OrganisationService.get_organisation_stats(test_org.id, None)
+        org_stats = OrganisationService.get_organisation_stats(self.test_org.id)
         # Assert
         self.assertEqual(org_stats.projects.published, 1)
         self.assertEqual(org_stats.projects.draft, 0)
@@ -128,7 +127,7 @@ def test_get_organisation_stats(self):
         test_project.save()
         # Act
         org_stats = OrganisationService.get_organisation_stats(
-            test_org.id, datetime.today().strftime("%Y")
+            self.test_org.id, datetime.today().strftime("%Y")
         )
         # Assert
         self.assertEqual(org_stats.projects.published, 0)
@@ -139,3 +138,67 @@ def test_get_organisation_stats(self):
         self.assertEqual(org_stats.active_tasks.badimagery, 0)
         self.assertEqual(org_stats.active_tasks.validated, 0)
         self.assertEqual(org_stats.active_tasks.invalidated, 0)
+
+    def assert_org_dto(self, org_dto):
+        """Asserts that the organisation DTO is correct"""
+        self.assertEqual(org_dto.organisation_id, self.test_org.id)
+        self.assertEqual(org_dto.name, self.test_org.name)
+        self.assertEqual(org_dto.description, None)
+        self.assertEqual(org_dto.url, None)
+
+    def test_get_organisation_dto(self):
+        """Test get_organisation_dto returns correct DTO"""
+        # Arrange
+        test_user = create_canned_user()
+        test_team_1 = return_canned_team("test_team_1", self.test_org.name)
+        test_team_1.visibility = TeamVisibility.PUBLIC.value
+        test_team_1.create()
+        test_team_2 = return_canned_team("test_team_2", self.test_org.name)
+        test_team_2.visibility = TeamVisibility.PRIVATE.value
+        test_team_2.create()
+
+        # Test with valid org and user id as non manager
+        org_dto = OrganisationService.get_organisation_dto(
+            self.test_org, test_user.id, abbreviated=False
+        )
+        self.assert_org_dto(org_dto)
+        self.assertFalse(org_dto.is_manager)
+        self.assertEqual(len(org_dto.teams), 1)
+        self.assertEqual(org_dto.teams[0].team_id, test_team_1.id)
+
+        # Test with valid org with abbreviated=True
+        org_dto = OrganisationService.get_organisation_dto(
+            self.test_org, test_user.id, abbreviated=True
+        )
+        self.assertEqual(org_dto.organisation_id, self.test_org.id)
+        self.assertEqual(org_dto.name, self.test_org.name)
+        self.assertEqual(org_dto.description, None)
+        self.assertEqual(org_dto.url, None)
+        self.assertFalse(org_dto.is_manager)
+        self.assertEqual(org_dto.teams, None)
+
+        # Test with valid org and user id as manager
+        add_manager_to_organisation(self.test_org, test_user)
+        org_dto = OrganisationService.get_organisation_dto(
+            self.test_org, test_user.id, abbreviated=False
+        )
+        self.assert_org_dto(org_dto)
+        self.assertTrue(org_dto.is_manager)
+        self.assertEqual(len(org_dto.teams), 2)
+        self.assertEqual(org_dto.teams[0].team_id, test_team_1.id)
+        self.assertEqual(org_dto.teams[1].team_id, test_team_2.id)
+
+        # Test with valid org and user id as 0
+        org_dto = OrganisationService.get_organisation_dto(
+            self.test_org, 0, abbreviated=False
+        )
+        self.assert_org_dto(org_dto)
+        self.assertFalse(org_dto.is_manager)
+        self.assertEqual(len(org_dto.teams), 1)
+        self.assertEqual(org_dto.teams[0].team_id, test_team_1.id)
+
+        # Test with invalid org
+        with self.assertRaises(NotFound):
+            OrganisationService.get_organisation_dto(
+                None, test_user.id, abbreviated=False
+            )