Add collections info to StartingMaterialTable.vue#1695
Add collections info to StartingMaterialTable.vue#1695
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1695 +/- ##
=======================================
Coverage 78.82% 78.82%
=======================================
Files 81 81
Lines 7038 7038
=======================================
Hits 5548 5548
Misses 1490 1490
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds collection membership visibility for starting materials by returning collection metadata from the API and rendering it in the Starting Materials table.
Changes:
- Add a “Collections” column to
StartingMaterialTable.vueusing the existingCollectionListcell renderer and enabling column filtering. - Update the
/starting-materials/aggregation pipeline to$lookupcollections and include them in the projected response.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| webapp/src/components/StartingMaterialTable.vue | Adds a Collections column and derives a comma-joined collectionsList field per row. |
| pydatalab/src/pydatalab/routes/v0_1/items.py | Adds collections lookup + projection to the /starting-materials/ list endpoint output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return this.$store.state.starting_material_list.map((item) => ({ | ||
| ...item, | ||
| collectionsList: (item.collections || []) | ||
| .map((collection) => collection.collection_id) | ||
| .join(", "), | ||
| })); |
| }, | ||
| "collections": { | ||
| "collection_id": 1, | ||
| "title": 1, |
There was a problem hiding this comment.
Pull request overview
Adds collection membership visibility for starting materials by wiring the backend /starting-materials/ summary response to include collection info and exposing it in the Starting Material table UI.
Changes:
- Add a “Collections” column to
StartingMaterialTable.vueusing the existingCollectionListcell renderer and filtering support. - Update Cypress component test expectations to include the new column header.
- Update the backend aggregation for
/starting-materials/to$lookupcollections and add a server test assertingcollections[].collection_idis present.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| webapp/src/components/StartingMaterialTable.vue | Adds a Collections column rendered via CollectionList. |
| webapp/cypress/component/StartingMaterialTableTest.cy.jsx | Updates expected table headers to include Collections. |
| pydatalab/tests/server/test_starting_materials.py | Adds an API test asserting /starting-materials/ includes collections[].collection_id. |
| pydatalab/src/pydatalab/routes/v0_1/items.py | Adds a collections $lookup and attempts to project collections into the response. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "collections": { | ||
| "collection_id": 1, | ||
| }, |
There was a problem hiding this comment.
wrong, but we should also get the collection title too
There was a problem hiding this comment.
| "collections": { | |
| "collection_id": 1, | |
| }, | |
| "collections": { | |
| "collection_id": 1, | |
| "title": 1, | |
| }, |
There was a problem hiding this comment.
The lookup doesn't currently return the title, only _id and collection_id.
Do you want me to update it to return title?
Displaying the title alongside collection_id looks a bit messy in the UI
There was a problem hiding this comment.
Huh, I guess that's a bug in the item routes too then (which tries to grab title). Let's just stick to collection_id only then.
| collection_data = json.loads(default_collection.json()) | ||
| collection_data["starting_members"] = [{"item_id": default_starting_material_dict["item_id"]}] | ||
| response = client.put("/collections", json={"data": collection_data}) | ||
| assert response.status_code == 201, response.json | ||
|
|
||
| response = client.get("/starting-materials/") | ||
| assert response.status_code == 200 | ||
| items = response.json["items"] | ||
| matching = [ | ||
| item for item in items if item["item_id"] == default_starting_material_dict["item_id"] | ||
| ] | ||
| assert len(matching) == 1 | ||
| item = matching[0] | ||
|
|
||
| assert "collections" in item, "collections key missing from /starting-materials/ response" | ||
| assert len(item["collections"]) == 1 | ||
| assert item["collections"][0]["collection_id"] == default_collection.collection_id | ||
|
|
||
| # Clean up the collection so later tests are unaffected | ||
| client.delete(f"/collections/{default_collection.collection_id}") | ||
|
|
||
|
|
There was a problem hiding this comment.
I wouldn't worry about clean up too much as long as it doesn't effect tests in this file -- our database is torn down and deleted after every test file anyway
…terial and equipment pages
…ocation and supplier
…entory. I.e the user can't type
ml-evs
force-pushed
the
bes/add_collections_to_sm_page
branch
from
fc620a9 to
32c4839
Compare
No description provided.