Delete orpaned tags when deleting a CO

client/src/pages/CollaborationForm.jsx

@@ -84,35 +84,36 @@ class CollaborationForm extends React.Component {
     componentDidMount = () => {
         const params = this.props.match.params;
         if (params.id) {
-            collaborationById(params.id).then(collaboration => {
-                const {user} = this.props;
-                const organisation = collaboration.organisation;
-                const orgOptions = this.mapOrganisationsToOptions([organisation])
-                this.updateBreadCrumb(orgOptions[0], collaboration, false, false);
-                if (user.admin) {
-                    allOrganisations().then(res => {
-                        const orgOptions = this.mapOrganisationsToOptions(res);
-                        this.setState({organisations: orgOptions});
+            collaborationById(params.id)
+                .then(collaboration => {
+                    const {user} = this.props;
+                    const organisation = collaboration.organisation;
+                    const orgOptions = this.mapOrganisationsToOptions([organisation])
+                    this.updateBreadCrumb(orgOptions[0], collaboration, false, false);
+                    if (user.admin) {
+                        allOrganisations().then(res => {
+                            const orgOptions = this.mapOrganisationsToOptions(res);
+                            this.setState({organisations: orgOptions});
+                        });
+                    }
+                    const expiryDate = collaboration.expiry_date ? moment(collaboration.expiry_date * 1000).toDate() : null;
+                    const tagOptions = collaboration.tags.map(tag => ({label: tag.tag_value, value: tag.id}));
+                    this.setState({
+                        ...collaboration,
+                        collaboration: collaboration,
+                        organisation: orgOptions[0],
+                        organisations: orgOptions,
+                        tags: tagOptions,
+                        tagsSelected: tagOptions,
+                        isNew: false,
+                        loading: false,
+                        expiry_date: expiryDate,
+                        allow_join_requests: !collaboration.disable_join_requests
+                    }, () => {
+                        this.updateUnits(organisation, collaboration);
+                        this.updateTags(organisation.id);
                     });
-                }
-                const expiryDate = collaboration.expiry_date ? moment(collaboration.expiry_date * 1000).toDate() : null;
-                const tagOptions = collaboration.tags.map(tag => ({label: tag.tag_value, value: tag.id}));
-                this.setState({
-                    ...collaboration,
-                    collaboration: collaboration,
-                    organisation: orgOptions[0],
-                    organisations: orgOptions,
-                    tags: tagOptions,
-                    tagsSelected: tagOptions,
-                    isNew: false,
-                    loading: false,
-                    expiry_date: expiryDate,
-                    allow_join_requests: !collaboration.disable_join_requests
-                }, () => {
-                    this.updateUnits(organisation, collaboration);
-                    this.updateTags(organisation.id);
-                });
-            });
+                }).catch(() => this.props.history.push("/404"));
         } else {
             myOrganisationsLite().then(json => {
                 if (json.length === 0) {

client/src/pages/OrganisationForm.jsx

@@ -96,7 +96,7 @@ class OrganisationForm extends React.Component {
                         {value: I18n.t("home.edit")}
                     ];
                 });
-            });
+            }).catch(() => this.props.history.push("/404"));
         } else {
             health().then(() => {
                 this.setState({loading: false})
@@ -212,7 +212,7 @@ class OrganisationForm extends React.Component {
     };
 
     isValid = () => {
-        const {required, alreadyExists, administrators, isNew, duplicatedUnit,invalidInputs } = this.state;
+        const {required, alreadyExists, administrators, isNew, duplicatedUnit, invalidInputs} = this.state;
         const inValid = Object.values(alreadyExists).some(val => val) || required.some(attr => isEmpty(this.state[attr]))
             || duplicatedUnit || Object.keys(invalidInputs).some(key => invalidInputs[key]);
         return !inValid && (!isNew || !isEmpty(administrators));

server/api/collaboration.py

@@ -96,6 +96,19 @@ def _get_collaboration_membership(co_identifier, user_uid) -> CollaborationMembe
     return membership
 
 
+def _tag_identifiers(collaboration_id):
+    collaboration = Collaboration.query.filter(Collaboration.id == int(collaboration_id)).one()
+    return [tag.id for tag in collaboration.tags]
+
+
+def delete_orphan_tags(tag_identifiers):
+    # We delete orphan tags, but need to look them up to prevent Parent instance is not bound to a Session
+    for tag_pk in tag_identifiers:
+        tag = Tag.query.filter(Tag.id == tag_pk).first()
+        if tag and not tag.collaborations:
+            db.session.delete(tag)
+
+
 @collaboration_api.route("/admins/<service_id>", strict_slashes=False)
 @json_endpoint
 def collaboration_admins(service_id):
@@ -164,8 +177,15 @@ def delete_collaboration_api(co_identifier):
     collaboration = confirm_organisation_api_collaboration(co_identifier)
     collaboration_id = collaboration.id
 
+    tag_identifiers = [tag.id for tag in collaboration.tags]
+
+    broadcast_collaboration_deleted(collaboration_id)
+    res = delete(Collaboration, collaboration_id)
+
+    delete_orphan_tags(tag_identifiers)
+
     broadcast_collaboration_deleted(collaboration_id)
-    return delete(Collaboration, collaboration_id)
+    return res
 
 
 @collaboration_api.route("/v1/<co_identifier>/members", methods=["PUT"], strict_slashes=False)
@@ -786,6 +806,11 @@ def update_collaboration():
 @json_endpoint
 def delete_collaboration(collaboration_id):
     confirm_collaboration_admin(collaboration_id)
+    tag_identifiers = _tag_identifiers(collaboration_id)
 
     broadcast_collaboration_deleted(collaboration_id)
-    return delete(Collaboration, collaboration_id)
+    res = delete(Collaboration, collaboration_id)
+
+    delete_orphan_tags(tag_identifiers)
+
+    return res

server/test/api/test_collaboration.py

@@ -318,10 +318,15 @@ def test_collaboration_update_short_name_not_allowd(self):
             self.assertFalse("changed" in group.global_urn)
 
     def test_collaboration_delete(self):
-        pre_count = Collaboration.query.count()
-        collaboration = self._find_by_identifier()
-        self.delete("/api/collaborations", primary_key=collaboration["id"])
-        self.assertEqual(pre_count - 1, Collaboration.query.count())
+        tag = Tag.query.filter(Tag.tag_value == "tag_uuc").one()
+        self.assertIsNotNone(tag)
+
+        collaboration = self.find_entity_by_name(Collaboration, co_ai_computing_name)
+        self.delete("/api/collaborations", primary_key=collaboration.id)
+
+        self.assertIsNone(self.find_entity_by_name(Collaboration, co_ai_computing_name))
+        tag = Tag.query.filter(Tag.tag_value == "tag_uuc").first()
+        self.assertIsNone(tag)
 
     def test_collaboration_delete_no_admin(self):
         collaboration = self._find_by_identifier()
@@ -1023,11 +1028,16 @@ def test_update_membership_wrong_role_api(self):
                  response_status_code=400)
 
     def test_delete_collaboration_api(self):
+        tag = Tag.query.filter(Tag.tag_value == "tag_uuc").one()
+        self.assertIsNotNone(tag)
+
         self.delete(f"/api/collaborations/v1/{co_ai_computing_uuid}",
                     headers={"Authorization": f"Bearer {unihard_secret}"},
                     with_basic_auth=False)
 
         self.assertIsNone(self.find_entity_by_name(Collaboration, co_ai_computing_name))
+        tag = Tag.query.filter(Tag.tag_value == "tag_uuc").first()
+        self.assertIsNone(tag)
 
     def test_delete_collaboration_api_forbidden(self):
         self.delete(f"/api/collaborations/v1/{co_ai_computing_uuid}",