-
Notifications
You must be signed in to change notification settings - Fork 28
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: Add organizations array resolver on Account type #863
Conversation
after: String | ||
last: Int | ||
before: String | ||
): AccountOrganizationConnection! @cost(complexity: 25, multipliers: ["first", "last"]) |
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.
complexity is same as on user organizations list. Seems to make sense to keep things consistent. Have run this by Ajay
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. 📢 Thoughts on this report? Let us know! |
❌ 4 Tests Failed:
View the top 3 failed tests by shortest run time
To view individual test run time comparison to the main branch, go to the Test Analytics Dashboard |
Test Failures Detected: Due to failing tests, we cannot provide coverage reports at this time. ❌ Failed Test Results:Completed 2461 tests with View the full list of failed testspytest
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #863 +/- ##
=======================================
Coverage 96.34% 96.34%
=======================================
Files 823 823
Lines 19157 19163 +6
=======================================
+ Hits 18457 18463 +6
Misses 700 700
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
||
|
||
__all__ = ["get_current_license", "account_bindable"] | ||
__all__ = ["get_current_license", "account_bindable", "account"] |
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.
I noticed this in a lot of GQL files but idk if we actually need get_current_license here, do we?
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.
You're right lol. I removed the unnecessary instances and tests are all green.
): | ||
return queryset_to_connection( | ||
account.organizations, | ||
ordering=(ordering,), |
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.
do we need the tuple here for ordering? vs ordering=ordering
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.
Yes we do. Not sure exactly how this works under the hood, but the queryset_to_connection helper does a for field in ordering
. Some other calls do pass multiple values in the tuple, so seems like there's a reason for this.
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.
Got it 👌 Thanks for clarifying!
def resolve_organizations( | ||
account: Account, | ||
info: GraphQLResolveInfo, | ||
ordering=AccountOrganizationOrdering.ACTIVATED_USERS, |
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.
just to confirm, default ordering is by plan_activated_users in descending order
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.
Yes!
6e72268
to
79e22d2
Compare
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.
send it ✅
Adds the paginated organizations array resolver on the Account type in the graphql API.
Closes codecov/engineering-team#2558