Skip to content

Commit

Permalink
allow apps to not include the user in a notification
Browse files Browse the repository at this point in the history
While some may find the current OS user helpful, in other contexts, that user is randomly assigned and not material to the notification itself. Consider a deployment enviroment like Heroku, which will use a different OS user for each deployable, and each deploy.

It may also cause grouping issues in services like Sentry, which use the exception / notification content to determine whether the situation has occurred already.

This does not change the default behavior, but rather introduces a new option to skip including that information in notifications, for those who want to.
  • Loading branch information
agrobbin committed Jan 15, 2024
1 parent eff5b01 commit feb8ed1
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 3 deletions.
1 change: 1 addition & 0 deletions README.md
Expand Up @@ -102,6 +102,7 @@ The code above will enable all of the Bullet notification systems:
* `Bullet.slack`: add notifications to slack
* `Bullet.raise`: raise errors, useful for making your specs fail unless they have optimized queries
* `Bullet.always_append_html_body`: always append the html body even if no notifications are present. Note: `console` or `add_footer` must also be true. Useful for Single Page Applications where the initial page load might not have any notifications present.
* `Bullet.skip_user_in_notification`: exclude the OS user (`whoami`) from notifications.


Bullet also allows you to disable any of its detectors.
Expand Down
2 changes: 1 addition & 1 deletion lib/bullet.rb
Expand Up @@ -40,7 +40,7 @@ class << self
:stacktrace_excludes,
:skip_html_injection
attr_reader :safelist
attr_accessor :add_footer, :orm_patches_applied, :skip_http_headers, :always_append_html_body
attr_accessor :add_footer, :orm_patches_applied, :skip_http_headers, :always_append_html_body, :skip_user_in_notification

available_notifiers =
UniformNotifier::AVAILABLE_NOTIFIERS.select { |notifier| notifier != :raise }
Expand Down
15 changes: 13 additions & 2 deletions lib/bullet/notification/base.rb
Expand Up @@ -51,11 +51,22 @@ def notify_out_of_channel
end

def short_notice
[whoami.presence, url, title, body].compact.join(' ')
parts = []
parts << whoami.presence unless Bullet.skip_user_in_notification
parts << url
parts << title
parts << body

parts.compact.join(' ')
end

def notification_data
{ user: whoami, url: url, title: title, body: body_with_caller }
hash = {}
hash[:user] = whoami unless Bullet.skip_user_in_notification
hash[:url] = url
hash[:title] = title
hash[:body] = body_with_caller
hash
end

def eql?(other)
Expand Down
12 changes: 12 additions & 0 deletions spec/bullet/notification/base_spec.rb
Expand Up @@ -70,6 +70,18 @@ def temp_env_variable(name, value)
allow(subject).to receive(:body_with_caller).and_return('body_with_caller')
expect(subject.notification_data).to eq(user: 'whoami', url: 'url', title: 'title', body: 'body_with_caller')
end

context 'when skip_user_in_notification is true' do
before { allow(Bullet).to receive(:skip_user_in_notification).and_return(true) }

it 'should return notification data without user' do
allow(subject).to receive(:url).and_return('url')
allow(subject).to receive(:title).and_return('title')
allow(subject).to receive(:body_with_caller).and_return('body_with_caller')

expect(subject.notification_data).to eq(url: 'url', title: 'title', body: 'body_with_caller')
end
end
end

context '#notify_inline' do
Expand Down

0 comments on commit feb8ed1

Please sign in to comment.