Add errors to FormResponse in PersonalKeyForm #1450
Conversation
el-mapache
force-pushed
the
ab-add-errors-to-pkey-form
branch
from
59a82bb
to
ba8419e
Compare
| end | ||
|
|
||
| def personal_key_format_matches? | ||
| personal_key =~ /\A#{PersonalKeyFormatter.new.regexp}\Z/ |
There was a problem hiding this comment.
I think the PersonalKeyFormatter#regexp should return a regex object. I'm assuming it returns a plain string for something in a view or with JS, should we break out two methods? regexp_str and regexp?
There was a problem hiding this comment.
I'm actually not sure, I just moved the declaration into a method to satisfy line length requirements! I was also confused why it didn't return a regex, I can look into it.
There was a problem hiding this comment.
@zachmargolis I update the PeronalKeyFormatter class. I split up the method and moved the variables into constants, since we shouldn't have to recompute the regex string.
el-mapache
force-pushed
the
ab-add-errors-to-pkey-form
branch
3 times, most recently
from
7e4c19a
to
e846836
Compare
| VALID_CHAR = '[a-zA-Z0-9]'.freeze | ||
| VALID_WORD = "#{VALID_CHAR}{#{CHAR_COUNT}}" | ||
| REGEXP = "(?:#{VALID_WORD}([\s-])?){#{WORD_COUNT - 1}}#{VALID_WORD}".freeze | ||
|
|
||
| def regexp |
There was a problem hiding this comment.
tiny nit .... there's no point in instantating this object, let's make these class methods?
def self.regexp
when when we use it we don't need the .new
PersonalKeyFormatter.regexp
There was a problem hiding this comment.
yeah totally fine with that, I wasn't sure why it was being called with .new in the first place, so I left it alone 😄
el-mapache
force-pushed
the
ab-add-errors-to-pkey-form
branch
from
e202583
to
beaaee6
Compare
**Why**: To better adhere to our conventions of returning an error object from from objects Return regex object from PersonalKeyFormatter **Why**: The method .regexp on PersonalKeyFormatter seems like it should return an actual regex and not a string **How**: Make two methods, one for use by ruby, returning the object, and one for use on the client, returning a string. Use class methods instead of new **Why**: This is simpler, instantiation wasn't really needed
Why: To better adhere to our conventions of returning an error
object from from objects