Skip to content

Comments

feat: add submitterEmail column to form response for efficient duplicate checking#27303

Open
joeauyeung wants to merge 11 commits intomainfrom
devin/1769530315-form-response-submitter-email
Open

feat: add submitterEmail column to form response for efficient duplicate checking#27303
joeauyeung wants to merge 11 commits intomainfrom
devin/1769530315-form-response-submitter-email

Conversation

@joeauyeung
Copy link
Contributor

@joeauyeung joeauyeung commented Jan 27, 2026

What does this PR do?

Optimizes the form submitted no event webhook trigger by storing the submitter email directly in the form response record, enabling efficient database queries instead of iterating through all responses.

Before: When checking for duplicate form submissions, the code fetched all form responses within a 60-minute window and iterated through each response's JSON to find matching emails.

After: The submitter email is extracted and stored in a dedicated indexed column when the form is submitted, allowing direct database queries by email.

Changes

  • Add submitterEmail column to App_RoutingForms_FormResponse and App_RoutingForms_QueuedFormResponse models
  • Add composite database index on (formId, submitterEmail, createdAt) for optimal query performance
  • Add findResponseWithBookingByEmail repository method for direct email-based queries
  • Update handleResponse.ts to extract and save submitter email when recording form responses
  • Update hasDuplicateSubmission to use the new email-based query

Updates since last revision

  • Refactored to reuse the existing getSubmitterEmail function from formSubmissionValidation.ts instead of having a duplicate extractSubmitterEmail function in handleResponse.ts
  • Made getSubmitterEmail accept a more generic ResponseWithValue type to work with both the webhook response format and the handleResponse format
  • Updated unit tests to expect the new submitterEmail parameter in repository method calls
  • Index optimization: Changed from single-column index on submitterEmail to composite index on (formId, submitterEmail, createdAt) for better query performance since queries always filter by formId first
  • Added comprehensive test coverage: 16 new unit tests for getSubmitterEmail and getSubmitterName functions covering edge cases (empty responses, numeric values, arrays, multiple emails, different response formats)
  • Fixed queued-response endpoint: Added submitterEmail extraction to apps/web/app/api/routing-forms/queued-response/route.ts which was missing the new parameter when calling recordFormResponse
  • Performance optimization: Store submitterEmail in lowercase and remove case-insensitive query mode for better index utilization (addresses reviewer feedback)

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. N/A - no documentation changes needed.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

  1. Submit a routing form with an email field (try with mixed case like "Test@Example.COM")
  2. Verify the submitterEmail column is populated in the database in lowercase
  3. Submit the same form again within 60 minutes with the same email (different case should still be detected as duplicate)
  4. Verify the duplicate detection still works correctly (FORM_SUBMITTED_NO_EVENT webhook should not fire for duplicates)

Human Review Checklist

  • Email extraction logic: The getSubmitterEmail function finds the first field value containing "@". Verify this matches the expected behavior for identifying submitter emails (note: could match non-email fields like Twitter handles).
  • Generic type: The ResponseWithValue type uses [key: string]: unknown to allow flexibility - verify this doesn't cause issues with type safety.
  • Migration: The migration adds the column but doesn't backfill existing records. Existing form responses will have submitterEmail as NULL.
  • Lowercase storage: Emails are now stored in lowercase and queries use exact match (no mode: "insensitive"). Verify this is acceptable for all use cases.
  • Composite index order: Verify (formId, submitterEmail, createdAt) is optimal for the query pattern (equality on formId, equality on email, range on createdAt).

Checklist

  • My code follows the style guidelines of this project
  • I have checked if my changes generate no new warnings

Link to Devin run: https://app.devin.ai/sessions/b837bc0ab2e647eb9374765432bebcef
Requested by: @joeauyeung

…ate checking

- Add submitterEmail column to App_RoutingForms_FormResponse and App_RoutingForms_QueuedFormResponse models
- Add index on submitterEmail for efficient querying
- Update RoutingFormResponseRepository to save submitterEmail when creating form responses
- Add findResponseWithBookingByEmail method to query by email directly
- Update hasDuplicateSubmission to use email-based query instead of iterating through all responses
- Extract and save submitter email in handleResponse when recording form responses

This improves performance by querying directly on the indexed email column instead of fetching all form responses within a time window and iterating through them to find matching emails.

Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
@devin-ai-integration
Copy link
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI' or '@devin'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@github-actions github-actions bot added the ❗️ migrations contains migration files label Jan 27, 2026
devin-ai-integration bot and others added 4 commits January 27, 2026 16:31
Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
…ter query performance

Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
…ions

Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
@joeauyeung joeauyeung marked this pull request as ready for review January 27, 2026 16:55
@joeauyeung joeauyeung requested a review from a team as a code owner January 27, 2026 16:55
@graphite-app graphite-app bot added enterprise area: enterprise, audit log, organisation, SAML, SSO core area: core, team members only labels Jan 27, 2026
@graphite-app graphite-app bot requested a review from a team January 27, 2026 16:55
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 7 files

@github-actions
Copy link
Contributor

github-actions bot commented Jan 27, 2026

E2E results are ready!

Copy link
Contributor

@Udit-takkar Udit-takkar left a comment

Choose a reason for hiding this comment

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

@joeauyeung What is apps/web/app/api/routing-forms/queued-response/route.ts path?

That endpoint doesn't set submitterEmail when calling recordFormResponse

Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
response,
// We use the queuedFormResponse's chosenRouteId because that is what decided routed team members
chosenRouteId: queuedFormResponse.chosenRouteId,
submitterEmail,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Udit-takkar adding the submitter email here as well

Copy link
Contributor

@Udit-takkar Udit-takkar left a comment

Choose a reason for hiding this comment

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

LGTM.

I'll wait for Hariom's approval

devin-ai-integration[bot]

This comment was marked as resolved.

Copy link
Member

@hariombalhara hariombalhara left a comment

Choose a reason for hiding this comment

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

Devin has valid concern around existing Form Responses without submitterEmail. We need to tackle that.

devin-ai-integration bot and others added 2 commits February 4, 2026 18:54
…uery

Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
hariombalhara
hariombalhara previously approved these changes Feb 9, 2026
Copy link
Member

@hariombalhara hariombalhara left a comment

Choose a reason for hiding this comment

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

LGTM now !!

@github-actions
Copy link
Contributor

Devin AI is resolving merge conflicts

This PR has merge conflicts with the main branch. A Devin session has been created to automatically resolve them.

View Devin Session

Devin will:

  1. Merge the latest main into this branch
  2. Resolve any conflicts intelligently
  3. Run lint/type checks to ensure validity
  4. Push the resolved changes

If you prefer to resolve conflicts manually, you can close the Devin session and handle it yourself.

@github-actions
Copy link
Contributor

Devin AI is resolving merge conflicts

This PR has merge conflicts with the main branch. A Devin session has been created to automatically resolve them.

View Devin Session

Devin will:

  1. Merge the latest main into this branch
  2. Resolve any conflicts intelligently
  3. Run lint/type checks to ensure validity
  4. Push the resolved changes

If you prefer to resolve conflicts manually, you can close the Devin session and handle it yourself.

…-response-submitter-email

Co-Authored-By: unknown <>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core area: core, team members only devin-conflict-resolution enterprise area: enterprise, audit log, organisation, SAML, SSO ❗️ migrations contains migration files ready-for-e2e size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants