Fix #4186: Uncheck all selection on developer options not working

[KevinGitonga]

Explanation

Fix #4186
This pr fixes :--
User not able to unselect currently selected developer options using the "All Checkbox". I added more checks when the
checkbox is clicked to check scenario when the checkbox is already clicked, if clicked already initiate removal of all items that had been selected and uncheck the checkboxes.

Essential Checklist

• The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
• Any changes to scripts/assets files have their rationale included in the PR explanation.
• The PR follows the style guide.
• The PR does not contain any unnecessary code changes from Android Studio (reference).
• The PR is made from a branch that's not called "develop" and is up-to-date with "develop".
• The PR is assigned to the appropriate reviewers (reference).

For UI-specific PRs only

If your PR includes UI-related changes, then:

• Add screenshots for portrait/landscape for both a tablet & phone of the before & after UI changes
• For the screenshots above, include both English and pseudo-localized (RTL) screenshots (see RTL guide)
• Add a video showing the full UX flow with a screen reader enabled (see accessibility guide)
• Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing

Before changes demo video

all_checkbox_uncheck_not_working.mp4

After Changes made demo video

developer_options_all_uncheck.mp4

Screenshots of failing tests before fix

Espresso Test Results

MarkChaptersCompletedFragmentTest results

MarkStoriesCompletedFragmentTest results

MarkTopicsCompletedFragmentTest results

[BenHenning]

Thanks @KevinGitonga! I think the approach looks good. My main pieces of feedback aside from the in-line comments are:

• The title is missing a ':' after the issue number (see other PRs for reference)
• The explanation for the PR should include 'Fix' with the issue being fixed, along with a description explaining what's being fixed, how, and why this is the best approach (+ any other relevant considerations). I suggest taking a look at other PRs for reference, particularly some of the larger ones submitted over the past couple of months that have really clear explanation sections.
• The checklist should use [ ] for unchecked items and [x] for checked items (I think you have extra spaces in yours). If it's easier, you can actually click on the checkboxes once the PR description is saved so that you don't need to manually edit each one.
• The recorded video is really nice--thanks for adding that!
• Please also include the other videos/screenshots requested in the PR description template (particularly for accessibility since tablet & landscape aren't likely to be different in behavior per the nature of the fix).
• The Espresso test screenshot isn't actually showing the results of running any Espresso tests. See other PRs that have provided screenshots for an idea on what's helpful to include.
• Please add tests for each of the updated presenters (we generally always add tests for all behaviors that are added, removed, or changed in the codebase, unless there's a specific reason why the change can't be tested).

[rt4914]

@KevinGitonga Please add tests for this functionality. Thanks.

[BenHenning]

Ah, not sure if you intentionally re-assigned me @rt4914.

@KevinGitonga please make sure to address all points mentioned, and respond to all comments, before sending a PR back for review (I know you didn't send this back in this case, but I thought I'd mention this, anyway, so that you have this in mind when you send this PR back into review).

[oppiabot]

Hi @KevinGitonga, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

[BenHenning]

@KevinGitonga are you still actively working on this? We should avoid PRs becoming stale.

[KevinGitonga]

@BenHenning was blocked on tests for this patch. Will proceed as i can run tests on my Ubuntu now.

[oppiabot]

Hi @KevinGitonga, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

[oppiabot]

Hi @KevinGitonga, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

[BenHenning]

@KevinGitonga PTAL at KevinGitonga#4 and assign back if it looks good & you can merge it.

I have a separate follow-up fix PR to send once that's merged.

[BenHenning]

@KevinGitonga PTAL at KevinGitonga#3 and merge it if it looks good to you.

I think that'll fix at least the obvious CI failures (though there may be others that I'm unaware of).

Note that per that PR's description I tried running the tests a bunch of times and didn't see any flakes. After merging the PR, could you check again to see if you're still running into the flake issues that led to this being routed over to me for investigating?

[KevinGitonga]

@BenHenning have merged the changes will sync and run the test's on another device to check the status of this flake.

[KevinGitonga]

@BenHenning noticed i still have some flakes when i run test's on MarkChaptersCompletedFragmentTest on the develop branch too this is both on my Mac and ubuntu. The tests have a tendency to fail and sometimes pass completely. Please check image below for the test results.

First run

Next run

[BenHenning]

Just to confirm @KevinGitonga are those results from Robolectric or Espresso?

[KevinGitonga]

Just to confirm @KevinGitonga are those results from Robolectric or Espresso?

@BenHenning this are from Espresso run from my Samsung device. Not sure what is causing this on espresso but since if ci is using robolectric and the same tests are passing and they are actually supposed to pass, my thinking is that the changes can merge to develop as the core issue is resolved. Thanks.

[BenHenning]

Agreed @KevinGitonga. Since the Espresso failures already exist on develop, approving & merging this. Thanks!