Skip to content

Commit

Permalink
FIX: Do not downcase group name in notification payload
Browse files Browse the repository at this point in the history
Followup to 3d39b4b
  • Loading branch information
pmusaraj committed Sep 21, 2020
1 parent 577293c commit f7ca93b
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 12 deletions.
2 changes: 1 addition & 1 deletion app/services/post_alerter.rb
Expand Up @@ -223,7 +223,7 @@ def group_stats(topic)
topic.allowed_groups.map do |g|
{
group_id: g.id,
group_name: g.name.downcase,
group_name: g.name,
inbox_count: DB.query_single(sql, group_id: g.id).first.to_i
}
end
Expand Down
39 changes: 28 additions & 11 deletions spec/services/post_alerter_spec.rb
Expand Up @@ -71,19 +71,36 @@ def create_post_with_alerts(args = {})

end

it "triggers :before_create_notifications_for_users" do
pm = Fabricate(:topic, archetype: 'private_message', category_id: nil)
op = Fabricate(:post, user: pm.user, topic: pm)
user1 = Fabricate(:user)
user2 = Fabricate(:user)
group = Fabricate(:group, users: [user2])
pm.allowed_users << user1
pm.allowed_groups << group
events = DiscourseEvent.track_events do
context "group inboxes" do
fab!(:user1) { Fabricate(:user) }
fab!(:user2) { Fabricate(:user) }
fab!(:group) { Fabricate(:group, users: [user2], name: "TestGroup") }
fab!(:pm) { Fabricate(:topic, archetype: 'private_message', category_id: nil, allowed_groups: [group]) }
fab!(:op) { Fabricate(:post, user: pm.user, topic: pm) }

it "triggers :before_create_notifications_for_users" do
pm.allowed_users << user1
events = DiscourseEvent.track_events do
PostAlerter.post_created(op)
end

expect(events).to include(event_name: :before_create_notifications_for_users, params: [[user1], op])
expect(events).to include(event_name: :before_create_notifications_for_users, params: [[user2], op])

end

it "triggers group summary notification" do
TopicUser.change(user2.id, pm.id, notification_level: TopicUser.notification_levels[:tracking])

PostAlerter.post_created(op)
group_summary_notification = Notification.where(user_id: user2.id)

expect(group_summary_notification.count).to eq(1)
expect(group_summary_notification.first.notification_type).to eq(Notification.types[:group_message_summary])

notification_payload = JSON.parse(group_summary_notification.first.data)
expect(notification_payload["group_name"]).to eq(group.name)
end
expect(events).to include(event_name: :before_create_notifications_for_users, params: [[user1], op])
expect(events).to include(event_name: :before_create_notifications_for_users, params: [[user2], op])
end
end

Expand Down

2 comments on commit f7ca93b

@discoursereviewbot
Copy link

Choose a reason for hiding this comment

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

Régis Hanol posted:

Is this just a UI change or was something broken?

@discoursereviewbot
Copy link

Choose a reason for hiding this comment

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

Penar Musaraj posted:

The group summary notification link for groups with uppercase characters in their name was broken.

(This is a recent regression, I think introduced by: 0398271)

Please sign in to comment.