Warn users when before signing out with unsynced notes #702
Conversation
|
Noticed that I said |
|
One other thing to note, the iOS and macOS PRs also check for notes with queued changes that haven't synced yet. I haven't dug into whether or not we can do that with |
…here's any local modifications to notes before signing out.
@beaucollins helped me get a check for this added to @dmsnell could you have a look a this one at your convenience? 🙏 |
lib/dialogs/settings.jsx
Outdated
| () => this.showUnsyncedWarning() // Show a warning to the user | ||
| ); | ||
| }) | ||
| ); |
There was a problem hiding this comment.
what's going on here? is there a missing if statement body? something is wrong.
There was a problem hiding this comment.
Whoops, I had an if there that didn't belong. Callbacks are silly.
lib/dialogs/settings.jsx
Outdated
| getVersion(note.id, (e, v) => (e || v === 0 ? reject() : resolve())) | ||
| ); | ||
|
|
||
| Promise.race(notes.map(noteHasSynced)).then( |
There was a problem hiding this comment.
Should this be Promise.all not Promise.race? You want to make sure that none of the notes are unsynced right?
lib/dialogs/settings.jsx
Outdated
| getVersion(note.id, (e, v) => (e || v === 0 ? reject() : resolve())) | ||
| ); | ||
|
|
||
| Promise.race(notes.map(noteHasSynced)).then( |
There was a problem hiding this comment.
Looks like I commented on an outdated change but the Promise.race remains.
It seems you want to check if any of the notes have a version of 0 and if any of them do have a version of 0 then you will want to show showUnsyncedWarning.
Promise.race will return the first promise that resolves or rejects and ignore all others where as Promise.all will wait until all Promises resolve or fail early upon the first reject.
There was a problem hiding this comment.
Hmm ok, I think I indeed want Promise.all then. If we ever see a version of zero we should stop and show the warning. Otherwise, keep checking.
There was a problem hiding this comment.
Updated in 5324686. Nice catch!
There was a problem hiding this comment.
good catch @beaucollins - the race was from my original idea where only the unsync'd notes would resolve and so we wouldn't have needed to wait for all Promises - an early abort. this was a wrong idea. 😄
it would also work if we only reject()ed on unsync'd notes and had a timeout, but that's another kind of race we don't want.
There was a problem hiding this comment.
I've learned a lot about Promise via this PR! :)
There was a problem hiding this comment.
@roundhill you don't need to learn that much about promises. just let your "yes" be "yes" and your "no" be "no" 😉
Some users are losing notes when they sign out because for one reason or another they have unsynced notes. This is a quick solution to that which searches for unsynced notes upon signout and warns the user if so. An option is presented to view the web app to reconcile their unsynced notes:
Windows/Linux:
On the web (currently not used in production), a humble confirm dialog is displayed:
One nice thing that we could also do is offer to export the notes for the user. Maybe in place of the 'Visit Web App' button?
To Test
Sign Out. You should see the alert dialog.Refs: Automattic/Simplenote-United#7