[GSOC] Implemented Service to store Deck Meta Data #11487

prateek-singh-3212 · 2022-05-31T19:16:27Z

Pull Request template

Purpose / Description

I am working on decoupling the notification ( currently It causes random notification triggring in our app). This PR also aims to solve the problem of tight coupling between notification and small widget.

Fixes

Fixes #8114
Fixes #6476

Approach

What is the issue?
The complete issue is documented in #6476. The main issue is that Current notification system is coupled with widget and notification is only triggered when user use widget.
Secondly, Notification is triggered when user is using the app. How? Whenever deck review completes the deck the widget data is updated Hence Notification is shown (Because it is tightly coupled).

Architectural: one "unit of work" per time, rather than per deck.

ALGO FOR DECK NOTIFICATION
Input: TreeMap<time, List>

We store the the time and list of decks whose notification needs to be send on that time.
The map is TreeMap which stores the data in sorted key form.

Working of algo:

Takes the input time (EPOCH Long)

            if (currentTimeMS < scheduleTimeMS) {
                    // Scheduled time is gone for today.
                    // Only add time in time deck map. No need to reschedule work manager.
                    // Update schedule time for next day `scheduleTimeMS += 3600000`
                    // Update data in map
            } else if (scheduleTimeMS < (first key of map {Map is sorted so it will contain the most upcomming deck reminder at **TOP**})) {
                    // Scheduled time will come today only. And it will come before the current schedule deck.
                    // Updating data in map
                    // Replacing old work manager with new one.
            } else {
                    // Scheduled time will come today. And it will come after the current schedule deck.
                    // Updating data in map
            }

ALGO FOR ALL DECK NOTIFICATION
All deck notification will work in the syncronization with above algo and addintion to this If Thier is no scheduled notification in next 1 hour then also work manager will run after 1 hour and check the all deck status.

How Has This Been Tested?

Tested on samsung galaxy M51(API 31)

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

AnkiDroid/src/main/java/com/ichi2/anki/DeckMetaDataPreference.kt

AnkiDroid/build.gradle

AnkiDroid/src/main/java/com/ichi2/anki/DeckMetaDataPreference.kt

AnkiDroid/src/main/java/com/ichi2/anki/services/DeckMetaDataService.kt

david-allison · 2022-06-01T12:01:37Z

AnkiDroid/src/main/AndroidManifest.xml

@@ -377,6 +377,11 @@

        <!-- Service to perform web API queries -->
        <service android:name="com.ichi2.widget.AnkiDroidWidgetSmall$UpdateService" />
+        <service


Why does this need to be a service? It's not in the diagram

Yes, I did'nt mentioned that I will use service. But when I did some reserch on the best possible way to fulfil our requirements. Then I came to know that 'foreground service' will be a best fit for our case. I will run in background [on main thread] without disturbing the flow of our application.

Intuitively, I believe that WorkManager would be a better fit, what are the downsides of this? Android has a lot of issues with both Foreground and Background services in general due to energy constraints.

what are the downsides of this?

Their is not major downside. I implemented the work manager too. Please check now.

For the context, the commit that was commented and discussed here is 257bb94

prateek-singh-3212 · 2022-06-02T18:07:17Z

Please check this video for clarification. (for now I am starting this service when app starts and when onDestroy() of activity is called. But I want to change this in future.)

WhatsApp.Video.2022-06-02.at.11.31.33.PM.mp4

Snip of deck meta data [Shared Preference]...

creating json with this model class ⬇️
https://github.com/prateek-singh-3212/Anki-Android/blob/257bb949313aaca19c954100c90e185631996fe8/AnkiDroid/src/main/java/com/ichi2/anki/services/DeckMetaDataService.kt#L113

Json body...

{
  "deckName": "dtc",
  "did": 1621523610764,
  "eta": 0,
  "lrn": 0,
  "new": 0,
  "rev": 2
}

PS You can check logs by using tag META

david-allison · 2022-06-03T00:22:00Z

Tests are falling

prateek-singh-3212 · 2022-06-03T09:10:44Z

@david-allison Fixed the Issue. Kindly check now.

Akshay0701 · 2022-06-04T17:08:57Z

conflict is still there

AnkiDroid/src/main/AndroidManifest.xml

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.java

AnkiDroid/src/main/java/com/ichi2/anki/DeckMetaDataPreference.kt

AnkiDroid/src/main/java/com/ichi2/anki/worker/DeckMetaDataWorker.kt

prateek-singh-3212 · 2022-06-04T18:52:33Z

@david-allison I was unable to found why tests are failing. Could you please help?

AnkiDroid/build.gradle

mikehardy · 2022-06-04T20:25:08Z

What the heck? Nothing but this over and over and over

AbstractFlashcardViewerKeyboardInputTest > spaceDoesNotShowAnswerIfTextFieldFocused FAILED
    java.lang.UnsatisfiedLinkError: 'long net.ankiweb.rsdroid.NativeMethods.openBackend(byte[])'
        at net.ankiweb.rsdroid.NativeMethods.openBackend(Native Method)
        at net.ankiweb.rsdroid.BackendV1Impl.ensureBackend(BackendV1Impl.java:91)
        at net.ankiweb.rsdroid.BackendV1Impl.openAnkiDroidCollection(BackendV1Impl.java:128)
        at net.ankiweb.rsdroid.BackendMutex.openAnkiDroidCollection(BackendMutex.java:977)
        at net.ankiweb.rsdroid.BackendUtils.openAnkiDroidCollection(BackendUtils.java:29)
        at net.ankiweb.rsdroid.database.RustV11SupportSQLiteOpenHelper.createRustSupportSQLiteDatabase(RustV11SupportSQLiteOpenHelper.java:45)
        at net.ankiweb.rsdroid.database.RustSupportSQLiteOpenHelper.getWritableDatabase(RustSupportSQLiteOpenHelper.java:69)
        at com.ichi2.libanki.DB.<init>(DB.java:80)
        at com.ichi2.libanki.backend.RustDroidBackend.openCollectionDatabase(RustDroidBackend.kt:42)
        at com.ichi2.libanki.Storage.Collection(Storage.java:97)
        at com.ichi2.anki.CollectionHelper.openCollection(CollectionHelper.java:138)
        at com.ichi2.anki.CollectionHelper.getCol(CollectionHelper.java:157)
        at com.ichi2.anki.CollectionHelper.getCol(CollectionHelper.java:128)
        at com.ichi2.anki.CollectionHelper.getTimeSafe(CollectionHelper.java:190)
        at com.ichi2.anki.worker.DeckMetaDataWorker$Companion.setupNewWorker(DeckMetaDataWorker.kt:[112](https://github.com/ankidroid/Anki-Android/runs/6739991733?check_suite_focus=true#step:8:113))
        at com.ichi2.anki.AnkiDroidApp.setupDeckMetaDataWorker(AnkiDroidApp.java:443)
        at com.ichi2.anki.AnkiDroidApp.onCreate(AnkiDroidApp.java:236)
        at org.robolectric.android.internal.AndroidTestEnvironment.lambda$installAndCreateApplication$2(AndroidTestEnvironment.java:350)
        at org.robolectric.util.PerfStatsCollector.measure(PerfStatsCollector.java:86)
        at org.robolectric.android.internal.AndroidTestEnvironment.installAndCreateApplication(AndroidTestEnvironment.java:350)
        at org.robolectric.android.internal.AndroidTestEnvironment.lambda$createApplicationSupplier$0(AndroidTestEnvironment.java:229)
        at org.robolectric.util.PerfStatsCollector.measure(PerfStatsCollector.java:53)
        at org.robolectric.android.internal.AndroidTestEnvironment.lambda$createApplicationSupplier$1(AndroidTestEnvironment.java:226)
        at com.google.common.base.Suppliers$NonSerializableMemoizingSupplier.get(Suppliers.java:167)
        at org.robolectric.RuntimeEnvironment.getApplication(RuntimeEnvironment.java:71)
        at org.robolectric.android.internal.AndroidTestEnvironment.setUpApplicationState(AndroidTestEnvironment.java:194)
        at org.robolectric.RobolectricTestRunner.beforeTest(RobolectricTestRunner.java:325)
        at org.robolectric.internal.SandboxTestRunner$2.lambda$evaluate$0(SandboxTestRunner.java:265)
        at org.robolectric.internal.bytecode.Sandbox.lambda$runOnMainThread$0(Sandbox.java:88)
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1[128](https://github.com/ankidroid/Anki-Android/runs/6739991733?check_suite_focus=true#step:8:129))
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
        at java.base/java.lang.Thread.run(Thread.java:829)

Does the ankidroid backend use a provider also, and this is disrupting it?

david-allison · 2022-06-05T11:49:03Z

Does the ankidroid backend use a provider also, and this is disrupting it?

No

Typical initialization is:

Anki-Android/AnkiDroid/src/main/java/com/ichi2/libanki/backend/DroidBackendFactory.kt

Lines 45 to 50 in 599c505

    
           try { 
        
               backendFactory = BackendFactory.createInstance() 
        
           } catch (e: RustBackendFailedException) { 
        
               Timber.w(e, "Rust backend failed to load - falling back to Java") 
        
               CrashReportService.sendExceptionReport(e, "DroidBackendFactory::getInstance") 
        
           }

https://github.com/ankidroid/Anki-Android-Backend/blob/0c095aa8eb6f6b72bd88661d44fdf9a630f34f45/rsdroid/src/main/java/net/ankiweb/rsdroid/BackendV11Factory.java#L29-L33

My first place to look would be to see if we're initialising anki-android-backend-testing library correctly:

Anki-Android/AnkiDroid/src/test/java/com/ichi2/anki/RobolectricTest.kt

Lines 92 to 120 in 8741aae

    
           // Allow an override for the testing library (allowing Robolectric to access the Rust backend) 
        
           // This allows M1 macs to access a .dylib built for arm64, despite it not existing in the .jar 
        
           val backendPath = System.getenv("ANKIDROID_BACKEND_PATH") 
        
           if (backendPath != null) { 
        
               if (BuildConfig.BACKEND_VERSION != System.getenv("ANKIDROID_BACKEND_VERSION")) { 
        
                   throw java.lang.IllegalStateException( 
        
                       "AnkiDroid backend testing library requires an update.\n" + 
        
                           "Please update the library at '$backendPath' from https://github.com/ankidroid/Anki-Android-Backend/releases/ (v ${System.getenv("ANKIDROID_BACKEND_VERSION")})\n" + 
        
                           "And then set \$ANKIDROID_BACKEND_VERSION to ${BuildConfig.BACKEND_VERSION}\n" + 
        
                           "Error: \$ANKIDROID_BACKEND_VERSION: expected '${BuildConfig.BACKEND_VERSION}', got '${System.getenv("ANKIDROID_BACKEND_VERSION")}'" 
        
                   ) 
        
               } 
        
               // we're the right version, load the library from $ANKIDROID_BACKEND_PATH 
        
               RustBackendLoader.loadRsdroid(backendPath) 
        
           } else { 
        
               // default (no env variable): Extract the backend testing lib from the jar 
        
               try { 
        
                   RustBackendLoader.init() 
        
               } catch (e: UnsatisfiedLinkError) { 
        
                   if (e.message.toString().contains("arm64e")) { 
        
                       // Giving the commands to user to add the required env variables 
        
                       val exception = "Please download the arm64 dylib file from https://github.com/ankidroid/Anki-Android-Backend/releases/tag/${BuildConfig.BACKEND_VERSION} and add the following environment variables to your device by using following commands: \n" + 
        
                           "export ANKIDROID_BACKEND_PATH=\"{Path to the dylib file}\"\n" + 
        
                           "export ANKIDROID_BACKEND_VERSION=\"${BuildConfig.BACKEND_VERSION}\"" 
        
                       throw IllegalStateException(exception, e) 
        
                   } 
        
                   throw e 
        
               } 
        
           }

I'd also say: we're adding a dependency on a valid collection inside AnkiDroidApp.

We need to handle if a collection is corrupt etc...

david-allison · 2022-06-05T11:52:19Z

@Arthur-Milchior, this is probably the time to decouple Time from the collection

AnkiDroid/src/main/java/com/ichi2/anki/NotificationHelper.kt

Arthur-Milchior

I've finally read all of this.

For the next code you upload for us to review, please add javadoc on every single method, every single parameter of method, every single class member. Write every assumption you make.

This would generally be more than what is required. But you have a strong tendency to add not enough documentation, even after I asked in the first reviews. And this means that reviewing is extremely slow because I've got to guess what you want to be doing.
I spent almost my entire week-end on this PR. And I won't do it again.

And, on the opposite side, I'm not asking you to write comment for each line of code. Only do it when doing so add informations that is not already clear to someone knowing how to read Kotlin.

Finally, now that I've read everything, I can give more feedback on the architecture.
One thing I fear is that testing will be hard.
All of the code I see now seems to assume we already know the list of deck for which we must send a notification. Even when you recalibrate, you don't check whether there are new decks to add.
I'd prefer very strongly if a function, maybe recalibrate, was in charge of restarting everything from scratch. Because the nice thing with starting things from scratch is that we won't have any problem with debugging; indeed, in case of bug we erase all data and we restart. We won't have problem with test where a failed first test impact a second test.
I don't know how hard such a change would be. but it would be really helpful.

I'm not approving for two reasons.
First: if I'm right Exception Occurred during fetching of data." will send far too many crash report for us to be able to accept it
Second: I really want more texts in all commit messages. And that's not something that can be done after the PR is merged. We can correct most documentation, but the commit message will stay forever. And given how important each commit is, I care about having a lot of explanations about what is done from the user point of view, from the developer using your library point of view, from the point of view of the data store, of the notification system, etc...

Arthur-Milchior · 2022-08-15T15:52:59Z

AnkiDroid/src/main/java/com/ichi2/anki/NotificationDatastore.kt

+                "Exception Occurred during fetching of data."
+            )
+            throw Exception("Unable to find schedule data of given deck id: $did")
+        }.firstOrNull()


I'm confused by why first arrives at the end. It seems usuless to use map above if we only consider the first element and are never filtering.
I'll admit it matters little because it's a flow, so the map will only be executed on the first element. Still, that's useless cost to create the map object if know we use it only once at most

Changed this thanks for pointing this out

Implicitly, I was expecting this remark to be valid for all "map followed by firstOrNull"

AnkiDroid/src/main/java/com/ichi2/anki/NotificationDatastore.kt

Arthur-Milchior · 2022-08-15T16:00:09Z

AnkiDroid/src/main/java/com/ichi2/anki/NotificationDatastore.kt

+    * We actually are not blocking the thread. This method throws an exception. It will not create problem for us.
+    */
+    @Suppress("BlockingMethodInNonBlockingContext")
+    suspend fun getDeckSchedData(did: Long): DeckNotification? {


Can you add todo for tests. I realize now I'm not clear enough about when is null and when are exception expected. Which is kind of a problem with a catch around the whole expression.

I guess I'd do something such as

/** * Fetches the details of particular deck scheduling. * @return Deck Notification model for particular deck. * */ /* * We actually are not blocking the thread. This method throws an exception. It will not create problem for us. * TODO: unit test that : * * if there is no preference at all, we return null * * if there is a preference without entry for this key we return null * * if there is a preference whose entry for this key can't be cast to DeckNotification, throw * * if there is a preference with entry for this key that can be cast, we get expected notification */ @Suppress("BlockingMethodInNonBlockingContext") suspend fun getDeckSchedData(did: Long): DeckNotification? { val deckPreferenceKey = stringPreferencesKey(did.toString()) val latestPreference = context.notificationDatastore.data.firstOrNull() ?: return null val deckPreferences = latestPreference[deckPreferenceKey] ?: return null try { return objectMapper.readValue(deckPreferences, DeckNotification::class.java) } catch (ex: JsonProcessingException) { // Let the exception throw CrashReportService.sendExceptionReport( ex, "Notification Datastore-getDeckSchedData", "Exception Occurred during fetching of data." ) throw Exception("Unable to find schedule data of given deck id: $did", ex) } }

instead

@Arthur-Milchior Added. I think the below situation will not occur because the JSON format will be correct almost every time because we are serializing and deserializing JSON using jackson.
* * if there is a preference whose entry for this key can't be cast to DeckNotification, throw

Please don't resolve when you answer me. Because that make it very hard to see what you answer, I have to open the 42 comments I made to check which one have answer

There is sadly always the risk of a corruption inside the database. It's really small indeed, but there are millions of smartphone, and this may be accessed every hours over years potentially, so on the scale of big numbers, it's not totally impossible.
I agree it should be pretty rare

AnkiDroid/src/main/java/com/ichi2/anki/worker/NotificationWorkManager.kt

AnkiDroid/src/main/java/com/ichi2/anki/receiver/TimeZoneChangeReceiver.kt

AnkiDroid/src/main/res/values/02-strings.xml

AnkiDroid/src/main/java/com/ichi2/anki/worker/NotificationWorkManager.kt

Arthur-Milchior · 2022-08-20T09:07:59Z

Linter has a real (and simple to solve) failure

Task :AnkiDroid:ktlintMainSourceSetCheck FAILED
/home/runner/work/Anki-Android/Anki-Android/AnkiDroid/src/main/java/com/ichi2/anki/worker/NotificationWorkManager.kt:19:1 Imports must be ordered in lexicographic order without any empty lines in-between with "java", "javax", "kotlin" and aliases in the end

prateek-singh-3212 · 2022-08-20T17:13:17Z

@ankidroid/gsoc-mentor Done with checks.

AnkiDroid/src/main/java/com/ichi2/anki/worker/NotificationWorkManager.kt

Arthur-Milchior

Thanks for all of those changes.

A few general remarks.

Regarding commit message.

Usually, commit messages starts with a one-line title. That's what's assumed by most GUI, in particular github. That makes long line not displayed properly

Then, there should be a message. Most commit do not change a lot of the behavior of the app, so often just the title is fine.
However, you're writing non trivial changes, adding new feature to the codebase and even new features. So it's relatively important for you to put potentially multiple paragraph by commit message. First because the reviewer know what to expect when they'll review the commit. Secondly because a future reader who wants to understand why a change was done and look at history later, should be able to undersatnd the context without reading the whole PR discussion.

what to do while waiting

While I still require a lot of changes, for example due to still one daylight saving bug, the behavior is now stable enough that you can still use this code and build on top of it without risking too much to have your work be useless.

So, while you wait for a reviewer to answer you, which I do as fast as I can, but not always easy due to the length, I would like to be sure you know what to do.

you can probably already add tests. I really would love to see tests, because that ensure that I finally am certain of what's the expected behavior. Plus, that'll force us to see what is required to create the test. For example, to ensure that when you start a new test, datastore is empty (and ideally that we only empty it at end of a test that have some datastore behavior, so that we don't pay extra cost in test)
You can also start already the UI to display and add decks to the data store. I suspect that you can do most of the UI with only the first two commits merged. So maybe it will be worth having a PR that only deals with Datastore. It can hopefully be merged faster (since it'll be quicker to review and we already agree on most of what should go in there). So that the current PR will only get 5 commits, and the UI can be done on top of it already
or whatever you consider the next step, I believe you can do it on top of this PR, assuming it won't change much, and even if it requires this entire PR to be merged, you can always put it in a draft PR

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.java

AnkiDroid/src/main/java/com/ichi2/anki/NotificationDatastore.kt

AnkiDroid/src/main/java/com/ichi2/anki/model/DeckNotification.kt

AnkiDroid/src/main/java/com/ichi2/anki/worker/NotificationWorkManager.kt

Arthur-Milchior · 2022-08-21T16:57:52Z

AnkiDroid/src/main/java/com/ichi2/anki/NotificationHelper.kt

+                    deckData.schedMinutes,
+                    newTimeDeckData
+                )
+                newTimeDeckData.append(deckData.did, notificationData.scheduleTime)


If you are recreating, why are you iterating over listOfDeck = oldTimeDeckData.values.flatten() instead of iterating over all top level decks.
Because you are almost recreating it from scratch. Except that any deck that currently had no notification still won't have any notification. Which is a shame, because it would answer the request that @david-allison and I had, to have at least one method to entirely reset the state from scratch, which would be great for testing purpose for example

AnkiDroid/src/main/java/com/ichi2/anki/NotificationHelper.kt

AnkiDroid/src/main/java/com/ichi2/anki/receiver/TimeZoneChangeReceiver.kt

AnkiDroid/src/main/java/com/ichi2/anki/NotificationHelper.kt

Arthur-Milchior · 2022-08-29T18:45:07Z

@prateek-singh-3212 you got merge conflict, sorry

Arthur-Milchior

I've done the correction myself, since they don't fundamentally change your PR, so that we can move on to other things.

Arthur-Milchior · 2022-08-29T20:52:34Z

AnkiDroid/src/main/java/com/ichi2/anki/NotificationDatastore.kt

+                    it[datastoreKey],
+                    DeckNotification::class.java
+                )
+            } catch (ex: Exception) {


Is there a specific exception you want to catch?
If contains fail, I think it's okay to fail the program, something goes so wrong that it's not worth taking this into consideration.
If contains return true but we can't actually access the data, same.
So I'd put everything that consists in accessing the data outside the try, and just let the reading of the value in the try since it's the only one where it slightly makes sense to eventually expect an exception, after all, we are parsing text.

Arthur-Milchior · 2022-08-29T20:55:08Z

AnkiDroid/src/main/java/com/ichi2/anki/NotificationDatastore.kt

+        val datastoreKey = stringPreferencesKey(did.toString())
+        return context.notificationDatastore.data.firstOrNull()?.let {
+            try {
+                if (!it.contains(datastoreKey)) {


[] returns null if the value is absent. So it's not useful to do a cortains then a [].
Instead, you can just do [] and check whether value is null or not. That would avoid a search through the map.

Arthur-Milchior · 2022-08-29T20:56:12Z

AnkiDroid/src/main/java/com/ichi2/anki/NotificationDatastore.kt

+
+    /**
+     * Stores the details of the [notification] scheduling of deck [did]
+     * @return operation successful of not.


"is successful". And "of" should be "or".
Also, simply "whether operation is successful" indicates that it can be true or false.

Arthur-Milchior · 2022-08-29T21:02:05Z

AnkiDroid/src/main/java/com/ichi2/anki/worker/NotificationWorker.kt

+ * It will run at most in one hour. Sooner if we'll have notification to send earlier @see [getTriggerTime].
+ * If the device is switched off at the time of notification then Notification work manager will run whenever
+ * devices gets turned on and it will fire all the previous pending notifications.
+ * If their is no deck notification within 1 hour then also notification work manager will run after 1 hour to trigger all deck notification.


Arthur-Milchior · 2022-08-29T21:16:04Z

AnkiDroid/src/main/java/com/ichi2/anki/worker/NotificationWorker.kt

+            timeDeckData.remove(it.key)
+            calendar.timeInMillis = it.key.toLong()
+//            TODO: We should ensure that we only plan the next notification for a time in the future.
+            timeDeckData[ONE_DAY_MS(calendar).toString()] = it.value


ONE_DAY_MS is a really strange name. Not the way we call functions usually.
Plus, it has a side effect that is not documented, it change calendars.

Also, it's really strange to take a calendar and return a long.
I understand that you don't want to take a long, because you are reusing the same calendar over and over which saves time, so that's great.

Since it's used a single time, I find this method too unatural to be worth it.

calendar.add(Calendar.DAY_OF_YEAR, 1) above would suffice

AnkiDroid/src/main/java/com/ichi2/anki/NotificationHelper.kt

Arthur-Milchior · 2022-08-29T21:30:16Z

AnkiDroid/src/main/java/com/ichi2/anki/NotificationHelper.kt

+     * Cancel/Removes the deck id from the [NotificationTodo]. To avoid notification triggering.
+     * */
+    fun removeDeckNotification(vararg deckIds: Long) {
+        CoroutineScope(Dispatchers.Default).launch {


Please don't resolve without correcting or explaining why you don't

Arthur-Milchior · 2022-08-29T21:31:43Z

AnkiDroid/src/main/java/com/ichi2/anki/NotificationHelper.kt

+    }.build()
+
+    /**
+     * Work that needs to be so that notification should trigger.


to be done

So that notification CAN trigger.

It already should, even if no work is done. Simply, it can't trigger if the work is not done

Arthur-Milchior · 2022-08-29T21:44:22Z

AnkiDroid/src/main/java/com/ichi2/anki/worker/NotificationWorker.kt

@@ -168,11 +194,32 @@ class NotificationWorker(val context: Context, workerParameters: WorkerParameter
            totalDueCount.addRev(it.revCount)
        }

+        // Creates an explicit intent for an DeckPiker Activity.


You don't need to indicate you "create". Just indicate what's the result in this variable, and only if the variable can't indicate explicitly.
So no comment and deckPickerIntent should be sufficient

Arthur-Milchior · 2022-08-29T21:45:57Z

AnkiDroid/src/main/java/com/ichi2/anki/worker/NotificationWorker.kt

        if (totalDueCount.count() < minCardsDue) {
            // Due card limit is higher.
            return
        }
-        // TODO: Build & Fire all deck notification.
+
+        // Build the notification


Same here, I don't believe the comment add anything that the variable name don't already indicate.

This commit simply adds all the tools we'll need to use datastore in workers. Gradle, manifest, and changing AnkidroidApp

Notification preference datastore is used to store all the data related to notification in the preference datastore. We both introduce helper methods to deal with primitive type, and an ad-hoc type to represents notification times for each decks in a way that is simple to store in the datastore and efficient to process.

* Worker to fire deck notification which is pending at time of execution of worker. * This worker also fire all deck notification when the due card is greater than min due cards. * After completion of work next worker is scheduled and time of next worker is calculated based upon the NotificationTodo data. * Also cleans decks that are not at top level anymore, so that should not leads to notification.

This is used in particular for Timezone Change broadcast receiver. Ensure notification are adapted to local timezone instead of absolute time. This is required because Android store notification in GMT time.

When AnkiDroid starts, notification worker will start if not scheduled previously. Note that currently we can't add any deck in the datastore, so no notification will occur until we actually start to add data.

github-actions · 2023-01-09T14:41:35Z

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

prateek-singh-3212 changed the title ~~Implemented Service to store Deck Meta Data~~ [GSOC] Implemented Service to store Deck Meta Data May 31, 2022

Akshay0701 reviewed Jun 1, 2022

View reviewed changes

AnkiDroid/src/main/java/com/ichi2/anki/DeckMetaDataPreference.kt Outdated Show resolved Hide resolved

AnkiDroid/build.gradle Outdated Show resolved Hide resolved

david-allison requested changes Jun 1, 2022

View reviewed changes

david-allison reviewed Jun 1, 2022

View reviewed changes

prateek-singh-3212 force-pushed the Sync_Deck_Meta_Data branch from 257bb94 to c550746 Compare June 2, 2022 18:42

prateek-singh-3212 requested review from david-allison and Akshay0701 June 2, 2022 21:38

prateek-singh-3212 force-pushed the Sync_Deck_Meta_Data branch from c141e7c to c32f6e9 Compare June 2, 2022 21:50

david-allison added the Review High Priority Request for high priority review label Jun 3, 2022

prateek-singh-3212 force-pushed the Sync_Deck_Meta_Data branch from c32f6e9 to f24e9b2 Compare June 3, 2022 08:40

lukstbit mentioned this pull request Jun 3, 2022

Reduce number of notification by caching previous values #11516

Closed

7 tasks

david-allison reviewed Jun 4, 2022

View reviewed changes

prateek-singh-3212 force-pushed the Sync_Deck_Meta_Data branch 2 times, most recently from 8deb0d5 to b0d990a Compare June 4, 2022 18:30

mikehardy reviewed Jun 4, 2022

View reviewed changes

AnkiDroid/build.gradle Outdated Show resolved Hide resolved

prateek-singh-3212 force-pushed the Sync_Deck_Meta_Data branch 4 times, most recently from 5289b36 to fd61fec Compare June 5, 2022 09:57

prateek-singh-3212 requested review from david-allison and mikehardy June 5, 2022 10:40

Arthur-Milchior mentioned this pull request Jun 5, 2022

Decouple Time from the collection #11541

Closed

Arthur-Milchior reviewed Aug 14, 2022

View reviewed changes

prateek-singh-3212 force-pushed the Sync_Deck_Meta_Data branch from 311b010 to 9a0eb65 Compare August 15, 2022 05:34

Arthur-Milchior reviewed Aug 15, 2022

View reviewed changes

dae reviewed Aug 18, 2022

View reviewed changes

AnkiDroid/src/main/java/com/ichi2/anki/worker/NotificationWorkManager.kt Outdated Show resolved Hide resolved

prateek-singh-3212 force-pushed the Sync_Deck_Meta_Data branch from 9a0eb65 to 7fa9e0f Compare August 19, 2022 21:18

prateek-singh-3212 force-pushed the Sync_Deck_Meta_Data branch from 7fa9e0f to 29c8a68 Compare August 20, 2022 16:36

dae reviewed Aug 21, 2022

View reviewed changes

AnkiDroid/src/main/java/com/ichi2/anki/worker/NotificationWorkManager.kt Outdated Show resolved Hide resolved

Arthur-Milchior reviewed Aug 21, 2022

View reviewed changes

prateek-singh-3212 force-pushed the Sync_Deck_Meta_Data branch 3 times, most recently from 72ad276 to 0808c37 Compare August 24, 2022 09:44

Arthur-Milchior force-pushed the Sync_Deck_Meta_Data branch 2 times, most recently from 59531bc to 3a8cced Compare August 29, 2022 21:48

Arthur-Milchior approved these changes Aug 29, 2022

View reviewed changes

Integrated WorkManager & Datastore Preference Library.

30684d9

This commit simply adds all the tools we'll need to use datastore in workers. Gradle, manifest, and changing AnkidroidApp

prateek-singh-3212 mentioned this pull request Aug 29, 2022

[GSoC] Implemented Notification Datastore and Work Manager #12234

Closed

5 tasks

prateek-singh-3212 and others added 6 commits August 29, 2022 23:53

Implemented NotificationHelper to Manage all Notifications

328e52e

Implemented Calibrated notification.

a7a1c60

This is used in particular for Timezone Change broadcast receiver. Ensure notification are adapted to local timezone instead of absolute time. This is required because Android store notification in GMT time.

Integrated NotificationWorkManager with NotificationHelper

44907df

Established Connection of Notification WorkManager in AnkiDroidApp

59023e5

When AnkiDroid starts, notification worker will start if not scheduled previously. Note that currently we can't add any deck in the datastore, so no notification will occur until we actually start to add data.

Arthur-Milchior force-pushed the Sync_Deck_Meta_Data branch from 3a8cced to 59023e5 Compare August 29, 2022 21:58

prateek-singh-3212 mentioned this pull request Nov 1, 2022

[GSoC] Global notification #12766

Closed

5 tasks

github-actions bot added the Stale label Jan 9, 2023

github-actions bot closed this Jan 16, 2023

[GSOC] Implemented Service to store Deck Meta Data #11487

[GSOC] Implemented Service to store Deck Meta Data #11487

Conversation

prateek-singh-3212 commented May 31, 2022 • edited Loading

Pull Request template

Purpose / Description

Fixes

Approach

How Has This Been Tested?

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prateek-singh-3212 commented Jun 2, 2022 • edited Loading

david-allison commented Jun 3, 2022

prateek-singh-3212 commented Jun 3, 2022

Akshay0701 commented Jun 4, 2022

prateek-singh-3212 commented Jun 4, 2022

mikehardy commented Jun 4, 2022

david-allison commented Jun 5, 2022 • edited Loading

david-allison commented Jun 5, 2022

Arthur-Milchior left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prateek-singh-3212 Aug 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Arthur-Milchior commented Aug 20, 2022

prateek-singh-3212 commented Aug 20, 2022

Arthur-Milchior left a comment

Choose a reason for hiding this comment

Regarding commit message.

what to do while waiting

Choose a reason for hiding this comment

Arthur-Milchior commented Aug 29, 2022

Arthur-Milchior left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 9, 2023

prateek-singh-3212 commented May 31, 2022 •

edited

Loading

prateek-singh-3212 commented Jun 2, 2022 •

edited

Loading

david-allison commented Jun 5, 2022 •

edited

Loading

prateek-singh-3212 Aug 18, 2022 •

edited

Loading