Refactor uploads #2981

ashishkumar468 · 2019-06-02T16:10:04Z

Refactor Upload Activities
Optimised UploadActivity, added fragments for various stages of upload, switched to MVP
Fixes #2886
To be tested
Uploads:

Camera upload
Camera nearby upload with successful wikidata edit
Single gallery upload
Google Photos upload
Multiple gallery upload
Gallery upload with successful wikidata edit - upload is successful but Wikidata edit is not. The toast is not even triggered.
Image taken date is correct
Suffixes are added if same title is used

In the current ShareActivity:

Tooltips for title, desc, license fields - tooltip not present for title
Language selector for desc
Prefilling of title and desc when upload initiated via Nearby
License selector
Link to license and Commons policies - works, but crashes app
Zoom in (and back out)
Open location of image on maps
Copy previous title/desc

Category suggestions/selection:

When search text field is empty, display:-Recent categories - does not work
Location-based category suggestions (if image has geolocation) - does not work
Title-based category suggestions - does not work
Wikidata item category (if image is uploaded via Nearby and if that Wikidata item has a category) - does not work
When user types in search text field:

Check for categories shortly after typing stops, repeatedly

Display exact matches at the top
Categories selected via both of these suggestion types need to be added to the list of category selections
BUG: Categories are not saved with multiple uploads, the final product in Commons is missing categories

Warnings:

If image is too dark
If image already exists in Commons (via hash check)
If user tries to submit empty title (do not allow to proceed)
If user does not select any categories

Image templates:

Ensure the same templates are added (not sure if this will be an issue if backend has not changed, but make sure you check)

* Fix duplicate param information (#2515) * Bug fix issue #2476 (#2526) * Added wikidataEntityID in all db versions, handled db.execSql via method runQuery * Versioning and changelog for v2.10.2 (#2531) * Update changelog.md * Versioning for v2.10.2 * Update changelog.md * Bugfix/issue 2580 (#2584) * Corrected string placedholders in certain string files * Corrected string placedholders in certain string files[Bug fix #2580] * Bug Fix #2585 (#2647) * Bug Fix #2585 * Added null checks on view in SearchImageFragment when updating views from external sources * Disposed the disposables in SearchActivity and SearchImageFragment when no longer in active lifecycle * use FragmentUtils to verify fragment active state * Bug Fix issue #2648 (#2678) * Bug Fix issue #2648 * Handled external storage permission before file download * * Removed redudant check for permission in MediaDetailPagerFragment (Dexter already does that) * Removed duplicate code in PermissionUtil$checkPermissionsAndPerformAction, used the existing function with conditional extra parameters * string name typo correction * BugFix issue #2652 (#2706) * Addded null check on bookmark before operating on it * BugFix issue #2711 (#2712) * Added null checks in OkHttpJsonApiClient$searchImages MwQueryResponse * BugFix #2718 (#2719) * Handled null auth cookies * Fix #2791: NPE when nominating for deletion and leaving screen (#2792) * Bug Fix issue #2789 (#2790) * Handled Illegal State Exception for non existent appropriate view parents in ViewUtils$showShortSnackbar * BugFix #2720 (#2831) BugFix deprecated licenes #2720 * ui fixes, wip, upload * *Issue #2886, BugFix #2832[wip] * updated UploadActivity code * modified ui * Updated UploadPresenterTest * * updated interfaces names to follow names suffixed with Contract * added test cases * card view elevation * view pager disabled swipe * bug fix, duplicate image * used existing non-swipable view pager * Avoid image view resize with keyboard, added adjustPan and stateVisible as softinputMode for UploadActivity * retain UploadBaseFragment instances on orientation changes * * Added test cases for UploadMediaPresenter * Injected io and main thread schedulers * categories presenter test cased wip * Added CategoriesPresenter test * * Added the logic to show open map (with to be uploaded image's coordinates while uploading image) * codacy suggested changes * added java docs * Added travis_wait fot android-wait-for-emulator * ranamed interface onResponseCallback to Callback * * Added api to delete picture in UploadModel * cleanUp in UploadModel. once upload has been initiated * Removed unused methods from UploadModel and the corresponding test class * * Added tests for UploadPresenter * Travis suggested changes * Addded copy previous title and description * * Made the upload add descriptions visible when keyboard visible * add description request focus only when user manually requests it * Added JavaDocs, review suggested changes * Fix dagger injection * use DialogUtil to show info in descriptions * use activity context for DialogUtil * Minor changes

pullrequest

✅ A review job has been created and sent to the PullRequest network.

Check the status or cancel PullRequest code review here.

ashishkumar468 · 2019-06-02T16:15:08Z

Attaching screenshots for the test performed (if applicable)
Dark Image Warning

No Categories Selected Warning

Next Button Disabled in case of empty caption

Duplicate Image Warning

Tooltips for title and description

Attaching screenshot for successful wikidata edit via nearby

ashishkumar468 · 2019-06-02T16:24:57Z

@maskaravivek Can you ebaborate this one Ensure the same templates are added (not sure if this will be an issue if backend has not changed, but make sure you check)

vanshikaarora · 2019-06-03T04:41:48Z

@ashishkumar468 I was checking for uploads for images. And currently I think we couldn't merge this branch into master. I have mostly focused on changing title to captions mostly the UI part. The image title for uploading the image is not the captions nor the filename. I'll take the task of First Caption as filename and sending through API. Till then we can focus on other Issues with this PR:)

misaochan · 2019-06-03T16:12:56Z

Just spoke to @ashishkumar468 and this is ready for testing now. I will start tomorrow - as it is a HUGE change, help with testing is always appreciated. :)

misaochan

Preliminary review: Looks good to me for the most part, thanks @ashishkumar468 ! :)

Javadocs are mostly present but missing from a few classes.
Code style needs to be fixed
Codecov bot does not seem to have made the usual comment re: code coverage. Not sure why that is. :/

I will proceed with the manual testing now

app/src/main/java/fr/free/nrw/commons/upload/ThumbnailsAdapter.java

app/src/main/java/fr/free/nrw/commons/upload/UploadModel.java

app/src/main/java/fr/free/nrw/commons/repository/UploadRemoteDataSource.java

app/src/main/java/fr/free/nrw/commons/repository/UploadRepository.java

app/src/main/java/fr/free/nrw/commons/upload/categories/CategoriesPresenter.java

app/src/main/java/fr/free/nrw/commons/upload/license/MediaLicensePresenter.java

pullrequest

	1 Message
⚠️	Due to its size, this pull request will likely have a little longer turnaround time and will probably require multiple passes from our reviewers.

misaochan

Pending fixes mentioned in bold in opening comment edit.

ashishkumar468 · 2019-06-06T05:58:10Z

Thanks for testing this out @misaochan. I will make the necessary changes and let you know :-)

* merged with master * BugFix IllegalStateException * setRetainState(true), not required with FragmentStatePagerAdapter * Increase the ViewPager's Offscreen Limit, we want all the fragments to be active * BugFix, clear selected categoris for previous upload session * Clear Selected Categories * Addded JavaDocs for CategoriesModel * Code Formatting in app/src/main/java/fr/free/nrw/commons/upload/UploadModel.java * Added class level JavaDoc UploadRemoteDataSource * Added class level JavaDoc for UploadRepository * Added JavaDocs for ThumbnailsAdapter * Added JavaDocs for MediaLicensePresenter, CategoriesPresenter * Removed null check on category query * Show default catgeories based on image title and gps location when category text empty * Allow search for empty category search * Attached image scale listener to upload media image * Bug fix, reduced the add description edit text clickable bound

maskaravivek · 2019-06-09T18:01:48Z

Just tested wikidata edits. They are working fine for me.

codecov-io · 2019-06-09T18:05:22Z

Codecov Report

Merging #2981 into master will increase coverage by 0.69%.
The diff coverage is 16.15%.

@@            Coverage Diff            @@
##           master   #2981      +/-   ##
=========================================
+ Coverage    3.74%   4.43%   +0.69%     
=========================================
  Files         249     259      +10     
  Lines       12102   12261     +159     
  Branches     1070    1051      -19     
=========================================
+ Hits          453     544      +91     
- Misses      11615   11678      +63     
- Partials       34      39       +5

Impacted Files	Coverage Δ
...fr/free/nrw/commons/media/MediaDetailFragment.java	`0% <ø> (ø)`	⬆️
...va/fr/free/nrw/commons/explore/SearchActivity.java	`0% <ø> (ø)`	⬆️
.../fr/free/nrw/commons/di/FragmentBuilderModule.java	`0% <ø> (ø)`	⬆️
...a/fr/free/nrw/commons/upload/UploadController.java	`39.08% <ø> (ø)`	⬆️
app/src/main/java/fr/free/nrw/commons/Utils.java	`18.03% <ø> (ø)`	⬆️
.../java/fr/free/nrw/commons/upload/GPSExtractor.java	`1.75% <ø> (ø)`	⬆️
...ree/nrw/commons/contributions/ContributionDao.java	`0% <ø> (ø)`	⬆️
.../fr/free/nrw/commons/upload/ThumbnailsAdapter.java	`0% <0%> (ø)`
...fr/free/nrw/commons/upload/UploadBaseFragment.java	`0% <0%> (ø)`
.../fr/free/nrw/commons/category/CategoriesModel.java	`0% <0%> (ø)`	⬆️
... and 37 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6619ccf...bf5bb76. Read the comment docs.

* merged with master * BugFix IllegalStateException * setRetainState(true), not required with FragmentStatePagerAdapter * Increase the ViewPager's Offscreen Limit, we want all the fragments to be active * BugFix, clear selected categoris for previous upload session * Clear Selected Categories * Addded JavaDocs for CategoriesModel * Code Formatting in app/src/main/java/fr/free/nrw/commons/upload/UploadModel.java * Added class level JavaDoc UploadRemoteDataSource * Added class level JavaDoc for UploadRepository * Added JavaDocs for ThumbnailsAdapter * Added JavaDocs for MediaLicensePresenter, CategoriesPresenter * Removed null check on category query * Show default catgeories based on image title and gps location when category text empty * Allow search for empty category search * Attached image scale listener to upload media image * Bug fix, reduced the add description edit text clickable bound * Added tooltip in Title in UploadMediaFragment * BugFix recent categories * Updated test methods

maskaravivek · 2019-06-10T05:45:11Z

Just checked the following:

title based suggestions
tooltip for title

These things are now working for me.

There's still a memory leak happening which needs to be fixed (This is different from what I fixed in #3001 ).

* merged with master * BugFix IllegalStateException * setRetainState(true), not required with FragmentStatePagerAdapter * Increase the ViewPager's Offscreen Limit, we want all the fragments to be active * BugFix, clear selected categoris for previous upload session * Clear Selected Categories * Addded JavaDocs for CategoriesModel * Code Formatting in app/src/main/java/fr/free/nrw/commons/upload/UploadModel.java * Added class level JavaDoc UploadRemoteDataSource * Added class level JavaDoc for UploadRepository * Added JavaDocs for ThumbnailsAdapter * Added JavaDocs for MediaLicensePresenter, CategoriesPresenter * Removed null check on category query * Show default catgeories based on image title and gps location when category text empty * Allow search for empty category search * Attached image scale listener to upload media image * Bug fix, reduced the add description edit text clickable bound * Added tooltip in Title in UploadMediaFragment * BugFix recent categories * Updated test methods * Avoid memory leak, free the adpater in MediaLicenseFragment.onDestroyView

ashishkumar468 · 2019-06-11T20:52:14Z

@maskaravivek Thanks for pointing out the memory leak. I have added a fix for it as well. @misaochan @neslihanturan, The PR is up for testing :-)

maskaravivek

Looks good to me. Am happy to merge once @misaochan's review is also completed. :)

neslihanturan · 2019-06-12T10:18:20Z

Hi @ashishkumar0207 , I get this error on your branch just after choosing and image from gallery. I am not sure if this is relevant with your changes though. On beta cluster, API 26 emulator.

 Caused by: java.lang.IllegalStateException: The application's PagerAdapter changed the adapter's contents without calling PagerAdapter#notifyDataSetChanged! Expected adapter item count: 0, found: 3 Pager id: fr.free.nrw.commons.beta:id/vp_upload Pager class: class fr.free.nrw.commons.contributions.UnswipableViewPager Problematic adapter: class fr.free.nrw.commons.upload.UploadActivity$UploadImageAdapter
        at androidx.viewpager.widget.ViewPager.populate(ViewPager.java:1143)
        at androidx.viewpager.widget.ViewPager.populate(ViewPager.java:1092)
        at androidx.viewpager.widget.ViewPager.setOffscreenPageLimit(ViewPager.java:856)
        at fr.free.nrw.commons.upload.UploadActivity$UploadImageAdapter.getCount(UploadActivity.java:434)
        at androidx.viewpager.widget.ViewPager.dataSetChanged(ViewPager.java:1023)
        at androidx.viewpager.widget.ViewPager$PagerObserver.onChanged(ViewPager.java:3097)
        at androidx.viewpager.widget.PagerAdapter.notifyDataSetChanged(PagerAdapter.java:291)
        at fr.free.nrw.commons.upload.UploadActivity$UploadImageAdapter.setFragments(UploadActivity.java:425)
        at fr.free.nrw.commons.upload.UploadActivity.receiveSharedItems(UploadActivity.java:343)
        at fr.free.nrw.commons.upload.UploadActivity.lambda$uFmJNhak7opat5jgMvaezcNPkNM(Unknown Source:0)
        at fr.free.nrw.commons.upload.-$$Lambda$UploadActivity$uFmJNhak7opat5jgMvaezcNPkNM.run(Unknown Source:2)
        at fr.free.nrw.commons.utils.PermissionUtils$1.onPermissionGranted(PermissionUtils.java:114)
        at com.karumi.dexter.MultiplePermissionsListenerToPermissionListenerAdapter.onPermissionsChecked(Unknown Source:35)

