Improve the dialog's features related to BackendSyncException #17017
Conversation
|
Message to maintainers, this PR contains strings changes.
Read more about updating strings on the wiki, |
Is this the server time or the device time? |
It's device time. |
|
So, what about using Or what about using a sentence: |
peyman79
force-pushed
the
Improve-clock-sync-error-dialog
branch
from
e9827c2 to
750ea0e
Compare
Yes you're right. I changed it, It's better now. |
Now this dialog has a "Go to setting" button. Also it shows the current time.
peyman79
force-pushed
the
Improve-clock-sync-error-dialog
branch
from
750ea0e to
b660a05
Compare
| val formattedTime = currentTime.format(dateFormatter) | ||
| val invalidClockMessage = context.getString(R.string.InvalidClockDialog) | ||
|
|
||
| val fullMessage = "$msg\n\n$invalidClockMessage\n$formattedTime" |
There was a problem hiding this comment.
🤔 normally I would be wary of string concatenation as it may break RTL languages, but it is on a new line so it should be okay
| is BackendSyncException -> { | ||
| // this exceptions do not generate worthwhile crash reports | ||
| showInvalidClockDialog(this, exc.localizedMessage!!, exc) |
There was a problem hiding this comment.
Are you certain that the only BackendSyncException is from invalid device time?
Even if you are certain of that now are we certain that will remain true forever?
Is there some way to constrain this new dialog to present only when we are certain that it was from an invalid device time?
Perhaps a string match on exception message "your clock is not set to correct time"? But I believe this message comes through localized and may be translated to whatever we set the backend language too (which is ideally the user's selected language) which means a string search is not valid
So, a specific question for @dae then: is BackendError.Kind.SYNC_OTHER_ERROR a set of errors of different sub-types - sort of a grab bag - or is it always device time related? If it is multiple different types is there any way to differentiate with certainty whether a specific SYNC_OTHER_ERROR is device time related ?
I trolled through and I couldn't see anything specific about BackendError.Kind.SYNC_OTHER_ERROR being constrained only to device time issues:
For this PR, if this is always a device time error, great. If it is not always device time but we can differentiate with certainty, then we need to only show this specific dialog when we are certain, otherwise use old code path. If there is no way to differentiate then the Dialog needs to be made much more ambiguous and say something like "if the error is device time related, you may wish to open time settings and update your time, current device time is xxxx" or similar
...that's the only reference I have really, as the ankiweb sync server code does not seem to be searchable
There was a problem hiding this comment.
Sorry, lost track of this one. If AD wants to be able to identify the clock message separately, it should send through a PR that moves the error out of OTHER and into a separate specific error, like we currently use to detect sync auth failures. Other is basically a catch-all for errors that we don't (currently) need to special-case in the frontend.
There was a problem hiding this comment.
🤔 pretty far out of my wheelhouse but after some spelunking, is that roundabouts here?
There was a problem hiding this comment.
Yep, that maps our internal error to the limited protobuf errors we export. With a new case in the .proto file, ClockIncorrect could be mapped to it.
I had AnkiDroid notifications turned off which is why I missed this (again). I should see future pings.
lukstbit
added
the
Blocked by dependency
david-allison
added
the
Needs Author Reply
|
Hello 👋, this PR has had no activity for more than 2 weeks and needs a reply from the author. If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing! You have 7 days until this gets closed automatically |
Purpose / Description
After clicking the sync button, if your current time is invalid, a dialog will appear notifying you that your time settings are incorrect. This dialog now includes a "Go to Settings" button, allowing you to easily adjust your date and time to the correct settings. Additionally, the dialog displays the current time for your reference.
Fixes
How Has This Been Tested?
Here is a screenshot of the result:
Checklist
Please, go through these checks before submitting the PR.