Skip to content

Commit 7b6f545

Browse files
authored
Fix join help queue race (#589)
1 parent 40f784f commit 7b6f545

File tree

4 files changed

+74
-37
lines changed

4 files changed

+74
-37
lines changed

app/course/[course_id]/office-hours/[queue_id]/new/newRequestForm.tsx

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010
useHelpRequestStudents,
1111
useHelpRequestTemplates,
1212
useHelpQueues,
13-
useHelpQueueAssignments,
13+
useActiveHelpQueueAssignments,
1414
useOfficeHoursController
1515
} from "@/hooks/useOfficeHoursRealtime";
1616
import {
@@ -119,12 +119,13 @@ export default function HelpRequestForm() {
119119
const isLoadingQueues = false; // Individual hooks don't expose loading state
120120
const connectionError = null; // Will be handled by connection status if needed
121121

122-
// Get help queue assignments to check for active staff
123-
const allHelpQueueAssignments = useHelpQueueAssignments();
122+
// Get active help queue assignments to check for active staff
123+
// Uses useActiveHelpQueueAssignments which subscribes to individual item changes
124+
// for proper reactivity when staff starts/stops working
125+
const activeHelpQueueAssignments = useActiveHelpQueueAssignments();
124126
const queueIdsWithActiveStaff = useMemo(() => {
125-
const activeAssignments = allHelpQueueAssignments.filter((assignment) => assignment.is_active);
126-
return new Set(activeAssignments.map((a) => a.help_queue_id));
127-
}, [allHelpQueueAssignments]);
127+
return new Set(activeHelpQueueAssignments.map((a) => a.help_queue_id));
128+
}, [activeHelpQueueAssignments]);
128129

129130
// Get all help requests and students data from realtime
130131
const allHelpRequests = useHelpRequests();
@@ -310,28 +311,39 @@ export default function HelpRequestForm() {
310311
).length;
311312
}, [selectedHelpQueue, allHelpRequests]);
312313

313-
// Validate that selected queue has active staff
314+
// Validate that selected queue is available and has active staff
314315
useEffect(() => {
315316
if (selectedHelpQueue) {
316317
const selectedQueue = helpQueues.find((q) => q.id === selectedHelpQueue);
317-
if (selectedQueue && !queueIdsWithActiveStaff.has(selectedHelpQueue)) {
318+
319+
// Check if queue exists in available queues list
320+
if (!selectedQueue) {
321+
// Queue is not available (available === false) or doesn't exist
322+
const queueFromAll = allHelpQueues.find((q) => q.id === selectedHelpQueue);
323+
if (queueFromAll) {
324+
setError("help_queue", {
325+
type: "manual",
326+
message: "This queue is not currently accepting new requests. Please select a different queue."
327+
});
328+
}
329+
} else if (!queueIdsWithActiveStaff.has(selectedHelpQueue)) {
318330
setError("help_queue", {
319331
type: "manual",
320332
message: "This queue is not currently staffed. Please select a queue with active staff members."
321333
});
322334
} else {
323-
// Clear the error if queue has active staff
335+
// Clear any manual queue errors if queue is available and staffed
324336
const errorMessage = errors.help_queue?.message;
325337
if (
326338
errors.help_queue?.type === "manual" &&
327339
typeof errorMessage === "string" &&
328-
errorMessage.includes("not currently staffed")
340+
(errorMessage.includes("not currently staffed") || errorMessage.includes("not currently accepting"))
329341
) {
330342
clearErrors("help_queue");
331343
}
332344
}
333345
}
334-
}, [selectedHelpQueue, helpQueues, queueIdsWithActiveStaff, setError, clearErrors, errors.help_queue]);
346+
}, [selectedHelpQueue, helpQueues, allHelpQueues, queueIdsWithActiveStaff, setError, clearErrors, errors.help_queue]);
335347

336348
const onSubmit = useCallback(
337349
async (e: React.FormEvent<HTMLFormElement>) => {

app/course/[course_id]/office-hours/[queue_id]/new/page.tsx

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import { useParams, useRouter } from "next/navigation";
44
import { useEffect, useMemo } from "react";
5-
import { useHelpQueue, useHelpQueueAssignments } from "@/hooks/useOfficeHoursRealtime";
5+
import { useHelpQueue, useActiveHelpQueueAssignments } from "@/hooks/useOfficeHoursRealtime";
66
import { Box, Card, Container, Text, Button } from "@chakra-ui/react";
77

88
import HelpRequestForm from "./newRequestForm";
@@ -11,25 +11,31 @@ export default function NewRequestPage() {
1111
const { queue_id, course_id } = useParams();
1212
const router = useRouter();
1313
const helpQueue = useHelpQueue(Number(queue_id));
14-
const allHelpQueueAssignments = useHelpQueueAssignments();
14+
// Use the specialized hook that subscribes to individual item changes
15+
const activeHelpQueueAssignments = useActiveHelpQueueAssignments();
1516

16-
// Check if queue has an active assignment
17+
// Check if queue has an active assignment (staff is working)
1718
const hasActiveAssignment = useMemo(() => {
18-
if (!allHelpQueueAssignments) return false;
19-
return allHelpQueueAssignments.some(
20-
(assignment) => assignment.help_queue_id === Number(queue_id) && assignment.is_active
21-
);
22-
}, [allHelpQueueAssignments, queue_id]);
19+
if (!activeHelpQueueAssignments) return false;
20+
return activeHelpQueueAssignments.some((assignment) => assignment.help_queue_id === Number(queue_id));
21+
}, [activeHelpQueueAssignments, queue_id]);
22+
23+
// Check if queue is available for new requests (both available flag AND has active staff)
24+
const isQueueOpen = helpQueue?.available && hasActiveAssignment;
2325

2426
useEffect(() => {
25-
if (helpQueue && !hasActiveAssignment) {
26-
// Redirect back to queue page if queue has no active assignment
27+
if (helpQueue && !isQueueOpen) {
28+
// Redirect back to queue page if queue is not open for new requests
2729
router.replace(`/course/${course_id}/office-hours/${queue_id}`);
2830
}
29-
}, [helpQueue, hasActiveAssignment, router, course_id, queue_id]);
31+
}, [helpQueue, isQueueOpen, router, course_id, queue_id]);
32+
33+
// Show error message if queue is not open for new requests
34+
if (helpQueue && !isQueueOpen) {
35+
const reason = !helpQueue.available
36+
? "This queue is not currently accepting new requests."
37+
: "This queue is not currently staffed.";
3038

31-
// Show error message if queue has no active assignment
32-
if (helpQueue && !hasActiveAssignment) {
3339
return (
3440
<Container>
3541
<Box py={8}>
@@ -39,7 +45,7 @@ export default function NewRequestPage() {
3945
Queue Closed for New Requests
4046
</Text>
4147
<Text color="fg.muted" mb={4}>
42-
This queue is currently closed for new requests. You can still view existing requests and queue status.
48+
{reason} You can still view existing requests and queue status.
4349
</Text>
4450
<Button onClick={() => router.push(`/course/${course_id}/office-hours/${queue_id}`)} variant="outline">
4551
Back to Queue

components/help-queue/office-hours-header.tsx

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import NextLink from "next/link";
88
import { FaPlus } from "react-icons/fa";
99
import { FiChevronRight, FiChevronDown } from "react-icons/fi";
1010
import { useMemo } from "react";
11-
import { useHelpQueueAssignments, useHelpQueues } from "@/hooks/useOfficeHoursRealtime";
11+
import { useActiveHelpQueueAssignments, useHelpQueues } from "@/hooks/useOfficeHoursRealtime";
1212
import { useClassProfiles } from "@/hooks/useClassProfiles";
1313
import { useParams, useRouter, usePathname } from "next/navigation";
1414
import { useOfficeHoursController } from "@/hooks/useOfficeHoursRealtime";
@@ -62,7 +62,9 @@ export function OfficeHoursHeader({
6262
const pathname = usePathname();
6363
const { private_profile_id } = useClassProfiles();
6464
const isInstructor = useIsInstructor();
65-
const allQueueAssignments = useHelpQueueAssignments();
65+
// Use the specialized hook that subscribes to individual item changes
66+
// This ensures the header updates when staff starts/stops working
67+
const activeQueueAssignments = useActiveHelpQueueAssignments();
6668
const helpQueues = useHelpQueues();
6769
const controller = useOfficeHoursController();
6870
const { helpQueueAssignments } = controller;
@@ -78,26 +80,26 @@ export function OfficeHoursHeader({
7880
}, [currentRequest?.queueId, queue_id]);
7981

8082
// Get queues with active staff assignments (for student mode)
83+
// Uses activeQueueAssignments which properly subscribes to is_active changes
8184
const queuesWithActiveStaff = useMemo(() => {
8285
if (isManageMode) return [];
83-
const activeAssignments = allQueueAssignments.filter((assignment) => assignment.is_active);
84-
const queueIdsWithActiveStaff = new Set(activeAssignments.map((a) => a.help_queue_id));
86+
const queueIdsWithActiveStaff = new Set(activeQueueAssignments.map((a) => a.help_queue_id));
8587
return helpQueues
8688
.filter((queue) => queue.available && queueIdsWithActiveStaff.has(queue.id))
8789
.sort((a, b) => a.name.localeCompare(b.name));
88-
}, [allQueueAssignments, helpQueues, isManageMode]);
90+
}, [activeQueueAssignments, helpQueues, isManageMode]);
8991

9092
// Get queues the TA is currently working
9193
const workingQueues = useMemo(() => {
9294
if (!isManageMode || !private_profile_id) return [];
93-
return allQueueAssignments
94-
.filter((assignment) => assignment.ta_profile_id === private_profile_id && assignment.is_active)
95+
return activeQueueAssignments
96+
.filter((assignment) => assignment.ta_profile_id === private_profile_id)
9597
.map((assignment) => {
9698
const queue = helpQueues.find((q) => q.id === assignment.help_queue_id);
9799
return queue;
98100
})
99101
.filter(Boolean);
100-
}, [allQueueAssignments, helpQueues, private_profile_id, isManageMode]);
102+
}, [activeQueueAssignments, helpQueues, private_profile_id, isManageMode]);
101103

102104
const handleStartStopWorking = async (queueId: number, isWorking: boolean, assignmentId?: number) => {
103105
// Guard: check if private_profile_id is present before proceeding
@@ -315,8 +317,9 @@ export function OfficeHoursHeader({
315317
// Stop all working queues
316318
for (const queue of workingQueues) {
317319
if (!queue) continue;
318-
const assignment = allQueueAssignments.find(
319-
(a) => a.ta_profile_id === private_profile_id && a.is_active && a.help_queue_id === queue.id
320+
// activeQueueAssignments already filtered for is_active === true
321+
const assignment = activeQueueAssignments.find(
322+
(a) => a.ta_profile_id === private_profile_id && a.help_queue_id === queue.id
320323
);
321324
if (assignment) {
322325
await handleStartStopWorking(queue.id, true, assignment.id);

hooks/useOfficeHoursRealtime.tsx

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
"use client";
22

33
import { OfficeHoursRealTimeController } from "@/lib/OfficeHoursRealTimeController";
4-
import TableController, { useTableControllerTableValues, useTableControllerValueById } from "@/lib/TableController";
4+
import TableController, {
5+
useTableControllerTableValues,
6+
useTableControllerValueById,
7+
useListTableControllerValues
8+
} from "@/lib/TableController";
59
import { createClient } from "@/utils/supabase/client";
610
import {
711
HelpRequestMessage,
@@ -11,7 +15,7 @@ import {
1115
import { Database } from "@/utils/supabase/SupabaseTypes";
1216
import { Box, Spinner } from "@chakra-ui/react";
1317
import { SupabaseClient } from "@supabase/supabase-js";
14-
import { createContext, useContext, useEffect, useRef, useState } from "react";
18+
import { createContext, useCallback, useContext, useEffect, useRef, useState } from "react";
1519
import { useCourseController } from "./useCourseController";
1620
import { ClassRealTimeController } from "@/lib/ClassRealTimeController";
1721

@@ -619,6 +623,18 @@ export function useHelpQueueAssignments() {
619623
return useTableControllerTableValues(controller.helpQueueAssignments);
620624
}
621625

626+
/**
627+
* Returns only active help queue assignments (is_active === true).
628+
* Uses useListTableControllerValues which subscribes to individual item changes,
629+
* ensuring the data stays fresh when assignments are activated/deactivated.
630+
*/
631+
export function useActiveHelpQueueAssignments() {
632+
const controller = useOfficeHoursController();
633+
// Memoize predicate to avoid re-subscribing on every render
634+
const isActivePredicate = useCallback((assignment: { is_active: boolean }) => assignment.is_active === true, []);
635+
return useListTableControllerValues(controller.helpQueueAssignments, isActivePredicate);
636+
}
637+
622638
export function useStudentKarmaNotes() {
623639
const controller = useOfficeHoursController();
624640
return useTableControllerTableValues(controller.studentKarmaNotes);

0 commit comments

Comments
 (0)