Skip to content

Add a notice about deleted users#1663

Open
nygrenh wants to merge 1 commit intomasterfrom
review-fix
Open

Add a notice about deleted users#1663
nygrenh wants to merge 1 commit intomasterfrom
review-fix

Conversation

@nygrenh
Copy link
Member

@nygrenh nygrenh commented Feb 27, 2026

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Improved handling of deleted or missing user accounts across the application. When a user cannot be found, the system now displays a clear notice indicating the user is likely deleted, rather than showing blank fields. This applies to course instance points pages, user management pages, and submission details.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 27, 2026

📝 Walkthrough

Walkthrough

The changes introduce a new component (DeletedUserNotice) and refactor the user details hook to use a discriminated union type that explicitly represents the "not-found" case. Multiple pages are updated to consume the new hook format using helper functions (extractUserDetail, isUserDetailsNotFound) to derive user data and render appropriate UI when a user is deleted.

Changes

Cohort / File(s) Summary
User Details Hook
services/main-frontend/src/hooks/useUserDetails.ts
Introduced UserDetailsResult discriminated union type with ok and not-found variants; added extractUserDetail and isUserDetailsNotFound helper functions; updated useUserDetails hook to return wrapped result and handle 404/RecordNotFound errors explicitly.
Deleted User Notification
services/main-frontend/src/components/DeletedUserNotice.tsx
New component that displays a localized message when a user is not found, with styled userId display using Emotion CSS.
Pages and Components Using Hook
services/main-frontend/src/app/manage/course-instances/.../CourseInstanceUserInfoBox.tsx, services/main-frontend/src/app/manage/users/[id]/page.tsx, services/main-frontend/src/app/submissions/[id]/page.tsx, services/main-frontend/src/components/UserDisplay/index.tsx
Updated to use new hook helpers and conditionally render DeletedUserNotice when user details are not found; replaced direct data property access with derived user object access.
Localization
shared-module/packages/common/src/locales/en/main-frontend.json
Added new translation key message-user-likely-deleted for user not-found message.

Sequence Diagram(s)

sequenceDiagram
    participant Component as Page/Component
    participant Hook as useUserDetails
    participant API as API Backend
    
    Component->>Hook: Call useUserDetails(userId)
    Hook->>API: GET user details
    alt User Found
        API-->>Hook: 200 with user data
        Hook-->>Hook: Return { kind: "ok", user: ... }
        Hook-->>Component: UserDetailsResult (ok variant)
        Component->>Component: extractUserDetail() → user object
        Component->>Component: Render user info with UserDetails
    else User Not Found
        API-->>Hook: 404 or RecordNotFound error
        Hook-->>Hook: Return { kind: "not-found", userId }
        Hook-->>Component: UserDetailsResult (not-found variant)
        Component->>Component: isUserDetailsNotFound() → true
        Component->>Component: Render DeletedUserNotice
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

feature, refactor

Suggested reviewers

  • Redande

Poem

🐰 When users vanish from the store,
No error shadows—we handle more!
With types that guard and messages clear,
Deleted ones get notice here.
A union born, the path now bright! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add a notice about deleted users' accurately summarizes the main change: introducing a DeletedUserNotice component and integrating it across multiple user-related pages to display when user details are not found.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch review-fix

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
services/main-frontend/src/hooks/useUserDetails.ts (1)

57-68: Consider more robust error detection for RecordNotFound.

The message.includes("RecordNotFound") check is fragile as it depends on the exact error message format from the backend. If the backend error message format changes, this detection could break silently.

Consider also checking for a structured error code if the backend provides one, or documenting this coupling with the backend error format.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@services/main-frontend/src/hooks/useUserDetails.ts` around lines 57 - 68, The
current catch in useUserDetails (handling axiosError: AxiosError<ErrorResponse>)
relies on message.includes("RecordNotFound"), which is brittle; update the
detection in the catch block to first check for a structured error code (e.g.,
axiosError.response?.data?.code === "RecordNotFound" or other backend-provided
code) and only fall back to inspecting the message if the code is absent,
preserving the existing 404 check and returning the same { kind: "not-found",
userId: safeUserId } shape; ensure you reference axiosError, ErrorResponse, and
safeUserId in the changes and handle undefined response/data safely.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@services/main-frontend/src/hooks/useUserDetails.ts`:
- Around line 57-68: The current catch in useUserDetails (handling axiosError:
AxiosError<ErrorResponse>) relies on message.includes("RecordNotFound"), which
is brittle; update the detection in the catch block to first check for a
structured error code (e.g., axiosError.response?.data?.code ===
"RecordNotFound" or other backend-provided code) and only fall back to
inspecting the message if the code is absent, preserving the existing 404 check
and returning the same { kind: "not-found", userId: safeUserId } shape; ensure
you reference axiosError, ErrorResponse, and safeUserId in the changes and
handle undefined response/data safely.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d09d72b and 5388509.

📒 Files selected for processing (7)
  • services/main-frontend/src/app/manage/course-instances/[id]/points/user_id/CourseInstanceUserInfoBox.tsx
  • services/main-frontend/src/app/manage/users/[id]/page.tsx
  • services/main-frontend/src/app/submissions/[id]/page.tsx
  • services/main-frontend/src/components/DeletedUserNotice.tsx
  • services/main-frontend/src/components/UserDisplay/index.tsx
  • services/main-frontend/src/hooks/useUserDetails.ts
  • shared-module/packages/common/src/locales/en/main-frontend.json

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant