Fixes #1064 #1063 #1060

client/src/components/OrganisationUnits.jsx

@@ -119,13 +119,13 @@ export const OrganisationUnits = ({units, setUnits, setDuplicated}) => {
         units = units.length > 0 ? units : [{name: ""}]
     }
 
-
+    const unitName = removalIndex !== -1 ? (units[removalIndex] || {}).name : "";
     return (
         <div className="organisation-units sds--text-field ">
             <ConfirmationDialog isOpen={confirmationDialogOpen}
                                 cancel={cancelRemoval}
                                 confirm={() => removeAfterConfirmation()}
-                                question={I18n.t("units.confirmation")}
+                                question={I18n.t("units.confirmation", {name: unitName})}
                                 closeTimeoutMS={0}
                                 isWarning={true}>
                 {confirmationDialogOpen && renderConfirmation()}

client/src/components/redesign/Collaborations.scss

@@ -142,11 +142,13 @@ table.collaborations, table.memberCollaborations {
                     display: flex;
                     flex-direction: column;
                     gap: 10px;
+
                     span.chip-container {
                         margin: 0 auto;
                     }
                 }
             }
+
             &.collaboration_memberships_count {
                 text-align: center;
             }

client/src/components/redesign/OrganisationAdmins.jsx

@@ -529,6 +529,19 @@ class OrganisationAdmins extends React.Component {
                 header: I18n.t("models.users.name_email"),
                 mapper: entity => <UserColumn entity={entity} currentUser={currentUser}/>
             },
+            isEmpty(organisation.units) ? null : {
+                key: "units",
+                class: "units",
+                header: I18n.t("units.column"),
+                mapper: membership => <div className="unit-container">
+                    {(membership.units || [])
+                        .sort((u1, u2) => u1.name.localeCompare(u2.name))
+                        .map((unit, index) => <span key={index} className="chip-container">
+                        <Chip
+                            type={ChipType.Status_default}
+                            label={unit.name}/></span>)}
+                </div>
+            },
             {
                 key: "user__schac_home_organisation",
                 header: I18n.t("models.users.institute"),
@@ -560,7 +573,7 @@ class OrganisationAdmins extends React.Component {
                 header: "",
                 mapper: entity => this.getImpersonateMapper(entity, currentUser, impersonation_allowed, isAdmin)
             },
-        ]
+        ].filter(coll => coll !== null)
         const entities = admins.concat(invites);
         return (<>
                 <ConfirmationDialog isOpen={confirmationDialogOpen}
@@ -575,9 +588,13 @@ class OrganisationAdmins extends React.Component {
                           searchAttributes={["user__name", "user__email", "invitee_email"]}
                           defaultSort="name"
                           columns={isAdmin ? columns : columns.slice(1)}
-                          rowLinkMapper={membership =>
-                              isUserAllowed(ROLES.ORG_ADMIN, currentUser, organisation) && !membership.invite && membership.role === "manager"
-                              && !isEmpty(organisation.units) && this.gotoMember
+                          rowLinkMapper={membership => {
+                              const isOrgAdmin = isUserAllowed(ROLES.ORG_ADMIN, currentUser, organisation.id);
+                              const allowed = isOrgAdmin && !membership.invite && membership.role === "manager"
+                                  && !isEmpty(organisation.units);
+                              return allowed && this.gotoMember;
+                          }
+
                           }
                           loading={false}
                           onHover={true}

client/src/components/redesign/OrganisationAdmins.scss

@@ -10,19 +10,24 @@
             }
 
             &.name {
-                width: 35%;
+                width: 25%;
+            }
+
+            &.units {
+                width: 17%;
+                text-align: center;
             }
 
             &.user__schac_home_organisation {
-                width: 20%;
+                width: 15%;
             }
 
             &.role {
                 width: 15%;
             }
 
             &.status {
-                width: 20%;
+                width: 18%;
                 text-align: center;
             }
 
@@ -39,6 +44,18 @@
                 height: 20px;
             }
 
+            &.units {
+                .unit-container {
+                    display: flex;
+                    flex-direction: column;
+                    gap: 10px;
+
+                    span.chip-container {
+                        margin: 0 auto;
+                    }
+                }
+            }
+
             &.status {
                 text-align: center;
             }

server/api/organisation.py

@@ -216,14 +216,16 @@ def organisation_by_id(organisation_id):
         optional_managers = list(filter(lambda m: m.user_id == user_id, organisation.organisation_memberships))
         if not optional_managers:
             raise Forbidden()
-        manager_unit_identifiers = set([unit.id for unit in optional_managers[0].units])
+        manager_unit_identifiers = [unit.id for unit in optional_managers[0].units]
         organisation_json = jsonify(organisation).json
 
         def collaboration_allowed(collaboration):
-            if "units" not in collaboration or not collaboration["units"]:
-                return True
-            co_units = set([unit["id"] for unit in collaboration["units"]])
-            return co_units.issubset(manager_unit_identifiers)
+            if manager_unit_identifiers:
+                collaboration_has_units = "units" in collaboration and collaboration["units"]
+                # one of units of the manager has to match one of the units of the collaboration
+                co_units = [unit["id"] for unit in collaboration["units"]] if collaboration_has_units else []
+                return bool([id for id in manager_unit_identifiers if id in co_units])
+            return True
 
         collaborations = [co for co in organisation_json["collaborations"] if collaboration_allowed(co)]
         collaboration_requests = [co for co in organisation_json["collaboration_requests"] if collaboration_allowed(co)]

server/auth/security.py

@@ -181,18 +181,18 @@ def override_func():
             if not collaboration:
                 return False
             org_id = collaboration.organisation_id
+            if is_organisation_admin(org_id):
+                return True
             if org_manager_allowed:
                 is_organisation_member = is_organisation_admin_or_manager(org_id)
                 if not is_organisation_member:
                     return False
                 unit_allowed = True
-                if collaboration.units:
-                    membership = list(
-                        filter(lambda m: m.user_id == user_id, collaboration.organisation.organisation_memberships))[0]
+                membership = list(
+                    filter(lambda m: m.user_id == user_id, collaboration.organisation.organisation_memberships))[0]
+                if membership.units:
                     unit_allowed = collaboration.is_allowed_unit_organisation_membership(membership)
                 return unit_allowed
-            else:
-                return is_organisation_admin(org_id)
         return True
 
     if read_only:

server/db/domain.py

@@ -358,10 +358,10 @@ def service_emails(self):
         return res
 
     def is_allowed_unit_organisation_membership(self, organisation_membership):
-        if not self.units:
+        if not organisation_membership.units:
             return True
-        membership_units = set(organisation_membership.units)
-        return set(self.units).issubset(membership_units)
+        units = self.units if self.units else []
+        return bool([unit for unit in units if unit in organisation_membership.units])
 
 
 class OrganisationMembership(Base, db.Model):

server/test/api/test_collaboration.py

@@ -777,8 +777,8 @@ def _access_allowed(self, urn, response_status_code=200):
 
     def test_access_allowed_organisation_admin(self):
         data = self._access_allowed("urn:mary")
-        # Mary has no units in her organisation_membership
-        self.assertEqual("lite", data["access"])
+        # Mary is organisation admin
+        self.assertEqual("full", data["access"])
 
     def test_access_allowed_collaboration_admin(self):
         data = self._access_allowed("urn:admin")

server/test/api/test_organisation.py

@@ -133,13 +133,14 @@ def test_organisation_by_id_manager(self):
         organisation_id = self.find_entity_by_name(Organisation, uuc_name).id
         organisation = self.get(f"/api/organisations/{organisation_id}")
         self.assertTrue(len(organisation["organisation_memberships"]) > 0)
-        self.assertEqual(3, len(organisation["collaborations"]))
+        self.assertEqual(1, len(organisation["collaborations"]))
 
     def test_organisation_by_id_manager_restricted_collaborations(self):
         self.login("urn:paul")
         organisation_id = self.find_entity_by_name(Organisation, uuc_name).id
         organisation = self.get(f"/api/organisations/{organisation_id}")
-        self.assertEqual(2, len(organisation["collaborations"]))
+        # Paul has a organisation membership with an unit, that is not used by any collaboration
+        self.assertEqual(0, len(organisation["collaborations"]))
 
     def test_organisation_by_id_404(self):
         self.login("urn:sarah")

server/test/db/test_collaboration.py

@@ -6,11 +6,27 @@
 class TestCollaboration(TestCase):
 
     def test_valid_membership(self):
+        organisation_membership = OrganisationMembership()
+        collaboration = Collaboration()
+        self.assertTrue(collaboration.is_allowed_unit_organisation_membership(organisation_membership))
+
         u1 = Unit(id=1)
         u2 = Unit(id=2)
-        collaboration = Collaboration(units=[u1, u2])
+        u3 = Unit(id=3)
         organisation_membership = OrganisationMembership(units=[u1])
+        collaboration = Collaboration()
         self.assertFalse(collaboration.is_allowed_unit_organisation_membership(organisation_membership))
 
-        organisation_membership = OrganisationMembership(units=[u1, u2])
+        organisation_membership = OrganisationMembership(units=[])
+        collaboration = Collaboration(units=[u1, u2])
+        self.assertTrue(collaboration.is_allowed_unit_organisation_membership(organisation_membership))
+
+        collaboration = Collaboration(units=[u1, u2])
+        organisation_membership = OrganisationMembership(units=[u1])
+        self.assertTrue(collaboration.is_allowed_unit_organisation_membership(organisation_membership))
+
+        organisation_membership = OrganisationMembership(units=[u1, u3])
         self.assertTrue(collaboration.is_allowed_unit_organisation_membership(organisation_membership))
+
+        organisation_membership = OrganisationMembership(units=[u3])
+        self.assertFalse(collaboration.is_allowed_unit_organisation_membership(organisation_membership))