Skip to content

feat(#9255): adds androidApi service to calculate task notifications#9987

Closed
jonathanbataire wants to merge 53 commits intomedic:masterfrom
jonathanbataire:9255-notifications
Closed

feat(#9255): adds androidApi service to calculate task notifications#9987
jonathanbataire wants to merge 53 commits intomedic:masterfrom
jonathanbataire:9255-notifications

Conversation

@jonathanbataire
Copy link
Copy Markdown
Contributor

@jonathanbataire jonathanbataire commented May 26, 2025

Description

  • Adds a task notification service that returns task notifications to allow notification within the web app or android app
  • Adds new androidApi method to allow cht-android access these notifications. Android app access this API here
  • Adds new translation key android.notification.tasks.contentText for translating notification content text.

fixes #9255

Code review checklist

  • UI/UX backwards compatible: Test it works for the new design (enabled by default). And test it works in the old design, enable can_view_old_navigation permission to see the old design. Test it has appropriate design for RTL languages.
  • Readable: Concise, well named, follows the style guide, documented if necessary.
  • Documented: Configuration and user documentation on cht-docs
  • Tested: Unit and/or e2e where appropriate
  • Internationalised: All user facing text
  • Backwards compatible: Works with existing data and configuration or includes a migration. Any breaking changes documented in the release notes.

License

The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.

@jonathanbataire jonathanbataire marked this pull request as ready for review June 4, 2025 06:20
@jonathanbataire
Copy link
Copy Markdown
Contributor Author

Hey @andrablaj this ready for review :)

@andrablaj
Copy link
Copy Markdown
Member

@dianabarsan, would you be available to review this PR? If not, please reach out to/assign other teammates who might be available. Thank you!

@andrablaj andrablaj requested a review from dianabarsan June 5, 2025 09:27
Copy link
Copy Markdown
Member

@dianabarsan dianabarsan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great start and awesome work! I have some questions and suggestions inline. Please let me know if you have any questions.


let notifications = taskDocs
.filter(task => {
return task.state === 'Ready' && task.emission.dueDate === today &&
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about tasks that are overdue? I'm curious what this system is really trying to achieve.
If a user doesn't log in for one day, when a specific task is due, and logs in the next day when the task is overdue, they never get notified for it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i see that's one scenario that wasn't considered
yeah we can add overdue tasks

Copy link
Copy Markdown
Member

@dianabarsan dianabarsan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very clean work!

@dianabarsan
Copy link
Copy Markdown
Member

@lorerod Do you have some time to acceptance test this new feature? There is a corresponding cht-android PR: medic/cht-android#394 but that seems to not have been reviewed yet, and is required to fully acceptance test this.

@lorerod
Copy link
Copy Markdown
Contributor

lorerod commented Aug 4, 2025

Thanks for looping me in, @dianabarsan !
Happy to help with acceptance testing, but as you mentioned, the Android PR still looks like a work in progress. Since that part is required to fully test the feature, I’ll hold off until it's ready and integrated. I’ll be keeping an eye on the status of the Android PR so I can jump in once it’s ready for testing.

@github-actions
Copy link
Copy Markdown

This PR is now marked "stale" after 30 days without activity. It will be closed automatically in 10 days unless you add a comment, push new changes or remove the "stale" label.

@jkuester
Copy link
Copy Markdown
Contributor

Are we collecting any telemetry data about how often we are checking for notifications? Basically, I would like to be able to tell from telemetry if notifications-checks are happening regularly throughout the day. (Looking for measurable data to be able to tell if the background notification checks in cht-android are happening as expected...) I did not see anything in the TasksNotificationService, but maybe I missed something... 🤔

@jkuester
Copy link
Copy Markdown
Contributor

jkuester commented Oct 1, 2025

On a similar note to ☝️, @jonathanbataire @dianabarsan what do you think about exposing the TelemetryService.record function on the AndroidApiService? This would let us push telemetry info from the Android app (as long as we can load the webapp in the WebView...). Maybe this should be a separate PR, though... 🤔 Let me know what you think and maybe I can get some new tickets logged around this. (Might make some "Good First Issues"...)

@jkuester
Copy link
Copy Markdown
Contributor

jkuester commented Oct 7, 2025

@dianabarsan had an interesting though when we were discussing this functionality today and I wanted to continue the conversation here in case it is worth a pivot.

The general idea is that the fetchNotifications() function could pre-load future tasks and return them to the cht-android app. So, the function could return tasks that were currently due and tasks that will be due in the future (for some set time window). Then, on the cht-android side, we could take the future tasks and cache them for future notifications.

Then, the background worker in cht-android would not need a WebView/webapp to calculate the current notifications. When the cht-android app is closed, the notifications would only be powered based on the pre-loaded future notifications. This seems potentially viable since, in most cases, the user's data that would affect tasks is not be changing unless the user actually opens the app. So, future tasks should remain accurate enough.

I do have a couple questions here:

  1. When we calculate future tasks (by giving a future date to the rules engine), that will still result in new task docs getting written to the DB for those future tasks, right?
  2. If the data in the DB changes between the time we calculate the task and the time the task is actually relevant such that the task is no longer relevant, will it be automatically resolved/deleted? (I think so, right?)
  3. Will suddenly pre-calculating a bunch of tasks trigger another task doc explosion? 😬 (Seems like it will add a bit of initial load, but if we keep a reasonably short window it should not be a huge problem...)

@dianabarsan
Copy link
Copy Markdown
Member

Thanks @jkuester !
I tagged @jonathanbataire in a similar comment here: medic/cht-android#394 (comment)

Just so we don't have the same conversation in multiple threads

@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 8, 2025

This PR is now marked "stale" after 30 days without activity. It will be closed automatically in 10 days unless you add a comment, push new changes or remove the "stale" label.

@github-actions github-actions bot added the Stale label Nov 8, 2025
@github-actions github-actions bot closed this Nov 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enable android notifications for task reminders/alerts

5 participants