Fixing issue on gen.resource.browser with some column types

[jwoertink]

Purpose

Fixes #995

Description

Prior to this, the gen.resource.browser task would fail if you tried to generate a JSON::Any column or any of the Array column types. We were just doing a simple split on : which didn't really work for JSON::Any.

Checklist

• - An issue already exists detailing the issue/or feature request that this PR fixes
• - All specs are formatted with crystal tool format spec src
• - Inline documentation has been added and/or updated
• - Lucky builds on docker with ./script/setup
• - All builds and specs pass on docker with ./script/test

[paulcsmith]

Thank you! This has definitely tripped people up!

I’d like to suggest a change to the approach though. The generators don’t really support json any or array types and I can’t think of a good way to do handle inputs and show those types. I think it would be ok for people to add those columns themselves since we can’t really do it for them reliably.

So I think we can have an array of “supported types” such as String, Time, Ints. Then we check that they type is in the supported types and if not raise an error letting them know what the supported types are

I think this would also make the test simpler since you could pass a non supported type and check for a printed error.

I also think you could still use split since JSON::Any isn’t supported anyway. Thoughts?

[jwoertink]

I'm down for that too. I forgot about creating the inputs. I guess we can say that the generators are only meant for a basic "getting started" point, and any complex thing needs to be done manually. We should probably do up a whole deal on the guides on generators and mention that.

Ok, so I'll change this up to check specifically for the simple types that we support, and then any others will raise an error message. Do you think the error message should say something like "If you need to use JSON or Arrays, you will need to add those in manually"?

[paulcsmith]

Yeah that’s a good idea. Maybe more generic though? Like “other complex types can be added manually” and maybe a link to a guide if you can think of a good one!

…

On Jan 6, 2020, at 9:33 AM, Jeremy Woertink ***@***.***> wrote:  I'm down for that too. I forgot about creating the inputs. I guess we can say that the generators are only meant for a basic "getting started" point, and any complex thing needs to be done manually. We should probably do up a whole deal on the guides on generators and mention that. Ok, so I'll change this up to check specifically for the simple types that we support, and then any others will raise an error message. Do you think the error message should say something like "If you need to use JSON or Arrays, you will need to add those in manually"? — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

[paulcsmith]

Also I think split takes a second argument that’s an int. So if you pass 2 it will stop splitting after the first colon. That’s how it is in ruby and hopefully in crystal too 😆

[paulcsmith]

I'm going to merge this as-is because it is a HUGE improvement. If you have time though could you add a followup PR that adjust the wording from valid/invalid to supported/unsupported?

• VALID_TYPES -> SUPPORTED_TYPES
• Must provide support columns for the resource

and changing method names like `validate_has_supported_columns. Thoughts?