Page MenuHomePhabricator

[native] Make sure we finish saving Entry before deleting
ClosedPublic

Authored by ashoat on May 31 2024, 6:41 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Nov 18, 9:42 AM
Unknown Object (File)
Mon, Nov 11, 5:47 AM
Unknown Object (File)
Mon, Nov 11, 5:21 AM
Unknown Object (File)
Mon, Nov 11, 3:56 AM
Unknown Object (File)
Mon, Nov 11, 3:42 AM
Unknown Object (File)
Sun, Nov 10, 8:21 PM
Unknown Object (File)
Tue, Nov 5, 8:47 PM
Unknown Object (File)
Tue, Nov 5, 7:12 PM
Subscribers
None

Details

Summary

Now that onBlur from TextInput fires earlier (see here), when you press the delete button on an Entry it first starts to save.

As a result of this, I was occasionally seeing a race condition where we'd get a concurrent_modification error on the entry deletion request to the keyserver. This would occur because the save already went through, and the prevText on the deletion (the text before modification) would not match the updated text in MariaDB. Other times, the query to check the text in MariaDB for the deletion request would run before the query that updates the text for the save request, and no issue would be observed.

This diff addresses the race condition by making sure we finish the save before we start the delete. I basically just extended the existing needsDeleteAfterCreation behavior, which I renamed to needsDeleteAfterSave.

Depends on D12263

Test Plan

Type an entry and then press the delete button. On web, confirm that it appears in the history, but it otherwise missing

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

Not sure this is necessary after this realization. Will investigate more before submitting to review again

This diff is still useful. While "delete after save" no longer happens if the user hits the delete button while saving an Entry, it is still a relevant scenario if the user hits the save button first before hitting the delete button.

ashoat requested review of this revision.Jun 3 2024, 5:20 AM
This revision is now accepted and ready to land.Jun 4 2024, 1:17 AM