-
Notifications
You must be signed in to change notification settings - Fork 309
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
fix: management methods throwing error on access #500
fix: management methods throwing error on access #500
Conversation
@@ -839,7 +838,7 @@ describe('ManagementClient', function() { | |||
'getUsers', | |||
'getUser', | |||
'deleteAllUsers', | |||
'deleteUsers', |
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.
This method does not exist, but deleteUser
does.
@@ -872,12 +878,10 @@ describe('ManagementClient', function() { | |||
this.client = new ManagementClient(config); | |||
}); | |||
|
|||
for (var i = 0, l = methods.length; i < l; i++) { | |||
method = methods[i]; |
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.
method
was always set to the last item in the methods
array.
dde3721
to
f66875d
Compare
@@ -50,6 +50,9 @@ utils.generateClientInfo = function() { | |||
*/ | |||
utils.wrapPropertyMethod = function(Parent, name, propertyMethod) { | |||
var path = propertyMethod.split('.'); | |||
if (path.length > 2) { | |||
throw new Error('wrapPropertyMethod() only supports one level of nesting for propertyMethod'); |
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.
🎉
Changes
A few of the methods defined directly on the ManagementClient are throwing errors when accessed:
Throws:
After debugging, I found that the error comes from the
utils.wrapPropertyMethod()
method that only supports a single level of nesting forpropertyMethod
(ex.a.b
works, but nota.b.c
).In this PR, I review all methods defined on
ManagementClient
and ensure they all use one level of nesting instead of 2.I also fixed the unit test that was supposed to catch this.
References
Please include relevant links supporting this change such as a:
Testing
Please describe how this can be tested by reviewers. Be specific about anything not tested and reasons why. If this library has unit and/or integration testing, tests should be added for new functionality and existing tests should complete without errors.
Checklist