Exports new DialogFieldAssociations data #15608
Conversation
|
@miq-bot assign @gmcculloug |
d-m-u
force-pushed
the
add_dialog_field_associations_to_serializer
branch
from
84e91b1 to
d8555ec
Compare
| @@ -12,9 +12,16 @@ def initialize(resource_action_serializer = ResourceActionSerializer.new) | |||
| def serialize(dialog_field, all_attributes = false) | |||
| serialized_resource_action = @resource_action_serializer.serialize(dialog_field.resource_action) | |||
|
|
|||
| trigger_associations = | |||
| { dialog_field.name.to_s => dialog_field.dialog_field_trigger_associations.map { |field| DialogField.find(field.id).name } } | |||
There was a problem hiding this comment.
Isn't dialog_field.name always a string? I do no think it needs the to_s. But more important, if this data is stored per-field is there a reason to even include the field name? Seems these could just be arrays. Same for respond_associations.
There was a problem hiding this comment.
That's what I thought, but your original plan had the name included.
|
@d-m-u This PR needs tests and also suggest adding some example data to the PR description. |
|
That's when I thought the associations would be stored at the dialog level, not per field. |
There was a problem hiding this comment.
Agree with @gmcculloug about the tests and about not needing the field name.
I also don't think we even need to export the respond associations, since they should get built by themselves once the trigger associations get created.
| @@ -12,9 +12,16 @@ def initialize(resource_action_serializer = ResourceActionSerializer.new) | |||
| def serialize(dialog_field, all_attributes = false) | |||
| serialized_resource_action = @resource_action_serializer.serialize(dialog_field.resource_action) | |||
|
|
|||
| trigger_associations = | |||
| { dialog_field.name.to_s => dialog_field.dialog_field_trigger_associations.map { |field| DialogField.find(field.id).name } } | |||
There was a problem hiding this comment.
I don't think we need to hit the database for the associated fields, you should just be able to call field.name
Ok so I was wrong here, but we should be doing field.dialog_field_trigger_id instead.
d-m-u
force-pushed
the
add_dialog_field_associations_to_serializer
branch
7 times, most recently
from
60ef08c to
c5058eb
Compare
| @@ -103,12 +106,24 @@ | |||
| 'resource_action' => 'serialized resource action', | |||
| )) | |||
| end | |||
|
|
|||
| context 'with associations' do | |||
| let(:dialog_field_responders) { [FactoryGirl.create(:dialog_field_button)] } | |||
There was a problem hiding this comment.
I think it's fine to use a dialog field here, but I think we should use .build instead of .create since I'm pretty sure we don't need these to be persisted in the database right now to export them.
Also, I have no idea what a dialog_field_button is, I think it's an old type of dialog_field that either doesn't exist anymore or at least isn't actually useable so we should at least pick a type we still use today like another text box or checkbox or something.
d-m-u
changed the title
d-m-u
force-pushed
the
add_dialog_field_associations_to_serializer
branch
from
c5058eb to
a3a710a
Compare
|
Checked commits d-m-u/manageiq@40ba409~...a3a710a with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
Allows us to include the relationship data set up between dialog fields for targeted auto refresh by the two associations (trigger and response) in the dialog export (here: corresponding main repo PR and necessary migration). The additional metadata that's introduced includes two fields: a "trigger_id" and "respond_id" added via the extra_attributes in the serializer with an array of field names in the associations.
Related to:
PR #15724
PR #15740
PR #15566 (merged)
ManageIQ/manageiq-schema#41 (merged)