Bug when composing topic while viewing PM - incorrect "mentioned user will not be notified" warning


(Joshua Rosenfeld) #1

Had a feature request related to a closed topic. Flagged it to reopen. Asked to instead make a new topic. Typed c to open composer and started typing. After mentioning a non-admin user, I was warned that they would not be notified because it was a personal message. However, I was writing a public topic, not a PM. I was simply looking at a PM.

I think the logic from this PR is not working for some reason. Is it somehow checking the current view instead of the composer type @cpradio?

Screenshots:

Edit: just happened for this topic as well.

Repro:

  1. Navigate to Messages.
  2. Open a message.
  3. Type c to open new topic composer.
  4. Mention a non-admin user not a participant in message that was opened in step 2.

(cpradio) #2

This actually isn’t supposed to work at all…

From a PM:

Right now there are three scenarios that behave differently with mentions in regards to informing the user they do not have access.

  1. Reply to a Topic/PM
  • The same workflow can work for topic and PM, in that, you check to see if the mention user can see the already existing topic/PM
  1. Create a new Topic
  • This isn’t the same workflow as above, as the topic doesn’t exist. So you have to check if the user can see the category the topic will be posted to. But what if the user changes the category? It should probably check again…
  1. Create a PM
  • This isn’t the same as either workflow above, as obviously this is dependent on the Recipient list, not a category or existing topic/PM. If the user isn’t in the recipient list (or part of a group in the recipient list), then they can’t see the PM.

So maybe we can redefine the scope a bit to make this a lot less complex? Which of these three are the most important? Or maybe I should post this in the feature discussion topic to find out if there is a better way to do this? Maybe a new notification that is send to the creator of the reply/topic/PM that says “hey, you mentioned X, Y, and Z and they can’t see your post” which would get sent after the post/topic/PM is posted, thus same workflow for all three scenarios.


It was decided to not worry about items #2 and #3… so I’m sort of surprised to see the dialog appear.


(Jeff Atwood) #3

Just make sure the check does not fire for new topics.


(cpradio) #4

Okay, I’m close to done with this fix, but the message is still considering it a PM, need to look at that part still and will then submit a PR.
https://github.com/discourse/discourse/compare/master...cpradio:cannot-see-mention?expand=1


(cpradio) #5

PR Submitted:


(Alan Tan) #6

Merged :slight_smile: Thank you for fixing the bug @cpradio :thumbsup:


(Jeff Atwood) #7