Handle remote deletions more gracefully #248
Conversation
|
I'm thinking the next step could be to add a preference for how to handle this situation generally: always delete, or always unlink. |
amberin
requested review from
alensiljak,
stefan2904,
chvp,
colonelpanic8 and
cbkerr
|
I haven't looked at the code but am looking at how to make the message more clear:
New dialogue might be something like: Repository file mismatch Choose whether to: Write file to repo | Unlink notebook | ........ | Delete notebook Not sure on order of options. |
|
@cbkerr Many thanks! I'll show a picture of a new suggestion soon. Yes, there may be many repos. (I sync two different git repos, for sharing files with different people.) Unlinking means that the Orgzly notebook will no longer be associated with any repo. The general behavior during sync for books with no repo link is the following:
The user can always select a notebook and choose "Set link" in the top right menu, to change or remove its repo link. (The user must always do this if they have multiple repos and want to sync a newly created notebook.) |
Repositories don't have names, only URIs. While putting the URI in the message would be informative, I fear it may be too much text. (content:https:// repo URIs can be very long.) |
|
After fiddling with alternative solutions for a while, I realized that taking action through an alert dialog becomes very complicated when multiple files have been deleted. Now I'm thinking maybe the notification/prompt should simply inform the user, and give hints on what needs to be done. But I also think the notebook should be immediately de-linked from the repo, to make it easier for Orgzly to correctly interpret the book's status during the next sync. It also somewhat decreases the risk of involuntarily "restoring" remotely deleted files, which is the main issue we're trying to solve. The message could be something like "The file %s no longer exists in the repository %s. The notebook %s has therefore been de-linked from the repository. Please either delete the notebook, or trigger a new sync if you wish to sync it back to the repository." |
|
I also think we should add a new sync status for books which have been synced before, but are no longer linked to a repo. They should not be automatically linked and synced, even if there is only one repo. The workflow of silently linking and syncing makes sense when the user has created a new book in Orgzly and wants to sync it. But in our scenario, something more unusual has clearly occurred, and the user should take action themselves. |
amberin
force-pushed
the
handle-remote-deletions
branch
from
9670f39
to
273ebed
Compare
amberin
changed the title
amberin
changed the title
|
I suppose BOOK_WITHOUT_LINK_AND_PREVIOUS_ERROR would be better called BOOK_WITH_PREVIOUS_ERROR_AND_NO_LINK... |
|
I have pushed a different solution which does not involve an alert dialog, but relies more on the regular sync status messages, which are explained in the user documentation. This is a smaller change, but it should deal with the main problem. Please see the commit message for my thinking behind the code changes. |
amberin
requested review from
maikol-solis,
bemyak,
Gigahawk,
NatKarmios,
devhyper,
sp1r1t and
oujunhao
|
I like the unlink in background approach described in the commit. The only thing I'd tweak about the message is to make it clear which way the "sync" is happening to make it clear that the file will be written back to the repository. (also removed "therefore") "The file %s no longer exists in the repository %s. The notebook %s has ** been de-linked from the repository. Please either delete the notebook, or trigger a new sync if you wish to write it back to the repository."
Agree! |
|
@cbkerr The message you're referring to was intended for an alert dialog, but I ended up not adding one. There is now only a sync status message: "Remote file no longer exists. Repository link removed." This could of course be extended. Maybe:
|
This resolves orgzly-revived#77. We are adding two new BookSyncStatuses: - ROOK_NO_LONGER_EXISTS - BOOK_WITHOUT_LINK_AND_PREVIOUS_ERROR The first one is pretty obvious, but the second one requires some explanation. When a remote file is deleted, I have chosen to do two things to Orgzly's namesake notebook: 1. remove the book's repository link 2. clear the book's "synced to" attribute (which typically holds the last seen revision ID of the repository file) I chose to do this because it then becomes much easier to draw conclusions about the book's state during subsequent syncing attempts. One of the things it allows us to do is to NOT automatically link the notebook to the configured repository and sync it. This was the annoying previous behavior which we want to get rid of. For background, when Orgzly sees a local notebook without a repository link, and there is only one configured repository, it will usually silently link the notebook to the repository and sync it. This is great when the user has created a new notebook in Orgzly, but it's not desirable when something more unusual has happened, such as a file deletion. Instead, we want the user to be alerted about the deleted file, and then take action themselves. They probably want to delete the notebook from Orgzly, but they may also want to link it to a different repo, or back to the same one, in case the deletion was a mistake. In other words, the new sync status BOOK_WITHOUT_LINK_AND_PREVIOUS_ERROR allows us to distinguish between a local book which can be automatically linked to a repo and synced, and one which should not be touched, but left in an error state until the user takes action.
amberin
force-pushed
the
handle-remote-deletions
branch
from
a05ba15
to
2d84a5b
Compare
amberin
force-pushed
the
handle-remote-deletions
branch
from
01b71d0
to
1da0da0
Compare
The book's sync status message is now: "Remote file no longer exists; repository link removed. Set a link if you want to sync to a repository." I feel we should merge this and improve on it later, as needed. Notebook status:
Notification:
|
|
Gonna merge this soon, if there are no protests... |
This is my attempt to solve #77 (orgzly/orgzly-android#287)
[Edit: I have force-pushed a new solution without an alert dialog. See the comments below for my reasoning. I will leave the original comments untouched.]
As usual, I don't really know what I'm doing, so I am probably going about this notification/prompting business the wrong way. Any sort of feedback is very welcome.
If the app has focus, an alert dialog is shown with two choices: delete the local book, or unlink the local book.
If the app does not have focus, a notification is sent. When clicking the notification, the aforementioned alert dialog is shown.
If the user dismisses the notification (or loses it) instead of clicking it, the alert dialog will not be shown. This is not great. But the notebook will clearly show an error status, and the most important problem is gone -- Orgzly will no longer silently re-create files which were deleted on the remote side.