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

Fix Notifications None #18035

Merged
merged 1 commit into from
Sep 28, 2018
Merged

Fix Notifications None #18035

merged 1 commit into from
Sep 28, 2018

Conversation

kbrock
Copy link
Member

@kbrock kbrock commented Sep 28, 2018

The notification type "NONE" (AUDIENCE_NONE) was introduced in #17673 , but it was not part of basic validations

This was causing the RFE to fail:
https://bugzilla.redhat.com/show_bug.cgi?id=1535177


describe "#enabled?" do
it "detects properly" do
expect(FactoryGirl.create(:notification_type, :audience => NotificationType::AUDIENCE_USER)).to be_enabled
Copy link
Member

Choose a reason for hiding this comment

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

Does these tests need to create?
How is enabled? related? Should it instead test instance.valid??
If not, should the enabled? tests test against all of the audiences?

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess I was testing the side effect of save.
not very straight forward

fixes ebd613a

The notification type "NONE" was introduced, but it was not part of
basic validations

https://bugzilla.redhat.com/show_bug.cgi?id=1535177
@miq-bot
Copy link
Member

miq-bot commented Sep 28, 2018

Checked commit kbrock@acd0a19 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. 🍰

@bdunne bdunne merged commit 8489078 into ManageIQ:master Sep 28, 2018
@bdunne bdunne self-assigned this Sep 28, 2018
@bdunne bdunne added this to the Sprint 96 Ending Oct 8, 2018 milestone Sep 28, 2018
@kbrock kbrock deleted the monitor_notifier_fix branch September 28, 2018 22:35
simaishi pushed a commit that referenced this pull request Oct 1, 2018
@simaishi
Copy link
Contributor

simaishi commented Oct 1, 2018

Hammer backport details:

$ git log -1
commit 33aa42d74dafb5cc00206c4af28dfef3ca079ace
Author: Brandon Dunne <brandondunne@hotmail.com>
Date:   Fri Sep 28 18:26:33 2018 -0400

    Merge pull request #18035 from kbrock/monitor_notifier_fix
    
    Fix Notifications None
    
    (cherry picked from commit 8489078c17dcc236541e3c3cebdf20bb2e6291c1)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1535177

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