Skip to content
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

Use config api for server settings WD-7389 instance defaults WD-7390 and project defaults #543

Merged
merged 2 commits into from
Dec 1, 2023

Conversation

edlerd
Copy link
Collaborator

@edlerd edlerd commented Nov 17, 2023

Done

  • rely on the config api for:
    • server settings (used to be hard coded in a json file)
    • instance form defaults and help texts (used to be hard coded in a js object)
    • profile form defaults and help texts (used to be hard coded in a js object)
    • project form defaults and help texts (used to be hard coded in a js object)

Fixes WD-7389 WD-7390

QA

  1. Run the LXD-UI:
    • On the demo server via the link posted by @webteam-app below. This is only available for PRs created by collaborators of the repo. Ask @lorumic or @edlerd for access.
    • With a local copy of this branch, run as described here.
  2. Perform the following QA steps:
    • check settings page
      • edit settings
      • reset settings
    • check create instance form: all default values (From: LXD) and help texts (the gray explanation)
      • create instance > advanced > all sections and all possible overrides
    • check create profile form: all default values (From: LXD) and help texts (the gray explanation)
      • profiles > create profile > advanced > all sections and all possible overrides
    • check edit project form: all default values (From: LXD) and help texts (the gray explanation)
      • configuration > edit > all sections and all possible overrides

@webteam-app
Copy link

Demo starting at https://lxd-ui-543.demos.haus

@edlerd edlerd force-pushed the config-api-settings branch 4 times, most recently from 53b5915 to 23f0a20 Compare November 17, 2023 22:03
@edlerd edlerd changed the title Use config api for server settings WD-7389 Use config api for server settings WD-7389 and instance defaults WD-7390 Nov 21, 2023
@edlerd edlerd force-pushed the config-api-settings branch 2 times, most recently from 7ec7d39 to c244175 Compare November 21, 2023 10:36
@edlerd edlerd changed the title Use config api for server settings WD-7389 and instance defaults WD-7390 Use config api for server settings WD-7389 instance defaults WD-7390 and project defaults Nov 21, 2023
@edlerd edlerd force-pushed the config-api-settings branch 5 times, most recently from 28bdc63 to dcae3e0 Compare November 22, 2023 15:41
@edlerd edlerd requested a review from mas-who November 29, 2023 19:16
src/util/config.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@mas-who mas-who left a comment

Choose a reason for hiding this comment

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

Just some minor comments, I'd feel more comfortable if someone else also have a review since I may not be the most familiar with the code yet

feat(instance) use config api for project defaults
feat(instance) use config api for instance defaults WD-7390
feat(config) use config api for server settings WD-7389

Signed-off-by: David Edler <david.edler@canonical.com>
@mas-who
Copy link
Contributor

mas-who commented Nov 30, 2023

Checked during QA:

  • Created a project test-config-api-project and checked every setting for edit saves and reset to default value.
  • Checked create instance form, create profile form and edit project form for available settings as well as overrides. All settings and override options seem to display correct explanations.
  • Checked all links within setting explanations that they navigate to the correct documentation.

Copy link
Contributor

@lorumic lorumic left a comment

Choose a reason for hiding this comment

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

Mostly a bunch of assignment shorthand suggestions (I recommend using the "Add suggestion to batch" button if you want to implement them, because they are a lot), and a few other questions/proposals for tiny improvements. Feel free to merge with or without these changes.

src/pages/projects/forms/ClusterRestrictionForm.tsx Outdated Show resolved Hide resolved
src/pages/projects/forms/ClusterRestrictionForm.tsx Outdated Show resolved Hide resolved
src/pages/projects/forms/DeviceUsageRestrictionForm.tsx Outdated Show resolved Hide resolved
src/pages/projects/forms/DeviceUsageRestrictionForm.tsx Outdated Show resolved Hide resolved
src/pages/projects/forms/DeviceUsageRestrictionForm.tsx Outdated Show resolved Hide resolved
src/pages/settings/Settings.tsx Show resolved Hide resolved
src/pages/settings/Settings.tsx Outdated Show resolved Hide resolved
src/types/config.d.ts Show resolved Hide resolved
src/util/config.tsx Outdated Show resolved Hide resolved
src/util/instanceConfigInheritance.tsx Show resolved Hide resolved
Update src/pages/settings/Settings.tsx

Co-authored-by: Michele Lo Russo <michele.lorusso@canonical.com>

Signed-off-by: David Edler <david.edler@canonical.com>
@edlerd edlerd merged commit 42c65c3 into canonical:main Dec 1, 2023
6 checks passed
@edlerd edlerd deleted the config-api-settings branch December 1, 2023 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants