Skip to content

Joel/user type changes#105

Merged
joelchem merged 4 commits into
mainfrom
joel/user-type-changes
May 24, 2026
Merged

Joel/user type changes#105
joelchem merged 4 commits into
mainfrom
joel/user-type-changes

Conversation

@joelchem
Copy link
Copy Markdown
Collaborator

@joelchem joelchem commented May 24, 2026

  • UserData schema — removed dob/phoneExtension; added phone (required), createdTime, signedWaiver,
    disabled
    • Account disabling — admins can disable/enable users from EditAccountModal (with confirm dialog); disabled users
      are blocked at ProtectedRoute and redirected to new /status/account-disabled; "Disabled" badge + Active/Disabled
      filter in All Accounts
    • Forgot password — new /forgot-password page; "Forgot Password?" on login now navigates there
    • Auth action handler — new /auth/action page handles Firebase verifyEmail and resetPassword action codes
      (email verification + password reset flow end-to-end)
    • Auth UX — human-readable error messages on login/signup via authErrorMessage() util; verify-email and
      account-pending pages auto-redirect on tab focus; spam folder hint added to verification copy
    • Phone formatting — shared formatPhone() util + consistent XXX-XXX-XXXX masking and native validation across
      signup, profile, donate, and client request form
    • Signup — removed phoneExtension/dob fields; phone now required; password min-length enforced at 8 characters
      (also updated in profile password reset)

Summary by CodeRabbit

  • New Features

    • Added password reset functionality with forgot password page
    • Added account disabled status page with logout capability
    • Added account status filtering to user management
    • Enhanced email verification flow with improved redirects
  • Bug Fixes

    • Increased password minimum length requirement to 8 characters
    • Improved phone number input formatting and validation
    • Enhanced authentication error messaging
  • Documentation

    • Updated documentation for routing behavior and Firestore schema

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 24, 2026

Warning

Review limit reached

@joelchem, we couldn't start this review because you've used your available PR reviews for now.

Your plan currently allows 1 review/hour. Refill in 37 minutes and 49 seconds.

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

⌛ How to resolve this issue?

After more review capacity refills, 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 have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 35925674-73a5-4066-ba05-fc85a9cb701b

📥 Commits

Reviewing files that changed from the base of the PR and between 920cc28 and 8f32681.

📒 Files selected for processing (7)
  • app/auth/action/page.tsx
  • app/forgot-password/page.tsx
  • app/profile/page.tsx
  • app/status/account-pending/page.tsx
  • app/status/verify-email/page.tsx
  • components/user-management/EditAccountModal.tsx
  • contexts/AuthContext.tsx

Walkthrough

This PR redesigns the user data schema to include account metadata (createdTime, signedWaiver, disabled), removes date-of-birth and phone-extension fields, and refactors authentication and form handling throughout the app. It introduces email verification and password-reset flows via a new auth action handler, adds an account-disabled status page with ProtectedRoute integration, and standardizes phone input formatting using a shared utility.

Changes

User Authentication and Account Management

Layer / File(s) Summary
User data schema and type definitions
types/user.ts
UserData adds createdTime, signedWaiver, disabled; makes phone required; removes phoneExtension and dob. AuthContextType.signup signature updated to match.
Shared utilities for phone formatting and error handling
lib/utils/phone.ts, lib/utils/auth-errors.ts, lib/services/auth.ts
New formatPhone utility normalizes phone input to XXX-XXX-XXXX. New authErrorMessage maps Firebase error codes to user-facing strings. signUp function signature simplified and sets new account fields (phone, createdTime, disabled, signedWaiver) explicitly. New resetPassword(email) function added.
Form component enhancements for validation
components/form/FormInput.tsx, components/auth/LongButton.tsx
FormInput supports optional placeholder, pattern, and onInvalid props for HTML5 validation. LongButton gains optional onClick handler and updated disabled styling via opacity.
Authentication action handler and forgot-password flow
app/auth/action/page.tsx, app/forgot-password/page.tsx, app/login/page.tsx
New auth action page handles email verification and password reset via query parameters (mode, oobCode). New forgot-password page with email submission and secure error handling. Updated login page uses authErrorMessage and adds forgot-password navigation.
Signup form and authentication context updates
app/signup/PickRole.tsx, app/signup/SignUpInformation.tsx, contexts/AuthContext.tsx
PickRole adds router-based logo navigation. SignUpInformation removes phone extension and DOB fields, adds first/last name inputs, uses formatPhone for phone validation. AuthContext._signup simplified to remove phoneExtension and dob parameters.
Account disabled status handling and routing
components/general/ProtectedRoute.tsx, app/status/account-disabled/page.tsx, app/status/account-pending/page.tsx, app/status/verify-email/page.tsx, app/status/account-created/page.tsx
ProtectedRoute checks disabled flag and redirects to /status/account-disabled. New account-disabled page with logout functionality. AccountPending and VerifyEmail pages add window focus listeners to re-check user status and redirect when conditions change. AccountCreated page adds logo navigation.
Admin user management with disabled accounts and filtering
components/user-management/EditAccountModal.tsx, components/user-management/AccountReqTable.tsx, components/user-management/UserTable.tsx, app/user-management/all-accounts/page.tsx
EditAccountModal adds created timestamp display, disabled toggle (with confirmation) for non-self users, and disables role dropdown for self-editing. AccountReqTable updates badge color conditionally based on pending value. UserTable adds disabled badge. AllAccountsPage adds status filtering dropdown.
Account and donation forms with phone formatting
app/profile/page.tsx, app/client-request-form/steps/Step1ClientInfo.tsx, app/donate/steps/Step1PersonalInfo.tsx, components/profile/PasswordResetSection.tsx
Profile page removes DOB and uses formatPhone for phone input. Client request and donation forms apply formatPhone with HTML validation patterns. PasswordResetSection updates minimum new password length to 8 characters.
Configuration, test data, and minor styling updates
firebase.json, next.config.ts, lib/queries/inventory.ts, app/debug/data/page.tsx, components/homepage/VolunteerHomePage.tsx, CLAUDE.md
Firebase emulator config adds host: "0.0.0.0" binding. Next.js config adds allowedDevOrigins for local development. Inventory queries switch to uuidv4(). Debug data seeding updated for new schema. VolunteerHomePage adds left border. CLAUDE.md documented with schema, routing, and convention updates.

🎯 4 (Complex) | ⏱️ ~60 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

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.
Title check ❓ Inconclusive The title 'Joel/user type changes' is vague and generic, using non-descriptive branch naming conventions rather than describing the actual changeset content. Consider a more descriptive title that highlights the main change, such as 'Update user schema and add account disabling functionality' or 'Refactor UserData type and implement account management features'.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
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.

Copy link
Copy Markdown

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

Actionable comments posted: 11

Caution

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

⚠️ Outside diff range comments (2)
app/debug/data/page.tsx (1)

40-52: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift

Move Firestore writes out of the component layer.

This page is writing to users directly from a TSX component (setDoc/doc). Please route this through lib/queries/lib/services/ to match the project’s data-access boundary.

As per coding guidelines, "**/*.{tsx,jsx}: Never call Firestore directly from components; use lib/queries/ (React Query) → lib/services/ (Firebase) → SDK".

🤖 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/debug/data/page.tsx` around lines 40 - 52, This component is directly
calling Firestore (setDoc, doc, Timestamp.now()) — move that logic into a
Firebase service and expose it via a React-Query wrapper: create a service
function (e.g., createUserInFirestore / setUser) in lib/services/ that accepts
the user payload and performs setDoc(doc(db, "users", uid), {...}) and
Timestamp.now(), then create a query/mutation wrapper (e.g., useCreateUser) in
lib/queries/ that calls the service; replace the direct setDoc call in page.tsx
with a call to the useCreateUser mutation (or await the service function only if
used outside React lifecycle) so all Firestore access lives under lib/services
and is consumed through lib/queries.
app/status/verify-email/page.tsx (1)

7-38: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Honor disabled in the new focus re-check.

After the tab regains focus, this code only routes by verification and pending. That leaves disabled accounts on the wrong status screen, and a disabled+pending user can be sent back to /status/account-pending, which is not protected.

Suggested fix
     const handleFocus = async () => {
       if (!auth.currentUser) return;
       const { verified } = await authContext.checkVerification();
+      const userData = await getUserByUID(auth.currentUser.uid);
+
+      if (userData?.disabled) {
+        router.push('/status/account-disabled');
+        return;
+      }
+
       if (verified) {
-        const userData = await getUserByUID(auth.currentUser.uid);
         router.push(userData?.pending ? '/status/account-pending' : '/status/account-created');
       }
     };
🤖 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/status/verify-email/page.tsx` around lines 7 - 38, The focus handler
currently only checks verification and pending status; update the handleFocus
logic to also fetch the user record (via getUserByUID) and honor a disabled flag
before deciding routes: after authContext.checkVerification() returns verified,
call getUserByUID(auth.currentUser.uid), then if userData?.disabled route to
'/status/account-disabled' (even if pending), else if userData?.pending route to
'/status/account-pending', otherwise route to '/status/account-created'; ensure
you still guard with auth.currentUser and keep existing cleanup/listener code.
🤖 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/auth/action/page.tsx`:
- Around line 160-163: The reset button stays clickable while a reset is in
flight; update the form handling so multiple submissions are prevented by
disabling the UI and short-circuiting duplicate calls: add a disabled={loading}
(or equivalent) prop to the LongButton instance and ensure the submit handler
(the function that calls confirmPasswordReset) checks/loading-guards so it
returns early if loading is true before invoking confirmPasswordReset; reference
LongButton, the loading state variable, and confirmPasswordReset to locate where
to add the guard and disabled prop.
- Around line 42-57: VerifyEmailView/applyActionCode and ResetPasswordView's
verifyPasswordResetCode and handleSubmit need to handle FirebaseError codes
explicitly: update the .catch handlers for applyActionCode and
verifyPasswordResetCode to inspect the caught error (typed as FirebaseError),
set the local status for known codes like 'auth/invalid-action-code' and
'auth/expired-action-code' (or the corresponding reset codes), and rethrow or
propagate any unexpected errors; likewise, in ResetPasswordView.handleSubmit
where confirmPasswordReset is called, branch on specific FirebaseError.code
values (e.g., 'auth/expired-action-code') to set user-facing messages and
rethrow any other error to surface unknown failures. Reference functions:
applyActionCode, VerifyEmailView, verifyPasswordResetCode, ResetPasswordView,
handleSubmit, confirmPasswordReset; ensure you check error.code on the caught
error and only treat known action-code values as handled.

In `@app/debug/data/page.tsx`:
- Around line 32-35: TEST_ACCOUNTS includes a waiver flag but the user creation
logic still writes signedWaiver as null; update the seeding/persistence code
that consumes TEST_ACCOUNTS (the routine that maps each TEST_ACCOUNTS entry to a
user record) to set the user's signedWaiver field from the source object's
waiver property instead of hardcoding null—find references to TEST_ACCOUNTS and
the signedWaiver assignment in the seed/import function and change the
assignment to use account.waiver (or the equivalent property) so volunteer seed
state reflects the waiver value.

In `@app/forgot-password/page.tsx`:
- Around line 102-106: The submit button isn't disabling when a request is in
progress—wire the loading state into the LongButton by passing
disabled={loading} on the LongButton used in the form (the same place using
name={loading ? 'Sending...' : 'Send Reset Email'}), and if LongButton doesn't
already accept a disabled prop, update its props to forward disabled to the
underlying <button> so handleSubmit's loading guard can actually prevent
duplicate submissions.
- Around line 33-42: In the resetPassword error handling, treat only the
FirebaseError with code 'auth/user-not-found' as success by calling
setSent(true); for any other caught error (not that specific FirebaseError)
rethrow the error instead of calling setError(...) so unexpected failures bubble
up (check the catch handling around resetPassword and FirebaseError). Also wire
the loading state into the submit button by passing disabled={loading} to the
LongButton component so the button is disabled while the request is in progress.

In `@app/profile/page.tsx`:
- Around line 40-42: handleSave currently allows an empty phoneNumber and writes
phone: "" which violates the required users.phone schema; update handleSave to
require a non-empty phone before validating/saving: add an early check in
handleSave that if (!phoneNumber) { toast.error("Phone number is required.");
return; } then run the existing regex test and only include phone: phoneNumber
in the update payload when it passes validation so you never write an empty
string to the users collection (refer to the phoneNumber variable, the regex
test, and the handleSave update logic).

In `@app/status/account-pending/page.tsx`:
- Around line 7-25: In AccountPending's useEffect/handleFocus, also detect
admin-disabled users by checking userData?.disabled (or the app's disabled flag)
and immediately redirect to "/status/account-disabled" instead of treating them
as pending; update the logic in handleFocus (which calls getUserByUID and
authContext.refreshUser and uses router.push) to first check for disabled and
call router.push("/status/account-disabled") when true, keeping the existing
pending check/refresh flow otherwise and ensuring this runs both on focus and
initial mount if desired.

In `@components/auth/LongButton.tsx`:
- Around line 15-16: The LongButton component currently always includes
"hover:cursor-pointer" in its className, so disabled buttons still appear
interactive; update the className construction in LongButton to conditionally
include "hover:cursor-pointer" only when disabled is false (or alternatively
append "cursor-not-allowed" when disabled) and keep the existing opacity-50
behavior; locate the className string in LongButton and modify it to branch on
the disabled prop to emit the correct cursor utility.

In `@components/user-management/EditAccountModal.tsx`:
- Around line 127-128: The current call to editAccount({...}).then(onClose) only
handles success; change to handle rejections so the modal doesn't close on
failure and unhandled promise rejections are avoided: either convert the handler
to async/await and wrap the editAccount call in try/catch (await
editAccount(...); onClose() on success; set an error state or toast in catch) or
keep promise style and append .then(() => onClose()).catch(err => {
setError(toast or state, err); /* keep modal open */ }); update both occurrences
that call editAccount (the calls referencing editAccount and onClose) to use the
chosen pattern.
- Line 127: When calling editAccount with the updated account in
EditAccountModal, ensure signedWaiver is normalized to null whenever the new
role is not "Volunteer": inspect the local variables account and role and
construct the payload so that signedWaiver is preserved only if role ===
"Volunteer" (otherwise set signedWaiver = null) before calling editAccount({
...account, firstName, lastName, role, disabled, signedWaiver }). This change
should be applied at the editAccount call sites (the one at
editAccount(...).then(onClose) and the other occurrence mentioned) so
non-volunteer users never retain a Timestamp in signedWaiver.

In `@contexts/AuthContext.tsx`:
- Around line 60-61: After awaiting signUp(...) in the signup flow, call
refreshUser() to force-sync the users document so consumers don't see stale
userData; specifically, in the block using setAuthState(old => ({...old,
loading: true})), change the return path so you await signUp(email, ...), then
await refreshUser(), then update auth state (clear loading or return the signup
result) before returning—refer to signUp, refreshUser, and setAuthState to
locate and modify the code.

---

Outside diff comments:
In `@app/debug/data/page.tsx`:
- Around line 40-52: This component is directly calling Firestore (setDoc, doc,
Timestamp.now()) — move that logic into a Firebase service and expose it via a
React-Query wrapper: create a service function (e.g., createUserInFirestore /
setUser) in lib/services/ that accepts the user payload and performs
setDoc(doc(db, "users", uid), {...}) and Timestamp.now(), then create a
query/mutation wrapper (e.g., useCreateUser) in lib/queries/ that calls the
service; replace the direct setDoc call in page.tsx with a call to the
useCreateUser mutation (or await the service function only if used outside React
lifecycle) so all Firestore access lives under lib/services and is consumed
through lib/queries.

In `@app/status/verify-email/page.tsx`:
- Around line 7-38: The focus handler currently only checks verification and
pending status; update the handleFocus logic to also fetch the user record (via
getUserByUID) and honor a disabled flag before deciding routes: after
authContext.checkVerification() returns verified, call
getUserByUID(auth.currentUser.uid), then if userData?.disabled route to
'/status/account-disabled' (even if pending), else if userData?.pending route to
'/status/account-pending', otherwise route to '/status/account-created'; ensure
you still guard with auth.currentUser and keep existing cleanup/listener code.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4a9985e5-1759-4355-9cf2-8650753cf69b

📥 Commits

Reviewing files that changed from the base of the PR and between 0dc7e76 and 920cc28.

📒 Files selected for processing (31)
  • CLAUDE.md
  • app/auth/action/page.tsx
  • app/client-request-form/steps/Step1ClientInfo.tsx
  • app/debug/data/page.tsx
  • app/donate/steps/Step1PersonalInfo.tsx
  • app/forgot-password/page.tsx
  • app/login/page.tsx
  • app/profile/page.tsx
  • app/signup/PickRole.tsx
  • app/signup/SignUpInformation.tsx
  • app/status/account-created/page.tsx
  • app/status/account-disabled/page.tsx
  • app/status/account-pending/page.tsx
  • app/status/verify-email/page.tsx
  • app/user-management/all-accounts/page.tsx
  • components/auth/LongButton.tsx
  • components/form/FormInput.tsx
  • components/general/ProtectedRoute.tsx
  • components/homepage/VolunteerHomePage.tsx
  • components/profile/PasswordResetSection.tsx
  • components/user-management/AccountReqTable.tsx
  • components/user-management/EditAccountModal.tsx
  • components/user-management/UserTable.tsx
  • contexts/AuthContext.tsx
  • firebase.json
  • lib/queries/inventory.ts
  • lib/services/auth.ts
  • lib/utils/auth-errors.ts
  • lib/utils/phone.ts
  • next.config.ts
  • types/user.ts

Comment thread app/auth/action/page.tsx
Comment on lines +42 to +57
useEffect(() => {
applyActionCode(auth, oobCode)
.then(async () => {
const currentUser = auth.currentUser;
if (currentUser) {
await currentUser.reload();
await updateEmailVerificationStatus(currentUser.uid, true);
await refreshUser();
const userData = await getUserByUID(currentUser.uid);
router.replace(userData?.pending ? '/status/account-pending' : '/status/account-created');
} else {
router.replace('/login');
}
})
.catch(() => setStatus('error'));
}, [oobCode, router, refreshUser]);
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 | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="app/auth/action/page.tsx"
if [ ! -f "$FILE" ]; then
  echo "Missing $FILE"
  exit 1
fi

echo "=== File: $FILE ==="
wc -l "$FILE"

echo
echo "=== Lines 1-220 (to capture referenced sections) ==="
nl -ba "$FILE" | sed -n '1,220p'

Repository: Hack4Impact-UMD/journey-home

Length of output: 256


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="app/auth/action/page.tsx"
echo "=== File: $FILE ==="
wc -l "$FILE"
echo
echo "=== Lines 1-220 (with line numbers) ==="
cat -n "$FILE" | sed -n '1,220p'

Repository: Hack4Impact-UMD/journey-home

Length of output: 9068


Fix Firebase auth action-code error handling in app/auth/action/page.tsx

  • VerifyEmailView (lines 42-57): applyActionCode(...).catch(() => setStatus('error')) treats all errors as “invalid/expired” instead of special-casing known FirebaseError.code values (e.g., auth/invalid-action-code, auth/expired-action-code) and rethrowing anything else.
  • ResetPasswordView (lines 86-90): verifyPasswordResetCode(...).catch(() => setStatus('invalid')) collapses all failures into “invalid/expired” rather than only handling known action-code error codes and rethrowing unexpected ones.
  • ResetPasswordView.handleSubmit (lines 107-113): confirmPasswordReset(...) only branches on auth/expired-action-code; all other Firebase errors become a generic message instead of being rethrown.
🤖 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/auth/action/page.tsx` around lines 42 - 57,
VerifyEmailView/applyActionCode and ResetPasswordView's verifyPasswordResetCode
and handleSubmit need to handle FirebaseError codes explicitly: update the
.catch handlers for applyActionCode and verifyPasswordResetCode to inspect the
caught error (typed as FirebaseError), set the local status for known codes like
'auth/invalid-action-code' and 'auth/expired-action-code' (or the corresponding
reset codes), and rethrow or propagate any unexpected errors; likewise, in
ResetPasswordView.handleSubmit where confirmPasswordReset is called, branch on
specific FirebaseError.code values (e.g., 'auth/expired-action-code') to set
user-facing messages and rethrow any other error to surface unknown failures.
Reference functions: applyActionCode, VerifyEmailView, verifyPasswordResetCode,
ResetPasswordView, handleSubmit, confirmPasswordReset; ensure you check
error.code on the caught error and only treat known action-code values as
handled.

Comment thread app/auth/action/page.tsx
Comment thread app/debug/data/page.tsx
Comment on lines +32 to 35
{ email: "admin@test.com", password: "password", first: "Admin", last: "Test", role: "Admin", phone: "555-000-0001", waiver: false },
{ email: "casemanager@test.com", password: "password", first: "CaseManager", last: "Test", role: "Case Manager", phone: "555-000-0002", waiver: false },
{ email: "volunteer@test.com", password: "password", first: "Volunteer", last: "Test", role: "Volunteer", phone: "555-000-0003", waiver: true },
];
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

waiver seed data is currently ignored when persisting users.

TEST_ACCOUNTS now includes waiver, but signedWaiver is always written as null, so volunteer seed state is inconsistent.

💡 Proposed fix
-            signedWaiver: null,
+            signedWaiver: user.waiver ? Timestamp.now() : null,

Also applies to: 47-47

🤖 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/debug/data/page.tsx` around lines 32 - 35, TEST_ACCOUNTS includes a
waiver flag but the user creation logic still writes signedWaiver as null;
update the seeding/persistence code that consumes TEST_ACCOUNTS (the routine
that maps each TEST_ACCOUNTS entry to a user record) to set the user's
signedWaiver field from the source object's waiver property instead of
hardcoding null—find references to TEST_ACCOUNTS and the signedWaiver assignment
in the seed/import function and change the assignment to use account.waiver (or
the equivalent property) so volunteer seed state reflects the waiver value.

Comment on lines +33 to +42
try {
await resetPassword(email);
setSent(true);
} catch (e: unknown) {
if (e instanceof FirebaseError && e.code === 'auth/user-not-found') {
setSent(true);
} else {
setError('Failed to send reset email. Please try again.');
}
} finally {
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 | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="app/forgot-password/page.tsx"
if [ ! -f "$FILE" ]; then
  echo "Missing file: $FILE" >&2
  exit 1
fi

echo "=== File: $FILE (lines 1-220) ==="
nl -ba "$FILE" | sed -n '1,220p'

Repository: Hack4Impact-UMD/journey-home

Length of output: 191


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="app/forgot-password/page.tsx"
python3 - <<'PY'
import itertools, pathlib
p = pathlib.Path("app/forgot-password/page.tsx")
text = p.read_text(encoding="utf-8").splitlines()
for i, line in enumerate(text[:260], start=1):
    print(f"{i:4d}\t{line}")
PY

Repository: Hack4Impact-UMD/journey-home

Length of output: 4760


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate LongButton component file
fd -a "LongButton" .

Repository: Hack4Impact-UMD/journey-home

Length of output: 122


🏁 Script executed:

python3 - <<'PY'
import pathlib
p = pathlib.Path("components/auth/LongButton.tsx")
text = p.read_text(encoding="utf-8").splitlines()
for i, line in enumerate(text, start=1):
    if i <= 220:
        print(f"{i:4d}\t{line}")
PY

Repository: Hack4Impact-UMD/journey-home

Length of output: 686


Rethrow unexpected resetPassword failures + disable submit while loading

  • auth/user-not-found is treated as success, but all other Firebase errors are swallowed via setError(...); per guidelines, unexpected errors should be rethrown instead of flattened.
  • Wire loading into the submit button by passing disabled={loading} to <LongButton> (currently only the label changes).
🤖 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/forgot-password/page.tsx` around lines 33 - 42, In the resetPassword
error handling, treat only the FirebaseError with code 'auth/user-not-found' as
success by calling setSent(true); for any other caught error (not that specific
FirebaseError) rethrow the error instead of calling setError(...) so unexpected
failures bubble up (check the catch handling around resetPassword and
FirebaseError). Also wire the loading state into the submit button by passing
disabled={loading} to the LongButton component so the button is disabled while
the request is in progress.

