Add show_as_button to organizations connection #706
Conversation
jpealing-fiscaltec
had a problem deploying
to
external
GitHub Actions
Failure
| @@ -152,26 +152,31 @@ public async Task Test_organization_connections_crud_sequence() | |||
| var createConnectionResponse = await fixture.ApiClient.Organizations.CreateConnectionAsync(ExistingOrganizationId, new OrganizationConnectionCreateRequest | |||
| { | |||
| ConnectionId = ExistingConnectionId, | |||
| AssignMembershipOnLogin = true | |||
| AssignMembershipOnLogin = true, | |||
| ShowAsButton = true | |||
There was a problem hiding this comment.
show_as_button defaults to true on the API, can we remove this line to ensure the default value is used when omitted?
There was a problem hiding this comment.
I've removed that line and also removed the equivalent in the update call so there is a test for backwards compatability, as well as making ShowAsButton nullable on OrganizationConnectionUpdateRequest and OrganizationConnectionCreateRequest. This means Test_organization_connections_crud_sequence no longer tests creates and updates with ShowAsButton set, so I've added Test_organization_connections_show_as_button_crud_sequence as an attempt to test this, or would you prefer this be tested by extending Test_organization_connections_crud_sequence?
There was a problem hiding this comment.
See #706 (comment), I think we can stick to that.
To verify that the default value of "true" is used.
…e default value on the API is respected
| // Unlink Connection | ||
| await fixture.ApiClient.Organizations.DeleteConnectionAsync(ExistingOrganizationId, ExistingConnectionId); | ||
| } | ||
| } |
There was a problem hiding this comment.
I think we can avoid this test, and instead use the original to:
- Leave the default on create and assert its true
- Set it to false on update and assert its false after
|
Thanks 👍 Ignore the failing CI, will look into it. PR looks good 🙏 🔥 |
|
Any update on when this is likely to be merged? |
|
Sorry, lost track of this. Let me fix CI and look into merging and releasing this. |
|
Tests are failing because the property is only supported for Enterprise Connections. I will revert the changes to the test and move forward without the tests, testing enterprise-only features are not always that trivial. |
frederikprijck
requested a deployment
to
external
GitHub Actions
Waiting
Changes
Adds
show_as_buttontoOrganizationConnection,OrganizationConnectionCreateRequestandOrganizationConnectionUpdateRequest.References
This property is documented here:
Testing
I have attempted to add a unit test by following the pattern used for
AssignMembershipOnLogin, however I was not able to run the tests to validate that they passed.This change adds unit test coverage
This change adds integration test coverage
This change has been tested on the latest version of the platform/language or why not
Checklist
I have read the Auth0 general contribution guidelines
I have read the Auth0 Code of Conduct
All existing and new tests complete without errors