Integrate API for displaying featured images #1456

maskaravivek · 2018-04-19T20:56:09Z

Description

Part of task described in #324.

Tests performed

Please test on prodDebug build. Apparently, the beta site doesn't have any featured images.

Screenshots showing what changed

codecov-io · 2018-04-19T21:24:13Z

Codecov Report

Merging #1456 into master will decrease coverage by 0.03%.
The diff coverage is 2.29%.

@@            Coverage Diff            @@
##           master   #1456      +/-   ##
=========================================
- Coverage    3.21%   3.17%   -0.04%     
=========================================
  Files         132     135       +3     
  Lines        7091    7297     +206     
  Branches      682     708      +26     
=========================================
+ Hits          228     232       +4     
- Misses       6848    7050     +202     
  Partials       15      15

Impacted Files	Coverage Δ
.../fr/free/nrw/commons/di/FragmentBuilderModule.java	`0% <ø> (ø)`	⬆️
.../fr/free/nrw/commons/di/ActivityBuilderModule.java	`0% <ø> (ø)`	⬆️
.../java/fr/free/nrw/commons/utils/ContinueUtils.java	`0% <0%> (ø)`
...va/fr/free/nrw/commons/category/QueryContinue.java	`0% <0%> (ø)`
...e/nrw/commons/category/CategoryImagesActivity.java	`0% <0%> (ø)`
.../fr/free/nrw/commons/category/GridViewAdapter.java	`0% <0%> (ø)`
.../free/nrw/commons/category/CategoryImageUtils.java	`0% <0%> (ø)`
...free/nrw/commons/theme/NavigationBaseActivity.java	`22.82% <0%> (ø)`	⬆️
...w/commons/category/CategoryImagesListFragment.java	`0% <0%> (ø)`
.../nrw/commons/category/CategoryImageController.java	`0% <0%> (ø)`
... and 10 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 228aa21...689df02. Read the comment docs.

misaochan · 2018-04-30T10:12:07Z

app/build.gradle

    implementation 'com.jakewharton.rxbinding2:rxbinding-appcompat-v7:2.0.0'
    implementation 'com.jakewharton.rxbinding2:rxbinding-design:2.0.0'

+    implementation 'org.jsoup:jsoup:1.11.3'


Hi @maskaravivek , what is the reason for needing to add this library?

This library parses the artist name. The API returns the artist as a HTML string.

misaochan · 2018-04-30T11:39:11Z

Thanks for this PR, @maskaravivek ! It works well for me in general, great job. A few issues:

It only loads 10 images, and the same images all the time. This will get boring very quickly for the user, and they will likely not bother to give this feature more than one cursory glance. I think that for our minimum viable product, we can load as many images as the user's "Set Recent Upload Limit" setting states? I know that that is meant for contributions, but IMO realistically if they can load 100 of their own images, they can load 100 of other people's images without any issue. Ideally we should randomize the pictures shown or dynamically load more after scrolling down, but that can be an enhancement for the future.
In the media details view (accessed when image is tapped), the author field says "Featured image author user name goes here". :)
The toast "Error! URL not found" is displayed when the media details view loads, even though the image and most of the fields are correct
After viewing a few images, I crashed with the error:

04-30 21:31:33.051 23958-23958/fr.free.nrw.commons.debug E/AndroidRuntime: FATAL EXCEPTION: main
    Process: fr.free.nrw.commons.debug, PID: 23958
    java.util.ConcurrentModificationException
        at java.util.HashMap$HashIterator.nextEntry(HashMap.java:851)
        at java.util.HashMap$KeyIterator.next(HashMap.java:885)
        at fr.free.nrw.commons.Media.setDescriptions(Media.java:359)
        at fr.free.nrw.commons.MediaDataExtractor.fill(MediaDataExtractor.java:303)
        at fr.free.nrw.commons.media.MediaDetailFragment$3.onPostExecute(MediaDetailFragment.java:256)
        at fr.free.nrw.commons.media.MediaDetailFragment$3.onPostExecute(MediaDetailFragment.java:229)
        at android.os.AsyncTask.finish(AsyncTask.java:667)
        at android.os.AsyncTask.-wrap1(AsyncTask.java)
        at android.os.AsyncTask$InternalHandler.handleMessage(AsyncTask.java:684)
        at android.os.Handler.dispatchMessage(Handler.java:102)
        at android.os.Looper.loop(Looper.java:154)
        at android.app.ActivityThread.main(ActivityThread.java:6119)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:886)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:776)

…y images

maskaravivek · 2018-05-01T12:48:51Z

@misaochan Thanks for reviewing and testing the PR.

I have fixed most of the issues reported by you.

"Error! URL not found" still isn't fixed since the MediaDetailFragment fetches the media details again from the server and is not able to fetch the license information. Thus the toast is displayed. Later on, we can pick refactoring of this fragment so that the data is fetched again only if there are missing data.
Have also implemented pagination here so that the user can scroll through as many images as he wishes. We can introduce randomization later on.
Have completely refactored the code so that it can be reused for displaying category images.

@ujjwalagrawal17 @neslihanturan Just adding you guys to the conversation so that you don't have to rewrite the whole thing when/if browsing images of a particular category are picked up.

Here's how to use it:

CategoryImagesActivity.startYourself(this, <page_title>, <category_name>);

misaochan · 2018-05-01T14:49:19Z

Hi @maskaravivek ,

The pagination works very well! :) Nice work.
The author field seems to disappear completely now when I load an image's details. Sorry to nitpick, but I think we really need to have an author field if we are displaying other people's images, especially as some of them have licenses that require attribution.
Just noticed this: Pressing the "back" button on the device closes the Commons app entirely at the moment, whereas it should be bringing users back to Contributions activity. (Actually, even going to "Home" from the nav drawer on Featured activity closes the app, which isn't intended)
Should we just remove the "Error! URL not found" toast in that case, as a temporary measure, and add a TODO? It doesn't appear to be terribly informative to the user, at any rate.

misaochan · 2018-05-01T15:01:56Z

app/src/main/java/fr/free/nrw/commons/category/CategoryImageUtils.java

+
+    private static Date getDateCreated(Node document) {
+        String dateTime = getMetaDataValue(document, "DateTime");
+        Timber.d("Date time is %s", dateTime);


Could you please make this verbose at least? It spams my entire logcat screen each time any page loads. :)

Will remove it altogether. Was just for debugging purposes :D

maskaravivek · 2018-05-01T21:07:48Z

@misaochan I have fixed all the issues reported by you. Thanks again for helping with testing. :)

misaochan · 2018-05-02T08:19:02Z

@maskaravivek Does the author field in media details view work for you currently?

maskaravivek · 2018-05-02T19:19:55Z

@misaochan Yes, it works for me. Moreover, now even the featured images list shows the author name(As designed by @neslihanturan)

Attached screenshots.

misaochan · 2018-05-03T07:23:33Z

Thanks @maskaravivek , works well for me after restarting. :) Great job, I think users will really enjoy browsing through them!

There is just an issue with tests, causing Travis to fail. Once this is fixed, I think we can go right ahead with the merge.

13 PMD rule violations were found. See the report at: file:///home/travis/build/commons-app/apps-android-commons/app/build/reports/pmd/pmd.html
:app:kaptGenerateStubsBetaDebugUnitTestKotline: /home/travis/build/commons-app/apps-android-commons/app/src/test/kotlin/fr/free/nrw/commons/mwapi/ApacheHttpClientMediaWikiApiTest.kt: (34, 155): No value passed for parameter 'p3'
e: /home/travis/build/commons-app/apps-android-commons/app/src/test/kotlin/fr/free/nrw/commons/mwapi/ApacheHttpClientMediaWikiApiTest.kt: (34, 155): No value passed for parameter 'p4'
 FAILED
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':app:kaptGenerateStubsBetaDebugUnitTestKotlin'.

maskaravivek · 2018-05-03T08:47:42Z

@misaochan Have fixed the travis build. :)

misaochan · 2018-05-03T09:31:53Z

The build was re-run 5 min ago, but it seems to still fail:

13 PMD rule violations were found. See the report at: file:///home/travis/build/commons-app/apps-android-commons/app/build/reports/pmd/pmd.html
:app:kaptGenerateStubsBetaDebugUnitTestKotlin
:app:kaptBetaDebugUnitTestKotlin
:app:compileBetaDebugUnitTestKotlin
:app:preBetaDebugUnitTestBuild UP-TO-DATE
:app:javaPreCompileBetaDebugUnitTest
:app:compileBetaDebugUnitTestJavaWithJavac NO-SOURCE
:app:mockableAndroidJar
:app:processBetaDebugJavaRes
:app:processBetaDebugUnitTestJavaRes NO-SOURCE
:app:testBetaDebugUnitTest
fr.free.nrw.commons.mwapi.ApacheHttpClientMediaWikiApiTest > editToken FAILED
    kotlin.UninitializedPropertyAccessException at ApacheHttpClientMediaWikiApiTest.kt:36
fr.free.nrw.commons.mwapi.ApacheHttpClientMediaWikiApiTest > simpleLogin FAILED
    kotlin.UninitializedPropertyAccessException at ApacheHttpClientMediaWikiApiTest.kt:36
fr.free.nrw.commons.mwapi.ApacheHttpClientMediaWikiApiTest > simpleLoginWithWrongPassword FAILED
    kotlin.UninitializedPropertyAccessException at ApacheHttpClientMediaWikiApiTest.kt:36
fr.free.nrw.commons.mwapi.ApacheHttpClientMediaWikiApiTest > validateLoginForLoggedInUser FAILED
    kotlin.UninitializedPropertyAccessException at ApacheHttpClientMediaWikiApiTest.kt:36
fr.free.nrw.commons.mwapi.ApacheHttpClientMediaWikiApiTest > twoFactorLogin FAILED
    kotlin.UninitializedPropertyAccessException at ApacheHttpClientMediaWikiApiTest.kt:36
fr.free.nrw.commons.mwapi.ApacheHttpClientMediaWikiApiTest > validateLoginForLoggedOutUser FAILED
    kotlin.UninitializedPropertyAccessException at ApacheHttpClientMediaWikiApiTest.kt:36
fr.free.nrw.commons.mwapi.ApacheHttpClientMediaWikiApiTest > getUploadCount FAILED
    kotlin.UninitializedPropertyAccessException at ApacheHttpClientMediaWikiApiTest.kt:36
fr.free.nrw.commons.mwapi.ApacheHttpClientMediaWikiApiTest > fileExistsWithName_FileNotFound FAILED
    kotlin.UninitializedPropertyAccessException at ApacheHttpClientMediaWikiApiTest.kt:36
fr.free.nrw.commons.mwapi.ApacheHttpClientMediaWikiApiTest > authCookiesAreHandled FAILED
    kotlin.UninitializedPropertyAccessException at ApacheHttpClientMediaWikiApiTest.kt:36
112 tests completed, 9 failed
:app:testBetaDebugUnitTest FAILED
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':app:testBetaDebugUnitTest'.
> There were failing tests. See the report at: file:///home/travis/build/commons-app/apps-android-commons/app/build/reports/tests/testBetaDebugUnitTest/index.html
* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output.
* Get more help at https://help.gradle.org
BUILD FAILED in 6m 28s
76 actionable tasks: 76 executed

neslihanturan · 2018-05-03T10:43:12Z

@maskaravivek , it gives me this screen in a loop:

maskaravivek · 2018-05-03T10:48:46Z

@neslihanturan Can you post the error logs from logcat. Are you testing on beta or prod?

maskaravivek · 2018-05-03T11:50:23Z

@misaochan Finally the build succeeds. Sorry for having you test it multiple times.

misaochan · 2018-05-03T11:56:02Z

Thanks @maskaravivek . All good on my end - we can merge after @neslihanturan 's issue is sorted. :)

neslihanturan · 2018-05-04T07:45:01Z

Sorry for not adding logs earlier, here:
Error occurred while loading featured images java.lang.NullPointerException: Attempt to invoke virtual method 'void fr.free.nrw.commons.category.GridViewAdapter.addItems(java.util.List)' on a null object reference at fr.free.nrw.commons.category.CategoryImagesListFragment.lambda$fetchMoreImages$4$CategoryImagesListFragment(CategoryImagesListFragment.java:127) at fr.free.nrw.commons.category.CategoryImagesListFragment$$Lambda$4.accept(Unknown Source) at io.reactivex.internal.observers.LambdaObserver.onNext(LambdaObserver.java:60) at io.reactivex.internal.operators.observable.ObservableObserveOn$ObserveOnObserver.drainNormal(ObservableObserveOn.java:200) at io.reactivex.internal.operators.observable.ObservableObserveOn$ObserveOnObserver.run(ObservableObserveOn.java:252) at io.reactivex.android.schedulers.HandlerScheduler$ScheduledRunnable.run(HandlerScheduler.java:109) at android.os.Handler.handleCallback(Handler.java:739) at android.os.Handler.dispatchMessage(Handler.java:95) at android.os.Looper.loop(Looper.java:148) at android.app.ActivityThread.main(ActivityThread.java:5417) at java.lang.reflect.Method.invoke(Native Method) at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:726) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:616)

There is a possibility that it happens because of Turkey ban, but in that case, we should be using an information window instead of loop.

maskaravivek · 2018-05-05T08:54:23Z

@neslihanturan I have added checks to gracefully handle the error cases.

misaochan · 2018-05-07T06:54:38Z

Thanks for adding the Javadocs! :) @neslihanturan I would like to merge this once you can confirm that the error handling works for you.

neslihanturan · 2018-05-07T08:09:18Z

Works as expected, thanks you to include a great feature to our project @maskaravivek :)

* Integrate API for displaying featured images * Add pagination and refactor code so that it can be reused for category images * Add license info to the images * Fix author view * Remove unused values * Fix minor issues with featured images * Fix null license url issue * Remove some log lines * Fix back navigation issue * fix tests * fix test inits * Gracefully handling various error situations * Added java docs

…rby (#1495) * Localisation updates from https://translatewiki.net. * Integrate API for displaying featured images (#1456) * Integrate API for displaying featured images * Add pagination and refactor code so that it can be reused for category images * Add license info to the images * Fix author view * Remove unused values * Fix minor issues with featured images * Fix null license url issue * Remove some log lines * Fix back navigation issue * fix tests * fix test inits * Gracefully handling various error situations * Added java docs * Update pull_request_template.md (#1476) * Update pull_request_template.md * Remove Javadocs mention * Added required/optional notes * resolves #1464 : MediaDataExtractor is making inefficient (redundant) server calls (#1496) * Open map of place where picture was taken (#1360) * Intent to map added * Merge conflicts resolved * Added the functionality to hide FAB incase of null coordinate * Merge Conflict resolved * Improve pr quality * Improve Quality * Added nested FAB animations * Nested FAB implemented * Improve Quality * Added up arrow * Javadocs Added * Add nearby tutorial (#1467) * Add dependency for MaterialShowcase * Add actionview class to get a reference to material showcase * Create a NearbyMaterialShowcaseSequence class * Apply sequence steps * Add first three steps of nearby showcase * Add sequence id constants to make sure they will be displayed only once * Add last step of sequence to explain plus fab * Create an object to prevent customize all sequences every time * Fix typo * Code cleanup * Add strings to strings.xml * Code cleanup * Revert irrelevant change * Revert irrelevant change * Remove showcaseview for recenter button * Use single showcaseView instead of sequence * Add single showcase view insted of sequence to be able to edit text style * Make sure it will be displayed only once * Cleanup * Update strings * Change dismiss text style * CONTRIBUTING: fix formatting of the gist of the guidelines (#1453) * CONTRIBUTING: fix formatting of the gist of the guidelines First level headings for a gist seems to be overkill. So, replace first level headings with an ordered-list which sounds more meaningful. * CONTRIBUTING: specify clearly that 'blame' is a feature of "Git" The contributing file specifies about the ability to know who wrote something without the need of @author javadoc tags but incorrectly attributes the feature to GitHub. Correctly attribute the feature to where it belongs, Git, and specify the name of the feature to help users easily take advantage of it. * Feature/switch to butterknife (#1494) * Implemented butterknife in MediaDetailFragment [issue #1491] * Implemented butterknife in MediaDetailPagerFragment [[issue #1491]] * post merge upstream master wip [[issue #1491]] * Localisation updates from https://translatewiki.net. * Bug fix #1504 (#1506) * Bug fix #1504 * Filtered messages with ConnectException [issue #1504] * A generalised message for exceptions in Nearby Activity [issue #1504] * Localisation updates from https://translatewiki.net. * Fix security exception crash while accessing network location provider (#1498) * Fix security exception crash while accessing network location provider * Added java docs * Localisation updates from https://translatewiki.net. * Log P18 edits to wikidata corresponding wikidata entity on uploading a nearby image * Added java docs * Fix test build * Refresh nearby * Refresh nearby list on successful edit * Java docs * Make authenticated wikidata edits * Updated toast message to show entity name that was edited

misaochan reviewed Apr 30, 2018

View reviewed changes

maskaravivek force-pushed the featuredImageImpl branch from fc9dffe to d8c7ba2 Compare May 1, 2018 12:41

commons-app deleted a comment May 1, 2018

maskara added 5 commits May 1, 2018 18:15

Integrate API for displaying featured images

c02141e

Add pagination and refactor code so that it can be reused for categor…

790cf96

…y images

Add license info to the images

31abde9

Fix author view

679602e

Remove unused values

d8c7ba2

misaochan reviewed May 1, 2018

View reviewed changes

maskara added 3 commits May 2, 2018 02:07

Fix minor issues with featured images

afd5bc5

Fix null license url issue

3ef7c40

Remove some log lines

14b9353

Fix back navigation issue

e23006b

commons-app deleted a comment May 3, 2018

fix tests

8106d3c

commons-app deleted a comment May 3, 2018

fix test inits

09fe2fe

misaochan approved these changes May 3, 2018

View reviewed changes

misaochan mentioned this pull request May 4, 2018

Items needed for v2.8 release #1478

Closed

16 tasks

commons-app deleted a comment May 4, 2018

maskaravivek force-pushed the featuredImageImpl branch from a9e3908 to 3bdaf9c Compare May 5, 2018 08:52

commons-app deleted a comment May 5, 2018

Gracefully handling various error situations

3bdaf9c

maskaravivek force-pushed the featuredImageImpl branch from 31d980c to 5e33d3f Compare May 6, 2018 19:55

maskara and others added 2 commits May 7, 2018 01:29

Added java docs

5e33d3f

Merge branch 'master' into featuredImageImpl

689df02

neslihanturan merged commit 9845a62 into commons-app:master May 7, 2018

maskaravivek deleted the featuredImageImpl branch September 12, 2018 20:25

Integrate API for displaying featured images #1456

Integrate API for displaying featured images #1456

Uh oh!

Conversation

maskaravivek commented Apr 19, 2018

Description

Tests performed

Screenshots showing what changed

Uh oh!

codecov-io commented Apr 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

misaochan Apr 30, 2018

Choose a reason for hiding this comment

Uh oh!

maskaravivek May 6, 2018

Choose a reason for hiding this comment

Uh oh!

misaochan commented Apr 30, 2018

Uh oh!

maskaravivek commented May 1, 2018

Uh oh!

misaochan commented May 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

misaochan May 1, 2018

Choose a reason for hiding this comment

Uh oh!

maskaravivek May 1, 2018

Choose a reason for hiding this comment

Uh oh!

maskaravivek commented May 1, 2018

Uh oh!

misaochan commented May 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maskaravivek commented May 2, 2018

Uh oh!

misaochan commented May 3, 2018

Uh oh!

maskaravivek commented May 3, 2018

Uh oh!

misaochan commented May 3, 2018

Uh oh!

neslihanturan commented May 3, 2018

Uh oh!

maskaravivek commented May 3, 2018

Uh oh!

maskaravivek commented May 3, 2018

Uh oh!

misaochan commented May 3, 2018

Uh oh!

neslihanturan commented May 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maskaravivek commented May 5, 2018

Uh oh!

misaochan commented May 7, 2018

Uh oh!

neslihanturan commented May 7, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-io commented Apr 19, 2018 •

edited

Loading

misaochan commented May 1, 2018 •

edited

Loading

misaochan commented May 2, 2018 •

edited

Loading

neslihanturan commented May 4, 2018 •

edited

Loading