refactor(robot): introduce listselect model for project prompts#708
refactor(robot): introduce listselect model for project prompts#708qcserestipy wants to merge 3 commits intogoharbor:mainfrom
Conversation
- add prompts for project names and IDs - move functions to prompt package - make list select reusable - refactor system robot create and update to use new model Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #708 +/- ##
=========================================
- Coverage 10.99% 7.17% -3.82%
=========================================
Files 173 262 +89
Lines 8671 13032 +4361
=========================================
- Hits 953 935 -18
- Misses 7612 11989 +4377
- Partials 106 108 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR refactors the project selection logic in robot account creation and updating by introducing a reusable multi-select list model based on Bubble Tea. The refactoring extracts inline project selection code into a dedicated listselect component that can be reused throughout the application.
Changes:
- Introduces a new reusable
listselectmodel inpkg/views/base/listselect/that provides multi-select functionality with toggle (space) and confirm (enter) support - Adds wrapper functions in
pkg/views/project/listselect/for project-specific multi-select operations - Extends the
promptpackage withGetProjectNamesFromUser,GetProjectIDsFromUser, andPromptForMoreProjectsfunctions - Refactors robot create and update commands to use the new reusable prompt functions instead of inline huh.MultiSelect
- Cleans up unused code in multiselect model (removes high performance rendering flag and unused max helper function)
- Adds
demo/directory to.gitignore
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/views/base/listselect/model.go | New reusable Bubble Tea multi-select list model with toggle and confirm functionality |
| pkg/views/project/listselect/view.go | Project-specific wrappers for the listselect model, providing name and ID selection functions |
| pkg/prompt/prompt.go | Added GetProjectNamesFromUser, GetProjectIDsFromUser, and PromptForMoreProjects helper functions |
| cmd/harbor/root/robot/create.go | Removed inline project selection code, now uses prompt.GetProjectNamesFromUser and prompt.PromptForMoreProjects |
| cmd/harbor/root/robot/update.go | Updated to use prompt.GetProjectNamesFromUser and prompt.PromptForMoreProjects |
| pkg/views/base/multiselect/model.go | Cleanup: removed unused useHighPerformanceRenderer constant and max helper function (now using Go built-in) |
| .gitignore | Added demo/ directory to ignore list |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@qcserestipy How can I check / review this pr? When I run create it gives me a Do I need elevated permissions in demo.goharbor.io? EDIT: Nvm it was some weird issue, had to create a new user |
- run tea with altscreen - return error instead of os.Exit - print warning if list selection is empty Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
| package project |
There was a problem hiding this comment.
This file declares 'package project' when it's in the 'pkg/views/project/listselect/' directory. While technically valid in Go, having the same package name ('project') in both 'pkg/views/project/select/' and 'pkg/views/project/listselect/' creates confusion. Consider using 'package listselect' to match the directory name and improve code clarity, similar to how 'pkg/views/project/list/' uses 'package list'.
| package project | |
| package listselect |
| for idx := range m.Selected { | ||
| if i, ok := m.List.Items()[idx].(Item); ok { | ||
| m.Choices = append(m.Choices, string(i)) | ||
| } | ||
| } |
There was a problem hiding this comment.
Iterating over a map in Go produces non-deterministic order. This means the items in m.Choices will be added in random order, which could lead to inconsistent behavior across runs. Consider sorting the indices before iterating, or maintain the selection order in a separate slice to preserve the order in which users selected items.
| func ProjectsList(projects []*models.Project) ([]string, error) { | ||
| items := make([]list.Item, len(projects)) | ||
| for i, p := range projects { | ||
| items[i] = listselect.Item(p.Name) | ||
| } | ||
|
|
||
| m := listselect.NewModel(items, "Project") | ||
|
|
||
| p, err := tea.NewProgram(m, tea.WithAltScreen()).Run() | ||
|
|
||
| if err != nil { | ||
| return nil, fmt.Errorf("error running selection program: %w", err) | ||
| } | ||
|
|
||
| if model, ok := p.(listselect.Model); ok { | ||
| if model.Aborted { | ||
| return nil, ErrUserAborted | ||
| } | ||
| if len(model.Choices) == 0 { | ||
| return nil, errors.New("no project selected") | ||
| } | ||
| return model.Choices, nil | ||
| } | ||
|
|
||
| return nil, errors.New("unexpected program result") | ||
| } | ||
|
|
||
| func ProjectsListWithId(projects []*models.Project) ([]int64, error) { | ||
| items := make([]list.Item, len(projects)) | ||
| itemsMap := make(map[string]int64) | ||
|
|
||
| for i, p := range projects { | ||
| items[i] = listselect.Item(p.Name) | ||
| itemsMap[p.Name] = int64(p.ProjectID) | ||
| } | ||
|
|
||
| m := listselect.NewModel(items, "Project") | ||
|
|
||
| p, err := tea.NewProgram(m, tea.WithAltScreen()).Run() | ||
| if err != nil { | ||
| return nil, fmt.Errorf("error running selection program: %w", err) | ||
| } | ||
|
|
||
| if model, ok := p.(listselect.Model); ok { | ||
| if model.Aborted { | ||
| return nil, ErrUserAborted | ||
| } | ||
| if len(model.Choices) == 0 { | ||
| return nil, errors.New("no project selected") | ||
| } | ||
| var selectedIDs []int64 | ||
| for _, choice := range model.Choices { | ||
| selectedIDs = append(selectedIDs, itemsMap[choice]) | ||
| } | ||
| return selectedIDs, nil | ||
| } | ||
|
|
||
| return nil, errors.New("unexpected program result") | ||
| } |
There was a problem hiding this comment.
The functions ProjectsList and ProjectsListWithId share nearly identical logic, differing only in how they process the selected items (returning names vs IDs). Consider refactoring to reduce duplication by creating a single generic function that accepts a transformation function to convert model.Choices to the desired output type.
2 participants
refactor(robot): introduce reusable listselect model for project prompts
Replaces the inline project selection logic in robot create/update with a reusable multi-select list model based on Bubble Tea.
Changes
listselectmodel with toggle (space) and confirm (enter) supportGetProjectNamesFromUserandGetProjectIDsFromUsertopromptpackage