Make trigger_id and action_name optional for actions index endpoint #478
Conversation
Use proper path for create_action endpoint
There was a problem hiding this comment.
Thanks for your patience here! Looks like the original implementation of this endpoint in our library has a few issues, so thanks for taking the time to clean it up. Left a few comments. In general we should be able to implement this in a non-breaking way with a couple of changes (we'd like to avoid releasing a new major version if possible).
Let me know if you have the time to fix it up or I can continue with it.
lib/auth0/api/v2/actions.rb
Outdated
| raise Auth0::MissingTriggerId, 'Must supply a valid trigger_id' if trigger_id.to_s.empty? | ||
| raise Auth0::MissingActionName, 'Must supply a valid action_name' if action_name.to_s.empty? | ||
|
|
||
| def actions(trigger_id: nil, action_name: nil, deployed: nil, per_page: nil, page: nil, installed: nil) |
There was a problem hiding this comment.
I agree that this is the ideal API for this method but I would prefer we implement it in a non-breaking way. How about we do optional position parameters instead of named ones for the first two? That way existing calls will continue to work while there is also the ability to optionally leave them out.
def actions(trigger_id = nil, action_name = nil, deployed: nil, per_page: nil, page: nil, installed: nil)
lib/auth0/api/v2/actions.rb
Outdated
| trigger_id: trigger_id, | ||
| action_name: action_name, |
There was a problem hiding this comment.
Unfortunately the original implementation looks broken here, as these should be (according to the API):
triggerId: trigger_id,
actionName: action_name,I'm curious though, did you test this against the API? As I can't get your implementation to work, I get the "Query validation error: 'Additional properties not allowed: trigger_id'." error with the PR as it is now.
There was a problem hiding this comment.
Hmmm, I just tested it and as long as trigger_id and action_name are not passed as parameters, it works.
There was a problem hiding this comment.
Yeah I can see the same. If they are passed, however, we get an error.
There was a problem hiding this comment.
I've provided a fix in cb1ade0 that makes these arguments optional positional for backwards compatibility.
|
@auth0/dx-sdks-engineer would appreciate a review from another team member here as I have provided additional code modifications. |
|
Sweet thanks! |
Use proper path for create_action endpoint
Changes
Please describe both what is changing and why this is important. Include:
References