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

fix: IdsFilter Query Param name #3676

Closed
wants to merge 1 commit into from
Closed

fix: IdsFilter Query Param name #3676

wants to merge 1 commit into from

Conversation

ItsZiroy
Copy link

This PR fixes #3675. It makes sure that the sdk generated names ids_filter are in sync with the actual query parameter.

If instead the name ids should continue to be used, the spec should be updated so that the sdk clients will generate correctly, but I am not sure how to type hint this.

I think the spec is defined here:

kratos/identity/pool.go

Lines 17 to 28 in 699e5d5

type (
ListIdentityParameters struct {
Expand Expandables
IdsFilter []string
CredentialsIdentifier string
CredentialsIdentifierSimilar string
KeySetPagination []keysetpagination.Option
// DEPRECATED
PagePagination *x.Page
ConsistencyLevel crdbx.ConsistencyLevel
}

I am not entirely sure in how many places this is feature is tested, I was searching through the repo and didn't really find any success, so if you could let me know where I find the tests, I can update them.

Related issue(s)

Checklist

  • I have read the contributing guidelines.
  • I have referenced an issue containing the design document if my change
    introduces a new feature.
  • I am following the
    contributing code guidelines.
  • I have read the security policy.
  • I confirm that this pull request does not address a security
    vulnerability. If this pull request addresses a security vulnerability, I
    confirm that I got the approval (please contact
    [email protected]) from the maintainers to push
    the changes.
  • I have added tests that prove my fix is effective or that my feature
    works.
  • I have added or changed the documentation.

Further Comments

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@ItsZiroy ItsZiroy changed the title Fixes IdsFilter Query Param name fix: IdsFilter Query Param name Dec 31, 2023
aeneasr added a commit that referenced this pull request Jan 8, 2024
@aeneasr aeneasr closed this in #3684 Jan 8, 2024
aeneasr added a commit that referenced this pull request Jan 8, 2024
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.

List Identities Id filter specifies wrong query parameter
2 participants