Skip to content
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

Add parameters for users imports #177

Merged

Conversation

makoto-matsumoto
Copy link
Contributor

@makoto-matsumoto makoto-matsumoto commented Jul 1, 2019

Changes

Add request parameters for users imports endpoint.
(upsert / external_id / send_completion_email)

References

https://auth0.com/docs/api/management/v2/#!/Jobs/post_users_imports

Testing

  • This change adds unit test coverage
  • This change adds integration test coverage
  • This change has been tested on the latest version of Ruby

Checklist

@makoto-matsumoto makoto-matsumoto requested a review from a team July 1, 2019 01:59
@joshcanhelp joshcanhelp added this to the v4.8.0 milestone Jul 1, 2019
joshcanhelp
joshcanhelp previously approved these changes Jul 1, 2019
Copy link
Contributor

@joshcanhelp joshcanhelp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @makoto-matsumoto! A couple of small changes here. I'm also looking into why CI is not running on this PR.

We're working on a release for this week so great timing on this!

@@ -17,10 +17,34 @@
it { expect(@instance).to respond_to(:import_users) }
it 'expect client to send post to /api/v2/jobs/users-imports' do
expect(@instance).to receive(:post_file).with(
'/api/v2/jobs/users-imports', users: 'file', connection_id: 'connnection_id'
'/api/v2/jobs/users-imports',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for tests!

raise Auth0::InvalidParameter, 'Must specify a valid file' if users_file.to_s.empty?
raise Auth0::InvalidParameter, 'Must specify a connection_id' if connection_id.to_s.empty?

request_params = {
users: users_file,
connection_id: connection_id
connection_id: connection_id,
upsert: options[:upsert].nil? ? false : options[:upsert],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this library, we typically use hash.fetch for values:

https://github.com/auth0/ruby-auth0/blob/master/lib/auth0/api/v2/users.rb#L30

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the review.

It was simpler to use hash.fetch

@joshcanhelp
Copy link
Contributor

Closing and re-opening to trigger CI

@joshcanhelp joshcanhelp closed this Jul 1, 2019
@joshcanhelp joshcanhelp reopened this Jul 1, 2019
@joshcanhelp joshcanhelp dismissed their stale review July 1, 2019 15:40

Meant to request changes :|

@joshcanhelp joshcanhelp self-assigned this Jul 1, 2019
@joshcanhelp
Copy link
Contributor

@makoto-matsumoto - Thank you once again. I'll merge in #178 when it's been reviewed, we can merge in the changes, and this (and your other one) will be good to go.

@joshcanhelp
Copy link
Contributor

@makoto-matsumoto - This one is ready as well! Just rebase master and push and tests should pass 👍

@makoto-matsumoto makoto-matsumoto force-pushed the add_parameters_for_users_imports branch from 483d729 to c7ecd35 Compare July 3, 2019 02:53
@makoto-matsumoto
Copy link
Contributor Author

@joshcanhelp

I rebased the master and did a push.
Please confirm!

@joshcanhelp joshcanhelp self-requested a review July 9, 2019 16:28
@joshcanhelp joshcanhelp merged commit 93ab04f into auth0:master Jul 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants