Add patch functionality to lists #691
Conversation
lib/services/lists.js
Outdated
| async function addToList(uri, data) { | ||
| // do a db call for the list | ||
| const list = await db.get(uri), | ||
| added = list.concat(data); |
There was a problem hiding this comment.
It might be a good idea to make sure the item being added to the list isn't already there.
There was a problem hiding this comment.
supposedly we want to allow dupes... which makes add / delete a little difficult. decided to allow dupes to be added, and deletion will remove all duplicates.
There was a problem hiding this comment.
yeah Chris lists aren't sets as far as clay is concerned i.e. they don't have semantics for duplicates, so dupes should be allowed
There was a problem hiding this comment.
How will this work with components like tags which has a count?
There was a problem hiding this comment.
is count used anywhere ? afaik it's only tracked, and I don't think it's tracked very well. At least I remember it being inaccurate.
If we care about count then a generic clay list isn't a good structure for this since there's no good way to update it while taking into account concurrent modifications. A new structure with an identifier would be necessary.
| * @param {Array<Object>} data | ||
| * @returns {Promise} | ||
| */ | ||
| function patchList(uri, data) { |
There was a problem hiding this comment.
this might be the first time I've seen PATCH implemented correctly haha
lib/services/lists.js
Outdated
| } | ||
|
|
||
| if (list.length === startLength) { | ||
| throw new Error('Nothing was removed from the list.'); |
There was a problem hiding this comment.
Personally I feel throwing an error in this case will prove more trouble than it's worth. Like if two editors end up removing the same item from the list that should work just fine rather than cause a 500.
not a strong opinion though
There was a problem hiding this comment.
I would tend to agree here. If the goal is to have it removed, it was already achieved.
jpope19
changed the title
|
Is there a PR or matching ticket to update Kiln to utilize PATCH instead of doing a PUT of the entire list? |
|
No description provided.