Do not allow spaces in external billing #13081
Conversation
pacodelaluna
force-pushed
the
do-not-allow-spaces-in-external-billing-id
branch
from
dbb2515 to
39825a9
Compare
rioug
added
the
user facing changes
|
@rioug I have requested the merge of the last master changes, and it seems that the Rubocop warning is gone, but now I have a test failing, it seems to be a flaky one. Do you know waht to do in this case? Can I trigger the checks without pushing a commit? |
On your fork you should be able to, but probably not in the OFN repo, but I can :) All good now, thanks ! |
app/models/enterprise.rb
Outdated
| @@ -127,6 +127,9 @@ class Enterprise < ApplicationRecord | |||
| message: Spree.t('errors.messages.invalid_instagram_url') | |||
| }, allow_blank: true | |||
| validate :validate_white_label_logo_link | |||
| validates :external_billing_id, | |||
| format: { with: /\A\S+\Z/ }, | |||
There was a problem hiding this comment.
I wasn't familiar with the \A or \Z, so I tested this out. A website like regexr.com is really handy for testing and perfecting regular expressions.
I don't think this is what was intended, and it would always be considered invalid. Can you please add at least one other test to ensure that a valid Billing ID is validated correctly.
There was a problem hiding this comment.
Yes, it seems I messed up with the regex, not sure how the tests is passing... I will update with a standard one : /^\S+$/.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -127,6 +127,9 @@ | |||
| message: Spree.t('errors.messages.invalid_instagram_url') | |||
| }, allow_blank: true | |||
| validate :validate_white_label_logo_link | |||
| validates :external_billing_id, | |||
| format: { with: /^\S+$/ }, | |||
Check failure
Code scanning / Brakeman
Insufficient validation for external_billing_id using /^\S+$/. Use \A and \z as anchors. Error
What? Why?
I forgot to add a basic validation on the external_billing_id field on my last PR.
What should we test?
Release notes
Changelog Category (reviewers may add a label for the release notes):