Skip to content

Commit 4522f8e

Browse files
Achintya-ChatterjeeAchintya-Chatterjee
committed
fix: honor OOO grace window in missed updates flow
- persist lastOooUntil when users leave OOO across status mutations and reconciler - backfill historical userStatus documents with a lastOooUntil timestamp - filter missed updates candidates using the persisted grace-period cutoff
1 parent dadf4d4 commit 4522f8e

File tree

4 files changed

+173
-19
lines changed

4 files changed

+173
-19
lines changed

‎models/discordactions.js‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ const { findSubscribedGroupIds } = require("../utils/helper");
88
const { retrieveUsers } = require("../services/dataAccessLayer");
99
const { BATCH_SIZE_IN_CLAUSE } = require("../constants/firebase");
1010
const { getAllUserStatus, getGroupRole, getUserStatus } = require("./userStatus");
11+
const { normalizeTimestamp } = require("../utils/userStatus");
1112
const { userState } = require("../constants/userStatus");
1213
const { ONE_DAY_IN_MS, SIMULTANEOUS_WORKER_CALLS } = require("../constants/users");
1314
const userModel = firestore.collection("users");
@@ -958,6 +959,7 @@ const getMissedProgressUpdatesUsers = async (options = {}) => {
958959
const stats = {
959960
tasks: 0,
960961
missedUpdatesTasks: 0,
962+
filteredByOoo: 0,
961963
};
962964
try {
963965
const discordUsersPromise = discordService.getDiscordMembers();
@@ -1031,8 +1033,7 @@ const getMissedProgressUpdatesUsers = async (options = {}) => {
10311033

10321034
const userIdChunks = chunks(Array.from(usersMap.keys()), FIRESTORE_IN_CLAUSE_SIZE);
10331035
const userStatusSnapshotPromise = userIdChunks.map(
1034-
async (userIdList) =>
1035-
await userStatusModel.where("currentStatus.state", "==", userState.OOO).where("userId", "in", userIdList).get()
1036+
async (userIdList) => await userStatusModel.where("userId", "in", userIdList).get()
10361037
);
10371038
const userDetailsPromise = userIdChunks.map(
10381039
async (userIdList) =>
@@ -1046,7 +1047,13 @@ const getMissedProgressUpdatesUsers = async (options = {}) => {
10461047

10471048
userStatusChunks.forEach((userStatusList) =>
10481049
userStatusList.forEach((doc) => {
1049-
usersMap.get(doc.data().userId).isOOO = true;
1050+
const userStatusData = doc.data();
1051+
const mappedUser = usersMap.get(userStatusData.userId);
1052+
if (!mappedUser) {
1053+
return;
1054+
}
1055+
mappedUser.isOOO = userStatusData.currentStatus?.state === userState.OOO;
1056+
mappedUser.lastOooUntil = userStatusData.lastOooUntil ?? null;
10501057
})
10511058
);
10521059

@@ -1083,9 +1090,18 @@ const getMissedProgressUpdatesUsers = async (options = {}) => {
10831090

10841091
await Promise.all(progressCountPromise);
10851092

1093+
const gracePeriodCutoff = Date.now() - convertDaysToMilliseconds(dateGap);
10861094
for (const [userId, userData] of usersMap.entries()) {
10871095
const discordUserData = discordUserMap.get(userData.discordId);
10881096
const isDiscordMember = !!discordUserData;
1097+
const normalizedLastOooUntil = normalizeTimestamp(userData.lastOooUntil);
1098+
const isWithinGracePeriod = normalizedLastOooUntil !== null && normalizedLastOooUntil >= gracePeriodCutoff;
1099+
1100+
if (userData.latestProgressCount === 0 && (userData.isOOO || isWithinGracePeriod)) {
1101+
stats.filteredByOoo++;
1102+
usersMap.delete(userId);
1103+
continue;
1104+
}
10891105
const shouldAddRole =
10901106
userData.latestProgressCount === 0 &&
10911107
!userData.isOOO &&

‎models/userStatus.js‎

Lines changed: 64 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ const {
1515
generateNewStatus,
1616
getNextDayTimeStamp,
1717
convertTimestampsToUTC,
18+
resolveLastOooUntil,
1819
} = require("../utils/userStatus");
1920
const { TASK_STATUS } = require("../constants/tasks");
2021
const userStatusModel = firestore.collection("usersStatus");
@@ -213,8 +214,13 @@ const updateUserStatus = async (userId, updatedStatusData) => {
213214
if (userStatusDoc) {
214215
const docId = userStatusDoc.id;
215216
const userStatusData = userStatusDoc.data();
217+
const previousCurrentStatus = userStatusData.currentStatus || {};
218+
const previousState = previousCurrentStatus.state;
219+
const previousUntil = previousCurrentStatus.until;
220+
let requestedNextState;
216221
if (Object.keys(newStatusData).includes("currentStatus")) {
217-
const newUserState = newStatusData.currentStatus.state;
222+
requestedNextState = newStatusData.currentStatus?.state;
223+
const newUserState = requestedNextState;
218224
const isNewStateOoo = newUserState === userState.OOO;
219225
const isNewStateNotOoo = newUserState === userState.ACTIVE || newUserState === userState.IDLE;
220226
const isCurrentStateOoo = userStatusData.currentStatus?.state === userState.OOO;
@@ -235,6 +241,15 @@ const updateUserStatus = async (userId, updatedStatusData) => {
235241
}
236242
}
237243
}
244+
const lastOooUntilUpdate = resolveLastOooUntil({
245+
previousState,
246+
previousUntil,
247+
nextState: requestedNextState,
248+
fallbackTimestamp: newStatusData.currentStatus?.updatedAt,
249+
});
250+
if (lastOooUntilUpdate !== undefined) {
251+
newStatusData.lastOooUntil = lastOooUntilUpdate;
252+
}
238253
if (
239254
userStatusData.currentStatus?.state === userState.IDLE &&
240255
newStatusData.currentStatus?.state !== userState.IDLE
@@ -255,7 +270,7 @@ const updateUserStatus = async (userId, updatedStatusData) => {
255270
}
256271
}
257272
}
258-
const { id } = await userStatusModel.add({ userId, ...newStatusData });
273+
const { id } = await userStatusModel.add({ userId, lastOooUntil: null, ...newStatusData });
259274
return { id, userStatusExists: false, data: newStatusData };
260275
}
261276
} catch (error) {
@@ -283,20 +298,30 @@ const updateAllUserStatus = async () => {
283298
summary.usersCount = userStatusDocs._size;
284299
const batch = firestore.batch();
285300
const today = new Date().getTime();
286-
userStatusDocs.forEach(async (document) => {
301+
for (const document of userStatusDocs.docs) {
287302
const doc = document.data();
288303
const docRef = document.ref;
289304
const userId = doc.userId;
290305
const newStatusData = { ...doc };
291306
let toUpdate = false;
292307
const { futureStatus, currentStatus } = doc;
293-
const { state: futureState } = futureStatus;
294-
const { state: currentState } = currentStatus;
308+
const futureState = futureStatus?.state;
309+
const currentState = currentStatus?.state;
310+
const currentUntil = currentStatus?.until;
295311
if (futureState === "ACTIVE" || futureState === "IDLE") {
296312
if (today >= futureStatus.from) {
297313
// OOO period is over and we need to update their current status
298314
newStatusData.currentStatus = { ...futureStatus, until: "", updatedAt: today };
299315
delete newStatusData.futureStatus;
316+
const lastOooUntilUpdate = resolveLastOooUntil({
317+
previousState: currentState,
318+
previousUntil: currentUntil,
319+
nextState: futureState,
320+
fallbackTimestamp: today,
321+
});
322+
if (lastOooUntilUpdate !== undefined) {
323+
newStatusData.lastOooUntil = lastOooUntilUpdate;
324+
}
300325
toUpdate = !toUpdate;
301326
summary.oooUsersAltered++;
302327
} else {
@@ -319,6 +344,7 @@ const updateAllUserStatus = async () => {
319344
}
320345
newStatusData.currentStatus = newCurrentStatus;
321346
newStatusData.futureStatus = newFutureStatus;
347+
newStatusData.lastOooUntil = null;
322348
toUpdate = !toUpdate;
323349
summary.nonOooUsersAltered++;
324350
} else {
@@ -333,7 +359,7 @@ const updateAllUserStatus = async () => {
333359
}
334360
batch.set(docRef, newStatusData);
335361
}
336-
});
362+
}
337363
if (batch._ops.length > 100) {
338364
logger.info(
339365
`Warning: More than 100 User Status documents to update. The max limit permissible is 500. Refer https://github.com/Real-Dev-Squad/website-backend/issues/890 for more details.`
@@ -505,15 +531,17 @@ const batchUpdateUsersStatus = async (users) => {
505531
const newUserStatusRef = userStatusModel.doc();
506532
const newUserStatusData = {
507533
userId,
534+
lastOooUntil: null,
508535
currentStatus: statusToUpdate,
509536
};
510537
state === userState.ACTIVE ? summary.activeUsersAltered++ : summary.idleUsersAltered++;
511538
if (state === userState.IDLE) await addGroupIdleRoleToDiscordUser(userId);
512539
batch.set(newUserStatusRef, newUserStatusData);
513540
} else {
514-
const {
515-
currentStatus: { state: currentState, until },
516-
} = data;
541+
const currentStatusData = data?.currentStatus || {};
542+
const currentState = currentStatusData.state;
543+
const currentUntil = currentStatusData.until;
544+
const nextState = state;
517545
if (currentState === state) {
518546
currentState === userState.ACTIVE ? summary.activeUsersUnaltered++ : summary.idleUsersUnaltered++;
519547
continue;
@@ -533,18 +561,25 @@ const batchUpdateUsersStatus = async (users) => {
533561
state === userState.ACTIVE ? summary.activeUsersAltered++ : summary.idleUsersAltered++;
534562

535563
const currentDate = new Date();
536-
const untilDate = new Date(until);
564+
const untilDate = new Date(currentUntil);
537565

538566
const timeDifferenceMilliseconds = currentDate.setUTCHours(0, 0, 0, 0) - untilDate.setUTCHours(0, 0, 0, 0);
539567
const timeDifferenceDays = Math.floor(timeDifferenceMilliseconds / (24 * 60 * 60 * 1000));
540568

541569
if (timeDifferenceDays >= 1) {
542570
if (state === userState.IDLE) await addGroupIdleRoleToDiscordUser(userId);
571+
const lastOooUntilUpdate = resolveLastOooUntil({
572+
previousState: currentState,
573+
previousUntil: currentUntil,
574+
nextState,
575+
fallbackTimestamp: currentTimeStamp,
576+
});
543577
batch.update(docRef, {
544578
currentStatus: statusToUpdate,
579+
...(lastOooUntilUpdate !== undefined && { lastOooUntil: lastOooUntilUpdate }),
545580
});
546581
} else {
547-
const getNextDayAfterUntil = getNextDayTimeStamp(until);
582+
const getNextDayAfterUntil = getNextDayTimeStamp(currentUntil);
548583
batch.update(docRef, {
549584
futureStatus: {
550585
...statusToUpdate,
@@ -559,6 +594,15 @@ const batchUpdateUsersStatus = async (users) => {
559594
const updatedStatusData = {
560595
currentStatus: statusToUpdate,
561596
};
597+
const lastOooUntilUpdate = resolveLastOooUntil({
598+
previousState: currentState,
599+
previousUntil: currentUntil,
600+
nextState,
601+
fallbackTimestamp: currentTimeStamp,
602+
});
603+
if (lastOooUntilUpdate !== undefined) {
604+
updatedStatusData.lastOooUntil = lastOooUntilUpdate;
605+
}
562606
batch.update(docRef, updatedStatusData);
563607
}
564608
}
@@ -661,6 +705,15 @@ const cancelOooStatus = async (userId) => {
661705
}
662706
const updatedStatus = generateNewStatus(isActive);
663707
const newStatusData = { ...docData, ...updatedStatus };
708+
const lastOooUntilUpdate = resolveLastOooUntil({
709+
previousState: docData.currentStatus?.state,
710+
previousUntil: docData.currentStatus?.until,
711+
nextState: updatedStatus.currentStatus?.state,
712+
fallbackTimestamp: Date.now(),
713+
});
714+
if (lastOooUntilUpdate !== undefined) {
715+
newStatusData.lastOooUntil = lastOooUntilUpdate;
716+
}
664717
if (futureStatus?.state) {
665718
newStatusData.futureStatus = {};
666719
}

‎test/unit/models/discordactions.test.js‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -692,6 +692,7 @@ describe("discordactions", function () {
692692
let userNotInDiscord;
693693
let activeUserId;
694694
let activeUserWithProgressUpdates;
695+
let activeMissedUpdatesUserId;
695696

696697
beforeEach(async function () {
697698
idleUser = { ...userData[9], discordId: getDiscordMembers[0].user.id };
@@ -710,6 +711,7 @@ describe("discordactions", function () {
710711
await addUser(userNotInDiscord),
711712
]);
712713
activeUserId = userIdList[2];
714+
activeMissedUpdatesUserId = userIdList[1];
713715
await Promise.all([
714716
await userStatusModel.updateUserStatus(userIdList[0], idleUserStatus),
715717
await userStatusModel.updateUserStatus(userIdList[1], activeUserStatus),
@@ -761,6 +763,7 @@ describe("discordactions", function () {
761763
expect(result.tasks).to.equal(4);
762764
expect(result.missedUpdatesTasks).to.not.equal(undefined);
763765
expect(result.missedUpdatesTasks).to.equal(3);
766+
expect(result.filteredByOoo).to.equal(1);
764767
expect(result.usersToAddRole.includes(activeUserWithMissedProgressUpdates.discordId)).to.equal(true);
765768
expect(result.usersToAddRole.includes(idleUser.discordId)).to.equal(true);
766769
});
@@ -771,6 +774,17 @@ describe("discordactions", function () {
771774
expect(result.usersToAddRole).to.not.contain(userNotInDiscord.discordId);
772775
});
773776

777+
it("should exclude users within the post-OOO grace window", async function () {
778+
const graceTimestamp = Date.now() - convertDaysToMilliseconds(2);
779+
const snapshot = await userStatusCollection.where("userId", "==", activeMissedUpdatesUserId).limit(1).get();
780+
const [doc] = snapshot.docs;
781+
await doc.ref.update({ lastOooUntil: graceTimestamp });
782+
783+
const result = await getMissedProgressUpdatesUsers();
784+
expect(result.usersToAddRole).to.not.contain(activeUserWithMissedProgressUpdates.discordId);
785+
expect(result.filteredByOoo).to.equal(2);
786+
});
787+
774788
it("should not list of users when exception days are added", async function () {
775789
const date = new Date();
776790
date.setDate(date.getDate() - 1);
@@ -788,6 +802,7 @@ describe("discordactions", function () {
788802
tasks: 4,
789803
missedUpdatesTasks: 0,
790804
usersToAddRole: [],
805+
filteredByOoo: 0,
791806
});
792807
});
793808

@@ -800,6 +815,7 @@ describe("discordactions", function () {
800815
tasks: 0,
801816
missedUpdatesTasks: 0,
802817
usersToAddRole: [],
818+
filteredByOoo: 0,
803819
});
804820
});
805821

@@ -823,6 +839,7 @@ describe("discordactions", function () {
823839
tasks: 5,
824840
missedUpdatesTasks: 0,
825841
usersToAddRole: [],
842+
filteredByOoo: 0,
826843
});
827844
});
828845

0 commit comments

Comments
 (0)