Todo for user #3851

neslihanturan · 2020-06-29T12:41:25Z

Description (required)
Fixes #1870
You can see the todo warning to improve the quality of the image below

After selecting edit icon, a filter will appear

Then after selecting and clicking to update categories button category will be added

Note: I didn't preferred to add an icon to contributions list to indicate the todo list, since #3783 also adds an icon to contributions list. Two different icons for almost same thing would be confusing. I think after this PR is merged, we should move wikipedia association button to inside (media details fragment) and todo (for missing category and wikipedia association) to outside (category list).

tests-checker

Could you please add tests to make sure this change works as expected?

misaochan · 2020-07-08T16:12:09Z

Thanks for the PR @neslihanturan . These are quite substantial changes, could you add unit tests to your PR please? :)

I will test this tomorrow, @maskaravivek or @ashishkumar468 it would be great if one of you could do a code review.

ashishkumar468 · 2020-07-08T20:56:49Z

.idea/codeStyles/Project.xml

        <option name="CONTINUATION_INDENT_SIZE" value="4" />
-        <option name="TAB_SIZE" value="2" />


Please reset this

app/src/main/res/values/strings.xml

neslihanturan · 2020-07-10T21:09:25Z

.idea/codeStyles/Project.xml

@@ -325,4 +325,4 @@
      </indentOptions>
    </codeStyleSettings>
  </code_scheme>
-</component>


I didnt add any new lines at the and of the file, I dont get what this change is. Any idea?

misaochan · 2020-07-11T11:47:50Z

Did you try to close the keyboard, then it should be there

Yes, as you can see in my second screenshot the keyboard is closed (otherwise the list of categories would not even be displayed).

Actually I intentionally implemented this feature (not a bug:D) to east give a way to close it.

IMO for a dialog that requires a lot of tapping, it would be a lot better to have an "x" button on the top right to close, rather than closing with every mis-tap. :)

app/src/main/java/fr/free/nrw/commons/category/CategoryClient.kt

codecov-commenter · 2020-07-14T19:49:45Z

Codecov Report

Merging #3851 into master will decrease coverage by 0.14%.
The diff coverage is 0.00%.

@@             Coverage Diff             @@
##             master   #3851      +/-   ##
===========================================
- Coverage      8.69%   8.55%   -0.15%     
  Complexity      366     366              
===========================================
  Files           319     321       +2     
  Lines         11774   11971     +197     
  Branches        914     937      +23     
===========================================
  Hits           1024    1024              
- Misses        10693   10890     +197     
  Partials         57      57

Impacted Files	Coverage Δ	Complexity Δ
...ava/fr/free/nrw/commons/category/CategoryClient.kt	`85.71% <ø> (ø)`	`8.00 <0.00> (ø)`
.../free/nrw/commons/category/CategoryEditHelper.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (?)`
...ategory/CategoryEditSearchRecyclerViewAdapter.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (?)`
.../commons/contributions/ContributionViewHolder.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...w/commons/contributions/ContributionsFragment.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...ommons/contributions/ContributionsListAdapter.java	`0.00% <ø> (ø)`	`0.00 <0.00> (ø)`
...mmons/contributions/ContributionsListFragment.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...fr/free/nrw/commons/media/MediaDetailFragment.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...ee/nrw/commons/media/MediaDetailPagerFragment.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...e/nrw/commons/notification/NotificationHelper.java	`0.00% <ø> (ø)`	`0.00 <0.00> (ø)`
... and 3 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 6f4c60b...672b7b0. Read the comment docs.

neslihanturan · 2020-07-15T12:07:19Z

@misaochan I recognized an issue which is not reported here:
Categories are actually added but since media object is retained from cache (instead of being re-fetched, which makes sense in terms of efficiency) media.getCategories() still returns old category list. There are two possible solution to this.

1- Simple, save categories to media object and display media.getCategories() + new categories to user
2- Not that simple, re-fetch categories, I will need help from @maskaravivek since he has experience about media.kt class

neslihanturan · 2020-07-27T11:41:22Z

app/src/main/java/fr/free/nrw/commons/media/MediaDetailFragment.java

+
+    public void updateCategories(List<String> selectedCategories) {
+        Single<Boolean> resultSingle =reasonBuilder.getReason(media, null)
+            .flatMap((reason) -> categoryEditHelper.makeCategoryEdit(getContext(), media, selectedCategories, this));


Here @ashishkumar468 , there is no need to build a reason. I just tried to mimic the same implementation as deleteHelper which does not raise exception. I don't know what I can put to this line, to be able to call flatMap.

For other implementation, see line 747 of same file

private void onDeleteClicked(Spinner spinner) {

I could not understand @neslihanturan . What is the issue with this implementation?

well, I don't need to buıild a reason as "reasonBuilder.getReason(media, null)". It is only a need for deletion request. I used that line just to be able to implement the same structure with line 747. But my rxjava knowledge doesnt help me to find what to put before .flatMap, if not needed. Is it clear now?

neslihanturan · 2020-08-11T06:23:05Z

@misaochan this one is ready to be tested and probably be merged, after @ashishkumar468 's help:)

misaochan · 2020-08-11T17:02:19Z

Hi @neslihanturan , thanks for making the changes! :) It works well for me now, there is just one item from #3851 (comment) that seems to not work:

The area for onClick to search for categories should be wider, probably the entire width of the search icon row. Currently if you tap anywhere except the search icon itself, the dialog disappears

This problem still happens for me. Once this is fixed I am happy to merge.

misaochan · 2020-08-11T19:15:48Z

app/src/main/res/values/strings.xml

@@ -598,6 +598,18 @@ Upload your first media by tapping on the add button.</string>
  <string name="delete_helper_ask_reason_copyright_logo">Logo</string>
  <string name="delete_helper_ask_reason_copyright_other">Other</string>
  <string name="delete_helper_ask_alert_set_positive_button_reason">Because it is</string>
+
+  <string name="category_edit_helper_make_edit_toast">Trying to update categories.</string>
+  <string name="category_edit_helper_show_edit_title">Categories are updated</string>


Based on my interpretation, the notification title would be "Categories are updated: Success" if the categories are successfully updated, and "Categories are updated: Could not add categories" if they are not, right? In that case I think this string should be Category update instead of Categories are updated.

misaochan · 2020-08-11T19:17:20Z

app/src/main/res/values/strings.xml

+    <item quantity="one">Category %1$s is added.</item>
+    <item quantity="other">Categories %1$s are added.</item>
+  </plurals>
+  <string name="category_edit_helper_show_edit_title_failed">Failed</string>


Is this string ever used?

misaochan · 2020-08-17T11:30:29Z

Great job @neslihanturan !

* Add a simple message * Categories are edited * Display categories * read whole page * Revert wrong changes * Add newly added category field * Doesnt displaey alreasy added categories * add strings for notifications * clean code * Readd accidentally removed imports * Fix edit layout style * Fix category update messages * Pass isWikipediaButtonDisplayed information to fragment * Remove unused class * Fix strings * Fix string * Add exeption for uncategorised images too * Revert project.xml changes * fix update buttonvisibility issue * Make sure it works for auto added categories too * make the button visible * Make the button appear when categories are entered * Include cancel button * Make view updated too * Make category view edited * Update categories in an hacky way * Fix unnecessary method call * Add notes for short term fix to display added category * Fix tests * Fix strings * Fix click issue

neslihanturan added 4 commits June 9, 2020 01:28

Add a simple message

4974662

Categories are edited

852c824

Display categories

b19d291

read whole page

5c71174

tests-checker bot suggested changes Jun 29, 2020

View reviewed changes

neslihanturan changed the title ~~Todo for user~~ [WIP] Todo for user Jun 29, 2020

neslihanturan marked this pull request as draft June 29, 2020 12:41

neslihanturan added 3 commits July 1, 2020 15:31

Revert wrong changes

17f0c2c

Add newly added category field

2df909e

Doesnt displaey alreasy added categories

6c2c4ee

neslihanturan changed the base branch from master to contributions-todo July 3, 2020 14:06

neslihanturan changed the base branch from contributions-todo to master July 3, 2020 14:37

neslihanturan added 11 commits July 3, 2020 17:39

add strings for notifications

413626e

Fix conflicts

3f0c3d2

clean code

085fea6

Merge remote-tracking branch 'upstream/master' into todoForUser

645a7dc

Readd accidentally removed imports

4d4888f

Fix edit layout style

a9bff0e

Fix category update messages

3bce4e5

Pass isWikipediaButtonDisplayed information to fragment

4ed9548

Remove unused class

9d70ea4

Fix strings

0729554

Fix conflicts

8cb80d5

neslihanturan marked this pull request as ready for review July 7, 2020 23:17

misaochan changed the title ~~[WIP] Todo for user~~ Todo for user Jul 8, 2020

misaochan requested review from maskaravivek and ashishkumar468 July 8, 2020 16:12

ashishkumar468 reviewed Jul 8, 2020

View reviewed changes

misaochan requested changes Jul 9, 2020

View reviewed changes

app/src/main/res/values/strings.xml Outdated Show resolved Hide resolved

app/src/main/res/values/strings.xml Outdated Show resolved Hide resolved

app/src/main/res/values/strings.xml Outdated Show resolved Hide resolved

neslihanturan commented Jul 10, 2020

View reviewed changes

misaochan reviewed Jul 11, 2020

View reviewed changes

app/src/main/java/fr/free/nrw/commons/category/CategoryClient.kt Show resolved Hide resolved

neslihanturan added 5 commits July 13, 2020 15:50

fix update buttonvisibility issue

ffc548a

Make sure it works for auto added categories too

94c7b02

make the button visible

332d3cb

Make the button appear when categories are entered

f292cd1

Include cancel button

672b7b0

neslihanturan added 3 commits July 22, 2020 12:29

Make view updated too

db2fe3c

Make category view edited

0aff3ed

Update categories in an hacky way

05fc9a7

neslihanturan commented Jul 27, 2020

View reviewed changes

neslihanturan added 2 commits August 3, 2020 13:24

Fix unnecessary method call

bd63a0a

Add notes for short term fix to display added category

6f453e0

neslihanturan mentioned this pull request Aug 3, 2020

Categories should be re-fetched again after updated #3890

Open

Fix tests

3096735

misaochan reviewed Aug 11, 2020

View reviewed changes

neslihanturan added 2 commits August 13, 2020 15:14

Fix strings

70a469a

Fix click issue

a551c76

misaochan approved these changes Aug 17, 2020

View reviewed changes

misaochan merged commit 1856196 into commons-app:master Aug 17, 2020

misaochan mentioned this pull request Aug 17, 2020

Modify categories/description afterwards #161

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Todo for user #3851

Todo for user #3851

neslihanturan commented Jun 29, 2020 •

edited by sivaraam

Loading

tests-checker bot left a comment

misaochan commented Jul 8, 2020

ashishkumar468 Jul 8, 2020

neslihanturan Jul 10, 2020

misaochan commented Jul 11, 2020 •

edited

Loading

codecov-commenter commented Jul 14, 2020

neslihanturan commented Jul 15, 2020

neslihanturan Jul 27, 2020

ashishkumar468 Jul 27, 2020

neslihanturan Jul 27, 2020 •

edited

Loading

neslihanturan commented Aug 11, 2020 •

edited

Loading

misaochan commented Aug 11, 2020

misaochan Aug 11, 2020 •

edited

Loading

misaochan Aug 11, 2020

misaochan commented Aug 17, 2020

		<option name="CONTINUATION_INDENT_SIZE" value="4" />
		<option name="TAB_SIZE" value="2" />

Todo for user #3851

Todo for user #3851

Conversation

neslihanturan commented Jun 29, 2020 • edited by sivaraam Loading

tests-checker bot left a comment

Choose a reason for hiding this comment

misaochan commented Jul 8, 2020

ashishkumar468 Jul 8, 2020

Choose a reason for hiding this comment

neslihanturan Jul 10, 2020

Choose a reason for hiding this comment

misaochan commented Jul 11, 2020 • edited Loading

codecov-commenter commented Jul 14, 2020

Codecov Report

neslihanturan commented Jul 15, 2020

neslihanturan Jul 27, 2020

Choose a reason for hiding this comment

ashishkumar468 Jul 27, 2020

Choose a reason for hiding this comment

neslihanturan Jul 27, 2020 • edited Loading

Choose a reason for hiding this comment

neslihanturan commented Aug 11, 2020 • edited Loading

misaochan commented Aug 11, 2020

misaochan Aug 11, 2020 • edited Loading

Choose a reason for hiding this comment

misaochan Aug 11, 2020

Choose a reason for hiding this comment

misaochan commented Aug 17, 2020

neslihanturan commented Jun 29, 2020 •

edited by sivaraam

Loading

misaochan commented Jul 11, 2020 •

edited

Loading

neslihanturan Jul 27, 2020 •

edited

Loading

neslihanturan commented Aug 11, 2020 •

edited

Loading

misaochan Aug 11, 2020 •

edited

Loading