Update APIs #348
Update APIs #348
Conversation
|
Minor suggestion @wasabigeek - after you make a code change you realize that you want to fix something, or maybe add a changelog line. I usually |
|
Looks like we have a bug in a spec generator that produces |
|
Will take a look in a bit! |
| @@ -12,7 +12,7 @@ RSpec.describe Slack::Web::Api::Endpoints::<%= group.gsub(".", "_").camelize %> | |||
| context '<%= group %>_<%= name %>' do | |||
| <% required_params.each do |arg_name, arg_v| %> | |||
| it 'requires <%= arg_name %>' do | |||
| <% params_except_required = required_params.reject{ |name, _| name == arg_name }.map{|var, opts| "#{var}: '#{opts['example']}'"}.join(', ') %> | |||
| <% params_except_required = required_params.reject{ |name, _| name == arg_name }.map{|var, opts| "#{var}: '#{opts['example'].gsub("'", '"')}'"}.join(', ') %> | |||
There was a problem hiding this comment.
Ugh. This is to handle an arg like:
"args": {
"prefs": {
"required": true,
"example": "{'who_can_post':'type:admin,user:U1234,subteam:S1234'}",
"desc": "The prefs for this channel in a stringified JSON format."
}
},Which gets rendered as (prefs: '{'who_can_post':'type:admin,user:U1234,subteam:S1234'}') in the spec, breaking the tests.
There doesn't seem to be a less intrusive way to fix this. I could write a custom spec, but that seemed like it would skip auto-generation entirely for that endpoint, so it would probably drift pretty quickly. Do you have any suggestions @dblock?
There was a problem hiding this comment.
Some possibilities:
- do the substitution upstream in slack-api-ref, so it ends up like
{\"who_can_post\": ...}instead - create an object for each json instead of rebuilding it into a hash, so we can encapsulate the sanitization logic and make it more readable (e.g.
WebApiMethod.argscould sanitize and return the string)
There was a problem hiding this comment.
This will generate false positives and will mangle examples in the future if they contain single quotes within the String itself. And we'll never catch it.
I think this should really be "#{opts['example']}", which will work for everything. Then Rubocop will get upset, so a rubocop -a will auto-correct " into ' where appropriate. You could also do that by checking whether the string contains any ' and using " if so instead of gsub.
There was a problem hiding this comment.
To clarify, the suggestion is to have a manual step to correct such cases?
There was a problem hiding this comment.
For rubocop? Thinking about it i don't love it. I'll merge and we can fix this later.
Found a few updates while working on #347