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 missing external_id property to the import users job #222

Merged
merged 1 commit into from
Jun 30, 2020

Conversation

lbalmaceda
Copy link
Contributor

Changes

The endpoint was missing the external_id property.

References

Resolves SDK-1625

Testing

  • 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

@lbalmaceda lbalmaceda added CH: Added small Small review labels Jun 29, 2020
@lbalmaceda lbalmaceda added this to the v3-Next milestone Jun 29, 2020
@lbalmaceda lbalmaceda requested a review from a team June 29, 2020 18:43
Copy link
Contributor

@stevehobbsdev stevehobbsdev left a comment

Choose a reason for hiding this comment

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

The criteria for this states:

When their values are not set, they should not be sent in the request. This will keep the SDK behaving as before, not introducing breaking changes.

Is there a way to test this scenario? All the tests I'm seeing validate the call where external_id is passed in but that doesn't seem enough to validate that the property is not included in the request.

@lbalmaceda
Copy link
Contributor Author

@stevehobbsdev the unit tests on this SDK only test the arguments passed to the networking client when starting a request. For python requests library, when you pass None you tell you don't want to include that value.

  • See here where the default of None is included as part of the call args. This is tested here
  • See here the library docs where it mentions the behavior of None. Quoting below:

Note that any dictionary key whose value is None will not be added to the URL’s query string.

e.g.

>>> req = requests.Request('POST', 'http://google.com', data=dict(a=None, b=1))
>>> req.prepare().body
'b=1'

@stevehobbsdev
Copy link
Contributor

Thanks for clarifying!

@stevehobbsdev stevehobbsdev self-requested a review June 30, 2020 13:52
@lbalmaceda lbalmaceda merged commit 5f9373f into master Jun 30, 2020
@lbalmaceda lbalmaceda deleted the miss-prop branch June 30, 2020 14:05
@lbalmaceda lbalmaceda modified the milestones: v3-Next, 3.12.0 Jul 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CH: Added small Small review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants