From 6efea75a40e621bd0fe90d2cdb43dc81407cb10e Mon Sep 17 00:00:00 2001
From: Cursor Agent <cursoragent@cursor.com>
Date: Mon, 2 Mar 2026 14:54:20 +0000
Subject: [PATCH 1/2] Fix stale rubric unsaved warning after save

Co-authored-by: Jonathan Bell <jon@jonbell.net>
---
 .../[assignment_id]/ManageAssignmentNav.tsx   |  13 ++-
 .../[assignment_id]/rubric/page.tsx           | 101 +++++++++++++++++-
 lib/rubricUnsavedChanges.ts                   |  36 +++++++
 tests/unit/rubric-unsaved-changes.test.ts     |  35 ++++++
 4 files changed, 182 insertions(+), 3 deletions(-)
 create mode 100644 lib/rubricUnsavedChanges.ts
 create mode 100644 tests/unit/rubric-unsaved-changes.test.ts

diff --git a/app/course/[course_id]/manage/assignments/[assignment_id]/ManageAssignmentNav.tsx b/app/course/[course_id]/manage/assignments/[assignment_id]/ManageAssignmentNav.tsx
index b218522fd..ef79f31cd 100644
--- a/app/course/[course_id]/manage/assignments/[assignment_id]/ManageAssignmentNav.tsx
+++ b/app/course/[course_id]/manage/assignments/[assignment_id]/ManageAssignmentNav.tsx
@@ -3,9 +3,10 @@
 import { useIsGraderOrInstructor, useIsInstructor } from "@/hooks/useClassProfiles";
 import { Box, Button, Flex, Heading, HStack, VStack } from "@chakra-ui/react";
 import { Select } from "chakra-react-select";
+import { hasRubricUnsavedChangesFlag, RUBRIC_UNSAVED_CHANGES_WARNING_MESSAGE } from "@/lib/rubricUnsavedChanges";
 import NextLink from "next/link";
 import { useParams, usePathname, useRouter } from "next/navigation";
-import React, { useState } from "react";
+import React, { useCallback, useState } from "react";
 import {
   FaCalendar,
   FaChartBar,
@@ -106,6 +107,15 @@ export function ManageAssignmentNav({
   const pathname = usePathname();
   const [selectedPage, setSelectedPage] = useState<string>("");
   const router = useRouter();
+  const confirmRubricNavigation = useCallback(
+    (nextHref: string) => {
+      if (!pathname.includes("/rubric")) return true;
+      if (pathname === nextHref) return true;
+      if (!hasRubricUnsavedChangesFlag(String(assignment_id))) return true;
+      return window.confirm(RUBRIC_UNSAVED_CHANGES_WARNING_MESSAGE);
+    },
+    [assignment_id, pathname]
+  );
 
   return (
     <>
@@ -156,6 +166,7 @@ export function ManageAssignmentNav({
           <Select
             onChange={(e) => {
               if (e) {
+                if (!confirmRubricNavigation(e.value)) return;
                 setSelectedPage(e.value);
                 router.replace(e.value);
               }
diff --git a/app/course/[course_id]/manage/assignments/[assignment_id]/rubric/page.tsx b/app/course/[course_id]/manage/assignments/[assignment_id]/rubric/page.tsx
index bb89be7fa..183d06d4f 100644
--- a/app/course/[course_id]/manage/assignments/[assignment_id]/rubric/page.tsx
+++ b/app/course/[course_id]/manage/assignments/[assignment_id]/rubric/page.tsx
@@ -41,7 +41,12 @@ import {
 import { useCreate, useDataProvider, useDelete, useInvalidate, useUpdate } from "@refinedev/core";
 import { configureMonacoYaml } from "monaco-yaml";
 import dynamic from "next/dynamic";
-import { memo, useCallback, useEffect, useMemo, useRef, useState } from "react";
+import { memo, useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from "react";
+import {
+  clearRubricUnsavedChangesFlag,
+  RUBRIC_UNSAVED_CHANGES_WARNING_MESSAGE,
+  setRubricUnsavedChangesFlag
+} from "@/lib/rubricUnsavedChanges";
 
 // Dynamic import of Monaco Editor to reduce build memory usage
 const Editor = dynamic(() => import("@monaco-editor/react").then((mod) => mod.default), {
@@ -532,6 +537,23 @@ function InnerRubricPage() {
       {} as Record<string, boolean>
     )
   );
+  const hasAnyUnsavedChanges = useMemo(
+    () => hasUnsavedChanges || Object.values(unsavedStatusPerTab).some(Boolean),
+    [hasUnsavedChanges, unsavedStatusPerTab]
+  );
+  const hasAnyUnsavedChangesRef = useRef(hasAnyUnsavedChanges);
+  const shouldSkipNextPopStateWarningRef = useRef(false);
+  const rubricPageRootRef = useRef<HTMLDivElement>(null);
+  const isRubricPageInstanceVisible = useCallback(() => {
+    const rootElement = rubricPageRootRef.current;
+    if (!rootElement) return true;
+    return rootElement.getClientRects().length > 0;
+  }, []);
+
+  useLayoutEffect(() => {
+    hasAnyUnsavedChangesRef.current = hasAnyUnsavedChanges;
+  }, [hasAnyUnsavedChanges]);
+
   const [stashedEditorStates, setStashedEditorStates] = useState<
     Record<
       string,
@@ -876,6 +898,81 @@ function InnerRubricPage() {
     }
   }, [value, initialActiveRubricSnapshot, activeReviewRound, assignment_id]);
 
+  useLayoutEffect(() => {
+    if (!assignment_id) return;
+    if (!isRubricPageInstanceVisible()) return;
+
+    setRubricUnsavedChangesFlag(assignment_id, hasAnyUnsavedChanges);
+    return () => {
+      clearRubricUnsavedChangesFlag(assignment_id);
+    };
+  }, [assignment_id, hasAnyUnsavedChanges, isRubricPageInstanceVisible]);
+
+  useEffect(() => {
+    const handleBeforeUnload = (event: BeforeUnloadEvent) => {
+      if (!isRubricPageInstanceVisible()) return;
+      if (!hasAnyUnsavedChangesRef.current) return;
+      event.preventDefault();
+      event.returnValue = "";
+    };
+
+    const handlePopState = () => {
+      if (!isRubricPageInstanceVisible()) return;
+      if (shouldSkipNextPopStateWarningRef.current) {
+        shouldSkipNextPopStateWarningRef.current = false;
+        return;
+      }
+      if (!hasAnyUnsavedChangesRef.current) return;
+
+      const shouldLeave = window.confirm(RUBRIC_UNSAVED_CHANGES_WARNING_MESSAGE);
+      if (shouldLeave) return;
+
+      shouldSkipNextPopStateWarningRef.current = true;
+      window.history.go(1);
+    };
+
+    const handleDocumentClick = (event: MouseEvent) => {
+      if (!isRubricPageInstanceVisible()) return;
+      if (!hasAnyUnsavedChangesRef.current || event.defaultPrevented) return;
+      if (event.button !== 0 || event.metaKey || event.ctrlKey || event.shiftKey || event.altKey) return;
+
+      const target = event.target;
+      if (!(target instanceof Element)) return;
+
+      const anchor = target.closest("a[href]");
+      if (!(anchor instanceof HTMLAnchorElement)) return;
+      if (anchor.target === "_blank" || anchor.hasAttribute("download")) return;
+
+      const href = anchor.getAttribute("href");
+      if (!href) return;
+
+      const currentUrl = new URL(window.location.href);
+      const nextUrl = new URL(href, window.location.href);
+      const isSameDocumentLocation =
+        currentUrl.origin === nextUrl.origin &&
+        currentUrl.pathname === nextUrl.pathname &&
+        currentUrl.search === nextUrl.search;
+
+      if (isSameDocumentLocation) return;
+
+      const shouldLeave = window.confirm(RUBRIC_UNSAVED_CHANGES_WARNING_MESSAGE);
+      if (shouldLeave) return;
+
+      event.preventDefault();
+      event.stopPropagation();
+    };
+
+    window.addEventListener("beforeunload", handleBeforeUnload);
+    window.addEventListener("popstate", handlePopState);
+    document.addEventListener("click", handleDocumentClick, true);
+
+    return () => {
+      window.removeEventListener("beforeunload", handleBeforeUnload);
+      window.removeEventListener("popstate", handlePopState);
+      document.removeEventListener("click", handleDocumentClick, true);
+    };
+  }, [isRubricPageInstanceVisible]);
+
   const updatePartIfChanged = useCallback(
     async (part: HydratedRubricPart, existingPart: HydratedRubricPart) => {
       if (part.id !== existingPart.id) {
@@ -1302,7 +1399,7 @@ function InnerRubricPage() {
   }
 
   return (
-    <Flex w="100%" minW="0" direction="column">
+    <Flex ref={rubricPageRootRef} w="100%" minW="0" direction="column">
       <HStack w="100%" mt={2} mb={2} justifyContent="space-between" pr={2}>
         <Toaster />
         <VStack align="start">
diff --git a/lib/rubricUnsavedChanges.ts b/lib/rubricUnsavedChanges.ts
new file mode 100644
index 000000000..109c63c01
--- /dev/null
+++ b/lib/rubricUnsavedChanges.ts
@@ -0,0 +1,36 @@
+export const RUBRIC_UNSAVED_CHANGES_WARNING_MESSAGE =
+  "You have unsaved rubric changes. Leave this page without saving?";
+
+const STORAGE_KEY_PREFIX = "pawtograder:rubric-unsaved-changes";
+
+function getStorageKeyOrNull(assignmentId: string | number): string | null {
+  const normalizedAssignmentId = String(assignmentId).trim();
+  if (!normalizedAssignmentId) return null;
+  return `${STORAGE_KEY_PREFIX}:${normalizedAssignmentId}`;
+}
+
+export function setRubricUnsavedChangesFlag(assignmentId: string | number, hasUnsavedChanges: boolean): void {
+  if (typeof window === "undefined") return;
+  const storageKey = getStorageKeyOrNull(assignmentId);
+  if (!storageKey) return;
+
+  if (hasUnsavedChanges) {
+    window.sessionStorage.setItem(storageKey, "true");
+    return;
+  }
+  window.sessionStorage.removeItem(storageKey);
+}
+
+export function hasRubricUnsavedChangesFlag(assignmentId: string | number): boolean {
+  if (typeof window === "undefined") return false;
+  const storageKey = getStorageKeyOrNull(assignmentId);
+  if (!storageKey) return false;
+  return window.sessionStorage.getItem(storageKey) === "true";
+}
+
+export function clearRubricUnsavedChangesFlag(assignmentId: string | number): void {
+  if (typeof window === "undefined") return;
+  const storageKey = getStorageKeyOrNull(assignmentId);
+  if (!storageKey) return;
+  window.sessionStorage.removeItem(storageKey);
+}
diff --git a/tests/unit/rubric-unsaved-changes.test.ts b/tests/unit/rubric-unsaved-changes.test.ts
new file mode 100644
index 000000000..df4adcc7d
--- /dev/null
+++ b/tests/unit/rubric-unsaved-changes.test.ts
@@ -0,0 +1,35 @@
+import {
+  clearRubricUnsavedChangesFlag,
+  hasRubricUnsavedChangesFlag,
+  setRubricUnsavedChangesFlag
+} from "@/lib/rubricUnsavedChanges";
+
+describe("rubric unsaved changes session state", () => {
+  const assignmentId = "123";
+
+  beforeEach(() => {
+    window.sessionStorage.clear();
+  });
+
+  it("sets and reads unsaved state", () => {
+    setRubricUnsavedChangesFlag(assignmentId, true);
+    expect(hasRubricUnsavedChangesFlag(assignmentId)).toBe(true);
+  });
+
+  it("clears unsaved state when set false", () => {
+    setRubricUnsavedChangesFlag(assignmentId, true);
+    setRubricUnsavedChangesFlag(assignmentId, false);
+    expect(hasRubricUnsavedChangesFlag(assignmentId)).toBe(false);
+  });
+
+  it("clears unsaved state explicitly", () => {
+    setRubricUnsavedChangesFlag(assignmentId, true);
+    clearRubricUnsavedChangesFlag(assignmentId);
+    expect(hasRubricUnsavedChangesFlag(assignmentId)).toBe(false);
+  });
+
+  it("ignores empty assignment ids", () => {
+    setRubricUnsavedChangesFlag("", true);
+    expect(hasRubricUnsavedChangesFlag("")).toBe(false);
+  });
+});

From 361cfc128fb987acbd1753d4e03802ba2f120caf Mon Sep 17 00:00:00 2001
From: Cursor Agent <cursoragent@cursor.com>
Date: Tue, 3 Mar 2026 02:41:12 +0000
Subject: [PATCH 2/2] Address PR feedback on rubric unsaved warning

Co-authored-by: Jonathan Bell <jon@jonbell.net>
---
 .../[assignment_id]/rubric/page.tsx           | 135 +++++++++++++++---
 lib/rubricUnsavedChanges.ts                   |  38 ++++-
 tests/unit/rubric-unsaved-changes.test.ts     |   5 +
 3 files changed, 154 insertions(+), 24 deletions(-)

diff --git a/app/course/[course_id]/manage/assignments/[assignment_id]/rubric/page.tsx b/app/course/[course_id]/manage/assignments/[assignment_id]/rubric/page.tsx
index 183d06d4f..29a399cf2 100644
--- a/app/course/[course_id]/manage/assignments/[assignment_id]/rubric/page.tsx
+++ b/app/course/[course_id]/manage/assignments/[assignment_id]/rubric/page.tsx
@@ -543,16 +543,66 @@ function InnerRubricPage() {
   );
   const hasAnyUnsavedChangesRef = useRef(hasAnyUnsavedChanges);
   const shouldSkipNextPopStateWarningRef = useRef(false);
-  const rubricPageRootRef = useRef<HTMLDivElement>(null);
-  const isRubricPageInstanceVisible = useCallback(() => {
-    const rootElement = rubricPageRootRef.current;
-    if (!rootElement) return true;
-    return rootElement.getClientRects().length > 0;
-  }, []);
+  const isRubricFlagOwnerRef = useRef(false);
+  const rubricFlagOwnerIdRef = useRef(`rubric-editor-${Math.random().toString(36).slice(2)}`);
+  const [rubricPageRootElement, setRubricPageRootElement] = useState<HTMLDivElement | null>(null);
+  const [isRubricPageInstanceVisible, setIsRubricPageInstanceVisible] = useState<boolean>(false);
+  const rubricUnsavedChangesOwnerStorageKey = useMemo(() => {
+    if (!assignment_id) return null;
+    return `pawtograder:rubric-unsaved-changes-owner:${assignment_id}`;
+  }, [assignment_id]);
+  const computeRubricPageInstanceVisibility = useCallback(() => {
+    if (!rubricPageRootElement) return false;
+    const computedStyle = window.getComputedStyle(rubricPageRootElement);
+    return (
+      rubricPageRootElement.getClientRects().length > 0 &&
+      computedStyle.display !== "none" &&
+      computedStyle.visibility !== "hidden"
+    );
+  }, [rubricPageRootElement]);
+  const syncRubricUnsavedChangesFlagOwner = useCallback(
+    (visibilityOverride?: boolean) => {
+      if (!assignment_id || !rubricUnsavedChangesOwnerStorageKey) return;
+      const isVisible = visibilityOverride ?? isRubricPageInstanceVisible;
+      const ownerId = rubricFlagOwnerIdRef.current;
+
+      if (isVisible) {
+        try {
+          window.sessionStorage.setItem(rubricUnsavedChangesOwnerStorageKey, ownerId);
+        } catch {
+          // Ignore restricted storage environments.
+        }
+        setRubricUnsavedChangesFlag(assignment_id, hasAnyUnsavedChangesRef.current);
+        isRubricFlagOwnerRef.current = true;
+        return;
+      }
+
+      if (!isRubricFlagOwnerRef.current) return;
+      let currentOwnerId: string | null = null;
+      try {
+        currentOwnerId = window.sessionStorage.getItem(rubricUnsavedChangesOwnerStorageKey);
+      } catch {
+        // Ignore restricted storage environments.
+      }
+      if (currentOwnerId && currentOwnerId !== ownerId) {
+        isRubricFlagOwnerRef.current = false;
+        return;
+      }
+      clearRubricUnsavedChangesFlag(assignment_id);
+      try {
+        window.sessionStorage.removeItem(rubricUnsavedChangesOwnerStorageKey);
+      } catch {
+        // Ignore restricted storage environments.
+      }
+      isRubricFlagOwnerRef.current = false;
+    },
+    [assignment_id, isRubricPageInstanceVisible, rubricUnsavedChangesOwnerStorageKey]
+  );
 
   useLayoutEffect(() => {
     hasAnyUnsavedChangesRef.current = hasAnyUnsavedChanges;
-  }, [hasAnyUnsavedChanges]);
+    syncRubricUnsavedChangesFlagOwner();
+  }, [hasAnyUnsavedChanges, syncRubricUnsavedChangesFlagOwner]);
 
   const [stashedEditorStates, setStashedEditorStates] = useState<
     Record<
@@ -898,26 +948,77 @@ function InnerRubricPage() {
     }
   }, [value, initialActiveRubricSnapshot, activeReviewRound, assignment_id]);
 
-  useLayoutEffect(() => {
-    if (!assignment_id) return;
-    if (!isRubricPageInstanceVisible()) return;
+  useEffect(() => {
+    if (!rubricPageRootElement) return;
+
+    const handleVisibilityChange = () => {
+      const isVisible = computeRubricPageInstanceVisibility();
+      setIsRubricPageInstanceVisible((previousVisibility) => {
+        if (previousVisibility === isVisible) return previousVisibility;
+        // Keep ownership state in sync immediately when visibility flips.
+        syncRubricUnsavedChangesFlagOwner(isVisible);
+        return isVisible;
+      });
+    };
+
+    handleVisibilityChange();
+
+    let resizeObserver: ResizeObserver | undefined;
+    if (typeof ResizeObserver !== "undefined") {
+      resizeObserver = new ResizeObserver(() => handleVisibilityChange());
+      resizeObserver.observe(rubricPageRootElement);
+    }
+
+    let intersectionObserver: IntersectionObserver | undefined;
+    if (typeof IntersectionObserver !== "undefined") {
+      intersectionObserver = new IntersectionObserver(() => handleVisibilityChange(), {
+        threshold: [0, 0.01]
+      });
+      intersectionObserver.observe(rubricPageRootElement);
+    }
 
-    setRubricUnsavedChangesFlag(assignment_id, hasAnyUnsavedChanges);
+    window.addEventListener("resize", handleVisibilityChange);
     return () => {
-      clearRubricUnsavedChangesFlag(assignment_id);
+      resizeObserver?.disconnect();
+      intersectionObserver?.disconnect();
+      window.removeEventListener("resize", handleVisibilityChange);
     };
-  }, [assignment_id, hasAnyUnsavedChanges, isRubricPageInstanceVisible]);
+  }, [computeRubricPageInstanceVisibility, rubricPageRootElement, syncRubricUnsavedChangesFlagOwner]);
+
+  useEffect(
+    () => () => {
+      if (!assignment_id || !rubricUnsavedChangesOwnerStorageKey || !isRubricFlagOwnerRef.current) return;
+      let currentOwnerId: string | null = null;
+      try {
+        currentOwnerId = window.sessionStorage.getItem(rubricUnsavedChangesOwnerStorageKey);
+      } catch {
+        // Ignore restricted storage environments.
+      }
+      if (currentOwnerId && currentOwnerId !== rubricFlagOwnerIdRef.current) {
+        isRubricFlagOwnerRef.current = false;
+        return;
+      }
+      clearRubricUnsavedChangesFlag(assignment_id);
+      try {
+        window.sessionStorage.removeItem(rubricUnsavedChangesOwnerStorageKey);
+      } catch {
+        // Ignore restricted storage environments.
+      }
+      isRubricFlagOwnerRef.current = false;
+    },
+    [assignment_id, rubricUnsavedChangesOwnerStorageKey]
+  );
 
   useEffect(() => {
     const handleBeforeUnload = (event: BeforeUnloadEvent) => {
-      if (!isRubricPageInstanceVisible()) return;
+      if (!isRubricPageInstanceVisible) return;
       if (!hasAnyUnsavedChangesRef.current) return;
       event.preventDefault();
       event.returnValue = "";
     };
 
     const handlePopState = () => {
-      if (!isRubricPageInstanceVisible()) return;
+      if (!isRubricPageInstanceVisible) return;
       if (shouldSkipNextPopStateWarningRef.current) {
         shouldSkipNextPopStateWarningRef.current = false;
         return;
@@ -932,7 +1033,7 @@ function InnerRubricPage() {
     };
 
     const handleDocumentClick = (event: MouseEvent) => {
-      if (!isRubricPageInstanceVisible()) return;
+      if (!isRubricPageInstanceVisible) return;
       if (!hasAnyUnsavedChangesRef.current || event.defaultPrevented) return;
       if (event.button !== 0 || event.metaKey || event.ctrlKey || event.shiftKey || event.altKey) return;
 
@@ -1399,7 +1500,7 @@ function InnerRubricPage() {
   }
 
   return (
-    <Flex ref={rubricPageRootRef} w="100%" minW="0" direction="column">
+    <Flex ref={setRubricPageRootElement} w="100%" minW="0" direction="column">
       <HStack w="100%" mt={2} mb={2} justifyContent="space-between" pr={2}>
         <Toaster />
         <VStack align="start">
diff --git a/lib/rubricUnsavedChanges.ts b/lib/rubricUnsavedChanges.ts
index 109c63c01..b409c9093 100644
--- a/lib/rubricUnsavedChanges.ts
+++ b/lib/rubricUnsavedChanges.ts
@@ -3,6 +3,33 @@ export const RUBRIC_UNSAVED_CHANGES_WARNING_MESSAGE =
 
 const STORAGE_KEY_PREFIX = "pawtograder:rubric-unsaved-changes";
 
+const safeSessionStorage = {
+  getItem(key: string): string | null {
+    if (typeof window === "undefined") return null;
+    try {
+      return window.sessionStorage.getItem(key);
+    } catch {
+      return null;
+    }
+  },
+  setItem(key: string, value: string): void {
+    if (typeof window === "undefined") return;
+    try {
+      window.sessionStorage.setItem(key, value);
+    } catch {
+      // No-op in restricted browsing contexts
+    }
+  },
+  removeItem(key: string): void {
+    if (typeof window === "undefined") return;
+    try {
+      window.sessionStorage.removeItem(key);
+    } catch {
+      // No-op in restricted browsing contexts
+    }
+  }
+};
+
 function getStorageKeyOrNull(assignmentId: string | number): string | null {
   const normalizedAssignmentId = String(assignmentId).trim();
   if (!normalizedAssignmentId) return null;
@@ -10,27 +37,24 @@ function getStorageKeyOrNull(assignmentId: string | number): string | null {
 }
 
 export function setRubricUnsavedChangesFlag(assignmentId: string | number, hasUnsavedChanges: boolean): void {
-  if (typeof window === "undefined") return;
   const storageKey = getStorageKeyOrNull(assignmentId);
   if (!storageKey) return;
 
   if (hasUnsavedChanges) {
-    window.sessionStorage.setItem(storageKey, "true");
+    safeSessionStorage.setItem(storageKey, "true");
     return;
   }
-  window.sessionStorage.removeItem(storageKey);
+  safeSessionStorage.removeItem(storageKey);
 }
 
 export function hasRubricUnsavedChangesFlag(assignmentId: string | number): boolean {
-  if (typeof window === "undefined") return false;
   const storageKey = getStorageKeyOrNull(assignmentId);
   if (!storageKey) return false;
-  return window.sessionStorage.getItem(storageKey) === "true";
+  return safeSessionStorage.getItem(storageKey) === "true";
 }
 
 export function clearRubricUnsavedChangesFlag(assignmentId: string | number): void {
-  if (typeof window === "undefined") return;
   const storageKey = getStorageKeyOrNull(assignmentId);
   if (!storageKey) return;
-  window.sessionStorage.removeItem(storageKey);
+  safeSessionStorage.removeItem(storageKey);
 }
diff --git a/tests/unit/rubric-unsaved-changes.test.ts b/tests/unit/rubric-unsaved-changes.test.ts
index df4adcc7d..952494050 100644
--- a/tests/unit/rubric-unsaved-changes.test.ts
+++ b/tests/unit/rubric-unsaved-changes.test.ts
@@ -32,4 +32,9 @@ describe("rubric unsaved changes session state", () => {
     setRubricUnsavedChangesFlag("", true);
     expect(hasRubricUnsavedChangesFlag("")).toBe(false);
   });
+
+  it("ignores whitespace-only assignment ids", () => {
+    setRubricUnsavedChangesFlag("   ", true);
+    expect(hasRubricUnsavedChangesFlag("   ")).toBe(false);
+  });
 });