Skip to content

fix: strip avatar and profile from children payload in managed event type updates#28239

Merged
alishaz-polymath merged 3 commits intomainfrom
devin/1772450657-strip-avatar-from-children-payload
Mar 2, 2026
Merged

fix: strip avatar and profile from children payload in managed event type updates#28239
alishaz-polymath merged 3 commits intomainfrom
devin/1772450657-strip-avatar-from-children-payload

Conversation

@alishaz-polymath
Copy link
Member

@alishaz-polymath alishaz-polymath commented Mar 2, 2026

What does this PR do?

When assigning users to a large managed event type (~85+ users), the update request fails with a "Body exceeded 1mb limit" error. The root cause is that the full ChildrenEventType objects—including avatar (potentially large base64 data URLs), profile, username, and membership—are sent in the mutation payload, even though the server Zod schema (childSchema) only needs owner.{id, name, email, eventTypeSlugs} and hidden.

This PR strips the children array down to only server-required fields in handleSubmit before sending the mutation, while keeping the full data in form state for UI display (e.g., avatar rendering in ChildrenEventTypeSelect).

The stripping logic is extracted into a shared stripChildrenForPayload utility in packages/features/eventtypes/lib/childrenEventType.ts, which is imported by both the hook and the test file—avoiding code duplication.

How should this be tested?

  1. Create a managed event type in an organization with ~85+ team members
  2. Assign all members to the managed event type
  3. Save the event type — previously this would fail with "Body exceeded 1mb limit"; now it should succeed
  4. Verify the assigned members list still displays correctly with avatars in the UI

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
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

Human Review Checklist

  • Verify the stripped fields (owner.{id, name, email, eventTypeSlugs}, hidden) match exactly what the server childSchema in packages/trpc/server/routers/viewer/eventTypes/types.ts expects
  • Confirm the full children data (with avatars) is still available in react-hook-form state for UI rendering — note that children is extracted from values on line 303 (before dirty field filtering), while strippedChildren is only used for the outgoing payload
  • Verify stripChildrenForPayload in childrenEventType.ts is correctly shared between the hook and the test file (no duplicated logic)

Updates since last revision

  • Addressed Cubic AI review feedback (confidence 9/10): Extracted inline stripping logic from useEventTypeForm.ts into a shared exported stripChildrenForPayload() function in packages/features/eventtypes/lib/childrenEventType.ts. The test file now imports and tests the actual production function instead of maintaining a mirrored copy.

Link to Devin session
Link to original Devin session
Requested by: @alishaz-polymath

…type updates

When assigning users to managed event types with ~85+ users, the request
body exceeds the 1MB server limit. The root cause is that the full
ChildrenEventType objects (including avatar, profile, username, membership)
are sent in the update payload, even though the server schema (childSchema)
only needs owner.{id, name, email, eventTypeSlugs} and hidden.

Avatar data can be particularly large when stored as base64 data URLs.
With 85 users each having ~10KB+ avatars, the payload easily exceeds 1MB.

This fix strips the children array down to only server-required fields
before sending the mutation, while keeping the full data in form state
for UI display purposes.

Co-Authored-By: ali@cal.com <alishahbaz7@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

@alishaz-polymath alishaz-polymath marked this pull request as ready for review March 2, 2026 11:31
@alishaz-polymath alishaz-polymath requested review from a team as code owners March 2, 2026 11:31
@graphite-app graphite-app bot added enterprise area: enterprise, audit log, organisation, SAML, SSO core area: core, team members only labels Mar 2, 2026
@graphite-app graphite-app bot requested a review from a team March 2, 2026 11:31
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.

1 issue found across 2 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/platform/atoms/event-types/hooks/useEventTypeForm.test.ts">

<violation number="1" location="packages/platform/atoms/event-types/hooks/useEventTypeForm.test.ts:11">
P1: Test duplicates production logic instead of importing it</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 2, 2026

Devin AI is addressing Cubic AI's review feedback

A Devin session has been created to address the issues identified by Cubic AI.

View Devin Session

… of duplicating in test

Addresses Cubic AI review feedback: the test file was duplicating the
stripping logic instead of importing it from production code.

- Extracted stripChildrenForPayload() into childrenEventType.ts
- Updated useEventTypeForm.ts to import and use the shared function
- Updated test file to import from production code instead of defining its own copy

Co-Authored-By: bot_apk <apk@cognition.ai>
@alishaz-polymath alishaz-polymath added run-ci Approve CI to run for external contributors ready-for-e2e and removed ready-for-e2e labels Mar 2, 2026
@alishaz-polymath alishaz-polymath removed ready-for-e2e run-ci Approve CI to run for external contributors labels Mar 2, 2026
Co-Authored-By: bot_apk <apk@cognition.ai>
@alishaz-polymath alishaz-polymath added ready-for-e2e run-ci Approve CI to run for external contributors labels Mar 2, 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 !!

@alishaz-polymath alishaz-polymath merged commit 5943a8a into main Mar 2, 2026
70 of 75 checks passed
@alishaz-polymath alishaz-polymath deleted the devin/1772450657-strip-avatar-from-children-payload branch March 2, 2026 13:05
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 enterprise area: enterprise, audit log, organisation, SAML, SSO ready-for-e2e run-ci Approve CI to run for external contributors size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants