Skip to content

Commit

Permalink
FIX: Not send notifications when it should never notify assignment (#620
Browse files Browse the repository at this point in the history
)

* FIX: Not send notifications when it should never notify assignment

- Added silenced_assignments table and migration to store silenced assignments
- Added tests for silenced assignments

* DEV: Add annotation to model

* DEV: add empty line after early return

* DEV: lint file
  • Loading branch information
Grubba27 authored Dec 12, 2024
1 parent 215e1e3 commit 0f4a1fc
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 0 deletions.
2 changes: 2 additions & 0 deletions app/jobs/regular/assign_notification.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ module Jobs
class AssignNotification < ::Jobs::Base
def execute(args)
raise Discourse::InvalidParameters.new(:assignment_id) if args[:assignment_id].nil?
return if SilencedAssignment.exists?(assignment_id: args[:assignment_id])

Assignment.find(args[:assignment_id]).create_missing_notifications!
end
end
Expand Down
19 changes: 19 additions & 0 deletions app/models/silenced_assignment.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# frozen_string_literal: true

class SilencedAssignment < ActiveRecord::Base
belongs_to :assignment
end

# == Schema Information
#
# Table name: silenced_assignments
#
# id :bigint not null, primary key
# assignment_id :bigint not null
# created_at :datetime not null
# updated_at :datetime not null
#
# Indexes
#
# index_silenced_assignments_on_assignment_id (assignment_id) UNIQUE
#
14 changes: 14 additions & 0 deletions db/migrate/20241212113000_create_silenced_assignments.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# frozen_string_literal: true
class CreateSilencedAssignments < ActiveRecord::Migration[7.2]
def up
create_table :silenced_assignments do |t|
t.bigint :assignment_id, null: false
t.timestamps
end
add_index :silenced_assignments, :assignment_id, unique: true
end

def down
drop_table :silenced_assignments
end
end
4 changes: 4 additions & 0 deletions lib/assigner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,10 @@ def assign(

first_post.publish_change_to_clients!(:revised, reload_topic: true)
queue_notification(assignment) if should_notify

# This assignment should never be notified
SilencedAssignment.create!(assignment_id: assignment.id) if !should_notify

publish_assignment(assignment, assign_to, note, status)

if assignment.assigned_to_user?
Expand Down
6 changes: 6 additions & 0 deletions spec/jobs/regular/assign_notification_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@
assignment.expects(:create_missing_notifications!)
execute_job
end

it "does not create notifications if assignment is silenced" do
SilencedAssignment.stubs(:exists?).with(assignment_id: assignment_id).returns(true)
assignment.expects(:create_missing_notifications!).never
execute_job
end
end
end
end
4 changes: 4 additions & 0 deletions spec/lib/assigner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -794,6 +794,10 @@ def assigned_to?(assignee)
assigner.assign(group, should_notify: false)
expect(topic.allowed_groups).to include(group)
expect(Notification.count).to eq(0)
expect(SilencedAssignment.count).to eq(1)

group.add(Fabricate(:user))
expect(Notification.count).to eq(0) # no one is ever notified about this assignment
end

it "doesn't invite group if all members have access to the PM already" do
Expand Down
3 changes: 3 additions & 0 deletions spec/requests/assign_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ def assign_user_to_post
it "notifies the assignee when the topic is assigned to a group" do
admins = Group[:admins]
admins.messageable_level = Group::ALIAS_LEVELS[:everyone]
admins.assignable_level = Group::ALIAS_LEVELS[:everyone]
admins.save!

SiteSetting.invite_on_assign = true
Expand Down Expand Up @@ -290,6 +291,7 @@ def assign_user_to_post
it "does not notify the assignee when the topic is assigned to a group if should_notify option is set to false" do
admins = Group[:admins]
admins.messageable_level = Group::ALIAS_LEVELS[:everyone]
admins.assignable_level = Group::ALIAS_LEVELS[:everyone]
admins.save!

SiteSetting.invite_on_assign = true
Expand All @@ -313,6 +315,7 @@ def assign_user_to_post
should_notify: false,
}
expect(Notification.count).to eq(0)
expect(SilencedAssignment.count).to eq(1)
end

it "fails with a specific error message if the topic is not a PM and the assignee can not see it" do
Expand Down

0 comments on commit 0f4a1fc

Please sign in to comment.