Notification can now take to reviewer Activity if review was in progress before.

[IAmJaishree]

Purpose / Description

Currently, tapping the notification opens Deck List instead of Review page when pending reviews are present.
This PR contains the code which will allow user to go to Review page instead of deck list page by tapping the notification when a review is already in progress.
If no review is in progress the deck list page will open.

Fixes #3593

Approach

1. Added a check in NotificationService.java => onReceive function which checks if there are pending card to review and decides the intent of Notification click based on this.

2. Step 1 happens while the Notification onReceive method gets triggered and this can contain the lagging data as well.

For example when this onReceive is triggered there may be pending cards to review and based on the logic it decides to open Review page on tapping the notification but there are cases where the user taps the notification after completing the deck also in this case the Review page will open and there will be no card to review.

For this case added a check in Review activity where it checks if the Review Activity is open through Notification and verifies if there are pending cards or not, if there are no pending cards then it opens Deck List page instead.

How Has This Been Tested?

This has been tested by me manually going through each of the following scenario:

1. When user is reviewing a deck => Goes out of the app and there are pending cards to review. Notification appears and on tapping the notification the user is resumed to the Review page to continue the review process.

2. When user is reviewing a deck => Completes the Deck Review => app closes automatically as the review is complete. Notification appears and on tapping the notification the deck list is opened.

3. There are no review in progress but there are decks to review. Notification appears and on tapping the notification the deck list is opened.

Checklist Please, go through these checks before submitting the PR.

• You have not changed whitespace unnecessarily (it makes diffs hard to read)
• You have a descriptive commit message with a short title (first line, max 50 chars). -
• Your code follows the style of the project (e.g. never omit braces in if statements) -
• You have commented your code, particularly in hard-to-understand areas
• You have performed a self-review of your own code
• UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
• UI Changes: You have tested your change using the [Google Accessibility Scanner] (https://play.google.com/store/apps/details?id=com.google.android.apps.accessibility.auditor)

[welcome]

First PR! 🚀 We sincerely appreciate that you have taken the time to propose a change to AnkiDroid! Please have patience with us as we are all volunteers - we will get to this as soon as possible.

[codecov]

Codecov Report

Merging #8238 (55559f8) into master (7921fc2) will increase coverage by 0.15%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##             master    #8238      +/-   ##
============================================
+ Coverage     36.95%   37.11%   +0.15%     
- Complexity     3783     3842      +59     
============================================
  Files           354      359       +5     
  Lines         35820    36244     +424     
  Branches       4736     4795      +59     
============================================
+ Hits          13239    13451     +212     
- Misses        21072    21266     +194     
- Partials       1509     1527      +18     

Impacted Files	Coverage Δ	Complexity Δ
...kiDroid/src/main/java/com/ichi2/anki/Reviewer.java	37.71% <0.00%> (-0.30%)	68.00 <0.00> (ø)
...a/com/ichi2/anki/services/NotificationService.java	2.94% <0.00%> (-0.64%)	1.00 <0.00> (ø)
...id/src/main/java/com/ichi2/libanki/DeckConfig.java	42.10% <0.00%> (-11.23%)	8.00% <0.00%> (ø%)
...Droid/src/main/java/com/ichi2/upgrade/Upgrade.java	33.33% <0.00%> (-6.67%)	1.00% <0.00%> (ø%)
...c/main/java/com/ichi2/anki/dialogs/HelpDialog.java	34.09% <0.00%> (-6.30%)	1.00% <0.00%> (ø%)
...in/java/com/ichi2/themes/StyledProgressDialog.java	30.76% <0.00%> (-4.02%)	2.00% <0.00%> (ø%)
...d/src/main/java/com/ichi2/utils/ClipboardUtil.java	0.00% <0.00%> (-2.78%)	0.00% <0.00%> (-1.00%)
AnkiDroid/src/main/java/com/ichi2/libanki/DB.java	86.06% <0.00%> (-1.54%)	41.00% <0.00%> (-1.00%)
...main/java/com/ichi2/anki/services/BootService.java	43.07% <0.00%> (-1.37%)	11.00% <0.00%> (ø%)
...roid/src/main/java/com/ichi2/utils/SyncStatus.java	55.81% <0.00%> (-1.33%)	8.00% <0.00%> (ø%)
... and 84 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 7921fc2...55559f8. Read the comment docs.

[haran2248]

@IAmJaishree Can you please a screenshot/video of the change if possible?

[IAmJaishree]

@haran2248 initially I thought there is no UI change so it doesn't require screenshots, but yes as there is a change in behavior I'll add a video in a while. thanks for the suggestion :)

[IAmJaishree]

When the user has an in progress review the notification tap takes to that deck instead of deck list page.

ankiNotify.mp4

[IAmJaishree]

When user has completed the Review of a deck but still there are pending cards to review from other decks, the user will be notified by the notification and on tapping the notification the Deck List Page will open.

ankiNotify2.mp4

[mrudultora]

Some small changes. You can format your code using ctrl+alt+L. This would help in proper code readability.

[IAmJaishree]

@mrudultora the thing is doing ctrl + alt + l in Reviewer.java added many extra lines as un necessary white spaces, so I didn't do it.
manually I can make these changes in that file. let's have some more reviews and then I'll make all these changes in one go. Thanks for the review :).

[mrudultora]

@mrudultora the thing is doing ctrl + alt + l in Reviewer.java added many extra lines as un necessary white spaces, so I didn't do it.
manually I can make these changes in that file. let's have some more reviews and then I'll make all these changes in one go. Thanks for the review :).

No, don't select the whole file. Assume, you added 5 lines of code. So, select those 5 lines only and then do ctrl+alt+L. I hope the point in clear now?

[Arthur-Milchior]

Hi,

Thanks a lot for your first PR.
I have never heard about this bug, as I personally don't use notification. It's really nice to see it solved, seems that'll indeed help quite some people.

A few remarks you can see in code comment.

I would like, if possible, that you edit your PR description in order to give a high level explanation of what you are doing. I have used notification in the past and improved them a little bit, but I don't remember exactly what Notificationservice, OnReceive and Notification are.

It would help me, I think, to simply read, for example:

If the notification is triggered during a review, clicking on the notification let the user in the reviewer, unless there is no card to see in which case it goes back to deck picker

and also, for example:

This is done as follow:

• If the notification is triggered during review, then the intent will contains the field "throughNotification" to indicate that the notification was sent through review
• If the activity is opened and the reviewer has no card, then we go to the deck picker

Currently, I am not sure what you are trying to do exactly, so it's hard to review and ensure that's correctly what you wanted to do.

In particular, I've got a question. What occurs if the user click on the notification later. For example later in the day, when some cards that were hard are due to review a second time. Or a day later, when cards are back in the deck to be reviewed. It is not clear to

@mrudultora and @haran2248 Thank you a lot for your help reviewing. Given the increase in amount of time AnkiDroid suddenly require, this is very welcome. However, I'd like to ask for a change. As a general rule, I'd really appreciate if you can start by a positive message before starting with remarks of problem. Of course, the politeness rules depends of plenty of things, and that may not be needed for everybody, but that's still something that would ensure that even people with doubt will know that there were no mistake, that the whole process is good even if there are details to change. I believe that this help being a welcoming community to every people who wants to contribute. It also ensures simply that it is worth working on the PR; because after all, there is nothing that confirmed that it is a good approach globally.
To be clear, this is a general rule, for all PR, and not about this one in particular.

[lily1604]

@IAmJaishree I am assigned for issue in this repository . But I am stuck ,would you like to work with me on that issue

[david-allison]

Code's really clean, thanks :)

[IAmJaishree]

@IAmJaishree I am assigned for issue in this repository . But I am stuck ,would you like to work with me on that issue

sure :)

[mikehardy]

What's the status on this one? I don't see any status labels or clear approvals / request changes on review status from maintainers, though there is lots of interaction ?

[david-allison]

Tiny change then good to go, thanks!!

[Arthur-Milchior]

@IAmJaishree Do you still intend to work on it? Normally, it's almost done

[IAmJaishree]

@Arthur-Milchior sorry for the delay I was involved in my college exams, will complete this in a while

[Arthur-Milchior]

No emergency, we are all volunteers. We wondered whether we drove a contributor away somehow, and whether we should reassign this task

[IAmJaishree]

@david-allison-1 @Arthur-Milchior could you please review this PR :)

[david-allison]

Thanks! Looks good to me

[david-allison]

nit: remove spacing change here

[mikehardy]

This looks good in general, but I left a comment about Intent handling - could you check it @IAmJaishree ?

Thanks for your patience on the review I did not see this until just now!

[mikehardy]

This is something really subtle, but I have experienced bugs before where Intents were passed around, and the extras were erased crazycodeboy/react-native-splash-screen#289 (comment)

Philosophically, if we cannot handle an Intent, we should do our best to pass the original Intent to some Activity that can handle it, in the same way that if you cannot handle an Exception it is best to log something about why but then re-throw the original exception. Wrapping the exception or throwing a new one actually destroys information (the stack trace), and in the same way, making a new Intent here destroys information (any other possible Extras on the Intent, it's flags, category etc)

I don't believe we can just re-launch the Intent, but we can at least make sure we pass everything possible, which is the Extras Bundle (if it exists) and any flags (if they exist?)

I think the correct way to do that is either:

1- call setComponent (or maybe setClassName?) https://developer.android.com/reference/android/content/Intent#setComponent(android.content.ComponentName) to simply take this intent but re-direct it to DeckPicker.class or

2- call fillIn on the new Intent (directed at DeckPicker) but with the contents of the one we could not handle here in Reviewer https://developer.android.com/reference/android/content/Intent#fillIn(android.content.Intent,%20int)

I strongly prefer solution 1 because we know we are passing everything without having to think more, but if it does not work then solution 2 could work, as long as extras were manually copied as well

[github-actions]

Hello 👋, this PR has been opened for more than 2 months with no activity on it. If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing! You have 7 days until this gets closed automatically