-
-
Notifications
You must be signed in to change notification settings - Fork 632
chore: api status-page locale #2003
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -33,6 +33,7 @@ import { | |||||||||||||||
| dbPageToProto, | ||||||||||||||||
| dbPageToProtoSummary, | ||||||||||||||||
| dbSubscriberToProto, | ||||||||||||||||
| protoLocaleToDb, | ||||||||||||||||
| } from "./converters"; | ||||||||||||||||
| import { | ||||||||||||||||
| componentGroupCreateFailedError, | ||||||||||||||||
|
|
@@ -169,6 +170,11 @@ export const statusPageServiceImpl: ServiceImpl<typeof StatusPageService> = { | |||||||||||||||
| published: false, | ||||||||||||||||
| homepageUrl: req.homepageUrl ?? null, | ||||||||||||||||
| contactUrl: req.contactUrl ?? null, | ||||||||||||||||
| defaultLocale: req.defaultLocale | ||||||||||||||||
| ? protoLocaleToDb(req.defaultLocale) | ||||||||||||||||
| : "en", | ||||||||||||||||
|
||||||||||||||||
| defaultLocale: req.defaultLocale | |
| ? protoLocaleToDb(req.defaultLocale) | |
| : "en", | |
| defaultLocale: | |
| req.defaultLocale !== undefined | |
| ? protoLocaleToDb(req.defaultLocale) | |
| : "en", |
Outdated
Copilot
AI
Mar 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CreateStatusPage also writes defaultLocale/locales without enforcing that the default is contained in the enabled locales list when locales is provided. Consider validating or auto-including the default locale to avoid persisting an inconsistent configuration.
cubic-dev-ai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
cubic-dev-ai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
Copilot
AI
Mar 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UpdateStatusPage only updates locales when req.locales.length > 0, which makes it impossible for clients to clear/reset locales (e.g., set it to empty/null) via the API. If clearing is a supported operation (DB uses null for “no locales configured”), consider adding an explicit way to clear it (e.g., treat an empty list as “clear”, or add a separate boolean/field-mask-style signal).
| if (req.locales.length > 0) { | |
| updateValues.locales = req.locales.map(protoLocaleToDb); | |
| if (req.locales !== undefined) { | |
| updateValues.locales = | |
| req.locales.length > 0 ? req.locales.map(protoLocaleToDb) : null; |
Copilot
AI
Mar 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Locale fields are written without validating the invariant that defaultLocale must be included in locales when locales is provided (this is enforced in other code paths via packages/db/src/schema/pages/validation.ts). As-is, UpdateStatusPage can persist inconsistent combinations when updating either field independently; add server-side validation/normalization before saving.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -118,6 +118,12 @@ message CreateStatusPageRequest { | |
|
|
||
| // URL to the contact page (optional). | ||
| optional string contact_url = 5; | ||
|
|
||
| // Default locale for the status page (optional, defaults to EN). | ||
| optional Locale default_locale = 6; | ||
|
|
||
| // Enabled locales for the status page (optional). | ||
| repeated Locale locales = 7; | ||
| } | ||
|
|
||
| // CreateStatusPageResponse is the response after creating a status page. | ||
|
|
@@ -185,6 +191,12 @@ message UpdateStatusPageRequest { | |
|
|
||
| // New contact URL (optional). | ||
| optional string contact_url = 6; | ||
|
|
||
| // New default locale for the status page (optional). | ||
| optional Locale default_locale = 7; | ||
|
|
||
| // New enabled locales for the status page (optional). | ||
| repeated Locale locales = 8; | ||
cubic-dev-ai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: Switching from Consider keeping a wrapper message (e.g., Prompt for AI agents |
||
| } | ||
|
|
||
| // UpdateStatusPageResponse is the response after updating a status page. | ||
|
|
||
Large diffs are not rendered by default.
Uh oh!
There was an error while loading. Please reload this page.