Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revamped notification service #118

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ishanvaghani
Copy link

@ishanvaghani ishanvaghani commented Dec 30, 2023

Description

Revamped Notification Service

  • Rewrote logic for scheduling/canceling/rescheduling notifications.
  • Notification permission flow handled.
  • Scheduling notification for next 2 weeks.
  • Canceling notification for that day after recording/uploading video.

Checklist

Before you create this PR, please confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I have added my name in the CONTRIBUTORS.md file, if it wasn't already present.
  • I have added a description of the change in CHANGELOG.md.

@@ -91,4 +91,7 @@ flutter {

dependencies {
implementation "org.jetbrains.kotlin:kotlin-stdlib-jdk7:$kotlin_version"
implementation 'androidx.window:window:1.0.0'
implementation 'androidx.window:window-java:1.0.0'
coreLibraryDesugaring 'com.android.tools:desugar_jdk_libs:1.2.2'

Choose a reason for hiding this comment

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

@ishanvaghani why are these needed?
Then, shouldn't we also add coreLibraryDesugaringEnabled true as it says here: https://www.geeksforgeeks.org/desugaring-in-android/

Copy link
Author

Choose a reason for hiding this comment

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

Have missed it. Adding

);
}

void cancelTodayNotification() async {

Choose a reason for hiding this comment

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

@ishanvaghani This is not working properly, this method is executed when adding a video for today, but the notification shows up (when it should be cancelled).

Future<void> rescheduleNotification(DateTime day) async {
final TimeOfDay timeOfDay = getScheduledTime();
await scheduleNotification(timeOfDay.hour, timeOfDay.minute, day);
int getNotificationId() {

Choose a reason for hiding this comment

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

Is there any reason to have a random id?

Copy link
Author

Choose a reason for hiding this comment

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

We are scheduling 2 weeks notifications and storing this in shared pref and canceling notification based on notification id. So for unique identification using random id.

@bianca7dev
Copy link

Hi @ishanvaghani , also, after changing to a profile and back to default profile, it schedule the notification for the day in a hour that has already passed (example: it is 5pm, the notification time in the settings is 10am, I changed the profiles and it scheduled for today at 10am.)

@ishanvaghani
Copy link
Author

@bianca7dev I have pushed one fix can you check notification is still coming after uploading video? secondly also check for changing profile still schedules invalid notification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants