We'll want to be able to delete stuff like pages for bug 1273507. We can do this by making a PUT request that doesn't have a value, so the timestamp will still update and notify others. We would then need to recognize the missing value to notify UI of the deletion.
Created attachment 8758159 [details] [review] WIP Ed, I'm not sure if this approach is what you mean with your comment. Could you take a look at the WIP patch and give me some feedback?
Attachment #8758159 - Flags: feedback?(edilee)
Comment on attachment 8758159 [details] [review] WIP This is different from what I originally intended, but after thinking about it, this should be fine. Originally, I was thinking of a general purpose delete, but the only thing we would really delete are tiles (tocPages? depends on what bug 1274179 sets the name.) Overall the approach seems fine with some minor name/logic changes.
Attachment #8758159 - Flags: feedback?(edilee) → feedback+
Assignee: nobody → b.mcb
Status: NEW → ASSIGNED
Comment on attachment 8758671 [details] [review] [loop] mancas:bug1274177 > mozilla:akita Holding off on review for now as this depends on bug 1274179 https://github.com/mozilla/loop/pull/477/files The names should match up - in particular just plain "tile" isn't right as there are other tiles.
Comment on attachment 8758671 [details] [review] [loop] mancas:bug1274177 > mozilla:akita The name from bug 1274179 ended up being page, so we'll want `DeletedPage` action with `deletePage` method. I believe `Object.assign` will work with the undefined value of a deleted record and end up just taking `dispatchExtra` although I suppose the unit test in the PR will check for exactly that. :)
Attachment #8758671 - Flags: feedback+
Comment on attachment 8758671 [details] [review] [loop] mancas:bug1274177 > mozilla:akita r=Mardak adding in `pageId` to the added and deleted actions. (Some reason I thought an earlier version did include the id in the deleted action already... ?)
Attachment #8758671 - Flags: review?(edilee) → review+
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.