Closed Bug 1274177 Opened 8 years ago Closed 8 years ago

Allow deleting records from firebase and detecting those deletions

Categories

(Hello (Loop) :: Client, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Mardak, Assigned: mancas)

References

Details

(Whiteboard: [akita])

Attachments

(2 files)

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.
Rank: 19
Priority: -- → P1
Attached file 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+
Depends on: 1274179
Assignee: nobody → b.mcb
Status: NEW → ASSIGNED
Attachment #8758671 - Flags: review?(edilee)
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.
Attachment #8758671 - Flags: review?(edilee)
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+
Attachment #8758671 - Flags: review?(edilee)
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+
https://github.com/mozilla/loop/commit/03ecafa4be2ecb0b3c1d46cfc26dcec207796acb
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: