Feedback Module: Add reverts rate parameter #1649

tanvidadu · 2018-06-21T13:19:19Z

Added Reverts Parameter to FeedBack Module

Fixes #85

Description (required)

Refactored mwApi to fetch count of images reverted
Refactored LevelController and Achievements to add the number of reverts as a parameter.

Tests performed (required)

Tested on BetaDebug and ProdDebug

Screenshots showing what changed (optional)

{Only for user interface changes, otherwise remove this section. See how to take a screenshot}

Note: Please ensure that you have read CONTRIBUTING.md if this is your first pull request.

codecov-io · 2018-06-21T13:45:05Z

Codecov Report

Merging #1649 into feedback-gamification will decrease coverage by 0.03%.
The diff coverage is 0%.

@@                   Coverage Diff                    @@
##           feedback-gamification   #1649      +/-   ##
========================================================
- Coverage                   3.04%   3.01%   -0.04%     
========================================================
  Files                        140     140              
  Lines                       7683    7761      +78     
  Branches                     720     721       +1     
========================================================
  Hits                         234     234              
- Misses                      7434    7512      +78     
  Partials                      15      15

Impacted Files	Coverage Δ
...nrw/commons/achievements/AchievementsActivity.java	`0% <0%> (ø)`	⬆️
...fr/free/nrw/commons/achievements/Achievements.java	`0% <0%> (ø)`	⬆️
...n/java/fr/free/nrw/commons/auth/LoginActivity.java	`0% <0%> (ø)`	⬆️
...free/nrw/commons/achievements/LevelController.java	`0% <0%> (ø)`	⬆️
...rw/commons/mwapi/ApacheHttpClientMediaWikiApi.java	`4.98% <0%> (-0.25%)`	⬇️
.../java/fr/free/nrw/commons/auth/SessionManager.java	`15.38% <0%> (-1.29%)`	⬇️

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 bd1555f...ad46087. Read the comment docs.

tanvidadu · 2018-06-21T16:34:33Z

@misaochan @maskaravivek pr is ready for review!

misaochan · 2018-06-23T08:20:48Z

Hi @tanvidadu , thanks for the PR. Looks good to me in general, but unfortunately I can't actually test this until Vivek fixes the API so it can retrieve the deletion rate for my account. So we will have to wait for that before we merge, I think.

Are you able to carry on working on the quiz/tutorial in the meantime?

tanvidadu · 2018-06-24T07:31:54Z

Yes! since both are separate modules I will continue with quiz/tutorial :)

misaochan · 2018-07-01T09:06:15Z

Just tested this, works perfectly for me! Great job, @tanvidadu ! :) (And also, this tells me that I need to work on reducing my deletion percentage, lol).

Just one final suggestion (and then I think we can merge and wrap up Phase 1 of your project, if @maskaravivek agrees) - what do you think about having 'i' icons next to each parameter for a brief description? This will allow us to keep the main text simple while explaining the statistics more clearly to users.

My thoughts on how the final texts could look like, with information text in italics:

Images uploaded (The number of images you have uploaded to Commons, via any upload software)
Images not deleted (The percentage of images you have uploaded to Commons that were not deleted)
Images used (The number of images you have uploaded to Commons that were used in Wikimedia articles)

misaochan · 2018-07-01T09:13:49Z

Oops, there also seems to be a bug with the levelling, when I tested with my secondary account (Misaochan2). According to the stats, I should have levelled up, because all 3 requirements are fulfilled. There might be an error with the revert rate checking for levelling up?

tanvidadu · 2018-07-01T12:09:27Z

@misaochan small error on my part :( I have fixed the bug
I think its a great idea to 'i' next to each parameter.
Below is the mock of the screen after adding 'i'

misaochan · 2018-07-01T12:35:15Z

@tanvidadu My suggestion is to have the 'i's in a different color from the text, in blue perhaps? :)

Thanks for fixing the bug, I will re-test once the information tooltips are completed.

maskaravivek

Nice work @tanvidadu. Changes look good to me. Have just added a few minor comments. :)

maskaravivek · 2018-06-30T18:35:06Z

app/src/main/java/fr/free/nrw/commons/achievements/AchievementsActivity.java

