-
Notifications
You must be signed in to change notification settings - Fork 33
Add external_id to user and organization #381
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -75,6 +75,7 @@ def get_organization(id:) | |
| # @option domain_data [String] domain The domain that belongs to the organization. | ||
| # @option domain_data [String] state The state of the domain. "verified" or "pending" | ||
| # @param [String] name A unique, descriptive name for the organization | ||
| # @param [String] external_id The organization's external ID. | ||
| # @param [String] idempotency_key An idempotency key | ||
| # @param [Boolean, nil] allow_profiles_outside_organization Whether Connections | ||
| # within the Organization allow profiles that are outside of the Organization's configured User Email Domains. | ||
|
|
@@ -85,11 +86,13 @@ def create_organization( | |
| domain_data: nil, | ||
| domains: nil, | ||
| name:, | ||
| external_id: nil, | ||
| allow_profiles_outside_organization: nil, | ||
| idempotency_key: nil | ||
| ) | ||
| body = { name: name } | ||
| body[:domain_data] = domain_data if domain_data | ||
| body[:external_id] = external_id if external_id | ||
|
|
||
| if domains | ||
| warn_deprecation '`domains` is deprecated. Use `domain_data` instead.' | ||
|
|
@@ -123,22 +126,26 @@ def create_organization( | |
| # @option domain_data [String] state The state of the domain. "verified" or "pending" | ||
| # @param [String] stripe_customer_id The Stripe customer ID associated with this organization. | ||
| # @param [String] name A unique, descriptive name for the organization | ||
| # @param [String] external_id The organization's external ID. | ||
| # @param [Boolean, nil] allow_profiles_outside_organization Whether Connections | ||
| # within the Organization allow profiles that are outside of the Organization's configured User Email Domains. | ||
| # Deprecated: If you need to allow sign-ins from any email domain, contact [email protected]. | ||
| # @param [Array<String>] domains List of domains that belong to the organization. | ||
| # Deprecated: Use domain_data instead. | ||
| # rubocop:disable Metrics/ParameterLists | ||
| def update_organization( | ||
| organization:, | ||
| stripe_customer_id: nil, | ||
| domain_data: nil, | ||
| domains: nil, | ||
| name: nil, | ||
| external_id: :not_set, | ||
| allow_profiles_outside_organization: nil | ||
| ) | ||
| body = { name: name } | ||
| body[:domain_data] = domain_data if domain_data | ||
| body[:stripe_customer_id] = stripe_customer_id if stripe_customer_id | ||
| body[:external_id] = external_id if external_id != :not_set | ||
|
|
||
| if domains | ||
| warn_deprecation '`domains` is deprecated. Use `domain_data` instead.' | ||
|
|
@@ -162,6 +169,7 @@ def update_organization( | |
|
|
||
| WorkOS::Organization.new(response.body) | ||
| end | ||
| # rubocop:enable Metrics/ParameterLists | ||
|
|
||
| # Delete an Organization | ||
| # | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,6 +33,21 @@ | |
| end | ||
| end | ||
|
|
||
| context 'with external_id' do | ||
| it 'creates an organization with external_id' do | ||
| VCR.use_cassette 'organization/create_with_external_id' do | ||
| organization = described_class.create_organization( | ||
| name: 'Test Organization with External ID', | ||
| external_id: 'ext_org_123', | ||
| ) | ||
|
|
||
| expect(organization.id).to start_with('org_') | ||
| expect(organization.name).to eq('Test Organization with External ID') | ||
| expect(organization.external_id).to eq('ext_org_123') | ||
| end | ||
| end | ||
| end | ||
|
|
||
| context 'with domains' do | ||
| it 'creates an organization and warns' do | ||
| VCR.use_cassette 'organization/create_with_domains' do | ||
|
|
@@ -310,6 +325,43 @@ | |
| end | ||
| end | ||
| end | ||
| context 'with an external_id' do | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Think it's worth tests for unsetting
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ya, that's a good callout, will do! |
||
| it 'updates the organization' do | ||
| VCR.use_cassette 'organization/update_with_external_id' do | ||
| organization = described_class.update_organization( | ||
| organization: 'org_01K0SQV0S6EPWK2ZDEFD1CP1JC', | ||
| name: 'Test Organization', | ||
| external_id: 'ext_org_456', | ||
| ) | ||
|
|
||
| expect(organization.id).to eq('org_01K0SQV0S6EPWK2ZDEFD1CP1JC') | ||
| expect(organization.name).to eq('Test Organization') | ||
| expect(organization.external_id).to eq('ext_org_456') | ||
| end | ||
| end | ||
| end | ||
|
|
||
| context 'can set external_id to null explicitly' do | ||
| it 'includes external_id null in request body' do | ||
| original_method = described_class.method(:put_request) | ||
| allow(described_class).to receive(:put_request) do |kwargs| | ||
| original_method.call(**kwargs) | ||
| end | ||
|
|
||
| VCR.use_cassette 'organization/update_with_external_id_null' do | ||
| described_class.update_organization( | ||
| organization: 'org_01K0SQV0S6EPWK2ZDEFD1CP1JC', | ||
| name: 'Test Organization', | ||
| external_id: nil, | ||
| ) | ||
| end | ||
|
|
||
| # Verify the spy captured the right call | ||
| expect(described_class).to have_received(:put_request).with( | ||
| hash_including(body: hash_including(external_id: nil)), | ||
| ) | ||
| end | ||
| end | ||
| end | ||
|
|
||
| describe '.delete_organization' do | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -358,6 +358,22 @@ | |
| ) | ||
| end | ||
|
|
||
| it 'creates a user with external_id' do | ||
| VCR.use_cassette 'user_management/create_user_with_external_id' do | ||
| user = described_class.create_user( | ||
| email: '[email protected]', | ||
| first_name: 'External', | ||
| last_name: 'User', | ||
| external_id: 'ext_user_123', | ||
| ) | ||
|
|
||
| expect(user.first_name).to eq('External') | ||
| expect(user.last_name).to eq('User') | ||
| expect(user.email).to eq('[email protected]') | ||
| expect(user.external_id).to eq('ext_user_123') | ||
| end | ||
| end | ||
|
|
||
| context 'with an invalid payload' do | ||
| it 'returns an error' do | ||
| VCR.use_cassette 'user_management/create_user_invalid' do | ||
|
|
@@ -426,6 +442,25 @@ | |
| ) | ||
| end | ||
|
|
||
| it 'can set external_id to null explicitly' do | ||
| original_method = described_class.method(:put_request) | ||
| allow(described_class).to receive(:put_request) do |kwargs| | ||
| original_method.call(**kwargs) | ||
| end | ||
|
|
||
| VCR.use_cassette 'user_management/update_user_external_id_null' do | ||
| described_class.update_user( | ||
| id: 'user_01K0SR53HJ58M957MYAB6TDZ9X', | ||
| first_name: 'John', | ||
| external_id: nil, | ||
| ) | ||
| end | ||
|
|
||
| expect(described_class).to have_received(:put_request).with( | ||
| hash_including(body: hash_including(external_id: nil)), | ||
| ) | ||
| end | ||
|
|
||
| context 'with an invalid payload' do | ||
| it 'returns an error' do | ||
| VCR.use_cassette 'user_management/update_user/invalid' do | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
Nice catch! Wonder how we support unsetting the other arguments. 🤔
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.
Right now that's just not possible. I haven't done a thorough look to see what can be set to null or not. I think we should likely revisit the sdk and do this not_set thing in a bunch of places, but didn't want to get into that in this PR specifically