Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mapping.tag is a category Tag, calling .category is ill-defined #3973

Merged
merged 2 commits into from
May 24, 2018

Conversation

cben
Copy link
Contributor

@cben cben commented May 22, 2018

There are category/entry Tags, and category/entry Classifications.
.category is supposed to get the "uncle", the category Classification from an entry Tag:

                                   .parent
 category Classification "cat"   <----------   entry Classification "ent"
           |^                ^                        |^
       .tag||.classification  \_______________    .tag||.classification
           v|                     .category   \       v|
 category Tag /managed/cat                     entry Tag /managed/cat/ent

mapping.tag is a category Tag (for any-value mappings which are the kind we use).
Currently .category on a category Tag ("/managed/cat") happens to work same as .classification, but it's ill-defined and will stop working after ManageIQ/manageiq#17418.

@miq-bot add-label technical debt
@kbrock @Fryguy please review

cben added 2 commits May 22, 2018 16:27
Currently .category on a category Tag ("/managed/cat") happens to
work same as .classification, but it's ill-defined and will
stop working after ManageIQ/manageiq#17418.

                                   .parent
 category Classification "cat"   <----------   entry Classification "ent"
           |^                ^                        |^
       .tag||.classification  \_______________    .tag||.classification
           v|                     .category   \       v|
 category Tag /managed/cat                     entry Tag /managed/cat/ent
It's an AR collection so should be plural.
@cben
Copy link
Contributor Author

cben commented May 22, 2018

Locally I also see failures in ./spec/services/privilege_checker_service_spec.rb which sound unrelated (?), those also fail without ManageIQ/manageiq#17418.

@miq-bot
Copy link
Member

miq-bot commented May 22, 2018

Checked commits cben/manageiq-ui-classic@eb8e89f~...0d3c824 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 👍

Copy link
Member

@kbrock kbrock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good change.

the mapping's classification is the tag's category

@martinpovolny martinpovolny self-assigned this May 24, 2018
@martinpovolny martinpovolny added this to the Sprint 87 Ending Jun 4, 2018 milestone May 24, 2018
@martinpovolny martinpovolny merged commit da1580e into ManageIQ:master May 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants