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

added include credentials support #3652

Closed
wants to merge 1 commit into from

Conversation

LukaJaj
Copy link

@LukaJaj LukaJaj commented Dec 2, 2023

Related issue(s)

adds include identities support on ListIdentities api

Fixes #3190

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.

Copy link

codecov bot commented Dec 2, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (bbf874f) 78.30% compared to head (812baf2) 78.35%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3652      +/-   ##
==========================================
+ Coverage   78.30%   78.35%   +0.05%     
==========================================
  Files         345      345              
  Lines       23560    23560              
==========================================
+ Hits        18449    18461      +12     
+ Misses       3715     3703      -12     
  Partials     1396     1396              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@LukaJaj LukaJaj changed the title fixed formatting added include credentials support Dec 2, 2023
Copy link
Contributor

@hperl hperl left a comment

Choose a reason for hiding this comment

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

Hi, and thanks for your first contribution to Ory Kratos!

I don't think you have found the correct place to add a new parameter yet. You edited the generated Go client (which we check in to do internal testing), but you need to actually add the parameter to the handler.

This is the handler for list identities: https://github.com/ory/kratos/blob/master/identity/handler.go#L185

Also please make sure to add tests and a docs PR (in general, make sure to tick all check boxes in the PR template). Thanks!

@borisroman
Copy link
Contributor

I think this can be closed once #3343 is merged.

@aeneasr aeneasr closed this Feb 20, 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.

Support include_credential param for ListIdentities API
5 participants