Ability to filter webhooks by Creative Concept #14
Conversation
ps-ruby
changed the title
|
@andrewculver Do I have to also update bullet_train with new migration file? Where do we write a test for this? I saw a test case that triggers these codes in bullet_train. Should I update the test out there or add new test cases in this repo? |
| @@ -4,6 +4,8 @@ | |||
| <% with_field_settings form: form do %> | |||
| <%= render 'shared/fields/text_field', method: :name, options: {autofocus: true} %> | |||
| <%= render 'shared/fields/text_field', method: :url %> | |||
| <%= render 'shared/fields/super_select', method: :scaffolding_absolutely_abstract_creative_concept_id, | |||
| choices: [['None', '']] + @endpoint.creative_concepts.map { |event_type| [event_type.name, event_type.id] }, other_options: {search: true} %> | |||
There was a problem hiding this comment.
Can we replace [['None', '']] + with allow_blank: true? Does that work with super_select? /cc @gazayas @pascallaliberte
There was a problem hiding this comment.
@andrewculver It looks like we can remove [['None', '']] and just use the allowClear option along with the placeholder option Select2 has available.
You can add the options in the stimulus controller after the options hash here (I ended up adding it as options.allowClear = true; and it worked fine), and this will pass an empty String to the params in the controller, just like with [['None'], ['']]
Concerning the Creative Concept's validation though, allow_blank: true isn't working for now because the empty string has no ID, just resulting in a nil record. For that reason I think the optional: true line that was added in this PR in the EndpointSupport module might be a viable option. That is if we're ok with the Creative Concept being saved to the database as nil.
There was a problem hiding this comment.
@ps-ruby If adding allow_blank: true doesn't fix the problem, then it's a bug IMO and I need to get that fixed in bullet_train-fields.
|
Great work! Love how you solved this. Left some feedback. Let's get this PR merged, and then we'll open a new issue and move onto the next step of making it easier for downstream developers to add filters for other model types. |
#13