Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

User select and sharing of sample page with creators #830

Merged
merged 5 commits into from
Nov 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 106 additions & 10 deletions pydatalab/src/pydatalab/routes/v0_1/items.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
52 changes: 49 additions & 3 deletions pydatalab/tests/server/test_permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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/")
Expand All @@ -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/")
Expand All @@ -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
9 changes: 4 additions & 5 deletions webapp/cypress/e2e/editPage.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down Expand Up @@ -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", () => {
Expand All @@ -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");
Expand All @@ -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);

Expand Down
11 changes: 4 additions & 7 deletions webapp/src/components/CellInformation.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down Expand Up @@ -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 {
Expand All @@ -127,7 +124,7 @@ export default {
ItemRelationshipVisualization,
FormattedRefcode,
ToggleableCollectionFormGroup,
Creators,
ToggleableCreatorsFormGroup,
},
props: {
item_id: {
Expand Down
2 changes: 1 addition & 1 deletion webapp/src/components/Creators.vue
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export default {
},
showNames: {
type: Boolean,
default: false,
default: true,
required: false,
},
showBubble: {
Expand Down
12 changes: 8 additions & 4 deletions webapp/src/components/DynamicDataTable.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
9 changes: 3 additions & 6 deletions webapp/src/components/SampleInformation.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down Expand Up @@ -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: {
Expand All @@ -77,7 +74,7 @@ export default {
ItemRelationshipVisualization,
FormattedRefcode,
ToggleableCollectionFormGroup,
Creators,
ToggleableCreatorsFormGroup,
},
props: {
item_id: { type: String, required: true },
Expand Down
Loading
Loading