Add instructor read-only "view as student" mode#779
Open
jon-bell wants to merge 14 commits into
Open
Conversation
Lets instructors see the course exactly as a specific student does — scoped to that student's dashboard, assignments, submissions, grades, discussion, and office hours — without being able to make changes. The active target is stored in a per-course cookie resolved identically on the server (role-branching layout/landing pages via getEffectiveCourseIdentity) and on the client (ClassProfileProvider overrides role/profile and exposes isViewingAsStudent / isReadOnly / enter/exit). Realtime works unchanged: an instructor is already authorized on the student's broadcast channels. Key student write surfaces are gated on isReadOnly, with RLS as the backstop. Entry point is a "View as this student" action on the enrollments roster. https://claude.ai/code/session_01BMhotQB8DyrrEgTY6uo85H
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (29)
WalkthroughAdds per-course "view as student" (cookie + SSR effective identity), expands ClassProfileProvider with read-only state and useIsReadOnly, renders a ViewAsBanner, adds an enrollments "View as" button, migrates the assignments dashboard to a SECURITY DEFINER RPC, gates many interactive components for read-only, and adds e2e tests. ChangesInstructor View-as-Student Mode
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
tests/e2e/view-as-student.spec.ts (1)
2-4: ⚡ Quick winUse root alias imports instead of relative imports.
Switch
../global-setupand./TestingUtilsto@/*aliases for consistency with project import rules.♻️ Proposed change
import { Course } from "`@/utils/supabase/DatabaseTypes`"; -import { test, expect } from "../global-setup"; +import { test, expect } from "`@/tests/global-setup`"; import dotenv from "dotenv"; -import { createClass, createUsersInClass, loginAsUser, TestingUser } from "./TestingUtils"; +import { createClass, createUsersInClass, loginAsUser, TestingUser } from "`@/tests/e2e/TestingUtils`";As per coding guidelines, "
**/*.{ts,tsx,js,jsx}: Use@/*path alias to reference files from project root".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/view-as-student.spec.ts` around lines 2 - 4, Replace the relative imports at the top of the test file with root-alias imports: change the module specifiers for the test harness import (currently "../global-setup") and the utilities import (currently "./TestingUtils") to use the project root alias (@"...") so that the imports still expose the same symbols (test, expect from global-setup; createClass, createUsersInClass, loginAsUser, TestingUser from TestingUtils); update only the import paths to the `@/` equivalents to conform to the project's path-alias rules.lib/viewAs.ts (1)
24-28: 💤 Low valueConsider adding
Secureflag for production environments.The cookie is set without the
Secureflag, which means it could be transmitted over unencrypted HTTP connections. While the cookie only contains a profile ID (not credentials) and the feature is gated to authenticated instructors, addingSecureis a defense-in-depth measure for production.A common pattern is to conditionally add it based on the environment:
🔒 Proposed conditional Secure flag
export function setViewAsCookie(courseId: number | string, profileId: string): void { if (typeof document === "undefined") return; - document.cookie = `${viewAsCookieName(courseId)}=${encodeURIComponent(profileId)}; path=/; SameSite=Lax`; + const secure = window.location.protocol === "https:" ? "; Secure" : ""; + document.cookie = `${viewAsCookieName(courseId)}=${encodeURIComponent(profileId)}; path=/; SameSite=Lax${secure}`; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/viewAs.ts` around lines 24 - 28, The setViewAsCookie function sets a session cookie without the Secure flag; update setViewAsCookie (and where it composes the cookie string) to append "; Secure" when running in a production/HTTPS context (e.g., check location.protocol === "https:" or a runtime prod flag) so the cookie is only sent over TLS in production while preserving current behavior in non-HTTPS/dev environments; reference setViewAsCookie and viewAsCookieName when making the change.app/course/[course_id]/discussion/new/page.tsx (1)
54-55: ⚡ Quick winUse
formDisabledin the Ctrl/Cmd+Enter submit guard for consistency.The shortcut gate still uses
formBusy, so read-only state is not part of the same decision path as the submit button/fieldset.Suggested fix
- if ((e.metaKey || e.ctrlKey) && e.key === "Enter" && !formBusy) { + if ((e.metaKey || e.ctrlKey) && e.key === "Enter" && !formDisabled) {Also applies to: 129-133
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/course/`[course_id]/discussion/new/page.tsx around lines 54 - 55, The submit shortcut currently gates on formBusy but the UI uses formDisabled (const formDisabled = formBusy || isReadOnly), so update the shortcut guard(s) to use formDisabled instead of formBusy; locate the keydown/shortcut handler(s) (e.g. the Ctrl/Cmd+Enter submit guard in the component, any references around the submit handler such as handleSubmit/onKeyDown or submitOnShortcut) and replace checks like "if (formBusy) return" with "if (formDisabled) return" so read-only state is respected consistently (also apply the same change to the other occurrence referenced around lines 129-133).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/course/`[course_id]/manage/course/enrollments/enrollmentsTable.tsx:
- Around line 809-822: The "View as this student" icon currently appears for
dropped/disabled enrollments; update the render condition to also require the
enrollment be active by checking the enrollment status/disabled flag before
showing the control—e.g., only render when realRole === "instructor",
userRoleEntry.role === "student", studentProfileId is present, and the
enrollment is not disabled (check userRoleEntry.disabled !== true and/or
userRoleEntry.status !== "dropped") so that enterViewAs(studentProfileId) is
only callable for active students.
---
Nitpick comments:
In `@app/course/`[course_id]/discussion/new/page.tsx:
- Around line 54-55: The submit shortcut currently gates on formBusy but the UI
uses formDisabled (const formDisabled = formBusy || isReadOnly), so update the
shortcut guard(s) to use formDisabled instead of formBusy; locate the
keydown/shortcut handler(s) (e.g. the Ctrl/Cmd+Enter submit guard in the
component, any references around the submit handler such as
handleSubmit/onKeyDown or submitOnShortcut) and replace checks like "if
(formBusy) return" with "if (formDisabled) return" so read-only state is
respected consistently (also apply the same change to the other occurrence
referenced around lines 129-133).
In `@lib/viewAs.ts`:
- Around line 24-28: The setViewAsCookie function sets a session cookie without
the Secure flag; update setViewAsCookie (and where it composes the cookie
string) to append "; Secure" when running in a production/HTTPS context (e.g.,
check location.protocol === "https:" or a runtime prod flag) so the cookie is
only sent over TLS in production while preserving current behavior in
non-HTTPS/dev environments; reference setViewAsCookie and viewAsCookieName when
making the change.
In `@tests/e2e/view-as-student.spec.ts`:
- Around line 2-4: Replace the relative imports at the top of the test file with
root-alias imports: change the module specifiers for the test harness import
(currently "../global-setup") and the utilities import (currently
"./TestingUtils") to use the project root alias (@"...") so that the imports
still expose the same symbols (test, expect from global-setup; createClass,
createUsersInClass, loginAsUser, TestingUser from TestingUtils); update only the
import paths to the `@/` equivalents to conform to the project's path-alias rules.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 55a2f0b3-43ce-4aa0-b26e-f63ec4d839d4
📒 Files selected for processing (15)
app/course/[course_id]/assignments/[assignment_id]/finalizeSubmissionEarly.tsxapp/course/[course_id]/discussion/discussion_thread.tsxapp/course/[course_id]/discussion/new/page.tsxapp/course/[course_id]/layout.tsxapp/course/[course_id]/manage/course/enrollments/enrollmentsTable.tsxapp/course/[course_id]/page.tsxcomponents/course/view-as-banner.tsxcomponents/help-queue/floating-help-request-widget.tsxcomponents/ui/RequestRegradeForCheckDialog.tsxcomponents/ui/line-comments-form.tsxcomponents/ui/request-regrade-dialog.tsxhooks/useClassProfiles.tsxlib/ssrUtils.tslib/viewAs.tstests/e2e/view-as-student.spec.ts
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
The "View as this student" roster control is a clickable icon (svg with aria-label), consistent with the other row actions, so it has no button role. Select it via getByLabel instead. https://claude.ai/code/session_01BMhotQB8DyrrEgTY6uo85H
…verage - Make the roster "View as this student" control a real, keyboard-operable IconButton (it was a clickable <svg> icon). The label-based selector from the prior commit still matches it. - Extend the read-only guarantee to two student-data write surfaces still reachable in view-as mode: survey response submission/editing and Feedbot hint feedback. (Office-hours help-chat replies remain a documented follow-up; the help-request creation widget is already gated.) - E2E: add coverage for write surfaces being read-only in view-as mode, and for a non-instructor's forged view_as cookie being ignored (server + client guards). - E2E: fix a router.refresh()/goto navigation race on exit (flaky on WebKit) by waiting for the instructor view to be restored before navigating. Verified locally: full view-as-student spec passes on chromium + webkit; tsc, ESLint, and Prettier clean for the changed files; production build succeeds. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Use the repo's visualScreenshot() helper (Argos-backed, with the standard stabilization) to capture one screenshot per new test: - "View as student - read-only discussion form" (banner + disabled compose form) - "View as student - forged cookie ignored for non-instructor" (no banner) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
hooks/useClassProfiles.tsx (1)
179-205:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't treat “view-as role still loading” as “not viewing as.”
When a valid course cookie is present, this lookup is async. Until it resolves,
viewAsRolestaysnull, so the provider renders the real instructor context for one paint and only then flips to read-only student mode. That briefly re-enables instructor UI/write gates and can recreate the controller churn this change is trying to avoid.🩹 Proposed fix
const [viewAsProfileId, setViewAsProfileId] = useState<string | null>(null); const [viewAsRole, setViewAsRole] = useState<UserRoleWithClassAndUser | null>(null); + const [isResolvingViewAs, setIsResolvingViewAs] = useState(false); @@ useEffect(() => { const isInstructor = realMyRole?.role === "instructor"; if (!isInstructor || !viewAsProfileId || !course_id) { setViewAsRole(null); + setIsResolvingViewAs(false); return; } let cancelled = false; const supabase = createClient(); + setIsResolvingViewAs(true); (async () => { const { data, error } = await supabase .from("user_roles") @@ .eq("disabled", false) .single(); if (cancelled) return; setViewAsRole(error || !data ? null : (data as UserRoleWithClassAndUser)); + setIsResolvingViewAs(false); })(); return () => { cancelled = true; }; }, [realMyRole?.role, viewAsProfileId, course_id]); @@ - if (isLoading) { + if (isLoading || isResolvingViewAs) { return <Skeleton height="100px" width="100%" />; }Also applies to: 295-308
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hooks/useClassProfiles.tsx` around lines 179 - 205, The effect currently sets viewAsRole to null immediately which treats "still loading" as "not viewing-as" and briefly re-enables instructor UI; change the logic in the useEffect that fetches the user_roles (the async IIFE inside useEffect, and the parallel block around lines 295-308) to preserve the previous viewAsRole while the fetch is pending by introducing a loading sentinel or a separate loading flag (e.g., viewAsRoleLoading) instead of setting setViewAsRole(null) at the start; only set viewAsRole to null when the fetch completes with error/no data or when the prerequisites (realMyRole?.role !== "instructor" or !viewAsProfileId or !course_id) truly indicate no view-as, and continue to honor the cancelled flag before applying the final setViewAsRole result.
🧹 Nitpick comments (1)
tests/e2e/view-as-student.spec.ts (1)
4-5: ⚡ Quick winUse the repo root alias for these new test helpers.
These new relative imports diverge from the repo-wide TS import rule. Please switch them to
@/*paths for consistency.♻️ Proposed fix
-import { createClass, createUsersInClass, insertAssignment, loginAsUser, TestingUser } from "./TestingUtils"; -import { visualScreenshot } from "./VisualTestUtils"; +import { + createClass, + createUsersInClass, + insertAssignment, + loginAsUser, + TestingUser +} from "`@/tests/e2e/TestingUtils`"; +import { visualScreenshot } from "`@/tests/e2e/VisualTestUtils`";As per coding guidelines,
**/*.{ts,tsx,js,jsx}: Use@/*path alias to reference files from project root🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/view-as-student.spec.ts` around lines 4 - 5, Replace the relative test helper imports with the repository root alias: update the import statements that bring in createClass, createUsersInClass, insertAssignment, loginAsUser, TestingUser (from "./TestingUtils") and visualScreenshot (from "./VisualTestUtils") to use the "`@/`..." path alias instead so they follow the project TS import rule; ensure the alias paths point to the same modules and run type-check/tests to verify the new imports resolve correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@supabase/migrations/20260522120000_view_as_student_dashboard_visibility.sql`:
- Around line 343-346: Change the overly-broad GRANTs on the view
assignments_for_student_dashboard so public-facing roles only get read access:
replace the two GRANT ALL lines for anon and authenticated with GRANT SELECT ON
TABLE public.assignments_for_student_dashboard TO anon; and GRANT SELECT ON
TABLE public.assignments_for_student_dashboard TO authenticated; (leave
service_role privileges as-is if it needs full access, or explicitly grant
appropriate higher privileges to service_role if intended).
---
Outside diff comments:
In `@hooks/useClassProfiles.tsx`:
- Around line 179-205: The effect currently sets viewAsRole to null immediately
which treats "still loading" as "not viewing-as" and briefly re-enables
instructor UI; change the logic in the useEffect that fetches the user_roles
(the async IIFE inside useEffect, and the parallel block around lines 295-308)
to preserve the previous viewAsRole while the fetch is pending by introducing a
loading sentinel or a separate loading flag (e.g., viewAsRoleLoading) instead of
setting setViewAsRole(null) at the start; only set viewAsRole to null when the
fetch completes with error/no data or when the prerequisites (realMyRole?.role
!== "instructor" or !viewAsProfileId or !course_id) truly indicate no view-as,
and continue to honor the cancelled flag before applying the final setViewAsRole
result.
---
Nitpick comments:
In `@tests/e2e/view-as-student.spec.ts`:
- Around line 4-5: Replace the relative test helper imports with the repository
root alias: update the import statements that bring in createClass,
createUsersInClass, insertAssignment, loginAsUser, TestingUser (from
"./TestingUtils") and visualScreenshot (from "./VisualTestUtils") to use the
"`@/`..." path alias instead so they follow the project TS import rule; ensure the
alias paths point to the same modules and run type-check/tests to verify the new
imports resolve correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 49c65b6f-76a8-45aa-b4e6-05aefb92241a
📒 Files selected for processing (4)
app/course/[course_id]/assignments/page.tsxhooks/useClassProfiles.tsxsupabase/migrations/20260522120000_view_as_student_dashboard_visibility.sqltests/e2e/view-as-student.spec.ts
Two reported bugs from running the feature end-to-end:
1. "TableController for table 'discussion_threads' is closed. Cannot call
getById(...). This indicates a stale reference is being used." — fired
immediately on entering view-as. enterViewAs/exitViewAs did a soft client
transition (setViewAsProfileId + router.push + router.refresh()) which
flipped the client identity while the existing CourseController was still
being torn down on its new profile_id/role props; components that still
held a reference to the closed discussion_threads TableController crashed
on the next getById. Fix: do a full document navigation
(window.location.assign) so the server recomputes the effective identity
from the cookie and every controller is rebuilt cleanly.
2. The /course/<id>/assignments view was empty when viewing as a student.
Two layered causes:
a) `public.assignments_for_student_dashboard` is `security_invoker=true`
and its `ur_students` CTE sourced from `public.user_privileges` with
`WHERE user_id = auth.uid()`. The only SELECT policy on
`public.user_privileges` is the inlinable `(user_id = auth.uid())`, so
an instructor saw zero student rows. Fix in migration
20260522120000_view_as_student_dashboard_visibility.sql: retarget the
CTE to `public.user_roles`, whose existing SELECT policy already lets
an instructor of a class read that class's student rows (its second
OR branch checks instructor/grader privilege via a user_privileges
sub-SELECT that is satisfied by the inlinable own-rows policy — no
recursion). Add an `authorizeforinstructorofstudent(user_id)` branch
to the CTE WHERE for the view-as path. `public.user_privileges` RLS
stays strictly the inlinable own-rows policy — no function-based or
otherwise opaque predicate is added to that hot table.
b) The Student assignments page filtered the view by
`student_user_id = useAuthState().user?.id`, i.e. the *signed-in*
user — which is the instructor in view-as. Use
`useClassProfiles().role.user_id` (the effective identity, which
follows view-as) instead. No change for normal students.
E2E: 4th test seeds an assignment in beforeAll and asserts it is visible on
/course/<id>/assignments while viewing as the student. All 8 tests (4 specs
× chromium + webkit) pass locally.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jon-bell
force-pushed
the
claude/instructor-student-view-DmoUl
branch
from
e1f5e85 to
bfe13e9
Compare
Replaces `public.assignments_for_student_dashboard` (security_invoker view) with
`public.get_assignments_for_student_dashboard(p_class_id, p_student_profile_id)`,
a SECURITY DEFINER plpgsql function. Per CLAUDE.md ("Prefer Postgres RPCs for
data operations") and Jon's explicit guidance:
- Authorization is one explicit gate at the top of the function: the caller
must be either the requested student themselves OR an instructor/grader of
the requested class. Unauthorized calls raise 42501.
- `public.user_privileges` RLS is untouched and stays strictly the inlinable
`(user_id = auth.uid())` only. The recursion trap from earlier drafts
(function-based or inline-EXISTS policies on user_privileges) is avoided
entirely because the RPC bypasses RLS by definer privilege.
- The CTE chain is bounded by the (p_class_id, p_student_profile_id) args,
so every downstream join is O(assignments) regardless of class size.
- Defensive cleanup at the top of the migration drops any leftover policies/
helpers that earlier exploratory iterations may have applied locally
(DROP ... IF EXISTS — no-op on fresh environments).
Frontend callers updated:
- app/course/[course_id]/assignments/page.tsx: replaces
`useList({ resource: "assignments_for_student_dashboard", ... })` with
`supabase.rpc("get_assignments_for_student_dashboard", { p_class_id, p_student_profile_id })`.
- hooks/useGradebookWhatIf.tsx: same switch. This also fixes a latent view-as
bug here (the prior code filtered by `courseController.userId`, which is
always the real signed-in user, so the gradebook what-if wouldn't have
matched a viewed-as student's row either).
Types regenerated via `npm run client-local`; the view entry is gone from
SupabaseTypes.d.ts and the new RPC appears under Functions.
All 8 view-as E2E tests (4 specs × chromium + webkit) pass locally.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
hooks/useClassProfiles.tsx (1)
171-205:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't publish the real instructor role before view-as resolves.
With an active
view_as_<course_id>cookie, this provider still renders once withviewAsProfileId === nulland again withviewAsRole === nulluntil the client lookup finishes. During that window — or permanently if the lookup errors — consumers see the real instructor role, so read-only gates can briefly re-enable instructor/write UI even though SSR already scoped the page to the student. Please keep the provider loading until the cookie target is resolved, or hydrate the effective identity from the server instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hooks/useClassProfiles.tsx` around lines 171 - 205, The provider currently exposes the real instructor role while the view-as cookie lookup is pending because viewAsProfileId is immediately set to null; change the flow so the lookup phase is distinguishable (e.g., use undefined as "resolving" and null as "no target") and keep viewAsRole in a loading/resolving state until the Supabase lookup finishes. Concretely: in the effect that calls getViewAsCookie (referenced by viewAsProfileId and setViewAsProfileId) do not set viewAsProfileId to null synchronously—set it to the cookie value when available and keep it undefined while resolving; in the effect that fetches the student role (the async useEffect that uses createClient(), .from("user_roles") and sets setViewAsRole) set setViewAsRole to an explicit loading sentinel (undefined) before the async call and only set a concrete UserRoleWithClassAndUser or null after the request completes (or error), and ensure consumers treat undefined as "still resolving" so realMyRole (realMyRole?.role) is not published until view-as resolution finishes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/course/`[course_id]/assignments/page.tsx:
- Around line 96-118: The useEffect that calls createClient() and
supabase.rpc("get_assignments_for_student_dashboard", ...) only clears loading
inside the .then path, so rejected RPCs can leave isLoading true; update the
call to handle rejections and always clear loading by adding a .catch(...)
and/or a finally block that calls setIsLoading(false), ensure
setAssignments(null) is set on error paths (including the catch), and keep the
cancelled guard; reference the existing symbols useEffect, createClient,
supabase.rpc, get_assignments_for_student_dashboard, setIsLoading,
setAssignments, and cancelled when making the change.
In `@hooks/useGradebookWhatIf.tsx`:
- Around line 75-89: The RPC call in initializeGradebookGrades assigns
this._assignments asynchronously but recalculateDependentColumns() and notify()
run before the RPC completes; update the promise handling for
client.rpc("get_assignments_for_student_dashboard", ...) so that in the
.then(...) after setting this._assignments = data ?? [] you immediately call
this.recalculateDependentColumns() and this.notify(), and also add a .catch(...)
to handle RPC errors (log or surface the error instead of treating it as empty
data) so failures don't silently leave stale formulas using assignments(...).
In `@supabase/migrations/20260522120000_view_as_student_dashboard_visibility.sql`:
- Around line 40-49: The OR predicate currently calls
public.authorizeforinstructorofstudent(ur.user_id) which ignores ur.class_id and
can wrongly grant access across classes; update the predicate to be class-scoped
by either calling a class-aware RPC (e.g.,
public.authorizeforinstructorofstudent(ur.user_id, ur.class_id) or a new
public.authorizeforinstructorofstudent_in_class(ur_user_id, class_id)) or
replace the RPC with an existence check that ensures the caller is an instructor
for ur.class_id (e.g., an EXISTS on class_instructors linking auth.uid() to
ur.class_id and ur.user_id); adjust or add the RPC signature and its SECURITY
DEFINER wrapper as needed so the view only returns rows when the instructor
relationship is for the same class (refer to authorizeforinstructorofstudent and
the user_roles alias ur.class_id).
---
Outside diff comments:
In `@hooks/useClassProfiles.tsx`:
- Around line 171-205: The provider currently exposes the real instructor role
while the view-as cookie lookup is pending because viewAsProfileId is
immediately set to null; change the flow so the lookup phase is distinguishable
(e.g., use undefined as "resolving" and null as "no target") and keep viewAsRole
in a loading/resolving state until the Supabase lookup finishes. Concretely: in
the effect that calls getViewAsCookie (referenced by viewAsProfileId and
setViewAsProfileId) do not set viewAsProfileId to null synchronously—set it to
the cookie value when available and keep it undefined while resolving; in the
effect that fetches the student role (the async useEffect that uses
createClient(), .from("user_roles") and sets setViewAsRole) set setViewAsRole to
an explicit loading sentinel (undefined) before the async call and only set a
concrete UserRoleWithClassAndUser or null after the request completes (or
error), and ensure consumers treat undefined as "still resolving" so realMyRole
(realMyRole?.role) is not published until view-as resolution finishes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 9a36eb97-e520-458f-a13a-e2fe20169366
📒 Files selected for processing (8)
app/course/[course_id]/assignments/page.tsxhooks/useClassProfiles.tsxhooks/useGradebookWhatIf.tsxsupabase/functions/_shared/SupabaseTypes.d.tssupabase/migrations/20260522120000_view_as_student_dashboard_visibility.sqlsupabase/migrations/20260522130000_assignments_dashboard_rpc.sqltests/e2e/view-as-student.spec.tsutils/supabase/SupabaseTypes.d.ts
- Flatten redundant migration: delete the view-rewrite migration whose output is immediately dropped by the subsequent SECURITY DEFINER RPC migration. The RPC migration already includes the same defensive cleanup statements. - useClassProfiles: keep the provider in a loading state while the view-as cookie target is being resolved. Initialize viewAsProfileId synchronously from the cookie and set an isResolvingViewAs flag so the real instructor identity is never published during the in-flight lookup (which previously briefly re-enabled instructor write UI). - enrollmentsTable: hide "View as this student" for disabled enrollments. - assignments/page.tsx and useGradebookWhatIf: wrap the dashboard RPC in try/catch (or .then with explicit error branch) so RPC rejections don't leave loading state stuck or formulas evaluating against []. After the RPC resolves in the what-if controller, rerun recalculateDependentColumns() and notify() so formulas using assignments(...) refresh. - view-as-student.spec.ts: use the `@/` alias for repo-local imports; refresh the comment describing how the instructor sees the student's assignments now that the data path is an RPC, not a view. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Move the view-as entry point from a small inline eye in the enrollments roster onto the per-student summary page as a labeled orange button, and restyle StudentSummaryTrigger as a clickable chip that wraps the name + icon (with a hover state) at every existing call site so instructors actually find their way to the summary from anywhere they see a student. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tograder/platform into claude/instructor-student-view-DmoUl
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Lets an instructor see the course exactly as a specific student does — their dashboard, assignments, submissions, grades, discussion, and office hours — scoped to that student and read-only.
view_as_<course_id>), resolved identically on the server and client so role-branching pages and client context agree. The override only applies when the real user is an instructor for the course and the cookie names a non-disabled student in it. Auth/RLS identity is unchanged — the override is purely presentation/scoping.getEffectiveCourseIdentity()inlib/ssrUtils.ts, used byapp/course/[course_id]/layout.tsx(controller scoping + prefetch) andapp/course/[course_id]/page.tsx(dashboard selection).ClassProfileProvider(hooks/useClassProfiles.tsx) overridesrole/profile and exposesisViewingAsStudent,isReadOnly,realRole,enterViewAs,exitViewAs, plus a newuseIsReadOnly()hook.enrollmentsTable.tsx), instructors only.components/course/view-as-banner.tsx).authorizeforprofile.role="student"and subscribe to the student's channels.check_realtime_authorizationalready authorizes an instructor onclass:<id>:students(viaauthorizeforclass) andclass:<id>:user:<studentProfileId>(viaauthorizeforclassgrader), so the read-only view is live. No schema/policy changes.Test plan
tests/e2e/view-as-student.spec.ts: instructor enters view-as from the roster, sees the banner + student-scoped view, asserts a successful realtime subscription to the student's channel (console capture), and exits cleanly./course/<id>/manage/course/enrollments→ "View as this student" → confirm banner, student nav, student dashboard, student submission + grades, live updates, and that write surfaces are disabled/hidden. "Exit student view" restores the instructor view; reload persists state.npx tsc --noEmitclean for changed files;eslintclean;npm run buildsucceeds.https://claude.ai/code/session_01BMhotQB8DyrrEgTY6uo85H
Generated by Claude Code
Summary by CodeRabbit
New Features
UI/UX Improvements
Tests
Chores