[WIP] Display mapped classifications and tags in tag assignments #4723
Conversation
|
@miq-bot assign @mzazrivec |
|
Checked commit lpichler@a1e5671 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
lpichler
changed the title
|
This pull request is not mergeable. Please rebase and repush. |
| @categories = {} # Classifications array for first chooser | ||
|
|
||
| mapped_categories = cats.select { |c| ContainerLabelTagMapping::TAG_PREFIXES.any? { |pattern| c.tag.name.include?(pattern) } } |
There was a problem hiding this comment.
I'd like TAG_PREFIXES to remain an implementation detail internal to the model.
I did this kludge after the "correct" join, through parent category, to ContainerLabelTagMapping proved too complicated to debug, but in any case my hope was to encapsulate that in ContainerLabelTagMapping.controls_tag? and Tag.controlled_by_mapping scope.
If these are not sufficient, perhaps we need more abstraction eg. adding Classification.controlled_by_mapping but let's not leak TAG_PREFIXES...
| cats.delete_if { |c| c.read_only? || c.entries.length == 0 } # Remove categories that are read only or have no entries | ||
| cats += mapped_categories |
There was a problem hiding this comment.
Could we instead drop read_only? check above?
AFAIK, checking read_only? is the only reason mapped tags don't appear in several places in UI, including here.
However, I think this was deliberate UX here:
Mapped tags are read_only to reflect the fact manually un/assigning them is meaningless — they're "owned" by the mapping mechanism and will be reset in next refresh.
AFAICT, being impossible to manually un/assign is the meaning of read_only field, and I strongly suspect mapped tags are the only use of read_only.
(well ideally they should not have been omitted but greyed out)
|
Apologies for drive-by review — I briefly saw ManageIQ/manageiq#17971, the BZ and these PRs, but I do NOT yet understand the cross-region nuances... Do ping me if you're actively working on this and want any help / better review / moral support (I accept all blame for the horrors of tag mapping 😢) |
|
@cben thank you very much for review I appreciated it but it is not needed it, I will back to these PRs when we will needed it. closing |
See ManageIQ/manageiq#18046 for more info.
depends on:
Links
#18015 (first part of 3)
[WIP] Support mapped tags manageiq#18046 is second part of 3