Bug #20653

UI Notifications not delivered for hosts owned by usergroup

Added by Timo Goebel 5 months ago. Updated 4 months ago.

Status:Closed
Priority:Normal
Assigned To:Timo Goebel
Category:Notifications
Target version:-
Difficulty: Bugzilla link:
Found in release:1.15.2 Pull request:https://github.com/theforeman/foreman/pull/4758
Story points-
Velocity based estimate-
Release1.16.0Release relationshipAuto

Description

When creating a ui notification where the host is the subject and the host is owned by a Usergroup, this currently fails:

subject = host
audience is set to AUDIENCE_GROUP by app/services/ui_notifications/hosts.rb
When building the recipient_ids:
For a user: initiator.id is used
For a usergroup: subject.all_users is used, but not defined as subject is host.

I've written a test to demonstrate the behavior:

1) Error:
UINotificationsHostsTest::deliver notification to host owner#test_0002_owner is usergroup:
NoMethodError: undefined method `all_users' for #<Host::Managed:0x007f89bdbc91e0>
app/models/notification.rb:49:in `subscriber_ids'
app/models/notification.rb:65:in `set_notification_recipients'
test_after_commit (1.1.0) lib/test_after_commit/database_statements.rb:11:in `block in transaction'
test_after_commit (1.1.0) lib/test_after_commit/database_statements.rb:5:in `transaction'
test/unit/ui_notifications/hosts/base_test.rb:7:in `create'
app/services/ui_notifications.rb:26:in `deliver!'
app/services/ui_notifications/hosts.rb:11:in `deliver!'
test/unit/ui_notifications/hosts/base_test.rb:53:in `block (3 levels) in <class:UINotificationsHostsTest>'
test/unit/ui_notifications/hosts/base_test.rb:52:in `block (2 levels) in <class:UINotificationsHostsTest>'

Associated revisions

Revision 2d2390b5
Added by Timo Goebel 4 months ago

fixes #20653 - ui notifications for hosts with usergroup owner

Revision d51d3c9b
Added by Timo Goebel 4 months ago

fixes #20653 - ui notifications for hosts with usergroup owner

History

#1 Updated by The Foreman Bot 5 months ago

  • Status changed from New to Ready For Testing
  • Pull request https://github.com/theforeman/foreman/pull/4758 added

#2 Updated by Timo Goebel 5 months ago

In a production environment, this error is only logged:

2017-08-22 09:43:26 16b83fb6 [notifications] [W] Failed to handle notifications - this is most likely a bug: undefined method `all_users' for nil:NilClass

Maybe we should increase the log level to error.

#3 Updated by Timo Goebel 4 months ago

  • Status changed from Ready For Testing to Closed
  • % Done changed from 0 to 100

#4 Updated by Marek Hulán 4 months ago

  • Release set to 1.17.0

#5 Updated by Timo Goebel 4 months ago

  • Release changed from 1.17.0 to 1.16.0

This was cherry-picked to 1.16.0.

#6 Updated by Marek Hulán 4 months ago

Thanks Timo!

Also available in: Atom PDF