-
Notifications
You must be signed in to change notification settings - Fork 179
Make OrganizationMember.Roles nullable #895
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
Conversation
|
Hi @g1n93r 👋 Thanks for this PR! We have reviewed the changes and tested them locally; everything looks great, and the tests are passing too. However, there is just one final step before we can merge. We require all commits to be verified (signed) as per our contribution guidelines. Could you please sign your commits? GitHub has a guide here. Do let us know if you run into any issues! |
The Roles property is optional and may be null when the organization member is retrieved without explicitly requesting the roles field. This change updates the property type to be nullable and adds XML documentation clarifying that the property will only be present when OrganizationGetAllMembersRequest.Fields includes "roles" and IncludeFields is true.
The Roles property is optional and may be null when the organization member is retrieved without explicitly requesting the roles field. This change updates the property type to be nullable and adds XML documentation clarifying that the property will only be present when OrganizationGetAllMembersRequest.Fields includes "roles" and IncludeFields is true.
Here you go :) Let me know if you need anything else |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #895 +/- ##
==========================================
- Coverage 79.20% 8.96% -70.25%
==========================================
Files 457 372 -85
Lines 5915 4484 -1431
Branches 277 176 -101
==========================================
- Hits 4685 402 -4283
- Misses 1136 4066 +2930
+ Partials 94 16 -78
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Changes
This PR fixes a bug in the
OrganizationMemberclass where theRolesproperty was incorrectly defined as non-nullable. TheRolesproperty is optional and may be null when organization members are retrieved without explicitly requesting the roles field via theOrganizationGetAllMembersRequest.Classes and methods changed:
OrganizationMember.Rolesproperty type changed fromIList<Role>toIList<Role>?(nullable)OrganizationMember.Rolesclarifying that:OrganizationGetAllMembersRequest.Fieldsincludes "roles" andOrganizationGetAllMembersRequest.IncludeFieldsis trueWhy this is important:
When retrieving organization members without explicitly requesting roles (which is the default behavior), the Auth0 API does not include the
rolesfield in the response. Previously, the non-nullable type could cause deserialization issues or force consumers to handle null values despite the type not indicating it could be null. This change makes the API contract more accurate and prevents potential runtime errors.Usage:
References
This change aligns the SDK with the Auth0 Management API behavior where the
rolesfield is only included in organization member responses when explicitly requested via thefieldsquery parameter.Testing
This change can be tested by:
Existing integration tests: The existing test
Test_organization_member_roles_crud_sequenceinOrganizationTests.cs(line 359) already tests the scenario where roles are explicitly requested and should continue to pass.Manual testing:
Rolesis nullFields = "roles"andIncludeFields = trueand verifyRolesis populatedRegression testing: All existing integration tests should continue to pass, particularly:
Test_organization_members_crud_sequence- tests member retrieval without rolesTest_organization_member_roles_crud_sequence- tests member retrieval with rolesChecklist