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

‼️📅 Rewrite - notifScheduler.ts #1092

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
4abe86d
Add notifScheduler.ts
sebastianbarry Oct 31, 2023
da87f47
Merge branch 'service_rewrite_2023' into notifScheduler-rewrite
sebastianbarry Nov 2, 2023
1bfb113
Merge branch 'service_rewrite_2023' into notifScheduler-rewrite
sebastianbarry Nov 2, 2023
131eca5
Format notifScheduler.ts
sebastianbarry Nov 2, 2023
cc7664f
Merge branch 'service_rewrite_2023' into notifScheduler-rewrite
sebastianbarry Nov 2, 2023
39986a3
Temporary any type, handled purely in notifScheduler getReminderPrefs
sebastianbarry Nov 2, 2023
58539ca
Undoing imports getting duplicated during prettier changes
sebastianbarry Nov 2, 2023
384ffe8
Replace console.log with logger.ts's displayErrorMsg
sebastianbarry Nov 2, 2023
9d3c718
Move scheduledNotifs into the only function that uses it
sebastianbarry Nov 3, 2023
a1e035f
Remove console.errors that I was using for debugging
sebastianbarry Nov 3, 2023
8b09cb3
Adding Luxon functions
sebastianbarry Nov 3, 2023
96d1d50
Update notifscheduler to export functions instead of using hooks
sebastianbarry Nov 3, 2023
a6d99e7
Run prettier on non-pretty code
sebastianbarry Nov 3, 2023
b0da6e9
Remove old Angular notifScheduler.js
sebastianbarry Nov 7, 2023
6b9d222
Add the reminderSchemes as an argument to called getReminderPrefs
sebastianbarry Nov 7, 2023
97571ee
Added typing for User object in getReminderPrefs
sebastianbarry Nov 7, 2023
a29705d
Restructure promise format and replaced moment with Luxon
sebastianbarry Nov 7, 2023
9b4b1d0
Merge branch 'service_rewrite_2023' of https://github.com/e-mission/e…
JGreenlee Nov 9, 2023
ba239e6
Resolve the app crashing errors
sebastianbarry Nov 9, 2023
2f75de9
Remove unnecessary imports
sebastianbarry Nov 10, 2023
5e572d4
notif-scheduling-state variables kept in ProfileSettings.jsx and hand…
sebastianbarry Nov 14, 2023
db91c8c
Remove empty objects from notifs, fixing the "n.trigger.at does not e…
sebastianbarry Nov 14, 2023
e4817da
Added removeEmptyObjects check to the other cordova plugin getSchedul…
sebastianbarry Nov 14, 2023
0ee468a
Added sorting to list of notifications right before they get scheduled
sebastianbarry Nov 16, 2023
e209f10
Change static variable isScheduling to a useState
sebastianbarry Nov 16, 2023
b8bf1cf
Replace console.logs with logDebugs
sebastianbarry Nov 16, 2023
b27ae9a
Add in more type declarations
sebastianbarry Nov 16, 2023
abbe618
Add Jest Testing - getScheduledNotifs
sebastianbarry Nov 16, 2023
fbd1b62
Add first updateScheduledNotifs test
sebastianbarry Nov 17, 2023
87fdae5
Add reminder scheme missing test for updateScheduledNotifs
sebastianbarry Nov 21, 2023
214081e
Add already scheduled notifs test
sebastianbarry Nov 22, 2023
f5ebb1e
Fixing comments
sebastianbarry Nov 22, 2023
6892580
Final overall test for updateScheduledNotifs
sebastianbarry Nov 24, 2023
e8b449f
Merge branch 'service_rewrite_2023' of https://github.com/e-mission/e…
JGreenlee Nov 30, 2023
85f0519
Added user exists test for getReminderPrefs
sebastianbarry Nov 30, 2023
0528fb0
Remove unnecessary console.log
sebastianbarry Nov 30, 2023
2c0ec40
Fixing moved location of commHelper causing jest tests to fail
sebastianbarry Nov 30, 2023
35ffc41
Merge branch 'service_rewrite_2023' of https://github.com/e-mission/e…
JGreenlee Dec 6, 2023
4e44dba
fix error on configs without reminderSchemes
JGreenlee Dec 6, 2023
5ee4d64
Add getReminderPrefs test for when user does not exist
sebastianbarry Dec 12, 2023
d3750c3
Add simple setReminderPrefs test
sebastianbarry Dec 12, 2023
ec9729d
Add test for updateScheduledNotifs to test if notifs successfully get…
sebastianbarry Dec 15, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion www/js/commHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ export function updateUser(updateDoc) {
});
}

export function getUser() {
export function getUser(): any {
Copy link
Collaborator

Choose a reason for hiding this comment

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

As you said, we'd ideally type this out properly. I understand if need to use any in the short term, but instead of adding an any return type here, I think you should be able to do this at the place where getUser() is being called.

Like so:

const user = getUser() as any;

Let me know if that works!

return new Promise((rs, rj) => {
window['cordova'].plugins.BEMServerComm.getUserPersonalData("/profile/get", rs, rj);
}).catch(error => {
Expand Down
8 changes: 5 additions & 3 deletions www/js/control/ProfileSettings.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { AppContext } from "../App";
import { shareQR } from "../components/QrCode";
import { storageClear } from "../plugin/storage";
import { getAppVersion } from "../plugin/clientStats";
import { useSchedulerHelper } from "../splash/notifScheduler";
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you export update, getScheduledNotifs, getReminderPrefs, and setReminderPrefs directly from notifScheduler.ts, you can import them as individual functions instead of useSchedulerHelper


//any pure functions can go outside
const ProfileSettings = () => {
Expand All @@ -33,6 +34,7 @@ const ProfileSettings = () => {
const appConfig = useAppConfig();
const { colors } = useTheme();
const { setPermissionsPopupVis } = useContext(AppContext);
const schedulerHelper = useSchedulerHelper();

//angular services needed
const CarbonDatasetHelper = getAngularService('CarbonDatasetHelper');
Expand Down Expand Up @@ -173,13 +175,13 @@ const ProfileSettings = () => {
const newNotificationSettings ={};

if (uiConfig?.reminderSchemes) {
const prefs = await NotificationScheduler.getReminderPrefs();
const prefs = await schedulerHelper.getReminderPrefs();
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will become await getReminderPrefs(uiConfig.reminderSchemes)

const m = moment(prefs.reminder_time_of_day, 'HH:mm');
newNotificationSettings.prefReminderTimeVal = m.toDate();
const n = moment(newNotificationSettings.prefReminderTimeVal);
newNotificationSettings.prefReminderTime = n.format('LT');
newNotificationSettings.prefReminderTimeOnLoad = prefs.reminder_time_of_day;
newNotificationSettings.scheduledNotifs = await NotificationScheduler.getScheduledNotifs();
newNotificationSettings.scheduledNotifs = await schedulerHelper.getScheduledNotifs();
updatePrefReminderTime(false);
}

Expand Down Expand Up @@ -243,7 +245,7 @@ const ProfileSettings = () => {
if(storeNewVal){
const m = moment(newTime);
// store in HH:mm
NotificationScheduler.setReminderPrefs({ reminder_time_of_day: m.format('HH:mm') }).then(() => {
schedulerHelper.setReminderPrefs({ reminder_time_of_day: m.format('HH:mm') }).then(() => {
refreshNotificationSettings();
});
}
Expand Down
286 changes: 286 additions & 0 deletions www/js/splash/notifScheduler.ts
shankari marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,286 @@
import angular from 'angular';
import React, { useEffect, useState } from "react";
import { getConfig } from '../config/dynamicConfig';
import useAppConfig from "../useAppConfig";
import { addStatReading, statKeys } from '../plugin/clientStats';
import { getUser, updateUser } from '../commHelper';
import { logDebug } from "../plugin/logger";
import { DateTime } from "luxon";
import i18next from 'i18next';

let scheduledPromise = new Promise<void>((rs) => rs());
let scheduledNotifs = [];
let isScheduling = false;

// like python range()
function range(start, stop, step) {
let a = [start], b = start;
while (b < stop)
a.push(b += step || 1);
return a;
}

// returns an array of moment objects, for all times that notifications should be sent
const calcNotifTimes = (scheme, dayZeroDate, timeOfDay) => {
const notifTimes = [];
for (const s of scheme.schedule) {
// the days to send notifications, as integers, relative to day zero
const notifDays = range(s.start, s.end, s.intervalInDays);
for (const d of notifDays) {
const date = DateTime.fromFormat(dayZeroDate, 'yyyy-MM-dd').plus({ days: d}).toFormat('yyyy-MM-dd')
const notifTime = DateTime.fromFormat(date+' '+timeOfDay, 'yyyy-MM-dd HH:mm');
notifTimes.push(notifTime);
}
}
return notifTimes;
}

// returns true if all expected times are already scheduled
const areAlreadyScheduled = (notifs, expectedTimes) => {
for (const t of expectedTimes) {
if (!notifs.some((n) => DateTime.fromMillis(n.trigger.at).equals(t))) {
return false;
}
}
return true;
}

/* remove notif actions as they do not work, can restore post routing migration */
// const setUpActions = () => {
// const action = {
// id: 'action',
// title: 'Change Time',
// launch: true
// };
// return new Promise((rs) => {
// cordova.plugins.notification.local.addActions('reminder-actions', [action], rs);
// });
// }
function debugGetScheduled(prefix) {
window['cordova'].plugins.notification.local.getScheduled((notifs) => {
if (!notifs?.length)
return logDebug(`${prefix}, there are no scheduled notifications`);
const time = DateTime.fromMillis(notifs?.[0].trigger.at).toFormat('HH:mm');
//was in plugin, changed to scheduler
scheduledNotifs = notifs.map((n) => {
const time = DateTime.fromMillis(n.trigger.at).toFormat('t');
const date = DateTime.fromMillis(n.trigger.at).toFormat('DDD');
return {
key: date,
val: time
}
});
//have the list of scheduled show up in this log
logDebug(`${prefix}, there are ${notifs.length} scheduled notifications at ${time} first is ${scheduledNotifs[0].key} at ${scheduledNotifs[0].val}`);
});
}

//new method to fetch notifications
const getScheduledNotifs = function() {
return new Promise((resolve, reject) => {
/* if the notifications are still in active scheduling it causes problems
anywhere from 0-n of the scheduled notifs are displayed
if actively scheduling, wait for the scheduledPromise to resolve before fetching prevents such errors
*/
if(isScheduling)
{
console.log("requesting fetch while still actively scheduling, waiting on scheduledPromise");
scheduledPromise.then(() => {
getNotifs().then((notifs) => {
console.log("done scheduling notifs", notifs);
resolve(notifs);
})
})
}
else{
getNotifs().then((notifs) => {
resolve(notifs);
})
}
})
}

//get scheduled notifications from cordova plugin and format them
const getNotifs = function() {
return new Promise((resolve, reject) => {
window['cordova'].plugins.notification.local.getScheduled((notifs) => {
if (!notifs?.length){
console.log("there are no notifications");
resolve([]); //if none, return empty array
}

const notifSubset = notifs.slice(0, 5); //prevent near-infinite listing
let scheduledNotifs = [];
scheduledNotifs = notifSubset.map((n) => {
const time = DateTime.fromMillis(n.trigger.at).toFormat('t');
const date = DateTime.fromMillis(n.trigger.at).toFormat('DDD');
return {
key: date,
val: time
}
});
resolve(scheduledNotifs);
});
})
}

// schedules the notifications using the cordova plugin
const scheduleNotifs = (scheme, notifTimes) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can also type notifTimes as an array of DateTimes.
If you had done this before, Typescript could have helped you catch the issue that caused the crash (because Typescript would have known DateTime.toISO returns a string, which does not have a property called ts.)
That's why type definitions are worth the effort!

return new Promise<void>((rs) => {
isScheduling = true;
const localeCode = i18next.resolvedLanguage;
console.error("notifTimes: ", notifTimes, " - type: ", typeof(notifTimes));
const nots = notifTimes.map((n) => {
console.error("n: ", n, " - type: ", typeof(n));
const nDate = n.toDate();
const seconds = nDate.getTime() / 1000;
return {
id: seconds,
title: scheme.title[localeCode],
text: scheme.text[localeCode],
trigger: {at: nDate},
// actions: 'reminder-actions',
// data: {
// action: {
// redirectTo: 'root.main.control',
// redirectParams: {
// openTimeOfDayPicker: true
// }
// }
// }
}
});
window['cordova'].plugins.notification.local.cancelAll(() => {
debugGetScheduled("After cancelling");
window['cordova'].plugins.notification.local.schedule(nots, () => {
debugGetScheduled("After scheduling");
isScheduling = false;
rs(); //scheduling promise resolved here
});
});
});
}

// determines when notifications are needed, and schedules them if not already scheduled
const update = async (reminderSchemes) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I see you already accept reminderSchemes as an argument to all the functions that need it, which a decision that I think is fine.
So why not export this function and call it directly from ProfileSettings? ProfileSettings has appConfig, which is all you need

const { reminder_assignment,
reminder_join_date,
reminder_time_of_day} = await getReminderPrefs(reminderSchemes);
var scheme = {};
try {
scheme = reminderSchemes[reminder_assignment];
} catch (e) {
console.log("ERROR: Could not find reminder scheme for assignment " + reminderSchemes + " - " + reminder_assignment);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Appreciate the addition of this catch block - but instead of console.log let's displayErrorMsg (from logger.ts)

const notifTimes = calcNotifTimes(scheme, reminder_join_date, reminder_time_of_day);
return new Promise<void>((resolve, reject) => {
window['cordova'].plugins.notification.local.getScheduled((notifs) => {
if (areAlreadyScheduled(notifs, notifTimes)) {
logDebug("Already scheduled, not scheduling again");
} else {
// to ensure we don't overlap with the last scheduling() request,
// we'll wait for the previous one to finish before scheduling again
scheduledPromise.then(() => {
if (isScheduling) {
console.log("ERROR: Already scheduling notifications, not scheduling again")
} else {
scheduledPromise = scheduleNotifs(scheme, notifTimes);
//enforcing end of scheduling to conisder update through
scheduledPromise.then(() => {
resolve();
})
}
});
}
});
});
}

/* Randomly assign a scheme, set the join date to today,
and use the default time of day from config (or noon if not specified)
This is only called once when the user first joins the study
*/
const initReminderPrefs = (reminderSchemes) => {
// randomly assign from the schemes listed in config
const schemes = Object.keys(reminderSchemes);
const randAssignment = schemes[Math.floor(Math.random() * schemes.length)];
const todayDate = DateTime.local().toFormat('yyyy-MM-dd');
const defaultTime = reminderSchemes[randAssignment]?.defaultTime || '12:00';
return {
reminder_assignment: randAssignment,
reminder_join_date: todayDate,
reminder_time_of_day: defaultTime,
};
}

/* EXAMPLE VALUES - present in user profile object
reminder_assignment: 'passive',
reminder_join_date: '2023-05-09',
reminder_time_of_day: '21:00',
*/
// interface ReminderPrefs {
// reminder_assignment: string;
// reminder_join_date: string;
// reminder_time_of_day: string;
// }

const getReminderPrefs = async (reminderSchemes): Promise<any> => {
const user = await getUser();
if (user?.reminder_assignment &&
user?.reminder_join_date &&
user?.reminder_time_of_day) {
console.log("User already has reminder prefs, returning them", user)
return user;
}
// if no prefs, user just joined, so initialize them
console.log("User just joined, Initializing reminder prefs")
const initPrefs = initReminderPrefs(reminderSchemes);
console.log("Initialized reminder prefs: ", initPrefs);
await setReminderPrefs(initPrefs, reminderSchemes);
return { ...user, ...initPrefs }; // user profile + the new prefs
}
const setReminderPrefs = async (newPrefs, reminderSchemes) => {
await updateUser(newPrefs)
const updatePromise = new Promise<void>((resolve, reject) => {
//enforcing update before moving on
update(reminderSchemes).then(() => {
resolve();
});
});
// record the new prefs in client stats
getReminderPrefs(reminderSchemes).then((prefs) => {
// extract only the relevant fields from the prefs,
// and add as a reading to client stats
const { reminder_assignment,
reminder_join_date,
reminder_time_of_day} = prefs;
addStatReading(statKeys.REMINDER_PREFS, {
reminder_assignment,
reminder_join_date,
reminder_time_of_day
}).then(logDebug("Added reminder prefs to client stats"));
});
return updatePromise;
}

export function useSchedulerHelper() {
const appConfig = useAppConfig();
const [reminderSchemes, setReminderSchemes] = useState();

useEffect(() => {
if (!appConfig) {
logDebug("No reminder schemes found in config, not scheduling notifications");
return;
}
setReminderSchemes(appConfig.reminderSchemes);
}, [appConfig]);

//setUpActions();
update(reminderSchemes);
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you remove the React hook, you'll need to do this update call somewhere else. You can do it in ProfileSettings somewhere in the whenReady function.


return {
setReminderPrefs: (newPrefs) => setReminderPrefs(newPrefs, reminderSchemes),
getReminderPrefs: () => getReminderPrefs(reminderSchemes),
getScheduledNotifs: () => getScheduledNotifs(),
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

After seeing the code, I actually think this would be simpler and more concise without using a React hook.
There isn't anything about this code that is necessarily React-based or having to do with the UI components. It's really a matter of conditionally formatting data, which we can do just as well in plain TS.

We already have access to the appConfig/uiConfig in ProfileSettings.jsx, so it's a bit redundant to invoke useAppConfig() again in this hook.
And reminderSchemes is just part of appConfig - it doesn't really need to be its own state.

Loading