Route capitalization error in private_messages_group

I am not very familiar with private group inboxes and I’m not sure how to reproduce this but the following is happening:

  • A private message inbox has been set up for a group, let’s call it ExampleGroup.
  • A member of that group gets a notification ‘there are 6 messages in your examplegroup inbox’
  • Clicking that notification tries to load /topics/private-messages-group/username/examplegroup.json
  • This gives an error

I think this is happening because there is no group called examplegroup. The group is called ExampleGroup.

ListController::generate_message_route does this:

group = Group.find_by(name: params[:group_name])

changing this to

group = Group.find_by(name: params[:group_name])
group = Group.where('lower(name) = ?', params[:group_name].downcase).first unless group

seems to resolve the issue. But I’m sure that is not the actual problem. Somewhere the parameter is converted to lower case where it should not be (I think…).

Does anyone have any suggestion where to look?

3 Likes

If you just create notifications directly in the console, the link respects capitalization (this works even if the data is fake)

Notification.create!(
  user_id: 1,
  notification_type: Notification.types[:group_message_summary],
  data: { inbox_count: 5, group_name: "MyGroup", group_id: 41, username: 'angus' }.to_json
)

The issue may on the creation side of things, i.e. in the post alerter: https://github.com/discourse/discourse/blob/master/app/services/post_alerter.rb#L252

However, I can’t seem to repro this one.

1 Like

Thanks for your help! But that’s not the correct link,

it’s not /u/username/messages/group/groupname
but /topics/private-messages-group/username/groupname.

I cannot find where the last one is coming from and how to generate it.

2 Likes

I believe the correct fix here is to not require capitalized group name in URL params. Done via:

Note that browsing to /u/username/messages/group/groupname will fire an AJAX request to /topics/private-messages-group/username/groupname, so what @angus did above to debug this issue does look correct.

Thanks for reporting this issue @RGJ :+1:

5 Likes