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

Issue 1711 api pagination updates #1728

Merged
merged 8 commits into from
Sep 4, 2023

Conversation

aindriu-aiven
Copy link
Contributor

About this change - What it does
This change adds multiple new end points for the coral UI to interact with and reduces code complexity by re-using a single piece of code to give a paginated resource to schema and connector environments as well.
It also reduces the number of loops of the list of environments and fixes a bug in the pagination.

Resolves: #1711
Why this way

programmiri
programmiri previously approved these changes Sep 4, 2023
Copy link
Contributor

@programmiri programmiri left a comment

Choose a reason for hiding this comment

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

FE is happy 🎉

.toLowerCase()
.contains(searchEnvParam.toLowerCase()))
env -> {
if (env.getName().toLowerCase().contains(searchEnvParam.toLowerCase())
Copy link
Contributor

@muralibasani muralibasani Sep 4, 2023

Choose a reason for hiding this comment

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

could there be a bug even before, if we use 'contains' here instead of equals ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

as this is partial match I think that contains makes more sense, equals would work if we were providing a drop down list but this is like type in "DE" and get everything that has DE in it.

So i think it is ok

Signed-off-by: Aindriu Lavelle <[email protected]>
Copy link
Contributor

@muralibasani muralibasani left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants