fix(dashboard): added dropdown for backupClasses#2104
Conversation
androndo
requested review from
IvanHunters,
kvaps,
lexfrei,
lllamnyp and
sircthulhu
as code owners
dosubot
bot
added
size:M
Summary of ChangesHello @androndo, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the Cozystack dashboard by integrating a user-friendly dropdown for selecting backup classes when creating or modifying BackupJobs and Plans. This change streamlines the workflow for users by providing a pre-populated list of available backup classes, eliminating the need for manual entry and ensuring data consistency. The update also includes the necessary backend logic and permissions to support this new UI element. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughWalkthroughThe changes refactor backup class name form configuration from API-driven to schema-based approach in the dashboard controller, and add RBAC permissions for the dashboard to read backupclass resources. Two helper functions support schema construction, and form-merging logic was adjusted to preserve schema properties during override creation. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](proxy.php?url=https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request successfully adds a dropdown menu for selecting backupClasses within the BackupJob and Plan creation forms in the dashboard. This is achieved by updating the CustomFormsOverride to use a listInput for the backupClassName field and adding the necessary RBAC permissions for the dashboard to fetch the list of backupclasses. The implementation is correct and addresses the feature requirement. I have one suggestion to refactor a small piece of duplicated code to improve maintainability.
| if strings.Contains(customizationId, "backups.cozystack.io/v1alpha1/backupjobs") { | ||
| schema := map[string]any{} | ||
| applyListInputOverrides(schema, "BackupJob", nil) | ||
| newSpec["schema"] = schema | ||
| } else if strings.Contains(customizationId, "backups.cozystack.io/v1alpha1/plans") { | ||
| schema := map[string]any{} | ||
| applyListInputOverrides(schema, "Plan", nil) | ||
| newSpec["schema"] = schema | ||
| } |
There was a problem hiding this comment.
The if and else if blocks for handling BackupJob and Plan are nearly identical, with only the kind string differing. This duplication can be removed by first determining the kind and then executing the common logic once. This will make the code more concise and easier to maintain.
| if strings.Contains(customizationId, "backups.cozystack.io/v1alpha1/backupjobs") { | |
| schema := map[string]any{} | |
| applyListInputOverrides(schema, "BackupJob", nil) | |
| newSpec["schema"] = schema | |
| } else if strings.Contains(customizationId, "backups.cozystack.io/v1alpha1/plans") { | |
| schema := map[string]any{} | |
| applyListInputOverrides(schema, "Plan", nil) | |
| newSpec["schema"] = schema | |
| } | |
| var kind string | |
| if strings.Contains(customizationId, "backups.cozystack.io/v1alpha1/backupjobs") { | |
| kind = "BackupJob" | |
| } else if strings.Contains(customizationId, "backups.cozystack.io/v1alpha1/plans") { | |
| kind = "Plan" | |
| } | |
| if kind != "" { | |
| schema := map[string]any{} | |
| applyListInputOverrides(schema, kind, nil) | |
| newSpec["schema"] = schema | |
| } |
| applyListInputOverrides(schema, "Plan", nil) | ||
| newSpec["schema"] = schema | ||
| } | ||
|
|
There was a problem hiding this comment.
Function applyListInputOverrides is called in customformsoverride.go in function ensureCustomFormsOverride.
It looks as if callers of the createCustomFormsOverride function only perform cleanup, so altering props is noop here
androndo
force-pushed
the
fix/dashboard-backup-classes-dropdown
branch
from
4c8e589 to
412a408
Compare
![coderabbitai[bot]](proxy.php?url=https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/controller/dashboard/static_refactored.go (1)
2101-2121: Rename typoed helper for readability (Scema→Schema).At Line 2101,
listInputScemaItemBackupClasshas a typo. Consider renaming it (and the parameter name increateSchema) to improve maintainability/searchability.♻️ Suggested rename
- "backupClassName": listInputScemaItemBackupClass(), + "backupClassName": listInputSchemaItemBackupClass(), - "backupClassName": listInputScemaItemBackupClass(), + "backupClassName": listInputSchemaItemBackupClass(), -func listInputScemaItemBackupClass() map[string]any { +func listInputSchemaItemBackupClass() map[string]any { return map[string]any{ "type": "listInput", "customProps": map[string]any{ "valueUri": "/api/clusters/{cluster}/k8s/apis/backups.cozystack.io/v1alpha1/backupclasses", "keysToValue": []any{"metadata", "name"}, "keysToLabel": []any{"metadata", "name"}, }, } } -// backupClassSchema returns the schema for spec.backupClassName as listInput (BackupJob/Plan). -func createSchema(customProps map[string]any) map[string]any { +// createSchema returns schema entries under spec.properties. +func createSchema(specProperties map[string]any) map[string]any { return map[string]any{ "properties": map[string]any{ "spec": map[string]any{ - "properties": customProps, + "properties": specProperties, }, }, } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/controller/dashboard/static_refactored.go` around lines 2101 - 2121, Rename the misspelled helper function listInputScemaItemBackupClass to listInputSchemaItemBackupClass (and update its comment accordingly), then update all references/calls to this function throughout the codebase; also check the createSchema function signature and any parameter names you intended to change (ensure createSchema(customProps map[string]any) still matches callers or rename the parameter consistently if you change it), and run tests/build to catch any remaining references.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@internal/controller/dashboard/static_refactored.go`:
- Around line 2101-2121: Rename the misspelled helper function
listInputScemaItemBackupClass to listInputSchemaItemBackupClass (and update its
comment accordingly), then update all references/calls to this function
throughout the codebase; also check the createSchema function signature and any
parameter names you intended to change (ensure createSchema(customProps
map[string]any) still matches callers or rename the parameter consistently if
you change it), and run tests/build to catch any remaining references.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
internal/controller/dashboard/customformsoverride.gointernal/controller/dashboard/static_refactored.gopackages/system/dashboard/templates/rbac.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- internal/controller/dashboard/customformsoverride.go
- packages/system/dashboard/templates/rbac.yaml
androndo
force-pushed
the
fix/dashboard-backup-classes-dropdown
branch
from
412a408 to
d366c46
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/controller/dashboard/static_refactored.go (1)
2101-2113: Fix helper naming/comment consistency to reduce maintenance friction.There’s a typo in
listInputScemaItemBackupClass, and the comment at Line 2112 references a different function name than the implementation.♻️ Proposed cleanup
- "backupClassName": listInputScemaItemBackupClass(), + "backupClassName": listInputSchemaItemBackupClass(), @@ - "backupClassName": listInputScemaItemBackupClass(), + "backupClassName": listInputSchemaItemBackupClass(), @@ -func listInputScemaItemBackupClass() map[string]any { +func listInputSchemaItemBackupClass() map[string]any { @@ -// backupClassSchema returns the schema for spec.backupClassName as listInput (BackupJob/Plan). +// createSchema returns the schema wrapper for spec properties (e.g. backupClassName listInput). func createSchema(customProps map[string]any) map[string]any {Also applies to: 510-510, 525-525
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/controller/dashboard/static_refactored.go` around lines 2101 - 2113, The helper function name listInputScemaItemBackupClass has a typo and its nearby comment references the wrong function name; rename the function to listInputSchemaItemBackupClass (fix "Scema"→"Schema") and update the comment above createSchema to correctly reference the renamed helper, then update all usages/references (including the other occurrences mentioned) to the new symbol so builds/tests compile and comments stay consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@internal/controller/dashboard/static_refactored.go`:
- Around line 2101-2113: The helper function name listInputScemaItemBackupClass
has a typo and its nearby comment references the wrong function name; rename the
function to listInputSchemaItemBackupClass (fix "Scema"→"Schema") and update the
comment above createSchema to correctly reference the renamed helper, then
update all usages/references (including the other occurrences mentioned) to the
new symbol so builds/tests compile and comments stay consistent.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
internal/controller/dashboard/customformsoverride.gointernal/controller/dashboard/static_refactored.gopackages/system/dashboard/templates/rbac.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- internal/controller/dashboard/customformsoverride.go
- packages/system/dashboard/templates/rbac.yaml
androndo
force-pushed
the
fix/dashboard-backup-classes-dropdown
branch
from
d366c46 to
cbf851a
Compare
Signed-off-by: Andrey Kolkov <[email protected]>
androndo
force-pushed
the
fix/dashboard-backup-classes-dropdown
branch
from
cbf851a to
f0e8e11
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/controller/dashboard/static_refactored.go (1)
2101-2101: Fix helper name typo to improve maintainability.
listInputScemaItemBackupClassappears to be a typo (Scema→Schema).✏️ Suggested rename
-func listInputScemaItemBackupClass() map[string]any { +func listInputSchemaItemBackupClass() map[string]any {- "backupClassName": listInputScemaItemBackupClass(), + "backupClassName": listInputSchemaItemBackupClass(),- "backupClassName": listInputScemaItemBackupClass(), + "backupClassName": listInputSchemaItemBackupClass(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/controller/dashboard/static_refactored.go` at line 2101, Rename the misspelled helper function listInputScemaItemBackupClass to listInputSchemaItemBackupClass and update every reference (calls, tests, comments) accordingly to avoid breakage; search for the symbol listInputScemaItemBackupClass in the repository and replace with listInputSchemaItemBackupClass, keep the function signature and behavior unchanged, and run tests/compile to ensure no remaining usages of the old name.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@internal/controller/dashboard/static_refactored.go`:
- Line 2101: Rename the misspelled helper function listInputScemaItemBackupClass
to listInputSchemaItemBackupClass and update every reference (calls, tests,
comments) accordingly to avoid breakage; search for the symbol
listInputScemaItemBackupClass in the repository and replace with
listInputSchemaItemBackupClass, keep the function signature and behavior
unchanged, and run tests/compile to ensure no remaining usages of the old name.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
internal/controller/dashboard/static_refactored.gopackages/system/dashboard/templates/rbac.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/system/dashboard/templates/rbac.yaml
2 participants
What this PR does
Release note
Summary by CodeRabbit