Skip to content

refactor(utils): extract shared pagination validation helper#895

Open
PrasunaEnumarthy wants to merge 2 commits into
goharbor:mainfrom
PrasunaEnumarthy:refactor/shared-pagination-helpers
Open

refactor(utils): extract shared pagination validation helper#895
PrasunaEnumarthy wants to merge 2 commits into
goharbor:mainfrom
PrasunaEnumarthy:refactor/shared-pagination-helpers

Conversation

@PrasunaEnumarthy

@PrasunaEnumarthy PrasunaEnumarthy commented May 7, 2026

Copy link
Copy Markdown
Contributor

Description

This PR centralizes and standardizes the pagination validation logic across multiple CLI commands. Currently, logic to validate page and pageSize is duplicated in several list.go files, leading to inconsistent error messages and maintenance overhead.

By moving this logic to a shared utility helper, we ensure a "Single Source of Truth" for pagination constraints, improve code reusability, and simplify the implementation of future commands.

Type of Change

  • Bug fix
  • New feature
  • Refactor
  • Documentation update
  • Chore / maintenance

Changes

  • Created ValidatePagination(page, pageSize int64) error in pkg/utils/helper.go.
  • Added comprehensive table-driven unit tests in pkg/utils/helper_test.go covering edge cases (page < 1, negative pageSize, and max pageSize).
  • Refactored user list, project list, registry list, and artifact list to use the new centralized validation helper.
  • Standardized pagination error messages across the CLI to improve User Experience (UX).

@codecov

codecov Bot commented May 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 82.35294% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 9.21%. Comparing base (60ad0bd) to head (d04ccc0).
⚠️ Report is 186 commits behind head on main.

Files with missing lines Patch % Lines
cmd/harbor/root/user/list.go 0.00% 2 Missing ⚠️
cmd/harbor/root/registry/list.go 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             main    #895      +/-   ##
=========================================
- Coverage   10.99%   9.21%   -1.78%     
=========================================
  Files         173     321     +148     
  Lines        8671   16073    +7402     
=========================================
+ Hits          953    1481     +528     
- Misses       7612   14459    +6847     
- Partials      106     133      +27     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@NucleoFusion NucleoFusion left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm!
Thanks for the contribution!

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR centralizes pagination (page, pageSize) validation into a shared utility to eliminate duplicated logic across CLI list commands and standardize related error messages.

Changes:

  • Added utils.ValidatePagination(page, pageSize int64) error in pkg/utils/helper.go.
  • Added table-driven unit tests for pagination validation in pkg/utils/helper_test.go.
  • Refactored user/project/registry/artifact list commands to call the shared helper.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pkg/utils/helper.go Introduces shared pagination validation logic (with consistent constraints/messages).
pkg/utils/helper_test.go Adds unit tests for the new pagination helper.
cmd/harbor/root/user/list.go Replaces inline pagination checks with utils.ValidatePagination.
cmd/harbor/root/project/list.go Replaces inline pagination checks with utils.ValidatePagination.
cmd/harbor/root/registry/list.go Replaces inline pagination checks with utils.ValidatePagination.
cmd/harbor/root/artifact/list.go Replaces inline pagination checks with utils.ValidatePagination.
Comments suppressed due to low confidence (1)

cmd/harbor/root/registry/list.go:43

  • The error message references "projects list" even though this command lists registries, which is confusing and inconsistent with other list commands.
			registry, err := api.ListRegistries(opts)

			if err != nil {
				return fmt.Errorf("failed to get projects list: %v", err)
			}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/utils/helper.go
Comment thread pkg/utils/helper_test.go
@PrasunaEnumarthy PrasunaEnumarthy force-pushed the refactor/shared-pagination-helpers branch 2 times, most recently from ce6dbb3 to 8483e48 Compare June 5, 2026 10:56
Signed-off-by: PrasunaEnumarthy <eswari.prasuna@gmail.com>
Signed-off-by: PrasunaEnumarthy <eswari.prasuna@gmail.com>
@PrasunaEnumarthy PrasunaEnumarthy force-pushed the refactor/shared-pagination-helpers branch from 8483e48 to d04ccc0 Compare June 9, 2026 14:33
@PrasunaEnumarthy

Copy link
Copy Markdown
Contributor Author

@bupd please ptal when you have time, i have resolved all review comments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[feature]:Centralize and standardize pagination validation in pkg/utils

3 participants