fix(shared-decks): show login snackbar when AnkiWeb rate-limits deck downloads - #22153
sanjaysargam wants to merge 4 commits into
Conversation
david-allison
left a comment
david-allison
left a comment
There was a problem hiding this comment.
I believe this is buggy if it occurs while the activity is backgrounded
david-allison
added
Needs Author Reply
d9257d6 to
92f73db
Compare
sanjaysargam
added
Needs Review
and removed
Has Conflicts
Needs Author Reply
@david-allison Fixed this by moving the UI out of the receiver, it now only records state using ViewModel |
david-allison
left a comment
david-allison
left a comment
There was a problem hiding this comment.
Cheers! Logic seems good, but it feels like a lot of code churn in a single commit for this.
| * | ||
| * The redirect is not performed if [redirectTimes] is 3 or more | ||
| */ | ||
| private fun redirectUserToSignUpOrLogin() { |
| isSuccessful: Boolean, | ||
| isInvalidDeckFile: Boolean = false, | ||
| isRateLimited: Boolean = false, |
There was a problem hiding this comment.
This has become too complex, add a commit to convert it to an enum
| Timber.i("Download failed, offer a retry") | ||
| if (isVisible) { | ||
| context?.let { showThemedToast(it, CommonString.something_wrong, false) } | ||
| // A 429 from AnkiWeb is only actionable for a logged-out user; a logged-in |
There was a problem hiding this comment.
Find one place for these explanations to live, the comments are repeated and it feels excessive
92f73db to
22db50d
Compare
22db50d to
849407b
Compare
Didn't get to re-review this one today, sorry!
Please ping me - I'll aim to have it done within 24 hours
mikehardy
left a comment
mikehardy
left a comment
There was a problem hiding this comment.
Thanks for splitting the commits, moving the snackbar off the receiver, and covering backgrounded 429. The 429 detection looks right.
Request changes
-
Login path incomplete — WebView 429 calls
redirectUserToSignUpOrLogin()(snackbar + login URL). Download 429 only shows the snackbar andpopBackStack().showLoginRequiredSnackbar()adds Sign up only when AnkiDroidisLoggedIn()is false, so a user with an AnkiDroid account and no AnkiWeb WebView session gets a message with nothing to tap. Please reuse the existing redirect after leaving the download screen (or add a Login action that loadsshared_decks_login_url), and add a regression for AnkiDroid-logged-in + WebView-logged-out. -
AI_POLICY — PR NOTE has
Assisted-by: Opus 5.5, but none of the four commits have anAssisted-by:trailer. Please add trailers (and preferably note what Opus wrote).
Local on rebased tip 849407b6 (4 commits): lint / unit / package / emulator (API36_GAPI_PS16K) all green.
mikehardy
added
the
Needs Author Reply
3 participants
Note
Assisted-by: Opus 5.5
Purpose / Description
Downloading a second shared deck while logged out fails with a generic "Something went wrong" toast. AnkiWeb actually replies HTTP 429, asking the user to log in
Fixes
Approach
Android's Downloads puts the raw HTTP status code in COLUMN_REASON for 4xx/5xx failures, so that download-failure path now reads the reason and detects 429
How Has This Been Tested?
Regression Test
Physical Device
WhatsApp.Video.2026-09-28.at.11.10.08.PM.mp4
Checklist
Please, go through these checks before submitting the PR.