Skip to content
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

Fix #1139: Nominate uploaded image for deletion #1297

Merged
merged 7 commits into from
Mar 19, 2018

Conversation

diddypod
Copy link

@diddypod diddypod commented Mar 13, 2018

Description

Fixes #1139 Add Option to delete the uploaded image.

Added DeleteTask and added new functions to MediaWikiApi to make edits to the pages

Tests performed

Tested on Xiaomi (Wingtech) Redmi 2, with betaDebug.

Screenshots showing what changed

screenshot_20180313-112023
screenshot_20180313-112028
screenshot_20180313-112031

@diddypod diddypod changed the title Delete request Fix #1139: Nominte uploaded image for deletion Mar 13, 2018
@diddypod diddypod changed the title Fix #1139: Nominte uploaded image for deletion Fix #1139: Nominate uploaded image for deletion Mar 13, 2018
@nicolas-raoul
Copy link
Member

Sounds reasonable, judging from the screenshots :-)
I guess needs for subsequent iterations will appear as users start to use the feature, but that's good for now I would say.

@codecov-io
Copy link

codecov-io commented Mar 13, 2018

Codecov Report

Merging #1297 into master will decrease coverage by 0.08%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1297      +/-   ##
=========================================
- Coverage    3.81%   3.73%   -0.09%     
=========================================
  Files         126     127       +1     
  Lines        5919    6054     +135     
  Branches      583     588       +5     
=========================================
  Hits          226     226              
- Misses       5678    5813     +135     
  Partials       15      15
Impacted Files Coverage Δ
...rw/commons/mwapi/ApacheHttpClientMediaWikiApi.java 5.43% <0%> (-0.38%) ⬇️
...in/java/fr/free/nrw/commons/delete/DeleteTask.java 0% <0%> (ø)
...fr/free/nrw/commons/media/MediaDetailFragment.java 0% <0%> (ø) ⬆️

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 80cc8b7...9a27f9c. Read the comment docs.

@neslihanturan
Copy link
Collaborator

@diddypod Thanks for this great job. I think we should display "Succesfully nominated" message in more visible way, ie. with an alert.
Besides, @nicolas-raoul is it possible to get any information as "nominated for deletion" via MWAPI? If so we can prevent several nominations. If this is not technically possible, we can add this information to "Succesfully nominated" message, as "Succesfully nominated, please do not nominate this image again" (In more polite English)

@neslihanturan
Copy link
Collaborator

There is a notification for nomination process, so maybe we can recognize if an image is nominated for deletion.

@diddypod
Copy link
Author

diddypod commented Mar 17, 2018

Okay @neslihanturan, I'll incorporate a check for that ASAP.

@diddypod
Copy link
Author

Fixes issue: Potential multiple nominations

The AsyncTask now checks if the Commons:Deleion_Requests subpage exists to verify if the file has previously been nominated for deletion, and proceeds accordingly. Thus, each file may be nominated for deletion only once.
Also, the results of the nomination process are displayed as an alert, as @neslihanturan suggested, with an option to open the file page in the browser.

@neslihanturan
Copy link
Collaborator

Great @diddypod , very good job:) Will be testing this immediately

@neslihanturan
Copy link
Collaborator

@diddypod it works quite nice, thanks for your contribution. Maybe we can notify the user about "already nominated issue" before entering the message second time. What do you say?

But this can be a seperate PR, merging this one.

@neslihanturan neslihanturan merged commit 31ac506 into commons-app:master Mar 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants