Skip to content

Support include query parameter for list roles #1228

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Fabian-K
Copy link

@Fabian-K Fabian-K commented Apr 8, 2024

Hi,

this PR adds support for the "include" query parameter to the list roles request API v3. This is my first contribution to this repository. Please let me know in case I missed something :)

Thanks,
Fabian

fixes #1204

Copy link

linux-foundation-easycla bot commented Apr 8, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@Kehrlann Kehrlann self-assigned this May 28, 2025
@Kehrlann Kehrlann self-requested a review June 11, 2025 05:54
@Kehrlann
Copy link
Contributor

Hey @Fabian-K , thanks for your contribution!

Could you please:

  • Rebase your branch on main (please rebase, don't merge)
  • Include an integration test

@Fabian-K Fabian-K force-pushed the feature/role-include branch 2 times, most recently from 39c1ca6 to 49427f3 Compare June 17, 2025 09:02
@Fabian-K
Copy link
Author

@Kehrlann Thanks for picking this up! I´ve rebased and added an integration test. I did however not run the integration test because I don´t have a cf landscape available with admin permissions. I only used a local build of the client and was able to do some testing there.

@Kehrlann
Copy link
Contributor

@Fabian-K Thanks! Will run the integration tests and let you know.

Please apply the formatting rules on the integration test project as well:

./mvnw spotless:apply -Pintegration-test

@Fabian-K Fabian-K force-pushed the feature/role-include branch from 49427f3 to eecd9d2 Compare June 18, 2025 07:56
@Fabian-K
Copy link
Author

Done, thank you!

Copy link
Contributor

@Kehrlann Kehrlann left a comment

Choose a reason for hiding this comment

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

One issue. Will run integration tests once I have an env provisioned.

@Fabian-K Fabian-K force-pushed the feature/role-include branch from eecd9d2 to dba4242 Compare June 18, 2025 11:48
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.

V3 API: How To Construct Query To List Roles Including User, Space Or Organization
2 participants