Merge f7aa49b into f58a5ed

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

@@ -13,6 +13,7 @@
 from pydatalab.logger import LOGGER
 from pydatalab.models import ITEM_MODELS
 from pydatalab.models.items import Item
+from pydatalab.models.people import Person
 from pydatalab.models.relationships import RelationshipType
 from pydatalab.models.utils import generate_unique_refcode
 from pydatalab.mongo import flask_mongo
@@ -204,14 +205,10 @@ def creators_lookup() -> Dict:
         "from": "users",
         "let": {"creator_ids": "$creator_ids"},
         "pipeline": [
-            {
-                "$match": {
-                    "$expr": {
-                        "$in": ["$_id", {"$ifNull": ["$$creator_ids", []]}],
-                    },
-                }
-            },
-            {"$project": {"_id": 0, "display_name": 1, "contact_email": 1}},
+            {"$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",
     }
@@ -603,6 +600,103 @@ def create_samples():
     )  # 207: multi-status
 
 
+@ITEMS.route("/items/<refcode>/permissions", methods=["PATCH"])
+def update_item_permissions(refcode: str):
+    """Update the permissions of an item with the given refcode."""
+
+    request_json = request.get_json()
+    creator_ids: list[ObjectId] = []
+
+    if not len(refcode.split(":")) == 2:
+        refcode = f"{CONFIG.IDENTIFIER_PREFIX}:{refcode}"
+
+    current_item = flask_mongo.db.items.find_one(
+        {"refcode": refcode, **get_default_permissions(user_only=True)},
+        {"_id": 1, "creator_ids": 1},
+    )  # type: ignore
+
+    if not current_item:
+        return (
+            jsonify(
+                {
+                    "status": "error",
+                    "message": f"No valid item found with the given {refcode=}.",
+                }
+            ),
+            401,
+        )
+
+    current_creator_ids = current_item["creator_ids"]
+
+    if "creators" in request_json:
+        creator_ids = [
+            ObjectId(creator.get("immutable_id", None))
+            for creator in request_json["creators"]
+            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):
+        return (
+            jsonify(
+                {
+                    "status": "error",
+                    "message": "One or more creator IDs not found in the database.",
+                }
+            ),
+            400,
+        )
+
+    # Make sure a user cannot remove their own access to an item
+    current_user_id = current_user.person.immutable_id
+    try:
+        creator_ids.remove(current_user_id)
+    except ValueError:
+        pass
+    creator_ids.insert(0, current_user_id)
+
+    # 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)
+
+    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(
+        {"refcode": refcode, **get_default_permissions(user_only=True)},
+        {"$set": {"creator_ids": creator_ids}},
+    )
+
+    if not result.modified_count == 1:
+        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
+
+
 @ITEMS.route("/delete-sample/", methods=["POST"])
 def delete_sample():
     request_json = request.get_json()  # noqa: F821 pylint: disable=undefined-variable
@@ -918,9 +1012,11 @@ def search_users():
                     "_id": 1,
                     "identities": 1,
                     "display_name": 1,
+                    "contact_email": 1,
                 }
             },
         ]
     )
-
-    return jsonify({"status": "success", "users": list(cursor)}), 200
+    return jsonify(
+        {"status": "success", "users": list(json.loads(Person(**d).json()) for d in cursor)}
+    ), 200

pydatalab/tests/server/test_permissions.py

@@ -7,7 +7,7 @@ def test_unverified_user_permissions(unverified_client):
     response = client.get("/samples/")
     assert response.status_code == 200
 
-    response = client.post("/new-sample/", json={"item_id": "test"})
+    response = client.post("/new-sample/", json={"type": "samples", "item_id": "test"})
     assert response.status_code == 401
 
     response = client.get("/starting-materials/")
@@ -20,7 +20,7 @@ def test_deactivated_user_permissions(deactivated_client):
     response = client.get("/samples/")
     assert response.status_code == 200
 
-    response = client.post("/new-sample/", json={"item_id": "test"})
+    response = client.post("/new-sample/", json={"type": "samples", "item_id": "test"})
     assert response.status_code == 401
 
     response = client.get("/starting-materials/")
@@ -33,8 +33,54 @@ def test_unauthenticated_user_permissions(unauthenticated_client):
     response = client.get("/samples/")
     assert response.status_code == 401
 
-    response = client.post("/new-sample/", json={"item_id": "test"})
+    response = client.post("/new-sample/", json={"type": "samples", "item_id": "test"})
     assert response.status_code == 401
 
     response = client.get("/starting-materials/")
     assert response.status_code == 401
+
+
+def test_basic_permissions_update(admin_client, admin_user_id, client, user_id):
+    """Test that an admin can share an item with a normal user."""
+
+    response = admin_client.post(
+        "/new-sample/", json={"type": "samples", "item_id": "test-admin-sample"}
+    )
+    assert response.status_code == 201
+
+    response = admin_client.get("/get-item-data/test-admin-sample")
+    assert response.status_code == 200
+    refcode = response.json["item_data"]["refcode"]
+
+    response = client.get(f"/items/{refcode}")
+    assert response.status_code == 404
+
+    # Add normal user to the item
+    response = admin_client.patch(
+        f"/items/{refcode}/permissions", json={"creators": [{"immutable_id": str(user_id)}]}
+    )
+    assert response.status_code == 200
+
+    # Check that they can now see it
+    response = client.get(f"/items/{refcode}")
+    assert response.status_code == 200
+
+    # Check that they cannot remove themselves/the admin from the creators
+    client.patch(f"/items/{refcode}/permissions", json={"creators": []})
+    assert response.status_code == 200
+
+    response = client.get(f"/items/{refcode}")
+    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": [{"immutable_id": str(admin_user_id)}]}
+    )
+    assert response.status_code == 200
+
+    response = client.get(f"/items/{refcode}")
+    assert response.status_code == 404
+
+    # but that the admin still remains the creator
+    response = admin_client.get(f"/items/{refcode}")
+    assert response.status_code == 200

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/CellInformation.vue

@@ -25,11 +25,8 @@
               <FormattedRefcode :refcode="Refcode" />
             </div>
           </div>
-          <div class="form-group col-md-3 col-sm-3 col-6 pb-3 pr-2">
-            <label id="cell-creators">Creators</label>
-            <div aria-labelledby="cell-creators" class="mx-auto">
-              <Creators :creators="ItemCreators" :size="36" />
-            </div>
+          <div class="form-group col-md-3 col-sm-3 col-6 pb-3">
+            <ToggleableCreatorsFormGroup v-model="ItemCreators" :refcode="Refcode" />
           </div>
           <div class="col-md-6 col-sm-7 pr-2">
             <ToggleableCollectionFormGroup v-model="Collections" />
@@ -115,7 +112,7 @@ import TableOfContents from "@/components/TableOfContents";
 import ItemRelationshipVisualization from "@/components/ItemRelationshipVisualization";
 import FormattedRefcode from "@/components/FormattedRefcode";
 import ToggleableCollectionFormGroup from "@/components/ToggleableCollectionFormGroup";
-import Creators from "@/components/Creators";
+import ToggleableCreatorsFormGroup from "@/components/ToggleableCreatorsFormGroup";
 import { cellFormats } from "@/resources.js";
 
 export default {
@@ -127,7 +124,7 @@ export default {
     ItemRelationshipVisualization,
     FormattedRefcode,
     ToggleableCollectionFormGroup,
-    Creators,
+    ToggleableCreatorsFormGroup,
   },
   props: {
     item_id: {

webapp/src/components/Creators.vue

@@ -26,7 +26,7 @@ export default {
     },
     showNames: {
       type: Boolean,
-      default: false,
+      default: true,
       required: false,
     },
     showBubble: {

webapp/src/components/DynamicDataTable.vue

@@ -459,12 +459,16 @@ export default {
 
       const config = propsConfig[componentName] || {};
 
-      const props = Object.entries(config).reduce((acc, [prop, dataKey]) => {
-        if (dataKey !== true) {
+      const props = Object.entries(config).reduce((acc, [prop, setting]) => {
+        if (typeof setting === "object" && setting !== null && "value" in setting) {
+          acc[prop] = setting.value;
+        } else if (typeof setting === "boolean") {
+          acc[prop] = setting;
+        } else if (typeof setting === "string") {
           if (prop === "itemType") {
             acc[prop] = data.type !== undefined ? data.type : "starting_materials";
-          } else if (data[dataKey] !== undefined) {
-            acc[prop] = data[dataKey];
+          } else if (data[setting] !== undefined) {
+            acc[prop] = data[setting];
           }
         }
         return acc;

webapp/src/components/SampleInformation.vue

@@ -30,10 +30,7 @@
             </div>
           </div>
           <div class="form-group col-md-3 col-sm-3 col-6 pb-3">
-            <label id="samp-creators">Creators</label>
-            <div aria-labelledby="samp-creators" class="mx-auto">
-              <Creators :creators="ItemCreators" :size="36" />
-            </div>
+            <ToggleableCreatorsFormGroup v-model="ItemCreators" :refcode="Refcode" />
           </div>
           <div class="col-md-6 col-sm-7 pr-2">
             <ToggleableCollectionFormGroup v-model="Collections" />
@@ -62,11 +59,11 @@ import { createComputedSetterForItemField } from "@/field_utils.js";
 import ChemFormulaInput from "@/components/ChemFormulaInput";
 import FormattedRefcode from "@/components/FormattedRefcode";
 import ToggleableCollectionFormGroup from "@/components/ToggleableCollectionFormGroup";
+import ToggleableCreatorsFormGroup from "@/components/ToggleableCreatorsFormGroup";
 import TinyMceInline from "@/components/TinyMceInline";
 import SynthesisInformation from "@/components/SynthesisInformation";
 import TableOfContents from "@/components/TableOfContents";
 import ItemRelationshipVisualization from "@/components/ItemRelationshipVisualization";
-import Creators from "@/components/Creators";
 
 export default {
   components: {
@@ -77,7 +74,7 @@ export default {
     ItemRelationshipVisualization,
     FormattedRefcode,
     ToggleableCollectionFormGroup,
-    Creators,
+    ToggleableCreatorsFormGroup,
   },
   props: {
     item_id: { type: String, required: true },