Track meeting attendance automatically via Slack huddles - #10
Track meeting attendance automatically via Slack huddles#10HanCotterell wants to merge 3 commits into
Conversation
| for (const stat of attendanceStats.filter((s) => s.isFlagged)) { | ||
| const mentor = stat.project?.mentors?.[0]; |
There was a problem hiding this comment.
flaggedStudents always returns undefined mentor
stat.project?.mentors?.[0] will always be undefined because the underlying call to statStudentAttendance does not include the mentors relation on projects — only meetings and its attendance are included. Every FlaggedStudent will therefore have mentor: undefined, making the mentor field in the response always null regardless of the project's actual mentors.
| scheduledStartAt: weekStart, | ||
| scheduledEndAt: weekStart, |
There was a problem hiding this comment.
scheduledEndAt is set to weekStart instead of weekEnd, so every auto-created meeting will have a zero-duration window (start equals end). This will make the meeting appear to end the moment it begins in any UI that surfaces scheduled times.
| scheduledStartAt: weekStart, | |
| scheduledEndAt: weekStart, | |
| scheduledStartAt: weekStart, | |
| scheduledEndAt: weekEnd, |
| @Authorized(AuthRole.ADMIN, AuthRole.MANAGER, AuthRole.MENTOR) | ||
| @Mutation(() => MeetingAttendance) | ||
| async recordMeetingAttendance( | ||
| @Ctx() { auth }: Context, | ||
| @Arg('data', () => MeetingAttendanceInput) data: MeetingAttendanceInput, | ||
| ): Promise<PrismaMeetingAttendance> { | ||
| DEBUG(`Recording attendance for meeting ${data.meetingId}, student ${data.studentId}: ${data.attended}`); | ||
|
|
||
| // Check for existing attendance record | ||
| const existing = await this.prisma.meetingAttendance.findFirst({ | ||
| where: { | ||
| meetingId: data.meetingId, | ||
| studentId: data.studentId, | ||
| }, | ||
| }); | ||
|
|
||
| if (existing) { | ||
| // Update existing record | ||
| return this.prisma.meetingAttendance.update({ | ||
| where: { id: existing.id }, | ||
| data: { | ||
| attended: data.attended, | ||
| prepared: data.prepared ?? existing.prepared, | ||
| source: data.source ?? existing.source, | ||
| confidence: data.confidence ?? existing.confidence, | ||
| metadata: data.metadata as any ?? existing.metadata, | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| // Create new record | ||
| return this.prisma.meetingAttendance.create({ | ||
| data: { | ||
| meetingId: data.meetingId, | ||
| studentId: data.studentId, | ||
| attended: data.attended, | ||
| prepared: data.prepared ?? false, | ||
| source: data.source ?? 'MANUAL', | ||
| confidence: data.confidence ?? 1.0, | ||
| metadata: data.metadata as any, | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Mentor can record attendance for any meeting
The recordMeetingAttendance mutation is accessible to the MENTOR role but performs no check that the targeted meetingId belongs to a project the authenticated mentor is actually assigned to. Any mentor with a valid token can overwrite attendance records for meetings in other mentors' projects or even other events, allowing incorrect data to be written at will.
| @Authorized(AuthRole.ADMIN, AuthRole.MANAGER, AuthRole.MENTOR) | ||
| @Query(() => [MeetingAttendance]) | ||
| async meetingAttendance( | ||
| @Ctx() { auth }: Context, | ||
| @Arg('meetingId', () => String) meetingId: string, | ||
| ): Promise<PrismaMeetingAttendance[]> { | ||
| return this.prisma.meetingAttendance.findMany({ | ||
| where: { meetingId }, | ||
| include: { student: true }, | ||
| orderBy: { createdAt: 'asc' }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Mentor can read attendance for any meeting
The meetingAttendance query accepts any meetingId and returns all attendance records for it without verifying that the requesting mentor is associated with that meeting's project. A mentor can enumerate attendance data for every other team in the event by guessing or iterating meeting IDs.
| @Authorized(AuthRole.ADMIN, AuthRole.MANAGER, AuthRole.MENTOR, AuthRole.STUDENT) | ||
| @Query(() => Meeting, { nullable: true }) | ||
| async meeting( | ||
| @Ctx() { auth }: Context, | ||
| @Arg('id', () => String) id: string, | ||
| ): Promise<PrismaMeeting | null> { | ||
| const meeting = await this.prisma.meeting.findUnique({ | ||
| where: { id }, | ||
| include: { project: true }, | ||
| }); | ||
|
|
||
| if (!meeting) return null; | ||
|
|
||
| // Verify access | ||
| if (!auth.isAdmin && !auth.isManager) { | ||
| if (auth.isMentor || auth.isStudent) { | ||
| if (meeting.eventId !== auth.eventId) { | ||
| throw new Error('No permission to view this meeting.'); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return meeting; |
There was a problem hiding this comment.
Overly broad event-level access for mentor/student on
meeting query
The access check for mentors and students only verifies that meeting.eventId === auth.eventId, but a meeting may belong to a different project within the same event. A mentor or student can therefore retrieve meeting details (including schedule and attendance) for any project in their event, not just their own team.
| // Check mentor reflection completion | ||
| const mentorReflections = await prisma.surveyResponse.count({ | ||
| where: { | ||
| authorMentorId: mentor.id, | ||
| surveyOccurence: { | ||
| survey: { | ||
| personType: 'MENTOR', | ||
| eventId: event.id, | ||
| }, | ||
| }, | ||
| }, | ||
| }); | ||
|
|
There was a problem hiding this comment.
prisma.surveyResponse.count() is called inside the for (const project of projects) loop, issuing one separate database round-trip per mentor. For an event with 100 mentors this becomes 100 sequential queries. The same pattern exists in scripts/testAttendanceSlack.ts. Consider grouping these counts with a single groupBy call outside the loop.
| --- | ||
| to: "akif@codeday.org" | ||
| subject: "Weekly Attendance Report for {{ event.name }}" | ||
| --- |
There was a problem hiding this comment.
Hardcoded recipient email address
The template's to: frontmatter is set to a hardcoded personal email address (akif@codeday.org). If this template is ever wired up to the email-sending pipeline it will route all weekly attendance reports directly to that inbox regardless of the event's configured contact. The address should be a template variable like {{ event.contactEmail }} or removed if the template is not yet used.
🤖 Augment PR SummarySummary: Adds meeting attendance tracking to Labs GraphQL, primarily sourced from mentor reflection reporting and used to drive weekly alerting. Changes:
🤖 Was this summary useful? React with 👍 or 👎 |
| import { DateTime } from 'luxon'; | ||
|
|
||
| const DEBUG = makeDebug('automation:tasks:sendAttendanceAlerts'); | ||
| const ATTENDANCE_ALERT_CHANNEL = 'stats'; |
There was a problem hiding this comment.
src/automation/tasks/sendAttendanceAlerts.ts:8 ATTENDANCE_ALERT_CHANNEL is set to 'stats', but Slack chat.postMessage expects a channel ID (and other Slack flows here persist IDs like slackMentorChannelId), so this may fail to post. Consider resolving #stats to an ID or using an event-configured channel ID.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| const flagged: FlaggedStudent[] = []; | ||
|
|
||
| for (const stat of attendanceStats.filter((s) => s.isFlagged)) { | ||
| const mentor = stat.project?.mentors?.[0]; |
There was a problem hiding this comment.
src/resolvers/Stats.ts:276 stat.project?.mentors?.[0] will always be undefined here because statStudentAttendance doesn’t include mentors on the loaded project, so flaggedStudents likely returns mentor: null even when a mentor exists. Consider including mentors when building the stats (or fetching the mentor separately) before populating FlaggedStudent.mentor.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| DEBUG(`Recording attendance for meeting ${data.meetingId}, student ${data.studentId}: ${data.attended}`); | ||
|
|
||
| // Check for existing attendance record | ||
| const existing = await this.prisma.meetingAttendance.findFirst({ |
There was a problem hiding this comment.
src/resolvers/Meeting.ts:98 recordMeetingAttendance updates/creates by meetingId+studentId without validating that the meeting/student belong to auth.eventId (and for mentors, that they’re associated with the meeting’s project), which could allow cross-project/event edits if IDs are known. The same scoping concern also applies to meetingAttendance (src/resolvers/Meeting.ts:139).
Severity: high
Other Locations
src/resolvers/Meeting.ts:139
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| let meeting = await prisma.meeting.findFirst({ | ||
| where: { | ||
| projectId: project.id, | ||
| scheduledStartAt: { gte: weekStart, lte: weekEnd }, |
There was a problem hiding this comment.
src/automation/tasks/processMentorReflections.ts:134 The meeting lookup keys off scheduledStartAt, but this column is newly added and may be NULL for existing Meeting rows, which can cause duplicate meetings to be created for the same project/week. Consider matching on the existing visibleAt/dueAt week window or explicitly handling scheduledStartAt being null.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| visibleAt: weekStart, | ||
| dueAt: weekEnd, | ||
| scheduledStartAt: weekStart, | ||
| scheduledEndAt: weekStart, |
There was a problem hiding this comment.
src/automation/tasks/processMentorReflections.ts:148 When auto-creating a weekly meeting, scheduledEndAt is set to weekStart, making start/end identical and potentially confusing downstream ordering/reporting. Consider setting a more representative end timestamp (e.g., weekEnd or a default duration).
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
Introduces the Meeting/MeetingAttendance data model used to record whether students attend their team meetings: - AttendanceSource enum (SLACK_HUDDLE, MANUAL) plus source/confidence/metadata on MeetingAttendance, so records carry their provenance. - AttendanceTrackingMode enum with Project.attendanceTracking and Event.defaultAttendanceTracking, letting teams which do not meet on Slack opt out of automated tracking. - Meeting gains projectId, slackHuddleId, and scheduled start/end times. - SlackHuddleParticipation records raw huddle join/leave events. - GraphQL types, inputs, and a Meeting resolver for reading meetings and recording attendance manually.
Attendance is derived from Slack huddle activity rather than asked for: - A Slack events webhook receives user_huddle_changed and acknowledges within Slack's 3 second budget, processing the event asynchronously. - A mentor joining a huddle in a project channel opens a Meeting; students joining are recorded as present with source=SLACK_HUDDLE. The meeting end time is set when the last participant leaves. - markAbsentStudents runs daily and marks students who never joined, so absence is recorded without anyone submitting a form.
Surfaces attendance to program managers while ensuring teams that do not meet on Slack are not misreported. Because absence is inferred from the absence of a huddle, an untracked team would otherwise appear to have 0% attendance and every student would be flagged. - resolveAttendanceTracking/isAttendanceTracked resolve a project's mode, falling back to its event default. - statStudentAttendance exposes trackingMode and never flags untracked projects; flaggedStudents and the weekly Slack alert exclude them, reporting only a count. - attendanceTracking is settable via editProject and readable on Project. - Documents setup, the opt-out, and its effect on each component.
4f338da to
a4484c4
Compare
Records whether students attend their team meetings, and flags those who don't.
Attendance is derived from Slack huddle activity rather than asked for. A mentor joining a huddle in a project channel opens a meeting; students who join are recorded present, and a daily task marks those who never joined as absent. Nobody has to submit anything.
Note
This PR previously captured attendance from mentor reflection surveys. That approach has been removed in favour of automatic tracking, so the survey path no longer exists here. The general
Survey/SurveyResponsesystem is untouched — only the attendance-extraction layer built on it was dropped.Opting out
Not every team meets on Slack. Since absence is inferred from the lack of a huddle join, an untracked team would otherwise show 0% attendance and have every student flagged.
Projects carry
attendanceTracking, inheriting fromEvent.defaultAttendanceTrackingwhen unset. Setting it toNOT_TRACKEDexcludes the team at every layer:NOT_TRACKEDhuddleHandlermarkAbsentStudentsstatStudentAttendancetrackingMode: NOT_TRACKED, never setsisFlaggedflaggedStudentssendAttendanceAlertsThe
trackingModefield lets the dashboard distinguish "this student missed meetings" from "we don't measure this team".Fixes found while consolidating
This branch was cut before 5 commits landed on
main, so its diff was silently proposing to revert them. Rebuilt on currentmain, which avoids:config.ts(Attio env vars),Review.ts(partnerCodefilter),getProjectMatches.ts, andyarn.lockmentorResume/mentorJudgingemail templates thatmaindeleted — these auto-send, so re-adding them would have started sending mail again.test.tsfilter in the automation task loaderAlso fixed: two test files sat in
src/automation/tasks/, where the loaderthrows on any file lacking a default export — this crashed the automation runner at boot. They now live inscripts/. A migration forSlackHuddleParticipationwas also missing entirely and has been written.The diff is now purely additive: 28 files, +2432 −21, zero deletions.
Verification
yarn test:attendancepasses (opt-out resolution + alert message formatting)npx prisma validatepassestscerror count unchanged frommain(378, all pre-existing in generated Prisma types andsrc/badgr)markAbsentStudentsat0 2 * * *andsendAttendanceAlertsat0 9 * * MONNot yet done
Deployment needs the migration applied, then Slack app config: event subscription URL
https://<host>/{WEBHOOK_KEY}/slack, subscribe touser_huddle_changed, and scopeschannels:read+users:read. End-to-end testing against a real huddle hasn't happened yet — worth piloting on one project before rollout. Setup steps are inSLACK_HUDDLE_ATTENDANCE.md.