Skip to content

feat(worfklows): Allow server side pagination and filtering for workflows #1512

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.

Sign up for GitHub

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

Merged
javirln merged 2 commits into from
Nov 14, 2024
Merged

feat(worfklows): Allow server side pagination and filtering for workflows #1512

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
805079f
feat(worfklows): Allow server side pagination and filtering for workf…
javirln Nov 13, 2024
e9db379
tackle feedback
javirln Nov 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
181 changes: 178 additions & 3 deletions app/controlplane/api/controlplane/v1/pagination.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 20 additions & 0 deletions app/controlplane/api/controlplane/v1/pagination.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,23 @@ message CursorPaginationRequest {
(buf.validate.field).ignore = IGNORE_IF_UNPOPULATED
];
}

// OffsetPaginationRequest is used to paginate the results
message OffsetPaginationRequest {
// The (zero-based) offset of the first item returned in the collection.
int32 page = 1 [(buf.validate.field).int32.gte = 1];
// The maximum number of entries to return. If the value exceeds the maximum, then the maximum value will be used.
int32 page_size = 2 [(buf.validate.field).int32.gt = 0, (buf.validate.field).int32.lte = 100];
Copy link
Member

@migmartri migmartri Nov 13, 2024

Choose a reason for hiding this comment

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

is it mandatory? why don't we have a default?

Copy link
Member Author

@javirln javirln Nov 14, 2024

Choose a reason for hiding this comment

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

The message itself is not required, only if you provide it. The only default values are in the biz layer. I can add the default values here.

Copy link
Member Author

@javirln javirln Nov 14, 2024

Choose a reason for hiding this comment

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

I have been searching and I couldn't find a way to specify a default value on proto3, apparently it was an option of proto2. I have found this resource https://stackoverflow.com/questions/33222551/why-are-there-no-custom-default-values-in-proto3

In any case, rest assure there are default values if the pagination is not provided a default pagination object is created:

	// Initialize the pagination options, with default values
	paginationOpts := pagination.NewDefaultOffsetPaginationOpts()

	// Override the pagination options if they are provided
	if req.GetPagination() != nil {
		paginationOpts, err = pagination.NewOffsetPaginationOpts(
			int(req.GetPagination().GetPage()),
			int(req.GetPagination().GetPageSize()),
		)
		if err != nil {
			return nil, handleUseCaseErr(err, s.log)
		}
	}

}

// OffsetPaginationResponse is used to return the pagination information
message OffsetPaginationResponse {
Copy link
Member

@migmartri migmartri Nov 13, 2024

Choose a reason for hiding this comment

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

out of curiosity, is this a common response type for this kind of pagination?

last_page could be calculated by total_count / page_size >= page?

Copy link
Member Author

@javirln javirln Nov 14, 2024

Choose a reason for hiding this comment

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

Depending on where you search there is a different response. It seems the most common pattern is:

  • page: current page
  • page_size: the number of results per page
  • total_count: total number of results

I've additionally added the last_page so clients don't need to that exact calculation that you're doing. Instead, another common element is to return the amount of pages. I will remove the last_page and add a total_pages.

// The current page number
int32 page = 1;
// The number of results per page
int32 page_size = 2;
// The total number of results
int32 total_count = 3;
// The total number of pages
int32 total_pages = 4;
}
Loading
Loading