Fix custom picker uploading wrong image when filtering actioned pictures - #6930
Fix custom picker uploading wrong image when filtering actioned pictures#6930Disha-Shukla wants to merge 2 commits into
Conversation
Use a stable actionableImages list indexed by adapter position so the thumbnail shown and the image selected for upload always match after scrolling with Show already actioned pictures disabled. Fixes commons-app#6289 Co-authored-by: Cursor <cursoragent@cursor.com>
|
Regarding the IP block, can you try registering from other connections (mobile data, home WiFi, etc)? Wide-range blocks usually affect registration and logged-out editiing only. A free WiFi is often blocked permanently. If that doesn't work, using the betaDebug built variant and Beta Commons can be a workaround. |
|
I actually tried with 3 different networks other then the Wifi but I got the same message. |
|
3 sounds a bit too many to be a coincidence. Do you use VPN or proxy? If you do, please disable it or register from a more vanilla device. (You can use the same account across devices later.) If nothing works, the beta option I mentioned above might work to some extent. (But you will eventually need the real Commons access to be able to test the app fully.) (Something like Discord #6561 might help non-code trouble shooting like this, but I believe we don't have one that is alive.) |
|
✅ Generated APK variants! |
|
Thanks! I am testing this branch now, I like it! I observe these two bugs, do you think they can be fixed too?
2387e823-ca23-495d-a71d-bb3974db1ce6.mp4
4ed9e03f-2fbe-49fa-b34a-3d5220bc4cf7.mp4 |
chrisdebian
left a comment
There was a problem hiding this comment.
Nice simplification overall — the old design (a TreeMap keyed by the underlying allImages index, with a separate alreadyAddedPositions/imagePositionAsPerIncreasingOrder pair trying to track adapter position independently) was genuinely fragile, and a single position-indexed actionableImages list with one background loader is a much clearer model. The generation counter for cancelling stale loads on init() (folder switch) is a nice touch too.
One behavioural change worth confirming is intentional: the old code loaded actionable images lazily, paced by onBindViewHolder — it only searched for the next actionable image when a genuinely new position was bound, so loading was naturally throttled to what the user had actually scrolled to. In this PR, startLoadingActionables() is called exactly once from init(), and its while loop runs to completion — it scans the entire folder for actionable images continuously in the background the moment the picker opens with the switch off, with no pacing tied to scroll position at all. For a small folder that's no different in practice, but for a folder with several thousand photos this means continuous background CPU/IO work (each nextActionableImage() call presumably touches the content resolver) starts immediately and keeps running regardless of whether the user ever scrolls past the first screen. Worth confirming this is the intended trade-off (simplicity + correctness over lazy pacing) rather than an accidental side effect of the refactor.
Also noticed the two processThumbnailForActionedImage tests were removed (that function no longer exists), but I don't see an equivalent test added for startLoadingActionables's scanning loop, which is now doing all the heavy lifting the old function did. Might be worth a test covering at least the "reaches end of folder" and "generation changes mid-load" cases, given those are exactly the kind of race conditions this refactor is trying to fix.
|
Beta commons login also gets blocked. Any new connection gets blocked now at least at my end (WiFi, mobile internet - both physical devices and emulators) and you need to raise a request to get your IPs unblocked. I'd raised a request and then got unblocked, so closed it, but this remains a concern for new contributors. I'll reopen that ticket. |
Summary
TreeMap-based pagination with a stableactionableImageslistImagefor display and selectionFixes Custom picker: Uploads image I did not pick #6289
Test plan
ImageAdapterTestpasses locallyNOTE: Please check I am unable to signup with the app as it shows your IP has been blocked.