Repository navigation
Shubham Jakhete - Create way to see deactivated people's last week on the Weekly Summaries Reports page (backend) - #2363
Conversation
Deactivated volunteers are supposed to appear on the Weekly Summaries Report tab for the week their final day falls in, so they get credit for their last work and admins know to stop reporting on them. This does not happen today. The report computes finalWeekIndex relative to the week being requested and takes the absolute value of the difference. Because the frontend loads one tab at a time, the same end date produces a different index on each tab. As a result, a user deactivated last week matches no tab, and an active user with a scheduled end date is hidden from every tab. This commit adds getFinalWeekIndex, which measures an end date from the current week instead: - Returns 0 to 3 for This Week through Three Weeks Ago, using Pacific time week boundaries (Sunday to Saturday) - Returns null for end dates in a future week, older than three weeks, missing, or invalid - Rounds the day count between week starts so a daylight saving week cannot shift the result by one - Parses strings strictly as ISO 8601 so malformed dates return null Unit tests cover the tab mapping, the Saturday/Sunday boundary in Pacific time and in UTC, both daylight saving changes, and bad input. Limitations: the report does not use this helper yet. The index is only meaningful for users who are no longer active; callers must not apply it to scheduled separations. Related PRs: #1850 (HGNRest) and #4270 (HighestGoodNetworkApp) introduced the current finalWeekIndex logic.
Deactivated volunteers should still appear on the Weekly Summaries Report tab for the week they left. They currently appear on no tab, and some active volunteers are hidden as well. Three problems in weeklySummaries cause this: - finalWeekIndex was measured from the requested week with an absolute value, so it changed with the tab being loaded - The index was set after an early return for users with no time entries, so a user who logged no hours in their final week never received one - Active users with a scheduled end date received an index, which the frontend uses to hide them from every other tab This commit uses getFinalWeekIndex, which measures from the current week, sets the index before the early return, and sets it to null for active users. It also narrows the query. Since #1850 the report has loaded every inactive user ever created. It now loads active users plus inactive, non-paused users whose end date falls in the requested weeks. The filter excludes Paused rather than requiring Separated, because most existing inactive users on dev have no inactiveReason set. Effects: the weekly summaries email uses the same function, so it now lists active users plus users who left in the previous week, instead of all inactive users. Limitations: the frontend still hides any user with an endDate whose index does not match the tab, so active users with a scheduled end date remain hidden until the frontend is updated.
|
adit24dhaya
left a comment
There was a problem hiding this comment.
Reviewed f19b661 with frontend #5577 at 7276aa42. The final-week index is now measured from the current Pacific week rather than the requested tab, is assigned before the no-time-entries return, and stays null for active users. The narrowed query includes active users and non-paused inactive users whose final date falls in the requested range, while retaining legacy inactive records without inactiveReason. I found no blocking issue.
Validation: 20/20 tests passed in the two changed suites on Node 20.19.4. Coverage includes Pacific Saturday/Sunday boundaries, both DST transitions, invalid/future/out-of-window dates, zero-hour users and scheduled active separations. Babel compiled 774 files; changed-file ESLint had zero errors/nine warnings, and git diff --check passed.
Twenty additional paired assertions exercised the real weeklySummaries helper and actual frontend visibility rules across four tabs using six fixtures. Active scheduled users remain visible throughout; zero-hour and legacy inactive users appear only in their final week; paused and old inactive users are excluded. Persistence is mocked, and the fixture interprets the first-match query predicates; this is not a real MongoDB integration test. I checked the requested-week controller calls and the weekly-email caller's reuse of this helper; the documented reduction of historical inactive users in that email is intentional scope, not a new mutation. No permission route or account-deactivation write is added here.
Paired frontend validation: 41/41 tests in seven files, build 9,156 modules, eight real-component browser cases in light/dark with 24 visibility and four notice assertions, plus five theme/contrast assertions. The actual dark notice measures 5.12:1 with the route stylesheet loaded. Frontend JS lint/diff checks passed; separate SCSS lint could not run because its parser is unavailable.
No backend server or real database was used, no accounts were changed and no external API calls were made. Full authenticated navigation, actual MongoDB aggregation and email rendering were not exercised.
Commit audit: both commits are focused, imperative, under 72 characters, without trailing periods, and include blank-separated what/why bodies. The history references the relevant #1850/#4270 work and explains the email effect. Approved.
AaditTrivedi
left a comment
There was a problem hiding this comment.
Reviewed by Aadit Trivedi.
Checked out the PR, installed with npm ci, and ran the two test files it adds: finalWeekIndex.test.js and reporthelper.test.js pass 20/20. I reviewed the backend on its own; the frontend PR #5577 needs this backend running, so I have not tested the report tabs end to end.
Verified
getFinalWeekIndexcomputes the week in company time (America/Los_Angeles), rounds a calendar-day difference so a 167- or 169-hour DST week cannot shift the result, parses strings strictly as ISO 8601, and returns null for missing, invalid, future, or out-of-window dates.- The report query now includes active users, plus inactive users whose
endDatefalls within the requested weeks, excluding those withinactiveReason'Paused'. Active users with a scheduled separation get no final week index. - The index is now set before the early return, so a user who logged no time in their final week is still placed correctly.
Pointers, not blocking
- Inactive users with no
endDateare now excluded from the report entirely. Normal deactivation always setsendDate, but a profile edit can clear it, so those users would disappear from the report. Probably fine, worth knowing. - The
'Paused'string is written out in the query; using the sharedInactiveReasonconstant, asuserProfileController.jsdoes, would keep it consistent if the value ever changes.
Approving.
vidiyala99
left a comment
There was a problem hiding this comment.
Tested f19b661 together with frontend #5577 (7276aa4); details and screenshots are in my review there. Ran this branch on a test server against the shared dev data (read-only; nothing was written) and compared GET /api/reports/weeklysummaries?week=0..3 with development:
development: every tab returns all 56 inactive users, and the user who left last week getsfinalWeekIndex: 0on the Last Week request.- This PR: each tab returns active users plus only the people who left that week, with
finalWeekIndex1 (Last Week), 2 (Week Before Last) and 0 (This Week) as expected. Active users with a scheduled end date getnull.
finalWeekIndex.test.js and reporthelper.test.js pass (20/20).
Note for the description: reporthelper.weeklySummaries also feeds the weekly summaries email job (userHelper.emailWeeklySummariesForAllUsers), which will now list only people who left that week instead of every inactive user. This should merge together with #5577, since the frontend relies on the new index.
Approving.
|
Thank you all, merging! |



