Skip to content

Feature/kanban 462 Create 'contact us' form linked to 'Contact Us' menu item#1746

Open
Joe-Wdz wants to merge 12 commits intodevelop/ui-wgfrom
feature/KANBAN-462
Open

Feature/kanban 462 Create 'contact us' form linked to 'Contact Us' menu item#1746
Joe-Wdz wants to merge 12 commits intodevelop/ui-wgfrom
feature/KANBAN-462

Conversation

@Joe-Wdz
Copy link
Copy Markdown
Contributor

@Joe-Wdz Joe-Wdz commented Apr 9, 2026

KANBAN-462: Create 'contact us' form linked to 'Contact Us' menu item.
Integrated reCAPTCHA v3 for bot protection. ensured spam protection without user interaction.

@Joe-Wdz Joe-Wdz requested review from jimflybase and sethrcamp April 9, 2026 00:08
@Joe-Wdz
Copy link
Copy Markdown
Contributor Author

Joe-Wdz commented Apr 9, 2026

@claude please review my changes

@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 9, 2026

Claude finished @Joe-Wdz's task in 1m 37s —— View job


Code Review: Contact Us Form (KANBAN-462)

  • Gather context and read changed files
  • Review code for bugs, security issues, and best practices
  • Post review feedback

Overall this is a solid implementation of the contact form with reCAPTCHA v3. The form structure, validation, and state management are clean. Here are some issues and suggestions:


Issues

1. useEnterprise flag may be incorrect

src/containers/contactPage/index.jsx:182

<GoogleReCaptchaProvider reCaptchaKey={RECAPTCHA_SITE_KEY} useEnterprise>

The useEnterprise prop enables reCAPTCHA Enterprise mode. The site key in constants.js (6Ld-r5EsAAAAA...) appears to be a standard reCAPTCHA v3 key, not an Enterprise key. If you're not using the Enterprise API, this flag will cause the reCAPTCHA to fail silently or throw errors. Please verify whether you're actually using reCAPTCHA Enterprise — if not, remove the useEnterprise prop.

Fix this →

2. Variable shadowing in sendContactEmail.js

src/lib/sendContactEmail.js:10

let message = 'Failed to send message.';

The parameter message (from the destructured input on line 3) is shadowed by this local let message inside the error handler. This won't cause a runtime bug in this specific code path, but it's a code smell and could confuse readers/linters. Rename the local variable (e.g., errorMessage).

Fix this →

3. Commented-out reCAPTCHA site key left in constants

src/constants.js:3

// export const RECAPTCHA_SITE_KEY = '6Leya4csAAAAAI1Me4cTcUF1BBcO-31Hu4Z8UliC';
export const RECAPTCHA_SITE_KEY = '6Ld-r5EsAAAAAMlCU84nH_F5o4_sLkJYnBhr3j8b';

Remove the commented-out old key. Leaving old keys in comments is a security concern (key exposure) and adds clutter.

Fix this →

4. Hardcoded API URL

src/constants.js:1

export const CONTACT_API_URL = 'https://1ythblxki4.execute-api.us-east-1.amazonaws.com/prod/contact';

Other parts of this codebase use environment-based configuration (e.g., API_URL in package.json scripts). Consider making CONTACT_API_URL configurable via an environment variable so it works across test/stage/prod environments, consistent with the rest of the project.


Suggestions

5. Duplicate email regex pattern

src/containers/contactPage/index.jsx:27 and :48

The email regex /^[^\s@]+@[^\s@]+\.[^\s@]+$/ is defined inline in both validateEmail and handleSubmit. Extract it to a constant to keep it DRY:

const EMAIL_PATTERN = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;

6. Consider using type="email" browser validation

The <input type="email"> on line 119 already provides browser-native validation. Your custom regex validation on blur is a nice addition for UX, but you might consider relying on the browser's validity API (e.target.validity.valid) instead of maintaining a separate regex, which would give you more robust validation for free.

7. Commit history cleanup

The commit history includes a "WIP" commit and a separate "prettier format" commit. Consider squashing these before merge to keep the history clean on the target branch.


What looks good

  • Clean form state management with useState and useCallback
  • Good UX: loading state on submit button, success/error alerts, form reset
  • Proper reCAPTCHA v3 integration pattern (invisible, no user friction)
  • Subject field pre-population via search params is a nice touch
  • Error handling in sendContactEmail.js properly parses multiple error formats
  • Route and navigation menu integration are correct

@Joe-Wdz Joe-Wdz force-pushed the feature/KANBAN-462 branch from 34efcb3 to 69d1fb7 Compare April 9, 2026 16:11
@Joe-Wdz
Copy link
Copy Markdown
Contributor Author

Joe-Wdz commented Apr 15, 2026

@claude review my changes

@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 15, 2026

Claude finished @Joe-Wdz's task in 1m 17s —— View job


Code Review: Contact Us Form (KANBAN-462)

  • Check for new commits since previous review
  • Read all changed files
  • Review code for bugs, security issues, and best practices
  • Post review feedback

Since my previous review on Apr 9, one new commit was pushed:

  • cfcd3dd9 (Apr 14) — "Fixed issues reviewed by Claude"

This commit addressed two items from my earlier review:

  • Variable shadowing (issue AGR-519 Re-added basic disease support to search #2): Fixed — renamed message to errMessage in sendContactEmail.js
  • Commented-out code: Removed the commented-out EMAIL_PATTERN constant in contactPage/index.jsx
  • Also fixed a typo in the error message (errMessagemessage) ✅

Items from previous review still open

1. Commented-out reCAPTCHA site key still in constants.js

This was not addressed. Looking at the current file, I see the old commented-out key has been removed — good. ✅ (Resolved — I can confirm constants.js is clean now.)

2. Hardcoded API URL

src/constants.js:1

export const CONTACT_API_URL = 'https://1ythblxki4.execute-api.us-east-1.amazonaws.com/prod/contact';

This is still hardcoded to a prod API Gateway URL. This is a minor concern — if you ever need to test against a staging environment, you'd need to change this line. Consider making it configurable via environment variable, consistent with how other API URLs are handled in the project. This is low priority if there's only one environment.

3. useEnterprise flag

src/containers/contactPage/index.jsx:175-177
A comment was added saying "false positive: useEnterprise is intentionally enabled". If you've confirmed this is the correct configuration for your reCAPTCHA setup, that's fine. ✅


New observations on current state

4. No input length limits

src/containers/contactPage/index.jsx:93-138

None of the text inputs (name, subject, message) have maxLength attributes. While the backend should validate input length, adding client-side limits improves UX and provides defense-in-depth against oversized payloads. Consider adding reasonable maxLength values (e.g., name: 100, subject: 200, message: 5000).

Fix this →

5. clearSubmitStatus only clears on success

src/containers/contactPage/index.jsx:20-24

const clearSubmitStatus = () => {
    if (submitStatus === 'success') {
      setSubmitStatus(null);
    }
  };

This function is called onFocus of the form (line 90). It only clears the status when it's 'success', not when it's 'error'. This means the error alert persists even when the user starts editing the form again. Consider clearing on both success and error states so the alert dismisses when the user re-engages with the form:

const clearSubmitStatus = () => {
    if (submitStatus === 'success' || submitStatus === 'error') {
      setSubmitStatus(null);
      setSubmitError('');
    }
  };

Fix this →

6. topBar.jsx refactor is clean

The conversion from a class component to a functional component and the change from mailto: link to a React Router <Link> are both well done. Good modernization. ✅

7. Route placement

src/routes.jsx:57

<Route exact path="/contact-us" element={<ContactPage />} />

This is placed just before the WordPress catch-all route (/:slug), which is correct — specific routes should come before the wildcard. ✅


Summary

The core implementation is solid and the previous review feedback was partially addressed in cfcd3dd9. The remaining items are minor:

  • Hardcoded API URL — low priority, consider making configurable
  • No input length limits — defense-in-depth suggestion
  • Error status not clearing on re-focus — minor UX improvement

The form logic, reCAPTCHA integration, error handling, and routing are all well structured. The topBar.jsx modernization from class to functional component is a nice bonus.


View job

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