Skip to content

fix: ai translations description for atoms#17875

Merged
emrysal merged 6 commits into
mainfrom
remove-ai-translations-for-booker
Dec 2, 2024
Merged

fix: ai translations description for atoms#17875
emrysal merged 6 commits into
mainfrom
remove-ai-translations-for-booker

Conversation

@Ryukemeister
Copy link
Copy Markdown
Contributor

@Ryukemeister Ryukemeister commented Nov 27, 2024

What does this PR do?

  • Translations for event types description using ai was causing atoms to break, hence removed ai translations for atoms for the time being.

Mandatory Tasks (DO NOT REMOVE)

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

How should this be tested?

build platform atoms

@keithwillcode keithwillcode added core area: core, team members only platform Anything related to our platform plan labels Nov 27, 2024
@Ryukemeister Ryukemeister marked this pull request as ready for review November 27, 2024 11:05
@graphite-app graphite-app Bot requested a review from a team November 27, 2024 11:05
@dosubot dosubot Bot added ai area: AI, cal.ai i18n area: i18n, translations labels Nov 27, 2024
@graphite-app
Copy link
Copy Markdown

graphite-app Bot commented Nov 27, 2024

Graphite Automations

"Add platform team as reviewer" took an action on this PR • (11/27/24)

1 reviewer was added to this PR based on Keith Williams's automation.

event={event.data}
isPending={event.isPending}
isPlatform={isPlatform}
i18nLocales={i18nLocales}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

double checking: you don't need to pass userLocale here?

eventMetaTitle?: string;
eventMetaTimezoneSelect?: string;
};
locale?: string | null;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

shouldn't this be userLocale? based on the other changes you added

isPlatform={false}
areInstantMeetingParametersSet={areInstantMeetingParametersSet}
userLocale={session?.user.locale}
i18nLocales={i18nLocales}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmm passing a prop doesn't seem optimal. I'd suggest importing it from i18n.json where it's needed

@github-actions github-actions Bot marked this pull request as draft November 27, 2024 11:24
@vercel
Copy link
Copy Markdown

vercel Bot commented Nov 27, 2024

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

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Visit Preview Dec 2, 2024 6:29pm
calcom-web-canary ⬜️ Ignored (Inspect) Visit Preview Dec 2, 2024 6:29pm

@Ryukemeister Ryukemeister marked this pull request as ready for review November 27, 2024 15:52
@dosubot dosubot Bot added event-types area: event types, event-types 🐛 bug Something isn't working labels Nov 27, 2024
(trans) =>
trans.field === EventTypeAutoTranslatedField.DESCRIPTION &&
i18nLocales.includes(trans.targetLocale) &&
i18nLocales.locale.targets.includes(trans.targetLocale) &&
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  1. i18nLocales name is misleading
  2. i18n.locale.targets.concat([i18n.locale.source])

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 27, 2024

E2E results are ready!

@ThyMinimalDev ThyMinimalDev requested a review from hbjORbj December 2, 2024 19:27
@emrysal emrysal dismissed hbjORbj’s stale review December 2, 2024 19:27

Approved by @morgan - looks like issues addressed.

@emrysal emrysal merged commit fe209df into main Dec 2, 2024
@emrysal emrysal deleted the remove-ai-translations-for-booker branch December 2, 2024 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai area: AI, cal.ai 🐛 bug Something isn't working core area: core, team members only event-types area: event types, event-types i18n area: i18n, translations platform Anything related to our platform plan ready-for-e2e

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants