Add annotations on container summary pages that contain labels #9214
Conversation
|
@GilbertCherrie While looking at this I noticed that Compliance is sort of buried on the left. For consistency, we should move it to the top-right. I'll open a separate issue for that, unless someone wants to tackle it beforehand. |
|
@liu-samuel In looking at your example, the annotations look identical to the custom attributes (or maybe a super-set?). Should we pull those out of custom attributes since they have a dedicated panel? The same really goes for labels too. cc @agrare |
| @@ -73,6 +73,13 @@ def textual_group_container_labels | |||
| TextualGroup.new(_("Labels"), textual_key_value_group(@record.labels.to_a)) | |||
| end | |||
|
|
|||
| def textual_group_annotations | |||
| if @record && @record.respond_to?(:custom_attributes) | |||
| annotations = @record.custom_attributes.select { |attribute| attribute.section == "annotations" } | |||
There was a problem hiding this comment.
We have methods at the model level for separating these out, so I think this separation belongs at the model level. See https://github.com/ManageIQ/manageiq/tree/master/app/models/container_group.rb#L20-L27
There was a problem hiding this comment.
And I think this is why custom_attributes is showing duplicate information. If you look at the textual_miq_custom_attributes method it looks at everything (the superset).
There was a problem hiding this comment.
In that case, should I just remove annotations and labels from the nodes summary page and just leave the custom attributes dropdown? I believe that was the only summary page that had custom attributes instead of annotations and labels separately.
There was a problem hiding this comment.
I was actually thinking the other way around - maybe we just remove custom attributes? Side q - maybe for @agrare, can we store custom attributes but for miq purposes? If so, then I would think we'd keep custom attributes but filter out the labels/node_selectors/annoatations
There was a problem hiding this comment.
Yeah custom_attributes are used for a bunch of purposes, ":source => "EVM" is for MIQ internal purposes
There was a problem hiding this comment.
Sure, I can just remove it from the UI so it only shows annotations and labels
liu-samuel
force-pushed
the
annotations-on-summary-page
branch
2 times, most recently
from
c7b5e50
to
c936664
Compare
liu-samuel
force-pushed
the
annotations-on-summary-page
branch
from
c936664
to
b02b935
Compare
|
Looks good but appears you picked up a merge commit pulling from master, recommend pull.rebase=true in your gitconfig (see https://www.manageiq.org/docs/guides/developer_setup/git_workflow) |
liu-samuel
force-pushed
the
annotations-on-summary-page
branch
from
b02b935
to
712d12f
Compare
I think I just made it worse, should I just create a new PR? |
liu-samuel
force-pushed
the
annotations-on-summary-page
branch
from
712d12f
to
131f1b1
Compare
Nevermind, fixed it |
|
Looks good now @liu-samuel |
|
Checked commit liu-samuel@131f1b1 with ruby 3.1.5, rubocop 1.56.3, haml-lint 0.51.0, and yamllint app/helpers/container_summary_helper.rb
|
Add annotations on container summary pages that contain labels, remove custom attributes tab from container nodes summary page
Resolves #9069
Relevant PRs:
Providers-Kubernetes: ManageIQ/manageiq-providers-kubernetes#530
Providers-Openshift: ManageIQ/manageiq-providers-openshift#262
Core ManageIQ: ManageIQ/manageiq#23074
Old:
New:
Cross-Repo Tests:
ManageIQ/manageiq-cross_repo-tests#879
All passing except for known failing test cases in core ManageIQ