feat: new platform settings page - #820
Open
dennisvankekem wants to merge 5 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new “Platform settings” page under Settings, built around react-hook-form + Yup validation, and updates the shared KeyValue form component styling/field sizing to support the new UI.
Changes:
- Introduces
PlatformSettingsPagefor editing a subset of Otomi settings (platform version, external DNS/IDP toggles, global pull secret, node selectors). - Adds unit tests for the new page and its Yup validator.
- Updates
KeyValueusage (and related input sizing) across several pages, plus minor sizing tweaks to shared form inputs.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pages/settings/platform/PlatformSettingsPage.tsx | New settings form for Otomi platform settings, including submit logic to preserve hidden fields. |
| src/pages/settings/platform/PlatformSettingsPage.test.tsx | Tests for form initialization and submit payload shaping (preserve hidden fields, null pull secret, strip empty selectors). |
| src/pages/settings/platform/platform-settings.validator.ts | Yup schema for platform settings form values. |
| src/pages/settings/platform/platform-settings.validator.test.ts | Unit tests validating schema behavior. |
| src/pages/services/create-edit/ServicesCreateEditPage.tsx | Enlarges KeyValue input sizing for HTTP response headers. |
| src/pages/secrets/team/create-edit/SecretTypeFields.tsx | Enlarges KeyValue input sizing for secret data fields. |
| src/pages/secrets/team/create-edit/SecretCreateEditPage.tsx | Enlarges KeyValue input sizing for labels/annotations. |
| src/components/forms/TextField.tsx | Slightly increases “large” text field width. |
| src/components/forms/TextArea.tsx | Increases default textarea min width. |
| src/components/forms/KeyValue.tsx | UI tweaks: optional top margin removal, subtitle spacing, add button restyling. |
| src/App.tsx | Adds a private route to the new platform settings page (/settings/otomi). |
Comments suppressed due to low confidence (1)
src/pages/settings/platform/platform-settings.validator.ts:22
- nodeSelector items are marked as required with required name/value, but PlatformSettingsPage explicitly strips completely empty selector rows before submit. As-is, adding a row and leaving it blank will block submission due to validation instead of being ignored.
nodeSelector: yup
.array()
.of(
yup
.object({
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+9
to
+16
| globalPullSecret: yup | ||
| .object({ | ||
| server: yup.string().required(), | ||
| username: yup.string().required(), | ||
| password: yup.string().required(), | ||
| email: yup.string().email().default(''), | ||
| }) | ||
| .required(), |
| * The API schema allows globalPullSecret to be null. Avoid | ||
| * storing an object containing only empty strings. | ||
| */ | ||
| globalPullSecret: hasGlobalPullSecretValues(values.globalPullSecret) ? values.globalPullSecret : null, |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (2)
src/pages/settings/platform/platform-settings.validator.ts:16
- The schema currently requires globalPullSecret.server/username/password, but PlatformSettingsPage is designed to allow submitting
globalPullSecret: nullwhen the form is left at default/empty values. With the currentrequired()constraints, react-hook-form will block submit before onSubmit can convert the object to null.
globalPullSecret: yup
.object({
server: yup.string().required(),
username: yup.string().required(),
password: yup.string().required(),
email: yup.string().email().default(''),
})
.required(),
src/pages/settings/platform/PlatformSettingsPage.tsx:60
- hasGlobalPullSecretValues treats whitespace-only input (e.g. " ") as a configured pull secret because non-empty strings are truthy. That can cause the page to submit a globalPullSecret object (instead of null) even though the user effectively cleared the fields.
const hasGlobalPullSecretValues = (globalPullSecret: PlatformSettingsFormValues['globalPullSecret']): boolean =>
Boolean(
globalPullSecret.username ||
globalPullSecret.password ||
globalPullSecret.email ||
(globalPullSecret.server && globalPullSecret.server !== 'docker.io'),
)
Comment on lines
+18
to
+28
| nodeSelector: yup | ||
| .array() | ||
| .of( | ||
| yup | ||
| .object({ | ||
| name: yup.string().required(), | ||
| value: yup.string().required(), | ||
| }) | ||
| .required(), | ||
| ) | ||
| .required(), |
Comment on lines
+17
to
+24
| jest.mock('@hookform/resolvers/yup', () => ({ | ||
| yupResolver: | ||
| () => | ||
| (values: unknown): { values: unknown; errors: Record<string, never> } => ({ | ||
| values, | ||
| errors: {}, | ||
| }), | ||
| })) |
Comment on lines
+184
to
+187
| expect.objectContaining({ path: 'globalPullSecret.password' }), | ||
| expect.objectContaining({ path: 'globalPullSecret.email' }), | ||
| expect.objectContaining({ path: 'nodeSelector[0].name' }), | ||
| expect.objectContaining({ path: 'nodeSelector[0].value' }), |
j-zimnowoda
reviewed
Jul 28, 2026
| ) | ||
| .required(), | ||
| }) | ||
| .required() |
Contributor
There was a problem hiding this comment.
nodeSelector is an optional object. Is this required() necessary?
j-zimnowoda
reviewed
Jul 28, 2026
| globalPullSecret.username || | ||
| globalPullSecret.password || | ||
| globalPullSecret.email || | ||
| (globalPullSecret.server && globalPullSecret.server !== 'docker.io'), |
Contributor
There was a problem hiding this comment.
why do we need to have a special case for globalPullSecret.server !== 'docker.io'?
j-zimnowoda
reviewed
Jul 28, 2026
| const settings: Settings[] = [ | ||
| { title: 'Cluster', path: '/settings/cluster', icon: getIcon('cluster_icon.svg'), id: 'cluster' }, | ||
| { title: 'Platform', path: '/settings/otomi', icon: getIcon('akamai_icon.svg'), id: 'aplSettings' }, | ||
| { title: 'Platform', path: '/settings/platform-settings', icon: getIcon('akamai_icon.svg'), id: 'aplSettings' }, |
Contributor
There was a problem hiding this comment.
platform-settings => platform
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Considerations