Final fixup of creator selection UI and API

- Make sure creators order is preserved in lookup

- Better handle case of item with no creators

- Better error messages and handling of edge cases

- Better client-side handling of edge cases and invalid inputs in creator selection

- Tweak permissions test

- Fix typo...

- Use data-test-id for better encapsulation of dropdown testing

pydatalab/src/pydatalab/routes/v0_1/items.py

@@ -205,13 +205,9 @@ def creators_lookup() -> Dict:
         "from": "users",
         "let": {"creator_ids": "$creator_ids"},
         "pipeline": [
-            {
-                "$match": {
-                    "$expr": {
-                        "$in": ["$_id", {"$ifNull": ["$$creator_ids", []]}],
-                    },
-                }
-            },
+            {"$match": {"$expr": {"$in": ["$_id", {"$ifNull": ["$$creator_ids", []]}]}}},
+            {"$addFields": {"__order": {"$indexOfArray": ["$$creator_ids", "$_id"]}}},
+            {"$sort": {"__order": 1}},
             {"$project": {"_id": 1, "display_name": 1, "contact_email": 1}},
         ],
         "as": "creators",
@@ -639,6 +635,17 @@ def update_item_permissions(refcode: str):
             if creator.get("immutable_id", None) is not None
         ]
 
+    if not creator_ids:
+        return (
+            jsonify(
+                {
+                    "status": "error",
+                    "message": "No valid creator IDs found in the request.",
+                }
+            ),
+            400,
+        )
+
     # Validate all creator IDs are present in the database
     found_ids = [d for d in flask_mongo.db.users.find({"_id": {"$in": creator_ids}}, {"_id": 1})]  # type: ignore
     if not len(found_ids) == len(creator_ids):
@@ -663,11 +670,15 @@ def update_item_permissions(refcode: str):
     # The first ID in the creator list takes precedence; always make sure this is included to avoid orphaned items
     if current_creator_ids:
         base_owner = current_creator_ids[0]
-    try:
-        creator_ids.remove(base_owner)
-    except ValueError:
-        pass
-    creator_ids.insert(0, base_owner)
+        try:
+            creator_ids.remove(base_owner)
+        except ValueError:
+            pass
+        creator_ids.insert(0, base_owner)
+
+    if set(creator_ids) == set(current_creator_ids):
+        # Short circuit if the creator IDs are the same
+        return jsonify({"status": "success"}), 200
 
     LOGGER.warning("Setting permissions for item %s to %s", refcode, creator_ids)
     result = flask_mongo.db.items.update_one(
@@ -676,7 +687,12 @@ def update_item_permissions(refcode: str):
     )
 
     if not result.modified_count == 1:
-        return jsonify({"status": "error", "message": "Failed to update permissions"}), 400
+        return jsonify(
+            {
+                "status": "error",
+                "message": "Failed to update permissions: you cannot remove yourself or the base owner as a creator.",
+            }
+        ), 400
 
     return jsonify({"status": "success"}), 200
 

pydatalab/tests/server/test_permissions.py

@@ -73,7 +73,9 @@ def test_basic_permissions_update(admin_client, admin_user_id, client, user_id):
     assert response.status_code == 200
 
     # Check that the admin can remove the user from the permissions
-    response = admin_client.patch(f"/items/{refcode}/permissions", json={"creators": []})
+    response = admin_client.patch(
+        f"/items/{refcode}/permissions", json={"creators": [{"immutable_id": str(admin_user_id)}]}
+    )
     assert response.status_code == 200
 
     response = client.get(f"/items/{refcode}")

webapp/cypress/e2e/editPage.cy.js

@@ -205,12 +205,12 @@ describe("Edit Page", () => {
     cy.findByText("editable_sample").click();
 
     cy.findByText("Add a block").click();
-    cy.get(".dropdown-menu").findByText("Comment").click();
+    cy.get('[data-testid="add-block-dropdown"]').findByText("Comment").click();
 
     cy.contains("Unsaved changes").should("not.exist");
 
     cy.findByText("Add a block").click();
-    cy.get(".dropdown-menu").findByText("Comment").click();
+    cy.get('[data-testid="add-block-dropdown"]').findByText("Comment").click();
 
     cy.contains("Unsaved changes").should("not.exist");
 
@@ -258,7 +258,6 @@ describe("Edit Page", () => {
 
     cy.findByText("Add files from server...").click();
     cy.findByText("Select files to add").should("exist");
-    cy.findAllByLabelText("Close").eq(1).click();
   });
 
   it("Uploads an XRD file, makes an XRD block and checks that the plot works", () => {
@@ -268,7 +267,7 @@ describe("Edit Page", () => {
     cy.findByText("editable_sample").click();
 
     cy.findByText("Add a block").click();
-    cy.get(".dropdown-menu").findByText("Powder XRD").click();
+    cy.get('[data-testid="add-block-dropdown"]').findByText("Powder XRD").click();
 
     cy.findByText("Select a file:").should("exist");
     cy.get("select.file-select-dropdown").select("example_data_XRD_example_bmb.xye");
@@ -285,7 +284,7 @@ describe("Edit Page", () => {
     cy.findByText("editable_sample").click();
 
     cy.findByText("Add a block").click();
-    cy.get(".dropdown-menu").findByText("Media").click();
+    cy.get('[data-testid="add-block-dropdown"]').findByText("Media").click();
     cy.findAllByText("Select a file:").eq(1).should("exist");
     cy.get("select.file-select-dropdown").eq(1).select(test_fname);
 

webapp/src/components/ToggleableCreatorsFormGroup.vue

@@ -37,6 +37,7 @@ import Creators from "@/components/Creators";
 import UserSelect from "@/components/UserSelect";
 import { OnClickOutside } from "@vueuse/components";
 import { updateItemPermissions } from "@/server_fetch_utils.js";
+import { toRaw } from "vue";
 
 export default {
   components: {
@@ -71,23 +72,28 @@ export default {
     },
   },
   watch: {
-    isEditingCreators(newValue, oldValue) {
+    async isEditingCreators(newValue, oldValue) {
       // Check we are leaving the editing state
       if (newValue === false && oldValue === true) {
         // Check that the permissions have actually changed
-        if (this.shadowValue === this.value) {
+        if (JSON.stringify(toRaw(this.value)) === JSON.stringify(toRaw(this.shadowValue))) {
           return;
         }
         try {
+          // Check if the user has just removed all creators and reset if so
+          if (this.shadowValue.length == 0) {
+            throw new Error("You must have at least one creator.");
+          }
           let answer = confirm("Are you sure you want to update the permissions of this item?");
           if (answer) {
-            updateItemPermissions(this.refcode, this.shadowValue);
+            await updateItemPermissions(this.refcode, this.shadowValue);
             this.$emit("update:modelValue", [...this.shadowValue]);
           } else {
             this.shadowValue = [...this.value];
           }
         } catch (error) {
           // Reset value to original
+          alert("Error updating permissions: " + error);
           this.shadowValue = [...this.value];
         }
       }

webapp/src/components/UserSelect.vue

@@ -4,6 +4,7 @@
     v-model="value"
     :options="filteredUsers"
     multiple
+    :min="minimumOptions"
     label="immutable_id"
     :filterable="false"
     @search="debouncedAsyncSearch"
@@ -37,6 +38,10 @@ export default {
       type: Array,
       default: () => [],
     },
+    minimumOptions: {
+      type: Number,
+      default: 0,
+    },
   },
   emits: ["update:modelValue"],
   data() {

webapp/src/server_fetch_utils.js

@@ -510,17 +510,14 @@ export function addABlock(item_id, block_type, index = null) {
 
 export function updateItemPermissions(refcode, creators) {
   console.log("updateItemPermissions called with", refcode, creators);
-  fetch_patch(`${API_URL}/items/${refcode}/permissions`, {
+  return fetch_patch(`${API_URL}/items/${refcode}/permissions`, {
     creators: creators,
-  })
-    .then(function (response_json) {
-      if (response_json.status === "error") {
-        throw new Error(response_json.message);
-      }
-    })
-    .catch(function (error) {
-      throw new Error(error);
-    });
+  }).then(function (response_json) {
+    if (response_json.status === "error") {
+      throw new Error(response_json.message);
+    }
+    return response_json;
+  });
 }
 
 export function saveItem(item_id) {

webapp/src/views/EditPage.vue

@@ -4,7 +4,6 @@
     class="navbar navbar-expand sticky-top navbar-dark py-0 editor-navbar"
     :style="{ backgroundColor: navbarColor }"
   >
-    <div v-show="false" class="navbar-nav"><LoginDetails /></div>
     <div v-show="false" class="navbar-nav"><LoginDetails /></div>
     <span class="navbar-brand clickable" @click="scrollToID($event, 'topScrollPoint')"
       >{{ itemTypeEntry?.navbarName || "loading..." }}&nbsp;&nbsp;|&nbsp;&nbsp;
@@ -28,6 +27,7 @@
           v-if="blockInfoLoaded"
           v-show="isMenuDropdownVisible"
           class="dropdown-menu"
+          data-testid="add-block-dropdown"
           style="display: block"
           aria-labelledby="navbarDropdown"
         >