-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fixing issue#2131: same description for all photos in multiple uploads #2132
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@domdomegg Well could you please let me know whether the changes that i had made are appropriate for the issue or not |
@sp2710 Am having a look now. Usually it can take some time for pull requests to be reviewed, especially if they have to do with uploading stuff as need to thoroughly test haven't broken anything there + actually need to upload files rather than just view some part of the app. So far your commits have been good, and your work is definitely appreciated :) A change that will be needed is formatting, we (and many other open source projects) follow the Google Java Style Guide. In some places whitespace needs to be added. First though I'll review fully to check this does fix the issue and doesn't create new ones. |
Tests performed Unit tests pass Upload multiple files works on This does solve the issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor whitespace fixes in line with GSG 4.6.2
app/src/main/java/fr/free/nrw/commons/upload/DescriptionsAdapter.java
Outdated
Show resolved
Hide resolved
app/src/main/java/fr/free/nrw/commons/upload/DescriptionsAdapter.java
Outdated
Show resolved
Hide resolved
I don't fully understand why this actually fixes the issue. I feel like this should be basically equivalent to what was already there. I'd like you either to explain what exactly wasn't working before that is now, or for someone else to review who does understand it. |
Codecov Report
@@ Coverage Diff @@
## master #2132 +/- ##
========================================
Coverage ? 5.69%
========================================
Files ? 233
Lines ? 11598
Branches ? 1076
========================================
Hits ? 660
Misses ? 10883
Partials ? 55
Continue to review full report at Codecov.
|
Actually when i put some log messages inside the description Text Changed Listener i saw that the description object for which the setDescriptionText was called was not always the same for which it was desired. I put both description and descriptions inside that method as logs and seen the logs. I saw that description object was not always reffering to the same descriptions |
Fixes #2131 Multiple Uploads: All pictures are having same description
What changes did you make and why?
Added a new setNewDescription() method to set description of the photo. Earlier when we were writing any description it was calling setDescriptionText method for all the photos in multiple uploads and all photos were having same description. Now setDescriptionText method is only called for the desired image and all images are not having the same description.
Tested on Motorola Moto G5 plus with API level 24