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

feat(api): filter layouts endpoint #2479

Merged

Conversation

p-fernandez
Copy link
Contributor

What change does this PR introduce?

Adds endpoint to filter layouts by environment.

Why was this change needed?

UI needs a layout manager to list all the layouts an organization has in certain environment. For that this endpoint will be needed to show and paginate all the layouts.

Other information (Screenshots)

@p-fernandez p-fernandez self-assigned this Jan 11, 2023
@linear
Copy link

linear bot commented Jan 11, 2023

@p-fernandez p-fernandez force-pushed the nv-1415-feature-get-layouts-per-environment branch from f79da7d to dbd7167 Compare January 11, 2023 19:49
Copy link
Contributor

@ainouzgali ainouzgali left a comment

Choose a reason for hiding this comment

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

Looks good. I'm just not sure filter is the best name for this, as it implies actual filtering for params.
What do you think?
Maybe GetLayouts(that's the format we use in other places) or GetLayoutsList

@p-fernandez
Copy link
Contributor Author

Looks good. I'm just not sure filter is the best name for this, as it implies actual filtering for params. What do you think? Maybe GetLayouts(that's the format we use in other places) or GetLayoutsList

Probably we will need to add functionality to filter based in layouts being deleted or and by isDefault value. (For this last case we might want a dedicated endpoint though). Also currently we are filtering by environment and organization, though the values are passed in the Auth header, so for me we are actually filtering. That's why I always choose filter. GetLayout and GetLayouts is so easy to make a confusing mistake that if you are adamant on changing it I'd go for GetLayoutList.

@ainouzgali
Copy link
Contributor

No, it's fine. We can keep it as filter❤️

@p-fernandez p-fernandez merged commit 3be185b into feat-layouts Jan 12, 2023
@p-fernandez p-fernandez deleted the nv-1415-feature-get-layouts-per-environment branch January 12, 2023 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants