Page MenuHomePhabricator

[native] Fix Entry deletion after creation
ClosedPublic

Authored by ashoat on May 31 2024, 6:40 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Nov 2, 6:12 AM
Unknown Object (File)
Sat, Nov 2, 6:12 AM
Unknown Object (File)
Sat, Nov 2, 6:12 AM
Unknown Object (File)
Sat, Nov 2, 6:10 AM
Unknown Object (File)
Sat, Nov 2, 5:46 AM
Unknown Object (File)
Thu, Oct 10, 9:51 PM
Unknown Object (File)
Thu, Oct 10, 9:51 PM
Unknown Object (File)
Thu, Oct 10, 9:51 PM
Subscribers
None

Details

Summary

Noticed that this.needsDeleteAfterCreation wasn't working as intended. The issue was that we were getting a concurrent_modification error because the prevText we were passing in for the delete did not match the text after creation. We were passing in the empty string since that is what this.props.entryInfo.text was prior to the CREATE_ENTRY_SUCCESS action being reduced.

To address this, this diff allows us to pass this.state.text in instead. This makes sure that we match what is in the keyserver MariaDB when we attempt the deletion.

Depends on D12262

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
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Something we should consider for this diff and the next one is whether it would be better behavior to discard pending changes when the user hits the "delete" button while editing. This may have been the prior behavior, when the TextInput onBlur fired after the button onPress.

For pending creations we could just avoid sending any requests to keyserver (no creation or deletion). For pending updates, we could discard the pending text and instead just delete the entry.

To do it this way, we'd need to add a sleep to the onBlur callback and wait to make sure a delete isn't triggered soon after. If a delete is triggered, the onBlur action is discarded.

We'd still want to keep the needsDeleteAfterCreation/needsDeleteAfterSave behavior, but it would only apply if the user hits delete after already initiating a keyserver request (eg. they press save and then delete).

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

ashoat requested review of this revision.Jun 3 2024, 5:13 AM

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

This revision is now accepted and ready to land.Jun 4 2024, 1:13 AM
This revision was automatically updated to reflect the committed changes.