Introduce value objects for complex parameter types

[adrianna-chang-shopify]

We can offer users some custom parameter type objects to handle simple input validation + appropriate casting for more complex parameter types (the two we've talked about so far are arrays of strings & arrays of integers).

test/dummy/app/tasks/maintenance/params_task.rb demonstrates how a Task author would use one of these types in their Task. This way, users don't have to copy-pasta a regex across Tasks, and they also get typecasting for free without having to cast their user input themselves in the Task.

The regexes right now are pretty simple.
For array of integers: We accept a comma-delimited series of integers. We allow zero or one whitespace at the start or end of the string, before and after commas, and before and after the integers. (We could make it accept any amount of whitespace here).
For array of strings: We accept a comma-delimited series of strings (alphanumeric characters). I've just applied the same whitespace pattern matching as the integer array, although these will be included in the typecast result (we could discard additional whitespace at the start / end of the strings we parse? Not sure how fancy we want to get here)

Feedback welcome! 🤗 I would like to get an initial version of this ship and iterate as necessary though without getting too invested in the details 😉

[dirceu]

LGTM! I think it makes sense to ship an initial version and iterate.

(There's a problem with tests though.)

[adrianna-chang-shopify]

@dirceu tests will pass after #413 is merged! (Should just need to require the ActiveModel libs in Task, but wanted to try and keep this PR as minimal as possible).

I'll rebase once we ship ☝️

[adrianna-chang-shopify]

cc @Shopify/rails @Shopify/core-stewardship

[adrianna-chang-shopify]

cc @etiennebarrie could I get your 👀 on this?

[etiennebarrie]

I understand we want to make it real easy to deal with array of ids, but I think we should try to keep true to the roles each part of the framework need to play (types vs validations, etc.).

There is something in ActiveModel:Type::Integer that raises but it's closer to correctness than validation: https://github.com/rails/rails/blob/v6.1.3.2/activemodel/lib/active_model/type/integer.rb#L49
The DB would raise otherwise anyway.

I think I need to think about it more, I'm still not entirely sure what the solution would look like.

[adrianna-chang-shopify]

@etiennebarrie I can get on board with your perspective 😄 My inclination was to treat input data with a lot of caution, being pretty strict about what was expected and raising if it didn't conform to those expectations. You're right about Active Record types though - they're super loose in terms of how they coerce data, and they mostly just resort to nil if the data can't be coerced properly.

Let's go with just performing the coercion for users, and returning nil if the input can't be parsed properly into the intended value type - it can be up to users to define further validations on their Tasks if they need more safeguards in place.

[adrianna-chang-shopify]

For now, I've opted to introduce the value types without any of the validations, and I've added a note in the README:

If you use one of these types for a parameter in your Task, your input will becoerced appropriately. You may still want write validations for your input, as type coercion is fairly lax. For example, if you input "xyz" for anIntegerArrayType parameter, it will be cast to [0].

We may want to offer custom validations eventually (as you'd suggested), but I'm not sure what makes the most sense right now and so I'd prefer to defer until we have some feedback from users on what sort of custom validators would bring value.

[adrianna-chang-shopify]

We've agreed to put this on pause until we have more info. I've opened #432 in the meantime to keep track of this feature.

[gmcgibbon]

@adrianna-chang-shopify can this be closed?

[adrianna-chang-shopify]

Closing, I think we've determined that there isn't enough of a demand for a feature like this, and we weren't quite happy with the API in this PR.