Added validation errors #2639
Added validation errors #2639
Conversation
Generated by 🚫 Danger |
app/models/node.rb
Outdated
| elsif tagname.split(':')[0] == 'google' | ||
| errors ? "Only Oauth can create such tags" : false | ||
| elsif tagname.split(':')[0] == 'github' | ||
| errors ? "Only Oauth can create such tags" : false |
There was a problem hiding this comment.
By using ["facebook", ...].include? tagname.split(':')[0] instead we can squeeze 8 lines into just 2 and avoid much repetition. What do you think?
There was a problem hiding this comment.
@seafr I want to know whether we want the same error message for all 4 cases or different for these. I will do the required patch accordingly.
Thanks
There was a problem hiding this comment.
Ok, I see your point. I thought this was the final version as the in progress label was removed.
There was a problem hiding this comment.
@jywarren what do you suggest?
There was a problem hiding this comment.
Hm, i like @seafr's suggestion! Let's do it!
There was a problem hiding this comment.
@SidharthBansal I suggest moving all 4 options into 1 elseif as I said above. You can also customise the error message to something like:
"... create the #{tagname...[0]} tag"
if you want to make it different for each case.
There was a problem hiding this comment.
Ok, updating in a minute. Thanks both of you
|
This looks good! Is it ready for merging? 👍 😄 |
|
Yeah |
|
Thanks all mentors for selecting me in Gsoc 2018 |
|
I'm going to test CodeClimate integration by closing and reopening... let's see! |
|
@publiclab/reviewers - I tried enabling CodeClimate -- it's configured in |
|
Maybe we should turn off some of the checks -- that way we can "ease in" more gradually. As files get cleaner, we can turn on more checks? https://github.com/publiclab/plots2/blob/master/.codeclimate.yml |
|
Also notice I was able to log into CodeClimate by clicking "Details" and then approve it there despite the issues it complained about. So we can work around this... my main concern is that it looks too intimidating to newcomers -- and I wish it offered more advice to people on how to improve their code, rather than just complaining about it! |
|
So now that I've approved it, to see the original error it complained about, click Show all checks and click on Details for CodeClimate. |
|
I believe all reviewers ought to be able to "approve" via CodeClimate. |
|
Yeah I am able to see that you have approved it and the errors are on its respective page. Can anyone from @publiclab/reviewers approve this too?? I will not be able to do in my own pr. |
|
OK, merging!! 😀 |
As asked in #2556 by @jywarren
Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!
rake testfixes #0000-style reference to original issue #@publiclab/reviewersfor help, in a comment belowIf tests do fail, click on the red
Xto learn why by reading the logs.Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software
We have a loose schedule of reviewing and pulling in changes every Tuesday and Friday, and publishing changes on Fridays.
Thanks!