Make options_for_select generic

[paulcsmith]

Closes #292

This works, the problem is some Fields are nilable, which would mean the
second item in the tuple would be nilable. That's probably not what the
user wants. On the other hand, it seems unlikely that a nil option
would end up there.

[paulcsmith]

Maybe the nil option could be handled like this:

field : LuckyRecord::AllowedField, select_options : Array(Tuple(String, Nil)

# Or if the AllowedField needs a type
field : LuckyRecord::AllowedField(Object?), select_options : Array(Tuple(String, Nil)

I think this might work. I'll try it out later since this PR is not a priority

[jwoertink]

Documenting this method, and realized that it's restrictive. This PR fixes it, so maybe we can revisit this? I think doing Tuple(String, AnyPKeyType) would be fine. Even if it's not a pkey type, String being in there would catch any other possible value.

[paulcsmith]

Thanks for brining this back up. Now that I look at it, I think having nil is a-ok. What if you want an option like "Disassociate user" or something. Then you should be able to do that by setting the value to nil. So I think if this is rebased it can be merged as-is!

Thoughts?

[jwoertink]

Yeah, that makes sense. Does this support a way to set the label like how rails has the include_blank: true option or the prompt: "Select your thing" option?

[paulcsmith]

No it doesn't so we'd have to document that.

I think what would be even better is a separate method rather than yet another option (Rails helpers have too many options IMO).

# If one is chosen then don't render anything, if not render the prompt
select_prompt op.country, "Select a country" 
select_options op.country, # blah blah blah

For including a blank item I'd add another tuple manually: {"-", nil}. Which is nice because it won't allow it if the field doesn't allow nil to be set.

Thoughts?

[jwoertink]

Totally agree on the option bloat. I love that idea of a separate method for adding a prompt option.

select_tag(op.opts) do
  select_prompt("Select an option")
  options_for_select(op.opts, my_options)
end

[paulcsmith]

Yeah that looks good! Only change is select_prompt needs the attribute so it can skip rendering if a value is already set. But maybe you can skip passing the attribute if you want it to always show up

[paulcsmith]

Ready for re-review. Could you create an issue for adding select_prompt?

[jwoertink]

Does this mean it always needs to be Tuple(String, String)? I'm just thinking of the example from a query

CarQuery.new.all.map do |car|
  {car.make, car.id.to_s}
end

[paulcsmith]

Nope! It means that it has to be whatever the type is on the attribute. In this case company_id is an attribute with the String type. I can change it to Int32 though so this makes more sense

[jwoertink]

Oh, ok. I like that.

[jwoertink]

Alright, this looks great to me!