+        try {
+            achievements.setRevertCount(object.getInt("deletedUploads"));
+        } catch (JSONException e) {
+


Add Timber logs here. :)

maskaravivek · 2018-07-01T16:27:07Z

app/src/main/java/fr/free/nrw/commons/achievements/AchievementsActivity.java

+                .observeOn(AndroidSchedulers.mainThread())
+                .subscribe(
+                   object -> parseJsonRevertCount(object)
+                ));


Add a throwable subscription too.

maskaravivek · 2018-07-01T16:27:54Z

app/src/main/java/fr/free/nrw/commons/achievements/AchievementsActivity.java

+     */
+    private void setRevertCount(){
+        compositeDisposable.add(mediaWikiApi
+                .getRevertCount(sessionManager.getCurrentAccount().name)


Can sessionManager.getCurrentAccount() ever return a null? If yes, add a null check.

It won't return a null till logging in mandatory :)

Check the java docs for this method. :)

@maskaravivek this kind of check?
Optional.ofNullable(sessionManager.getCurrentAccount().name.orElse("")
It requires min API 24 where as min API of Commons app is 15. Any idea how to go about it ?

A simple null check like this should be enough.

Account currentAccount = sessionManager.getCurrentAccount(); if(currentAccount == null) { // do something }

Recently we experienced a few crashes on play store and we suspect that getCurrentAccount is returning null in some cases even for logged in users.

Refer to this PR for more context:
https://github.com/commons-app/apps-android-commons/pull/1684/files

maskaravivek · 2018-07-01T16:28:54Z

app/src/main/java/fr/free/nrw/commons/achievements/AchievementsActivity.java

+     * used to set the non revert image percentage
+     * @param notRevertPercentage
+     */
+    private void setImageRevertPercentage( int notRevertPercentage){


remove whitespace. :)

I can't find white spaces here but throughout the code, I tried to remove all extra white spaces :)

I think he means the extra whitespace between ( and int ;)

In general it's a good idea to follow the style of existing code. For instance, observe the places there are whitespaces (and no whitespaces) here:
public void onCreate(Bundle savedInstanceState) {

maskaravivek · 2018-07-01T16:46:31Z

app/src/main/java/fr/free/nrw/commons/mwapi/ApacheHttpClientMediaWikiApi.java

+                    Locale.ENGLISH,
+                    fetchRevertCountUrlTemplate,
+                    new PageTitle(userName).getText());
+            String userNameCaps = Character.toString(userName.charAt(0)).toUpperCase()+userName.substring(1);


Is this actually required? MediaWiki usernames always start with a capital letter. Our shared prefs should already have username in this format.

https://en.wikipedia.org/wiki/Wikipedia:Naming_conventions_(technical_restrictions)#Lowercase_first_letter

misaochan

Javadocs requested

misaochan · 2018-07-02T10:39:33Z

app/src/main/java/fr/free/nrw/commons/achievements/Achievements.java

@@ -1,5 +1,7 @@
 package fr.free.nrw.commons.achievements;

+import android.util.Log;
+
 /**


Javadocs are spotty for this class, please have a more descriptive Javadoc here, and add them to all the new methods as well. Thanks! :)

@misaochan I have updated the javadocs :)

misaochan · 2018-07-03T14:35:44Z

Thanks @tanvidadu . :) Are we ready for me to re-test this?

tanvidadu · 2018-07-03T19:11:40Z

Yes! @misaochan thanks :)

misaochan · 2018-07-04T14:56:18Z

Works for me now, well done @tanvidadu ! :)

* Implemented Statistics * Basic Structure Implemented * Layout made screen independent and menu inflated * Share Screenshot using cache * Improved the Image Bound and added strings * Improved the quality of Pr * Wired to navigation drawer * Changed the bounds of the image * Added Info icon * Removed the unecessary functionality * Updated JavaDocs and fetch the username * Fetch JsonObject from the api using JavaRx and OkHttp * Added JavaDocs and improved quality * fixed strings file * Improved the quality of pr * Render thanks , images used in articles on screen * fetch and rendered the upload count * FeaturedImages statistics rendered and Javadocs added * added ProgressBar * Added Class for calculating level * added level info and returned level info * level up info rendered on achievement activity * Inflated Level Number * Added the structure for badge * Added LevelUpInfo Programmetically on Drawable * aligned the text * changed the text * Implemented the structure for changing colour of drawable * Added functionality to change colours of badge during runtime * Added custom alert for share option * Improved the UI of screen * Added the alertDialog for info button * Improved the quality of PR * Added Builder model * Added Enum Model and increased levels to 15 * removed redundant class * Changed strings and added subtext * Feedback Module: Add reverts rate parameter (#1649) * Fetched Revert Count * Refactored Achievements class and display the fetched results * Refactored the levelController to include revert as parameter * Fixed error * Fixed bug * Added information for parameters and improved code quality * Javadocs added * Added null check and javadocs * Removed extra spaces

tanvidadu added 3 commits June 21, 2018 17:11

Fetched Revert Count

65700c0

Refactored Achievements class and display the fetched results

9512389

Refactored the levelController to include revert as parameter

46d62ee

Fixed error

1b1c158

commons-app deleted a comment Jun 30, 2018

Fixed bug

54b8175

commons-app deleted a comment Jul 1, 2018

maskaravivek approved these changes Jul 1, 2018

View reviewed changes

tanvidadu added 2 commits July 1, 2018 23:58

Added information for parameters and improved code quality

b132089

Javadocs added

88ce3f7

commons-app deleted a comment Jul 1, 2018

misaochan requested changes Jul 2, 2018

View reviewed changes

Added null check and javadocs

ad46087

commons-app deleted a comment Jul 2, 2018

misaochan approved these changes Jul 3, 2018

View reviewed changes

misaochan merged commit 12fb75a into commons-app:feedback-gamification Jul 4, 2018

tanvidadu deleted the feedback3 branch August 3, 2018 13:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feedback Module: Add reverts rate parameter #1649

Feedback Module: Add reverts rate parameter #1649

tanvidadu commented Jun 21, 2018

codecov-io commented Jun 21, 2018 •

edited

Loading

tanvidadu commented Jun 21, 2018

misaochan commented Jun 23, 2018 •

edited

Loading

tanvidadu commented Jun 24, 2018

misaochan commented Jul 1, 2018 •

edited

Loading

misaochan commented Jul 1, 2018

tanvidadu commented Jul 1, 2018

misaochan commented Jul 1, 2018

maskaravivek left a comment

maskaravivek Jun 30, 2018

maskaravivek Jul 1, 2018

maskaravivek Jul 1, 2018

tanvidadu Jul 1, 2018

maskaravivek Jul 2, 2018

tanvidadu Jul 2, 2018

maskaravivek Jul 2, 2018

maskaravivek Jul 1, 2018

tanvidadu Jul 1, 2018

misaochan Jul 2, 2018

maskaravivek Jul 1, 2018

misaochan left a comment

misaochan Jul 2, 2018

tanvidadu Jul 3, 2018

misaochan commented Jul 3, 2018

tanvidadu commented Jul 3, 2018

misaochan commented Jul 4, 2018

Feedback Module: Add reverts rate parameter #1649

Feedback Module: Add reverts rate parameter #1649

Conversation

tanvidadu commented Jun 21, 2018

Added Reverts Parameter to FeedBack Module

Description (required)

Tests performed (required)

Screenshots showing what changed (optional)

codecov-io commented Jun 21, 2018 • edited Loading

Codecov Report

tanvidadu commented Jun 21, 2018

misaochan commented Jun 23, 2018 • edited Loading

tanvidadu commented Jun 24, 2018

misaochan commented Jul 1, 2018 • edited Loading

misaochan commented Jul 1, 2018

tanvidadu commented Jul 1, 2018

misaochan commented Jul 1, 2018

maskaravivek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

misaochan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

misaochan commented Jul 3, 2018

tanvidadu commented Jul 3, 2018

misaochan commented Jul 4, 2018

codecov-io commented Jun 21, 2018 •

edited

Loading

misaochan commented Jun 23, 2018 •

edited

Loading

misaochan commented Jul 1, 2018 •

edited

Loading