Page MenuHomePhabricator

[lib] DMOperationSpec for delete entry operation
ClosedPublic

Authored by will on Sep 16 2024, 1:16 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Oct 23, 9:11 AM
Unknown Object (File)
Wed, Oct 23, 9:11 AM
Unknown Object (File)
Sun, Oct 20, 9:37 PM
Unknown Object (File)
Fri, Oct 18, 10:23 PM
Unknown Object (File)
Fri, Oct 18, 1:58 PM
Unknown Object (File)
Thu, Oct 17, 4:45 PM
Unknown Object (File)
Sun, Oct 13, 4:33 AM
Unknown Object (File)
Sun, Oct 13, 2:54 AM
Subscribers
None

Details

Summary

Implements the delete entry DM operation spec

Depends on D13316

Test Plan

Tested later in stack. Will test notifs before landing

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

will requested review of this revision.Sep 16 2024, 1:34 AM
tomek requested changes to this revision.Sep 16 2024, 3:41 AM

It is a bit confusing to see all this info being sent while deleting an entry. It might be invalid when one user edits an entry while someone else deletes it.

The proper solution would require:

  1. Modifying the payload to contain just the ID of the deleted entry without too many details
  2. Introducing a util that provides all the entry infos
  3. Introducing a separate timestamp for deleted property

This will take some time, and we're short on it, so we should search for a simpler solution.
What we can do for now, is to keep the current approach and introduce a lastUpdatedTime timestamp check.

This revision now requires changes to proceed.Sep 16 2024, 3:41 AM

It is a bit confusing to see all this info being sent while deleting an entry. It might be invalid when one user edits an entry while someone else deletes it.

The proper solution would require:

  1. Modifying the payload to contain just the ID of the deleted entry without too many details
  2. Introducing a util that provides all the entry infos
  3. Introducing a separate timestamp for deleted property

This will take some time, and we're short on it, so we should search for a simpler solution.
What we can do for now, is to keep the current approach and introduce a lastUpdatedTime timestamp check.

Same comment for edit. Did a timestamp check up the stack:
https://phab.comm.dev/D13348

will requested review of this revision.Sep 16 2024, 5:46 AM

Rerequesting review

This revision is now accepted and ready to land.Sep 16 2024, 5:57 AM