-
-
Notifications
You must be signed in to change notification settings - Fork 963
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: have the /admin/identities list call return all fields #3343
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #3343 +/- ##
==========================================
- Coverage 78.37% 78.34% -0.03%
==========================================
Files 348 347 -1
Lines 23985 23925 -60
==========================================
- Hits 18798 18744 -54
+ Misses 3768 3767 -1
+ Partials 1419 1414 -5 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thank you for your contribution! This looks pretty good and I have some ideas how to improve it further :)
ce055cb
to
1e1f74e
Compare
@aeneasr Can you please review the new changes in this PR? |
@borisroman could you resolve the merge conflicts? After that, this PR should be good to go. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! This is a great idea! It would be good if the list and get API of identities had the same parameters, which makes it easier to use the API - thanks! :)
1e1f74e
to
95674f0
Compare
@aeneasr @jonas-jonas I've rebased the PR against main and implemented the requested changes. Requesting your review once more. |
Any update on this? This currently impacts my integration massively, I need to provide some GraphQL queries for fetching identities in my API and it would be nice if I could return the full data on paginated responses without having to fetch them for each single identity. |
95674f0
to
a5cdda6
Compare
I've rebased against master again. Any chance this can get merged @aeneasr? Or give feedback if something needs to change... |
Yes! Unfortunately there is a test still failing to pass |
a5cdda6
to
3bd41c5
Compare
40e2d37
to
cc8275f
Compare
cc8275f
to
0a23fcc
Compare
@aeneasr Check, I added a test for improved code coverage; all green now! |
Related issue(s)
Fixes #3342
Checklist
introduces a new feature.
contributing code guidelines.
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.
works.
Further Comments