Fix ae engine tags #555
Fix ae engine tags #555
Conversation
| @@ -320,7 +327,13 @@ def self.create_ae_attrs(attrs, name, vmdb_object, objects = [MiqServer.my_serve | |||
| array_objects.each do |array_object| | |||
| # Each array attribute is tagged with Array:: before the attribute key unless it already starts with Array:: | |||
| array_attr_key = array_object | |||
| if !array_object.starts_with?("Array::") | |||
| if !ae_attrs[array_object].empty? && ae_attrs[array_object].first.split("::").first == "Classification" | |||
There was a problem hiding this comment.
New line 326 hurts my brain but let's not change that right now...
326 guarantees this first conditional is an array, so the empty check is fine, but, the next one is a bit concerning.
Maybe something like this:
if !ae_attrs[array_object].empty? && ae_attrs[array_object].first.to_s.split("::").first == "Classification"if ae_attrs[array_object].first is somehow not a string, such as nil, we'll still be able to split and call first and this conditional will be false as desired.
| expect(MiqAeEngine.create_automation_attributes_string(hash)).to eq(result_str) | ||
| expect(result["tags"]).to eq(result_arr) | ||
| end | ||
|
|
There was a problem hiding this comment.
@GilbertCherrie Did we completely revert the test changes so it's testing as it was before? This test looks new but others are changes. Basically, it's hard to determine the changes we're doing on code pre #545. I can't tell if you did a full revert of the prior changes in the repos and then made changes on top or we're keeping some of the changes from #545 and just fixing it to work with the original tests.
GilbertCherrie
force-pushed
the
fix_ae_engine_tags
branch
from
999141d to
8d0cf45
Compare
| @@ -329,6 +337,9 @@ def self.create_ae_attrs(attrs, name, vmdb_object, objects = [MiqServer.my_serve | |||
| if !array_object.starts_with?("Array::") | |||
| ae_attrs.delete(array_object) | |||
| end | |||
| if !array_object.starts_with?("TagArray::") && array_attr_key.starts_with?("TagArray::") | |||
| ae_attrs.delete(array_object) | |||
| end | |||
There was a problem hiding this comment.
I'm not sure what's being deleted here. It looks like it's removing non-array objects. If we need to do all of those conditions, you can combine them:
if !array_object.starts_with?("Array::") || (!array_object.starts_with?("TagArray::") && array_attr_key.starts_with?("TagArray::"))
ae_attrs.delete(array_object)
end
GilbertCherrie
force-pushed
the
fix_ae_engine_tags
branch
3 times, most recently
from
1c8bcb8 to
debff0e
Compare
GilbertCherrie
force-pushed
the
fix_ae_engine_tags
branch
from
c8a4cb2 to
0b8f603
Compare
|
Checked commit GilbertCherrie@0b8f603 with ruby 3.1.5, rubocop 1.56.3, haml-lint 0.51.0, and yamllint |
|
This pull request is not mergeable. Please rebase and repush. |
Depends on: ManageIQ/manageiq-ui-classic#9250 and ManageIQ/manageiq#23155
Fixed ae engine tag field values after this pr changed how they work: #545.
Also, updated and added new cases to ensure this tag functionality is not accidentally broken in the future.