Skip to content

Conversation

iethree
Copy link
Contributor

@iethree iethree commented Mar 17, 2025

Fix string definitions for Day, Month, and Year to be compatible with backend strings.

This will fix this error in the i18n job.

Screenshot 2025-03-17 at 7 25 44 AM

The developer ergonomics here aren't great, but we can't just define a singular where a plural exists on the backend, we have to define the localization string to say "use the singular form of this singular/plural group"

@iethree iethree requested a review from a team March 17, 2025 15:27
@metabase-bot metabase-bot bot added the .Team/AdminWebapp DEPRECATED Please use .Team/UXWest instead label Mar 17, 2025
@iethree iethree requested a review from rafpaf March 17, 2025 15:27
@iethree iethree added the backport Automatically create PR on current release branch on merge label Mar 17, 2025
Copy link
Contributor

github-actions bot commented Mar 17, 2025

e2e tests failed on 194841fafceb1118b886a26a88b09afbef42b7a5-1

e2e test run

File Test Name
dashboard.cy.spec.js (flaky) scenarios > dashboard > warn before leave > should warn a user before leaving after adding, removed, moving, or duplicating a tab
line-bar-tooltips.cy.spec.js (flaky) scenarios > visualizations > line/bar chart > tooltips > should be enterable and scollable to view all rows in long tooltips (#53586) (#48347)

@@ -30,15 +30,15 @@ export function getShortcutGroups(): ShortcutGroup[] {
columns: 3,
shortcuts: [
{
label: t`Week`,
label: ngettext(msgid`Week`, `Weeks`, 1),
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe write a doc string explaining this unusual code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i think it would be spammy. I updated the PR description to explain it a little more, so people can get there via blame if they need

@iethree iethree enabled auto-merge (squash) March 17, 2025 15:56
@iethree iethree merged commit fd976b7 into master Mar 17, 2025
143 of 146 checks passed
@iethree iethree deleted the fix-i18n-job branch March 17, 2025 15:58
Copy link

trunk-io bot commented Mar 17, 2025

Static BadgeStatic BadgeStatic BadgeStatic Badge

Flaky Test Failure Summary Logs
scenarios > visualizations > line/bar chart > tooltips should be enterable and scollable to view all rows in long tooltips (metabase#53586) (metab... The test timed out while trying to find an element with data-testid "echarts-tooltip". Logs ↗︎
scenarios > dashboard warn before leave should warn a user before leaving after adding, removed, moving, or duplicating a tab Unable to find an element with the text "You're editing this dashboard" in the expected time. Logs ↗︎

View Full Report ↗︎Docs

iethree added a commit that referenced this pull request Mar 18, 2025
Co-authored-by: Ryan Laurie <30528226+iethree@users.noreply.github.com>
@github-actions github-actions bot added this to the 0.53.8 milestone Mar 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport Automatically create PR on current release branch on merge Customization/i18n .Team/AdminWebapp DEPRECATED Please use .Team/UXWest instead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants