Skip to content

fix: add logic for when the doc id does not exist and fix form per ticket id 674#1152

Merged
carddev81 merged 2 commits into
mainfrom
carddev81/ticket_id674_confirmbytyping
Jun 7, 2026
Merged

fix: add logic for when the doc id does not exist and fix form per ticket id 674#1152
carddev81 merged 2 commits into
mainfrom
carddev81/ticket_id674_confirmbytyping

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

Added custom conditional logic for when DOC ID does not exist the user name is used instead. Also fixed form to use onSubmit rather than button

@carddev81 carddev81 requested a review from a team as a code owner June 2, 2026 21:38
@carddev81 carddev81 requested review from corypride and removed request for a team June 2, 2026 21:38
@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • Refactor
    • Streamlined form submission in resident edit dialogs: save now uses standard form submit behavior.
    • Improved confirmation prompts for resident actions (deactivate, delete, transfer) with clearer, context-aware labels and required confirmation tokens.
    • Cleaned up type definitions and formatting for bulk session updates, improving reliability of session-related controls.

Walkthrough

This PR updates form submission wiring in EditResidentDialog to use react-hook-form's form-level handler, refactors confirmation flows in Deactivate/Delete/Transfer dialogs to derive confirmToken/confirmLabel from doc_id or username, and applies type/formatting refinements to BulkSessionFieldModal.

Changes

Resident Dialog Form and Confirmation Refinements

Layer / File(s) Summary
useConfirmToken helper and wiring
frontend/src/components/residents/ResidentDialogs.tsx
Adds local useConfirmToken that derives confirmToken/confirmLabel from resident.doc_id or resident.username and wires the token into useTypeToConfirm.
EditResidentDialog form submission refactoring
frontend/src/components/residents/ResidentDialogs.tsx
EditResidentDialog submits via the form onSubmit handler that calls form.handleSubmit(...); "Save Changes" becomes type="submit" and "Cancel" becomes type="button".
Confirmation updates for Deactivate/Delete/Transfer
frontend/src/components/residents/ResidentDialogs.tsx
DeactivateDialog, DeleteDialog, and TransferDialog use useConfirmToken(resident, open) instead of displayId, update prompts to show {confirmLabel} and {confirmToken}, and pass the derived confirm into confirmation logic.
BulkSessionFieldModal type and formatting updates
frontend/src/pages/class-detail/BulkSessionFieldModal.tsx
reasonOptions prop type changed from Array<{ value; label }> to { value: string; label: string }[]; bulkPatchEvents, SelectValue, and SessionList calls reformatted without logic changes.
🚥 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 accurately reflects the main changes: adding fallback logic for missing doc IDs and fixing form submission behavior.
Description check ✅ Passed The description is relevant to the changeset, explaining the conditional logic for missing doc IDs and the form submission fix.
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.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
frontend/src/components/residents/ResidentDialogs.tsx (1)

406-414: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Input placeholder still hardcodes "Resident ID" after the label was made conditional.

The prompt now uses {confirmLabel} (which becomes "username" when doc_id is absent), but the placeholder on Line 412 remains "Type Resident ID to confirm", which is misleading in the username-fallback case. Derive it from confirmLabel instead.

🩹 Proposed fix
                     <Input
                         id="deactivate-confirm"
                         {...confirm.inputProps}
-                        placeholder="Type Resident ID to confirm"
+                        placeholder={`Type ${confirmLabel} to confirm`}
                         className="mt-2"
                     />
🤖 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/components/residents/ResidentDialogs.tsx` around lines 406 -
414, The placeholder text is hardcoded to "Type Resident ID to confirm" while
the label is dynamic via confirmLabel; update the Input with id
"deactivate-confirm" (the one using confirm.inputProps) to derive its
placeholder from confirmLabel (e.g., `Type ${confirmLabel} to confirm`, with
appropriate capitalization) so the prompt matches the displayed {confirmLabel}
and works when confirmLabel becomes "username".
🤖 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/residents/ResidentDialogs.tsx`:
- Around line 321-324: Extract the repeated confirm-token logic into a single
helper hook (e.g., function useConfirmToken(resident: User, open: boolean)) that
computes hasDocId, confirmToken (resident.doc_id || resident.username),
confirmLabel ('Resident ID' or 'username') and calls useTypeToConfirm({ open,
expected: confirmToken }) returning { confirmToken, confirmLabel, confirm };
then replace the four-line blocks inside DeactivateDialog, DeleteDialog, and
TransferDialog with a call to this hook and use the returned
confirmToken/confirmLabel/confirm variables instead of duplicating the logic.
- Around line 93-98: Remove the redundant e.preventDefault() call in the submit
handler inside ResidentDialogs.tsx: replace the current onSubmit that manually
calls e.preventDefault() and then form.handleSubmit(...) with a direct call to
form.handleSubmit(handleSave) (or equivalent wrapping used elsewhere) so that
react-hook-form handles the event prevention; ensure you still pass the same
handleSave callback used in the existing handler (reference form.handleSubmit
and handleSave to locate the code).

---

Outside diff comments:
In `@frontend/src/components/residents/ResidentDialogs.tsx`:
- Around line 406-414: The placeholder text is hardcoded to "Type Resident ID to
confirm" while the label is dynamic via confirmLabel; update the Input with id
"deactivate-confirm" (the one using confirm.inputProps) to derive its
placeholder from confirmLabel (e.g., `Type ${confirmLabel} to confirm`, with
appropriate capitalization) so the prompt matches the displayed {confirmLabel}
and works when confirmLabel becomes "username".
🪄 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: d241a4a1-f7d3-4dd7-b1e7-82dc73d84757

📥 Commits

Reviewing files that changed from the base of the PR and between ec4e2d9 and c70a0bd.

📒 Files selected for processing (2)
  • frontend/src/components/residents/ResidentDialogs.tsx
  • frontend/src/pages/class-detail/BulkSessionFieldModal.tsx

Comment thread frontend/src/components/residents/ResidentDialogs.tsx
Comment thread frontend/src/components/residents/ResidentDialogs.tsx Outdated

@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.

🐛 Bugs Caused by PR #1152

No PR-caused bugs identified during pre-test analysis. Code rabbit had a good suggestion that should be made, and this PR is good to go


🐛 Discovered Bugs — Unrelated to PR #1152

Bug: resident-verify returns 404 when resident has no doc_id

  • Severity: Medium
  • Blocker: No — this is a pre-existing bug unrelated to PR #1152. The PR can be merged as-is.
  • Description: GET /api/users/resident-verify?user_id=X&facility_id=Y&doc_id= returns 404 because GetUserByDocIDAndID fails on an empty doc_id. Surfaced when opening the Transfer dialog for a resident without a doc_id and selecting a destination facility.
  • Repro: Transfer dialog for any resident with doc_id = NULL → select destination facility → 404 in browser console.
  • Location: backend/src/database/users.goGetUserByDocIDAndID always queries WHERE LOWER(doc_id) = '' even when doc_id is empty, which never matches a NULL column.
  • Recommendation: When doc_id is an empty string, fall back to looking up by user_id alone. The endpoint's purpose is to verify the resident exists and check program transfer conflicts — doc_id is just a lookup key, not a security boundary.
    // backend/src/database/users.go
    func (db *DB) GetUserByDocIDAndID(ctx context.Context, docID string, userID int) (*models.User, error) {
        user := models.User{}
        query := db.WithContext(ctx)
        if docID != "" {
            query = query.Where("LOWER(doc_id) = ? AND id = ?", strings.ToLower(docID), userID)
        } else {
            query = query.Where("id = ?", userID)
        }
        if err := query.First(&user).Error; err != nil {
            return nil, newNotFoundDBError(err, "users")
        }
        return &user, nil
    }

🗓️ Testing completed on:

  • Date: 2026-06-04

📊 Overall Results

GTG!

@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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
frontend/src/components/residents/ResidentDialogs.tsx (1)

419-420: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Make confirm input placeholders dynamic to match token fallback.

These placeholders still hardcode “Resident ID”, but the required token can now be username. Use confirmLabel/confirmToken in the placeholder text so the instruction is consistent in fallback cases.

💡 Suggested patch
-                        placeholder="Type Resident ID to confirm"
+                        placeholder={`Type ${confirmLabel} to confirm`}
-                        placeholder="Type Resident ID to confirm"
+                        placeholder={`Type ${confirmLabel} to confirm`}
-                                placeholder="Enter Resident ID"
+                                placeholder={`Enter ${confirmLabel}`}

Also applies to: 507-508, 722-723

🤖 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/components/residents/ResidentDialogs.tsx` around lines 419 -
420, Update the hardcoded placeholder "Type Resident ID to confirm" to use the
dynamic confirmLabel/confirmToken values so the instruction matches the current
token fallback; find the placeholder usages in ResidentDialogs.tsx (around the
locations where the input element with className "mt-2" appears) and replace the
static text with a placeholder that references confirmLabel (or confirmToken if
that is the intended token name) so it reads dynamically (e.g., "Type
{confirmLabel} to confirm") in all occurrences (also update the other two
instances at the same pattern near the noted locations).
🤖 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.

Outside diff comments:
In `@frontend/src/components/residents/ResidentDialogs.tsx`:
- Around line 419-420: Update the hardcoded placeholder "Type Resident ID to
confirm" to use the dynamic confirmLabel/confirmToken values so the instruction
matches the current token fallback; find the placeholder usages in
ResidentDialogs.tsx (around the locations where the input element with className
"mt-2" appears) and replace the static text with a placeholder that references
confirmLabel (or confirmToken if that is the intended token name) so it reads
dynamically (e.g., "Type {confirmLabel} to confirm") in all occurrences (also
update the other two instances at the same pattern near the noted locations).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2d616910-da00-4ede-9ddf-c1d981ac2285

📥 Commits

Reviewing files that changed from the base of the PR and between c70a0bd and 7828746.

📒 Files selected for processing (1)
  • frontend/src/components/residents/ResidentDialogs.tsx

@carddev81 carddev81 merged commit 0c34ad1 into main Jun 7, 2026
10 checks passed
@carddev81 carddev81 deleted the carddev81/ticket_id674_confirmbytyping branch June 7, 2026 21:47
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