Comment thread app/forgot-password/page.tsx
Comment thread app/status/account-pending/page.tsx
Comment on lines +15 to +16
className={`w-full h-10 rounded-xs bg-primary text-white hover:cursor-pointer ${
disabled ? 'opacity-50' : ''
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

Don't show a pointer cursor on disabled buttons.

The base class always keeps hover:cursor-pointer, so disabled buttons still look interactive. Gate the hover class or add a disabled cursor.

Suggested fix
-      className={`w-full h-10 rounded-xs bg-primary text-white hover:cursor-pointer ${
-        disabled ? 'opacity-50' : ''
-      }`}
+      className={`w-full h-10 rounded-xs bg-primary text-white ${
+        disabled ? 'opacity-50 cursor-not-allowed' : 'hover:cursor-pointer'
+      }`}
📝 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
className={`w-full h-10 rounded-xs bg-primary text-white hover:cursor-pointer ${
disabled ? 'opacity-50' : ''
className={`w-full h-10 rounded-xs bg-primary text-white ${
disabled ? 'opacity-50 cursor-not-allowed' : 'hover:cursor-pointer'
}`}
🤖 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 `@components/auth/LongButton.tsx` around lines 15 - 16, The LongButton
component currently always includes "hover:cursor-pointer" in its className, so
disabled buttons still appear interactive; update the className construction in
LongButton to conditionally include "hover:cursor-pointer" only when disabled is
false (or alternatively append "cursor-not-allowed" when disabled) and keep the
existing opacity-50 behavior; locate the className string in LongButton and
modify it to branch on the disabled prop to emit the correct cursor utility.

Comment thread components/user-management/EditAccountModal.tsx
Comment on lines +127 to +128
editAccount({ ...account, firstName, lastName, role, disabled }).then(onClose);
}
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 | 🟠 Major | ⚡ Quick win

Handle editAccount rejections instead of chaining only .then(...).

Line 127 and Line 146 currently close only on success and do not handle failures, which can leave the modal state inconsistent and surface unhandled promise rejections.

Proposed fix
+    const [saveError, setSaveError] = useState<string | null>(null);
+
+    async function persistAccountUpdate(updated: UserData) {
+        try {
+            setSaveError(null);
+            await editAccount(updated);
+            onClose();
+        } catch (error) {
+            // Handle known write/auth errors explicitly; rethrow unexpected ones if needed upstream.
+            setSaveError("Failed to save account changes. Please try again.");
+        }
+    }
...
-                                            editAccount({ ...account, firstName, lastName, role, disabled }).then(onClose);
+                                            void persistAccountUpdate({ ...account, firstName, lastName, role, disabled });
...
-                    onConfirm={() => editAccount({ ...account, firstName, lastName, role, disabled }).then(onClose)}
+                    onConfirm={() => void persistAccountUpdate({ ...account, firstName, lastName, role, disabled })}

Also applies to: 146-146

🤖 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 `@components/user-management/EditAccountModal.tsx` around lines 127 - 128, The
current call to editAccount({...}).then(onClose) only handles success; change to
handle rejections so the modal doesn't close on failure and unhandled promise
rejections are avoided: either convert the handler to async/await and wrap the
editAccount call in try/catch (await editAccount(...); onClose() on success; set
an error state or toast in catch) or keep promise style and append .then(() =>
onClose()).catch(err => { setError(toast or state, err); /* keep modal open */
}); update both occurrences that call editAccount (the calls referencing
editAccount and onClose) to use the chosen pattern.

Comment thread contexts/AuthContext.tsx Outdated
@joelchem joelchem merged commit 2f2a3e4 into main May 24, 2026
3 checks passed
@joelchem joelchem deleted the joel/user-type-changes branch May 24, 2026 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant