Skip to content

Fix lab leader grading review assignmetns #621

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.

Sign up for GitHub

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

Open
jon-bell wants to merge 3 commits into
base: staging
Choose a base branch
from
Open

Fix lab leader grading review assignmetns #621

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f1b10ee
WIP
jon-bell Feb 27, 2026
48058d1
.
jon-bell Feb 28, 2026
287983f
Improvre validation
jon-bell Mar 1, 2026
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
41 changes: 40 additions & 1 deletion app/course/[course_id]/manage/assignments/[assignment_id]/reviews/bulk-assign/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1681,6 +1681,45 @@ function BulkAssignGradingForm({ handleReviewAssignmentChange }: { handleReviewA
Submissions will be automatically assigned to all lab leaders of each student's lab section. No
manual grader selection is needed.
</Alert>
<Field.Root>
<Field.Label>Review due date ({course.time_zone ?? "America/New_York"})</Field.Label>
<Input
type="datetime-local"
value={
dueDate
? new Date(dueDate)
.toLocaleString("sv-SE", {
timeZone: course.time_zone ?? "America/New_York"
})
.replace(" ", "T")
: ""
}
onChange={(e) => {
const value = e.target.value;
if (value) {
// Treat inputted date as course timezone regardless of user location
const [date, time] = value.split("T");
const [year, month, day] = date.split("-");
const [hour, minute] = time.split(":");

// Create TZDate with these exact values in course timezone
const tzDate = new TZDate(
parseInt(year),
parseInt(month) - 1,
parseInt(day),
parseInt(hour),
parseInt(minute),
0,
0,
course.time_zone ?? "America/New_York"
);
setDueDate(tzDate.toISOString());
} else {
setDueDate("");
}
}}
/>
</Field.Root>
</VStack>
</Fieldset.Content>
</Fieldset.Root>
Expand Down Expand Up @@ -2104,7 +2143,7 @@ function BulkAssignGradingForm({ handleReviewAssignmentChange }: { handleReviewA
0,
course.time_zone ?? "America/New_York"
);
setDueDate(tzDate.toString());
setDueDate(tzDate.toISOString());
} else {
setDueDate("");
}
Expand Down
243 changes: 243 additions & 0 deletions supabase/migrations/20260225000000_complete_sibling_review_assignments.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,243 @@
-- Migration: Complete sibling review assignments with same or subset rubric parts
--
-- When a review_assignment is completed, this trigger now:
-- 1. Finds other review_assignments for the same submission_review where the sibling's
-- rubric parts are a SUBSET of (or equal to) the completing assignment's parts
-- 2. Marks those sibling assignments as complete (redundant grading support)
-- 3. Checks if ALL review_assignments are now complete
-- 4. Validates that any rubric parts WITHOUT review_assignments have no blocking criteria
-- (no required checks or min_checks_per_submission requirements unmet)
-- 5. If both conditions are satisfied, marks the submission_review as complete
--
-- Example: If Grader A completes parts {A, B, C}, then:
-- - Grader B with parts {A} gets marked complete (subset)
-- - Grader C with parts {A, B} gets marked complete (subset)
-- - Grader D with parts {A, B, C} gets marked complete (equal)
-- - Grader E with parts {A, D} does NOT get marked complete (D not in {A,B,C})
--
-- Uncovered parts validation: If rubric has parts {A,B,C,D} but only review_assignment for {D}:
-- - Parts A,B,C are "uncovered" (no review_assignment)
-- - submission_review can only complete if A,B,C have no required checks or min_checks requirements
--
-- Uses pg_trigger_depth() to prevent redundant work in nested trigger executions.

CREATE OR REPLACE FUNCTION public.check_and_complete_submission_review()
RETURNS trigger
LANGUAGE plpgsql
SECURITY DEFINER
SET search_path = public, pg_temp
AS $$
declare
target_submission_review_id bigint;
target_rubric_id bigint;
completing_user_id uuid;
completing_review_assignment_id bigint;
current_assignment_part_ids bigint[];
covered_part_ids bigint[];
has_blocking_uncovered_parts boolean := false;
begin
-- Only proceed if completed_at was just set (not updated from one non-null value to another)
if OLD.completed_at is not null or NEW.completed_at is null then
return NEW;
end if;

-- Get the submission review and rubric info
target_submission_review_id := NEW.submission_review_id;
completing_user_id := NEW.completed_by;
completing_review_assignment_id := NEW.id;

-- Add advisory lock to prevent race conditions during concurrent updates
perform pg_advisory_xact_lock(target_submission_review_id);

-- Get the rubric_id for this submission review with existence check
select rubric_id into target_rubric_id
from submission_reviews
where id = target_submission_review_id;

-- Check if submission_review exists and raise warning if not
if not found then
raise warning 'submission_review with id % does not exist', target_submission_review_id;
return NEW;
end if;

if target_rubric_id is null then
return NEW;
end if;

-- Only perform sibling completion at the top level trigger (depth = 1)
-- Nested triggers (from sibling completions) skip this to avoid redundant work
if pg_trigger_depth() = 1 then
-- Get the rubric part IDs assigned to the completing review_assignment
-- NULL/empty array means "entire rubric" (no specific parts)
select array_agg(rubric_part_id order by rubric_part_id)
into current_assignment_part_ids
from review_assignment_rubric_parts
where review_assignment_id = completing_review_assignment_id;

-- STEP 1: Find and complete sibling review_assignments where sibling's parts
-- are a SUBSET of (or equal to) the completing assignment's parts.
-- This means all the work for the sibling has been done by the completing grader.
update review_assignments ra_target
set completed_at = NEW.completed_at,
completed_by = completing_user_id
where ra_target.submission_review_id = target_submission_review_id
and ra_target.id != completing_review_assignment_id
and ra_target.completed_at is null
and (
-- Case 1: Completing assignment covers entire rubric (no specific parts)
-- Any sibling (with or without specific parts) is a subset
(current_assignment_part_ids is null)
or
-- Case 2: Completing assignment has specific parts
(current_assignment_part_ids is not null and (
-- Check if sibling has specific parts that are a subset of ours
-- Using @> operator: our_parts @> sibling_parts means "our parts contain all sibling's parts"
exists (
select 1
from review_assignment_rubric_parts rarp
where rarp.review_assignment_id = ra_target.id
)
and
current_assignment_part_ids @> (
select array_agg(rarp.rubric_part_id)
from review_assignment_rubric_parts rarp
where rarp.review_assignment_id = ra_target.id
)
))
);
end if;

-- STEP 2: Check if ALL review_assignments for this submission_review are now complete
-- This runs at all trigger depths to ensure submission_review gets marked complete
if not exists (
select 1
from review_assignments ra
where ra.submission_review_id = target_submission_review_id
and ra.completed_at is null
) then
-- STEP 3: Before marking complete, check that uncovered rubric parts have no blocking criteria
-- Get all rubric part IDs covered by ANY review_assignment for this submission_review
-- A NULL/empty array in review_assignment_rubric_parts means "entire rubric"
select case
when exists (
-- If any review_assignment has no specific parts, it covers the entire rubric
select 1
from review_assignments ra
where ra.submission_review_id = target_submission_review_id
and not exists (
select 1 from review_assignment_rubric_parts rarp
where rarp.review_assignment_id = ra.id
)
) then null -- NULL means all parts are covered
else (
-- Otherwise, aggregate all specific parts from all review_assignments
select array_agg(distinct rarp.rubric_part_id)
from review_assignments ra
join review_assignment_rubric_parts rarp on rarp.review_assignment_id = ra.id
where ra.submission_review_id = target_submission_review_id
)
end into covered_part_ids;

-- Check for blocking criteria in uncovered parts (only if not all parts are covered)
if covered_part_ids is not null then
-- Check if any uncovered part has required checks that haven't been applied
select exists (
select 1
from rubric_checks rc
join rubric_criteria rcrit on rc.rubric_criteria_id = rcrit.id
where rc.rubric_id = target_rubric_id
and rc.is_required = true
and rcrit.rubric_part_id is not null
and not (rcrit.rubric_part_id = any(covered_part_ids))
and not exists (
select 1 from submission_comments sc
where sc.submission_review_id = target_submission_review_id
and sc.rubric_check_id = rc.id
and sc.deleted_at is null
union
select 1 from submission_file_comments sfc
where sfc.submission_review_id = target_submission_review_id
and sfc.rubric_check_id = rc.id
and sfc.deleted_at is null
union
select 1 from submission_artifact_comments sac
where sac.submission_review_id = target_submission_review_id
and sac.rubric_check_id = rc.id
and sac.deleted_at is null
)
) into has_blocking_uncovered_parts;

-- Also check if any uncovered part has min_checks_per_submission not met
if not has_blocking_uncovered_parts then
select exists (
select 1
from rubric_criteria rcrit
where rcrit.rubric_id = target_rubric_id
and rcrit.min_checks_per_submission is not null
and rcrit.rubric_part_id is not null
and not (rcrit.rubric_part_id = any(covered_part_ids))
and (
select count(distinct rc.id)
from rubric_checks rc
where rc.rubric_criteria_id = rcrit.id
and exists (
select 1 from submission_comments sc
where sc.submission_review_id = target_submission_review_id
and sc.rubric_check_id = rc.id
and sc.deleted_at is null
union
select 1 from submission_file_comments sfc
where sfc.submission_review_id = target_submission_review_id
and sfc.rubric_check_id = rc.id
and sfc.deleted_at is null
union
select 1 from submission_artifact_comments sac
where sac.submission_review_id = target_submission_review_id
and sac.rubric_check_id = rc.id
and sac.deleted_at is null
)
) < rcrit.min_checks_per_submission
) into has_blocking_uncovered_parts;
end if;
end if;

-- Only mark submission_review complete if no blocking uncovered parts
if not has_blocking_uncovered_parts then
update submission_reviews
set
completed_at = NEW.completed_at,
completed_by = completing_user_id
where id = target_submission_review_id
and completed_at is null;
end if;
end if;

return NEW;
end;
$$;

COMMENT ON FUNCTION public.check_and_complete_submission_review() IS
'Trigger function that handles review assignment completion with subset support:
1. When a review_assignment is marked complete, finds sibling assignments whose rubric parts
are a SUBSET of (or equal to) the completing assignment''s parts
2. Marks those siblings as complete (supports redundant grading where multiple graders review the same work)
3. If ALL review_assignments for the submission_review are now complete, validates that any
uncovered rubric parts have no blocking criteria (required checks or min_checks_per_submission)
4. Only marks the submission_review as complete if both conditions are satisfied
5. The complete_remaining_review_assignments trigger on submission_reviews handles any remaining stragglers

Example: If grader completes parts {A,B,C}, siblings with {A}, {A,B}, or {A,B,C} are auto-completed.
Siblings with {A,D} or {D} are NOT auto-completed (not subsets).

Uncovered parts validation: If rubric has parts {A,B,C,D} but only review_assignments for {D}:
- Parts A,B,C are "uncovered" (no review_assignment targets them)
- submission_review will only complete if A,B,C have no required checks or min_checks requirements unmet
- If A has a required check not applied, submission_review stays incomplete even though all review_assignments are done

Uses pg_trigger_depth() = 1 to only perform sibling completion at the top level, avoiding redundant work
when nested triggers fire for the completed siblings.';

-- Index to optimize sibling lookup: finding incomplete review_assignments by submission_review_id
CREATE INDEX IF NOT EXISTS idx_review_assignments_submission_review_incomplete
ON public.review_assignments (submission_review_id)
WHERE completed_at IS NULL;