ashishkumar468 · 2019-06-12T10:35:46Z

@neslihanturan Thanks for helping with the logs, I have raise a PR fixing the Issue. please have a look #3012

* merged with master * BugFix IllegalStateException * setRetainState(true), not required with FragmentStatePagerAdapter * Increase the ViewPager's Offscreen Limit, we want all the fragments to be active * BugFix, clear selected categoris for previous upload session * Clear Selected Categories * Addded JavaDocs for CategoriesModel * Code Formatting in app/src/main/java/fr/free/nrw/commons/upload/UploadModel.java * Added class level JavaDoc UploadRemoteDataSource * Added class level JavaDoc for UploadRepository * Added JavaDocs for ThumbnailsAdapter * Added JavaDocs for MediaLicensePresenter, CategoriesPresenter * Removed null check on category query * Show default catgeories based on image title and gps location when category text empty * Allow search for empty category search * Attached image scale listener to upload media image * Bug fix, reduced the add description edit text clickable bound * Added tooltip in Title in UploadMediaFragment * BugFix recent categories * Updated test methods * Avoid memory leak, free the adpater in MediaLicenseFragment.onDestroyView * BugFix Illegal State Exception in ViewpPagerAdapter * Remove irrelevant comment

neslihanturan · 2019-06-12T13:44:19Z

~~Closing this one since same changes are merged on another pull request.~~

Sorry my mistake, it is not solved.

neslihanturan

Sorry I forgot approving it earlier:)

ashishkumar468 added 2 commits May 28, 2019 15:51

Bug fix, reduced the add description edit text clickable bound (#2973)

b53803b

pullrequest bot reviewed Jun 2, 2019

View reviewed changes

ashishkumar468 requested review from maskaravivek and neslihanturan June 2, 2019 16:12

misaochan changed the title ~~Refactor uploads [WIP]~~ Refactor uploads Jun 3, 2019

misaochan reviewed Jun 4, 2019

View reviewed changes

pullrequest bot reviewed Jun 5, 2019

View reviewed changes

misaochan requested changes Jun 5, 2019

View reviewed changes

maskaravivek and others added 2 commits June 9, 2019 17:34

Merge branch 'master' into refactor_uploads

e05b337

maskaravivek and others added 2 commits June 10, 2019 00:36

Fix memory leak (#3001)

7dd42c3

maskaravivek approved these changes Jun 12, 2019

View reviewed changes

ashishkumar468 added 2 commits June 12, 2019 14:32

merge conflict with strings (#3016)

95f272a

neslihanturan closed this Jun 12, 2019

neslihanturan reopened this Jun 12, 2019

Merge branch 'master' into refactor_uploads

bf5bb76

neslihanturan approved these changes Jun 13, 2019

View reviewed changes

maskaravivek merged commit 7a5dc77 into master Jun 13, 2019

ashishkumar468 deleted the refactor_uploads branch June 19, 2019 09:54

Refactor uploads #2981

Refactor uploads #2981

Uh oh!

Conversation

ashishkumar468 commented Jun 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pullrequest bot left a comment

Choose a reason for hiding this comment

Uh oh!

ashishkumar468 commented Jun 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ashishkumar468 commented Jun 2, 2019

Uh oh!

vanshikaarora commented Jun 3, 2019

Uh oh!

misaochan commented Jun 3, 2019

Uh oh!

misaochan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pullrequest bot left a comment

Choose a reason for hiding this comment

Uh oh!

misaochan left a comment

Choose a reason for hiding this comment

Uh oh!

ashishkumar468 commented Jun 6, 2019

Uh oh!

maskaravivek commented Jun 9, 2019

Uh oh!

codecov-io commented Jun 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

maskaravivek commented Jun 10, 2019

Uh oh!

ashishkumar468 commented Jun 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maskaravivek left a comment

Choose a reason for hiding this comment

Uh oh!

neslihanturan commented Jun 12, 2019

Uh oh!

ashishkumar468 commented Jun 12, 2019

Uh oh!

neslihanturan commented Jun 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

neslihanturan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ashishkumar468 commented Jun 2, 2019 •

edited

Loading

ashishkumar468 commented Jun 2, 2019 •

edited

Loading

codecov-io commented Jun 9, 2019 •

edited

Loading

ashishkumar468 commented Jun 11, 2019 •

edited

Loading

neslihanturan commented Jun 12, 2019 •

edited

Loading