Description
Bug list: "Create way to see deactivated people's last week on the Weekly Summaries Reports page" (priority high, Jae).
When people are deactivated, they should still show up on the "Last Week" tab of the Weekly Summaries Report with a clear final week notice, so they get credit for their final work in the weekly blog reporting and admins know they will no longer be reporting on them.
PRs #1850 and OneCommunityGlobal/HighestGoodNetworkApp#4270 added this, but it doesn't work. While fixing it I found three problems in
weeklySummaries:finalWeekIndexwas measured from the requested week withMath.abs, so it changed with the tab being loaded. A user deactivated last week matched no tab.Related PRS (if any):
This backend PR is related to frontend PR OneCommunityGlobal/HighestGoodNetworkApp#5577.
To test this backend PR you need to check out frontend PR OneCommunityGlobal/HighestGoodNetworkApp#5577.
Link: OneCommunityGlobal/HighestGoodNetworkApp#5577
Main changes explained:
src/utilities/finalWeekIndex.js: returns which tab (0 to 3) an end date falls in, measured from the current week in Pacific time. Returnsnullfor future, out-of-range, or invalid dates.src/helpers/reporthelper.js:nullfor active users.Pausedrather than requiringSeparated, because 46 of 54 inactive users on dev have noinactiveReason.How to test:
npm install, thennpm run dev.npx jest src/utilities/__tests__/finalWeekIndex.test.js src/helpers/__tests__/reporthelper.test.js.Note:
inactiveReason, scheduled separation, paused, and normal active. I can share the seed script if it helps review.