[WIP] Add routes to add an item to a collection from the collections page

[ml-evs]

Builds on #553, still some outstanding work to do.

One way of hacking in a "add item to collection" route would be to treat the following cases in /save-item:

• If /save-item is called with collections: [{"collection_id": "coll1"}] then after saving the item should only be in coll1 even if it was already in coll2
• If /save-item is called with collections: {"collection_id": "coll1"} then after saving the item should be added to coll1 but not removed from any other colls
• If /save-item is called with collections: [] then remove the item from all collections.

We will run into this pattern a lot when writing the Python API, i.e., whether you should be able to modify only the provided parts of a sample, or whether you always have to provide the full state to avoid deleting other fields (e.g., whether it should behave like a dictionary assignment or .update() in Python)

[cypress]

Passing run #777 ↗︎

0

26

0

0

0

Details:

Merge 63f170d into 1bda051...
Project: datalab	Commit: c1e10088fc ℹ️
Status: Passed	Duration: 01:31 💡
Started: Jan 25, 2024 9:25 PM	Ended: Jan 25, 2024 9:27 PM

Review all test suite changes for PR #557 ↗︎

[jdbocarsly]

One way of hacking in a "add item to collection" route would be to treat the following cases in /save-item:

* If `/save-item` is called with `collections: [{"collection_id": "coll1"}]` then after saving the item should only be in coll1 even if it was already in coll2

* If `/save-item` is called with `collections: {"collection_id": "coll1"}` then after saving the item should be added to coll1 but not removed from any other colls

* If `/save-item` is called with `collections: []` then remove the item from all collections.

Since adding or removing collections via save item is already working in the previous PR, I would propose to keep it as is (which is already quite a large function), and instead write two new routes:

• /add-item-to-collection
• /remove-item-from-collection

These wouldn't necessarily be used when managing collections from the sample edit page, but they would be used on the collection edit page and anywhere else where the mental model is: "I'm managing a collection"

The actual under-the-hood implementation currently is that the collections are stored in the item mongo documents, but from the perspective of a caller (the frontend collection page, python api interacting with collections, etc.), I think it should still "feel" like you are calling routes that deal with the collection, if that makes sense. Otherwise, it gets messy to try to do stuff on the collections page, but have to write complicated save_item API requests to do that stuff.

[jdbocarsly]

We will run into this pattern a lot when writing the Python API, i.e., whether you should be able to modify only the provided parts of a sample, or whether you always have to provide the full state to avoid deleting other fields (e.g., whether it should behave like a dictionary assignment or .update() in Python)

Yeah, that's tricky. IMO updating is a better pattern, but harder to write the routes for. If we do this, we should also use more expressive names for the functions, i.e. update_item_data() . This could be supplemented by having a separate set of functions that are replace_item_data() if there are still use cases for that.

[ml-evs]

Yeah, that's tricky. IMO updating is a better pattern, but harder to write the routes for. If we do this, we should also use more expressive names for the functions, i.e. update_item_data() . This could be supplemented by having a separate set of functions that are replace_item_data() if there are still use cases for that.

On the API side, I think using the HTTP semantics for PUT, PATCH and DELETE would make this clear too, with more expressive URLs so that DELETE /collections/<coll_id>/items/<item_id> is what removes the item from the collection and PUT /collections/<coll_id>/items/<item_id> is what adds it