Skip to content

fix: clean up password reset logic per ticket id 671#1158

Open
carddev81 wants to merge 1 commit into
mainfrom
carddev81/ticket_id671_resetmodal
Open

fix: clean up password reset logic per ticket id 671#1158
carddev81 wants to merge 1 commit into
mainfrom
carddev81/ticket_id671_resetmodal

Conversation

@carddev81

Copy link
Copy Markdown
Contributor

Pre-Submission PR Checklist

  • No debug/console/fmt.Println statements
  • Unnecessary development comments removed
  • All acceptance criteria verified
  • Functions according to ticket specifications
  • Tested manually where applicable
  • Branch rebased with latest main
  • No business logic exists within the database layer

Description of the change

Cleaned up password reset logic per ticket 671.

@carddev81 carddev81 requested a review from a team as a code owner June 3, 2026 22:00
@carddev81 carddev81 requested review from CK-7vn and removed request for a team June 3, 2026 22:00
@coderabbitai

coderabbitai Bot commented Jun 3, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@carddev81, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 19 minutes and 57 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: acd21705-5802-49a2-880e-325e9f769d5c

📥 Commits

Reviewing files that changed from the base of the PR and between 86fbe21 and dcc75b6.

📒 Files selected for processing (7)
  • frontend/src/components/residents/BulkActionDialogs.tsx
  • frontend/src/components/residents/ResidentDialogs.tsx
  • frontend/src/components/shared/ResetPasswordModal.tsx
  • frontend/src/components/shared/index.ts
  • frontend/src/pages/admin/AdminManagement.tsx
  • frontend/src/pages/admin/ResidentProfile.tsx
  • frontend/src/pages/admin/StudentManagement.tsx
📝 Walkthrough

Walkthrough

This PR consolidates the password reset UI by refactoring ResetPasswordDialog into a flexible shared ResetPasswordModal component that supports both confirm-then-reset and result-only modes. The old resident-specific reset dialogs are removed, and the new modal is integrated across AdminManagement, ResidentProfile, and StudentManagement pages.

Changes

Password reset modal consolidation

Layer / File(s) Summary
ResetPasswordModal contract and implementation
frontend/src/components/shared/ResetPasswordModal.tsx, frontend/src/components/shared/index.ts
New ResetPasswordModal component replaces the old ResetPasswordDialog with flexible props supporting both confirm-then-reset mode (userId) and result-only mode (presetPassword). State initialization and reset are tied to modal lifecycle. API call to users/{userId}/student-password posts new password request and transitions to result phase. Copy-to-clipboard flow now marks password as copied even in fallback branch. FormModal wiring uses dynamic title, description, and resultTitle, with text interpolating name and subject instead of hardcoded "resident" wording.
Remove old resident reset dialogs
frontend/src/components/residents/ResidentDialogs.tsx
Delete ResetPasswordConfirmDialog and ResetPasswordResultDialog components and remove unused ResetPasswordResponse type and Copy icon imports.
Resident dialog toast formatting updates
frontend/src/components/residents/ResidentDialogs.tsx
Reformat success toast messages in EditResidentDialog, DeactivateDialog, DeleteDialog, and TransferDialog to multi-line template literals.
AdminManagement integration
frontend/src/pages/admin/AdminManagement.tsx
Import ResetPasswordModal from shared components. Remove local clipboard state and refactor handleResetPassword to synchronous state setter that prepares context and opens modal. Update toast formatting in add, edit, and delete handlers. Replace inline password display modal with ResetPasswordModal component, wiring presetPassword for create flow and userId for reset flow, with titles derived from passwordModalContext.
Bulk action UI updates
frontend/src/components/residents/BulkActionDialogs.tsx
Add "Close" button alongside "Download Password File" in bulk reset footer and reformat delete-error toast to multi-line call.
ResidentProfile integration
frontend/src/pages/admin/ResidentProfile.tsx
Switch from ResetPasswordDialog to ResetPasswordModal imported from shared components, wiring resident name and userId instead of passing user object. Reformat facilities useSWR call and breadcrumb/name rendering.
StudentManagement refactor
frontend/src/pages/admin/StudentManagement.tsx
Remove legacy ResetPasswordConfirmDialog and ResetPasswordResultDialog imports and add ResetPasswordModal from shared. Remove reset-result and temp-password state variables. Reorganize main JSX into clearer sections (header, search, stats, table, dialogs, bulk actions) and wire reset action to ResetPasswordModal with resident name and userId. Reformat type imports, useSWR generic, and helper functions.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically references consolidating/cleaning up password reset logic per ticket 671, which aligns with the main changes of refactoring reset-password components across multiple files.
Description check ✅ Passed The description references ticket 671 and the consolidation of reset-password modal implementations, which directly relates to the changeset's refactoring of password reset components from separate dialogs into unified modal implementations.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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 `@frontend/src/components/shared/ResetPasswordModal.tsx`:
- Around line 82-100: The frontend uses handleReset which calls
API.post(`users/${userId}/student-password`) but the backend handler is
handleResetStudentPassword and its authorization (canResetUserPassword) already
allows admin resets, so the route name is misleading; rename the endpoint and
handler to a neutral name (e.g., users/{id}/reset-password or
handleResetUserPassword) and update all references including API.post in
handleReset and any docs/UI copy (or alternatively add clear docs/comments
linking the existing student-password route to admin usage) so the route name
accurately reflects its purpose and prevents confusion.

In `@frontend/src/pages/admin/StudentManagement.tsx`:
- Around line 650-654: The BulkImportDialog success handler only calls mutate()
so the stats cards don't refresh; change its onSuccess to call the existing
handleMutate function (which refreshes both the list and the resident totals)
instead of mutate — update the BulkImportDialog prop from onSuccess={() => void
mutate()} to onSuccess={() => void handleMutate()} (or call handleMutate() in
addition to mutate if you want to keep the original behavior).
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 41bd81f5-6b2e-47e1-bab4-c2a4b59e6e7a

📥 Commits

Reviewing files that changed from the base of the PR and between 92d5275 and 86fbe21.

📒 Files selected for processing (7)
  • frontend/src/components/residents/BulkActionDialogs.tsx
  • frontend/src/components/residents/ResidentDialogs.tsx
  • frontend/src/components/shared/ResetPasswordModal.tsx
  • frontend/src/components/shared/index.ts
  • frontend/src/pages/admin/AdminManagement.tsx
  • frontend/src/pages/admin/ResidentProfile.tsx
  • frontend/src/pages/admin/StudentManagement.tsx

Comment thread frontend/src/components/shared/ResetPasswordModal.tsx
Comment on lines +650 to +654
<BulkImportDialog
open={bulkImportOpen}
onOpenChange={setBulkImportOpen}
onSuccess={() => void mutate()}
/>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Refresh the stats cards after bulk import.

BulkImportDialog only revalidates the table query here. The resident totals above stay stale until SWR happens to refetch, even though handleMutate already encapsulates the correct list + stats refresh pair.

Suggested fix
             <BulkImportDialog
                 open={bulkImportOpen}
                 onOpenChange={setBulkImportOpen}
-                onSuccess={() => void mutate()}
+                onSuccess={handleMutate}
             />
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<BulkImportDialog
open={bulkImportOpen}
onOpenChange={setBulkImportOpen}
onSuccess={() => void mutate()}
/>
<BulkImportDialog
open={bulkImportOpen}
onOpenChange={setBulkImportOpen}
onSuccess={handleMutate}
/>
🤖 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 `@frontend/src/pages/admin/StudentManagement.tsx` around lines 650 - 654, The
BulkImportDialog success handler only calls mutate() so the stats cards don't
refresh; change its onSuccess to call the existing handleMutate function (which
refreshes both the list and the resident totals) instead of mutate — update the
BulkImportDialog prop from onSuccess={() => void mutate()} to onSuccess={() =>
void handleMutate()} (or call handleMutate() in addition to mutate if you want
to keep the original behavior).

@carddev81 carddev81 requested review from corypride and removed request for CK-7vn June 8, 2026 19:49
@carddev81 carddev81 force-pushed the carddev81/ticket_id671_resetmodal branch from 86fbe21 to dcc75b6 Compare June 8, 2026 20:19

@corypride corypride left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

frontend/src/pages/admin/resident-profile/ResetPasswordDialog.tsx is no longer imported anywhere after this PR — ResidentProfile.tsx switches to ResetPasswordModal but the old file isn't deleted. Safe to remove.

Otherwise the consolidation looks good — the new modal handles both modes cleanly and the clipboard fallback is an improvement over the old implementations.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants