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.

Sign up for GitHub

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

Jump to bottom

Check if class is taggable before attempting to process tag expression #18114

Merged
merged 1 commit into from
Oct 22, 2018
Merged

Check if class is taggable before attempting to process tag expression #18114

merged 1 commit into from
Oct 22, 2018

Conversation

gtanzillo
Copy link
Member

@gtanzillo gtanzillo commented Oct 19, 2018 •
edited
Loading

This eliminates an error being raised when a tag expression was used in RBAC and is
applied to a class that is not taggable

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1641012.

/cc @jrafanie @kbrock @lpichler

@gtanzillo gtanzillo force-pushed the fix-tag-expression-on-not-taggable-class branch from c2be27d to eaf4ad6 Compare October 19, 2018 20:19
jrafanie
jrafanie approved these changes Oct 19, 2018
View reviewed changes
Copy link
Member

@jrafanie jrafanie left a comment

Choose a reason for hiding this comment

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

LGTM, I'd like @kbrock's 👍

@gtanzillo
Check if class is taggable before attempting to process tag expression
ed5fbff 
This eliminates an error being raised when a tag expression was used in RBAC and is
applied to a class that is not taggable

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1641012
@gtanzillo gtanzillo force-pushed the fix-tag-expression-on-not-taggable-class branch from eaf4ad6 to ed5fbff Compare October 19, 2018 21:19
@miq-bot
Copy link
Member

miq-bot commented Oct 19, 2018

Checked commit gtanzillo@ed5fbff with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 👍

lpichler
lpichler approved these changes Oct 22, 2018
View reviewed changes
Copy link
Contributor

@lpichler lpichler left a comment

Choose a reason for hiding this comment

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

👍

kbrock
kbrock approved these changes Oct 22, 2018
View reviewed changes
Copy link
Member

@kbrock kbrock left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Good stuff.

My knee jerk is to convert the return nil to return klass.none but I think we should make as little change as possible.

(then again, my next reaction is to make the object taggable - but alas, not possible)

So yes, :shipit:

@jrafanie jrafanie self-assigned this Oct 22, 2018
@jrafanie jrafanie added this to the Sprint 97 Ending Oct 22, 2018 milestone Oct 22, 2018
@jrafanie jrafanie merged commit 6fa29c2 into ManageIQ:master Oct 22, 2018
@jrafanie
Copy link
Member

@gtanzillo I added gap and hammer, that should be ok, right?

@gtanzillo
Copy link
Member Author

@jrafanie 👍

simaishi pushed a commit that referenced this pull request Oct 22, 2018
@jrafanie @simaishi
Merge pull request #18114 from gtanzillo/fix-tag-expression-on-not-ta…
256263f 
…ggable-class

Check if class is taggable before attempting to process tag expression

(cherry picked from commit 6fa29c2)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1641810
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit 256263f4d31da72725f0aaa7362ac55f7c5b12a0
Author: Joe Rafaniello <[email protected]>
Date:   Mon Oct 22 11:26:24 2018 -0400

    Merge pull request #18114 from gtanzillo/fix-tag-expression-on-not-taggable-class
    
    Check if class is taggable before attempting to process tag expression
    
    (cherry picked from commit 6fa29c26a8dbb3c8935dfc4d521b6971073d3655)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1641810

simaishi pushed a commit that referenced this pull request Oct 22, 2018
@jrafanie @simaishi
Merge pull request #18114 from gtanzillo/fix-tag-expression-on-not-ta…
371a21f 
…ggable-class

Check if class is taggable before attempting to process tag expression

(cherry picked from commit 6fa29c2)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1641012
@simaishi
Copy link
Contributor

Hammer backport details:

$ git log -1
commit 371a21f17a26376ccd8e473b645f8988ed61c5f1
Author: Joe Rafaniello <[email protected]>
Date:   Mon Oct 22 11:26:24 2018 -0400

    Merge pull request #18114 from gtanzillo/fix-tag-expression-on-not-taggable-class
    
    Check if class is taggable before attempting to process tag expression
    
    (cherry picked from commit 6fa29c26a8dbb3c8935dfc4d521b6971073d3655)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1641012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lpichler lpichler lpichler approved these changes

@jrafanie jrafanie jrafanie approved these changes

@kbrock kbrock kbrock approved these changes

Assignees

@jrafanie jrafanie

Labels
blocker bug core/rbac gaprindashvili/backported hammer/backported
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@gtanzillo @miq-bot @jrafanie @simaishi @kbrock @lpichler