Skip to content

Hotfix/Event Type Save Error with Multiple Durations#6154

Merged
emrysal merged 2 commits intomainfrom
hotfix/event-type-saving-error
Dec 22, 2022
Merged

Hotfix/Event Type Save Error with Multiple Durations#6154
emrysal merged 2 commits intomainfrom
hotfix/event-type-saving-error

Conversation

@hariombalhara
Copy link
Copy Markdown
Member

What does this PR do?

Fixes # (issue)
See loom

Environment: Production

Type of change

  • Bug fix (non-breaking change which fixes an issue)

@vercel
Copy link
Copy Markdown

vercel Bot commented Dec 22, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
cal ✅ Ready (Inspect) Visit Preview Dec 22, 2022 at 11:13AM (UTC)

Copy link
Copy Markdown
Member Author

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

Self review done

onChange={(e) => {
formMethods.setValue("length", Number(e.target.value));
}}
{...formMethods.register("length")}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Use standard RHF approach. This ensures that length is defined without onChange as well. But it causes length to be become a string and tRPC requires it to be a number. That has been handled by introducing zodResolver and adding a string to number transform.

Recent duration-related code checks length and it is undefined if onchange hasn't occurred.

I could have fixed the duration code as well, but it seems unintuitive to have length be undefined.

selectedMultipleDuration.find((opt) => opt.value === option?.value) ?? null
);
formMethods.setValue("length", Number(option?.value));
formMethods.setValue("length", option?.value);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It was already a number here, because it is picked from options[number].value which is a number.

.object({
// Length if string, is converted to a number or it can be a number
// Make it optional because it's not submitted from all tabs of the page
length: z.union([z.string().transform((val) => +val), z.number()]).optional(),
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

In Multiple durations case, it is already a number. In single duration case, it is a string and thus requires conversion to number.

@PeerRich PeerRich requested a review from leog December 22, 2022 09:34
@emrysal emrysal enabled auto-merge (squash) December 22, 2022 11:02
@emrysal emrysal merged commit a076418 into main Dec 22, 2022
@emrysal emrysal deleted the hotfix/event-type-saving-error branch December 22, 2022 11:13
@PeerRich PeerRich added the core area: core, team members only label Jul 24, 2023
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants