Fixed #4836 & #4840 Added the functionality of cancel upload and also solved the small bug of pausing upload by arinmodi · Pull Request #4843 · commons-app/apps-android-commons

arinmodi · 2022-02-20T13:31:03Z

Added the functionality of cancel upload and also solved the small bug of pausing upload

Fixes #4836 & #4840

What changes did you make and why?

I made the cancel button visible after the pause and also added confirmation dialog

For second issue to solve I change the id with string in setTag function of pauseResume button.

Tests performed

Tested build variant on ASUS_X00TD with API level 28

codecov · 2022-02-20T13:46:23Z

Codecov Report

Merging #4843 (f151111) into master (4a69bc5) will increase coverage by 0.43%.
The diff coverage is 73.91%.

@@             Coverage Diff              @@
##             master    #4843      +/-   ##
============================================
+ Coverage     50.14%   50.57%   +0.43%     
- Complexity     2207     2225      +18     
============================================
  Files           338      338              
  Lines         15475    15546      +71     
  Branches       1348     1359      +11     
============================================
+ Hits           7760     7863     +103     
+ Misses         7141     7100      -41     
- Partials        574      583       +9

Impacted Files	Coverage Δ
...n/java/fr/free/nrw/commons/CommonsApplication.java	`42.99% <ø> (ø)`
...mmons/contributions/ContributionsListFragment.java	`59.21% <55.55%> (-0.44%)`	⬇️
.../commons/contributions/ContributionViewHolder.java	`79.16% <85.71%> (-0.48%)`	⬇️
...mmons/profile/leaderboard/LeaderboardFragment.java	`46.03% <0.00%> (-14.33%)`	⬇️
...ree/nrw/commons/contributions/ContributionDao.java	`6.25% <0.00%> (-12.50%)`	⬇️
...commons/contributions/ContributionsRepository.java	`41.66% <0.00%> (-8.34%)`	⬇️
...ns/contributions/ContributionsLocalDataSource.java	`16.66% <0.00%> (-4.17%)`	⬇️
...mons/explore/recentsearches/RecentSearchesDao.java	`61.29% <0.00%> (+1.07%)`	⬆️
...va/fr/free/nrw/commons/category/CategoriesModel.kt	`59.32% <0.00%> (+1.69%)`	⬆️
... and 8 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 4a69bc5...f151111. Read the comment docs.

arinmodi · 2022-02-21T08:50:29Z

@nicolas-raoul, please review this....

arinmodi · 2022-02-22T03:55:42Z

Hey @nicolas-raoul, is this working ? Or i have to change something ?

nicolas-raoul · 2022-02-22T04:44:31Z

I tested it yesterday and thought it was not working.
But when I tested it again today, I realized that I was just not waiting for long enough.

Yesterday I thought that Pause was not working on the first tap, so I was always tapping it several times.

Since pausing sometimes takes several seconds, how about showing a modal "Pausing..." popup so that people like me do not tap the button twice? Thanks a lot!

arinmodi · 2022-02-22T04:49:58Z

Ok, will push a changes

arinmodi · 2022-02-22T06:49:01Z

@nicolas-raoul, I tried to display the progress dialog but still the user have to wait for few seconds cause the progress bar still doing it's process even after the pausing, should i fix that progress bar in this PR ?

nicolas-raoul · 2022-02-22T07:21:27Z

Can you close the "Pausing..." popup just when the "Pause" button disappears?

arinmodi · 2022-02-22T07:50:00Z

I think we should put a delay of 2 second, so that before closing pausing dialog,

what do you think ?

nicolas-raoul · 2022-02-22T08:05:12Z

I don't think introducing delays is a good idea, as it might be too short or too long depending on the bandwidth and other factors.

arinmodi · 2022-02-22T08:22:01Z

@nicolas-raoul, added the pausing pop up as you said..

but why this progress bar is showing it's progress for few seconds after the pausing it,

i think it may be cause of the uploadWorker ?

what do you think ?

nicolas-raoul · 2022-02-22T08:27:48Z

Yes, possibly. Making it stop immediately might be a solution, but may be more complex than just monitoring the worker.

arinmodi · 2022-02-22T08:42:24Z

So, what changes required in this ?

nicolas-raoul

Please showing a modal "Pausing..." popup that closes exactly when the "Pause" button disappears.
Thanks!

arinmodi · 2022-02-22T09:33:39Z

@nicolas-raoul, I think doing that, user will not be able to see the pop up

arinmodi · 2022-02-22T09:39:21Z

@nicolas-raoul, done requested changes

nicolas-raoul · 2022-02-22T11:05:22Z

Strangely even with your new commit I am not seeing the popup.

arinmodi · 2022-02-22T11:33:33Z

Yes, that's what i am trying to say,

I think this heppen cause of the, setup of the upload Worker, cause the pause button just changing the state of the button, the actual pause is done by uploadWorker, which takes some time, and also before uploadWorker complete it's task, the pauseUpload completing it's task and returning which eventually calls the closing popup,

In short, currently we are displaying the popup for just setting up the global variable and state, which don't require much time

arinmodi · 2022-02-22T11:43:46Z

So, what do you think about this ?

arinmodi · 2022-02-22T15:39:35Z

Hey @nicolas-raoul, finally it's working now,

You can check test now, it's working better than ever

Thanks...

madhurgupta10 · 2022-02-22T16:09:54Z

    private final CompositeDisposable compositeDisposable = new CompositeDisposable();
    private final MediaClient mediaClient;
    private boolean isWikipediaButtonDisplayed;
+    private ProgressDialog pausingPopUp;


Progress Dialog is deprecated, I suggest using the progress bar.

So, I have to create a custom dialog with the progress bar, right ?

Thanks for your review

Looking at the scope of the PR, if it's too much trouble to use ProgressBar then I guess ProgressDialog would be fine since there are other places we use it. There is already an issue for it - #3487

@arinmodi Let us know if it possible to use Progress Bar for this PR :)

let me try, I will tell in some time

I can make the progress dialog using alert dialog but in that case i also need to create another xml file, is it ok ?

arinmodi · 2022-02-22T17:45:09Z

Hey @madhurgupta10, successfully replaced progress Dialog with the progress Bar and I build it using the alert dialog.

Can you please review now,

Thanks.

arinmodi · 2022-02-23T04:51:29Z

Hey @nicolas-raoul, can you please review this

arinmodi · 2022-02-23T07:22:46Z

@nicolas-raoul, is anything needs to change ?

nicolas-raoul · 2022-02-23T10:15:39Z


    /**
-     * In memory list of contributios whose uploads ahve been paused by the user
+     *  In memory list of contribution whose uploads have been paused by the user


Note for the future: Thanks for fixing these! Please do not hesitate to make such changes in a separate pull request, so that this pull request does not contain unrelated changes :-)

By the way:
In memory -> In-memory
contribution -> contributions

nicolas-raoul · 2022-02-23T10:16:33Z

    private void setResume() {
        pauseResumeButton.setImageResource(R.drawable.play_icon);
-        pauseResumeButton.setTag(R.string.resume);
+        pauseResumeButton.setTag("resume");


Is this really needed?

nicolas-raoul · 2022-02-23T10:17:27Z

+                ViewUtil.showShortToast(getContext(), R.string.cancelling_upload);
+                contributionsListPresenter.deleteUpload(contribution);
+            }, () -> {
+                ///do nothing


// Do nothing.

nicolas-raoul · 2022-02-23T10:18:01Z

    public void deleteUpload(final Contribution contribution) {
-        contributionsListPresenter.deleteUpload(contribution);
+        DialogUtil.showAlertDialog(getActivity(),
+            "Cancel Upload",


Would you mind reminding me what this string is used for, and why is does not need localization?

this string is the title of dialog util, And I forgot to do localization

nicolas-raoul · 2022-02-23T10:19:54Z

        ButterKnife.bind(this, parent);
        this.callback = callback;
+
+        // setting the pausingPopUp dialog


// Set a dialog indicating that the upload is being paused. This is needed because pausing an upload might take a dozen seconds.

arinmodi · 2022-02-23T10:46:26Z

Added the changes

arinmodi · 2022-02-23T14:59:24Z

Hey @madhurgupta10, I am blocked to upload the picture, so how can I work on the bugs of the upload feature

madhurgupta10 · 2022-02-23T15:24:09Z

Hey @madhurgupta10, I am blocked to upload the picture, so how can I work on the bugs of the upload feature

I think you can make an appeal to unblock your account, @nicolas-raoul could you please share more information on this?

nicolas-raoul · 2022-02-24T02:31:15Z

@arinmodi Are you blocked on beta or prod? For both you can appeal indeed.

arinmodi · 2022-02-24T05:04:56Z

Hey @nicolas-raoul, is anything else requires ?

arinmodi · 2022-02-24T06:41:11Z

@nicolas-raoul, changed a bit, please review this....

arinmodi · 2022-02-24T07:35:01Z

@nicolas-raoul, how can i appeal for unblock my account in beta version

nicolas-raoul · 2022-02-24T08:16:43Z

What is your username? "Anonymous me" does not seem to be blocked on beta: https://commons.wikimedia.beta.wmflabs.org/wiki/Special:Log?type=block&user=&page=User%3AAnonymous+me&wpdate=&tagfilter=&subtype=

arinmodi · 2022-02-24T08:19:02Z

my user name Arin Modi

nicolas-raoul · 2022-02-24T08:22:03Z

Apparently you uploaded a meme. In the future, please only upload pictures you have taken yourself (even pictures of a spoon) :-) Thanks! I will try to get you unblocked.

arinmodi · 2022-02-24T08:24:24Z

ok, I just uploaded by mistake

arinmodi · 2022-02-24T09:43:25Z

Hey @nicolas-raoul, any updates on this PR ?

arinmodi · 2022-02-25T04:51:38Z

Hey @nicolas-raoul, please review this

Fixed commons-app#4836 & commons-app#4840

45c5edd

nicolas-raoul changed the title ~~Fixed #4836 & #4840~~ Fixed #4836 & #4840 Added the functionality of cancel upload and also solved the small bug of pausing upload Feb 22, 2022

Added Pausing PopUp

3a5fd20

nicolas-raoul requested changes Feb 22, 2022

View reviewed changes

Closing Pausing PopUp exactly when pause button disappears

d14ef22

Managed to display the Pausing Upload

93f26fe

madhurgupta10 reviewed Feb 22, 2022

View reviewed changes

arinmodi added 2 commits February 22, 2022 23:09

Added the Pausing Dialog Using Progress Bar

04ba8e1

Added the Pausing Dialog Using Progress Bar

6707bec

arinmodi requested a review from nicolas-raoul February 23, 2022 06:13

nicolas-raoul requested changes Feb 23, 2022

View reviewed changes

arinmodi added 2 commits February 23, 2022 16:11

Added Required Changes

d4cd2c3

Added Required Changes-wording change

b75dc81

arinmodi requested a review from nicolas-raoul February 23, 2022 11:51

Added Required Changes-wording change

f151111

nicolas-raoul approved these changes Feb 25, 2022

View reviewed changes

nicolas-raoul merged commit 9431e37 into commons-app:master Feb 25, 2022

arinmodi mentioned this pull request Feb 26, 2022

[Bug]: Uploading is not paused after the resuming the upload #4840

Closed

arinmodi deleted the new_branch branch February 27, 2022 11:15

Conversation

arinmodi commented Feb 20, 2022

Uh oh!

codecov Bot commented Feb 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

arinmodi commented Feb 21, 2022

Uh oh!

arinmodi commented Feb 22, 2022

Uh oh!

nicolas-raoul commented Feb 22, 2022

Uh oh!

arinmodi commented Feb 22, 2022

Uh oh!

arinmodi commented Feb 22, 2022

Uh oh!

nicolas-raoul commented Feb 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arinmodi commented Feb 22, 2022

Uh oh!

nicolas-raoul commented Feb 22, 2022

Uh oh!

arinmodi commented Feb 22, 2022

Uh oh!

nicolas-raoul commented Feb 22, 2022

Uh oh!

arinmodi commented Feb 22, 2022

Uh oh!

nicolas-raoul left a comment

Choose a reason for hiding this comment

Uh oh!

arinmodi commented Feb 22, 2022

Uh oh!

arinmodi commented Feb 22, 2022

Uh oh!

nicolas-raoul commented Feb 22, 2022

Uh oh!

arinmodi commented Feb 22, 2022

Uh oh!

arinmodi commented Feb 22, 2022

Uh oh!

arinmodi commented Feb 22, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arinmodi commented Feb 22, 2022

Uh oh!

arinmodi commented Feb 23, 2022

Uh oh!

arinmodi commented Feb 23, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arinmodi commented Feb 23, 2022

Uh oh!

arinmodi commented Feb 23, 2022

Uh oh!

codecov Bot commented Feb 20, 2022 •

edited

Loading

nicolas-raoul commented Feb 22, 2022 •

edited

Loading