Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mentions resolve incorrectly if specified post ID is authored by a different user #2198

Closed
davwheat opened this issue Jun 15, 2020 · 12 comments

Comments

@davwheat
Copy link
Member

Bug Report

Current Behavior
With reply mentions (@username#id), if the ID doesn't match the correct username, the mention resolves to the post author instead of the specified username.

I can see why this could be "intentional" (or desired) but I think it's better to resolve to the username in the case of a bad ID.

For example, if someone clicked reply but accidentally removed a digit from the ID without noticing, the mention could ping off to a random person on the forum.

Example: https://discuss.flarum.org/d/23962-give-me-an-answer/6

In the example above, Pollux created post 133408, so the mention resolves to them.

Steps to Reproduce
2. Find a random post ID
3. Create a post with @anyusername#id
4. See that it resolves to the creator of the post, not the username

Expected Behavior
Debatable, but I think it should resolve to @anyusername. This makes it more likely to go to the right person if a mistake was made.

Environment

@dsevillamartin
Copy link
Member

dsevillamartin commented Jun 15, 2020

A bit related to https://github.com/flarum/core/issues/1734, perhaps.

With the post mentions, we'll probably want to do the same as my comment there. Now, I think the text that preceeds the post ID doesn't matter at all (even though it should).

Now, more specific to this issue: I think what we want to do here is error if the user associated with the post mention (@user#postid) does not exist - that way we can avoid unintended consequences by mentioning the wrong person or associating the wrong post as a reply.

I know this issue has been talked about before, but I couldn't find any issue on GitHub for it. Odd 🤔

@davwheat
Copy link
Member Author

davwheat commented Jun 15, 2020

I would have thought it had been mentioned before but after I couldn't find it, I decided to just roll with it in case :P

An error would be a good idea, but maybe confusing to non-technical users. Seeing "Invalid post/mention" would confuse some users into just not posting, which could be an issue for some forums.

@dsevillamartin
Copy link
Member

It would help having some kind of visual feedback when you are replying. Perhaps in the textarea or preview we could have it show as a red error or something 🤷

@davwheat
Copy link
Member Author

I think it would be better as a warning. Maybe highlight it red or something?

It should still let you post anyway as people will still get confused by that. Some people get frustrated at the "can't start thread" button...

Maybe this could be the start of post syntax validation for markdown (I wish).

@stale
Copy link

stale bot commented Sep 14, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We do this to keep the amount of open issues to a manageable minimum.
In any case, thanks for taking an interest in this software and contributing by opening the issue in the first place!

@stale stale bot added the stale Issues that have had over 90 days of inactivity label Sep 14, 2020
@davwheat
Copy link
Member Author

Not stale :)

@stale stale bot removed the stale Issues that have had over 90 days of inactivity label Sep 14, 2020
@stale
Copy link

stale bot commented Dec 13, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We do this to keep the amount of open issues to a manageable minimum.
In any case, thanks for taking an interest in this software and contributing by opening the issue in the first place!

@stale stale bot added the stale Issues that have had over 90 days of inactivity label Dec 13, 2020
@davwheat
Copy link
Member Author

Not stale :)

@stale stale bot removed the stale Issues that have had over 90 days of inactivity label Dec 13, 2020
@stale
Copy link

stale bot commented Mar 19, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We do this to keep the amount of open issues to a manageable minimum.
In any case, thanks for taking an interest in this software and contributing by opening the issue in the first place!

@stale stale bot added the stale Issues that have had over 90 days of inactivity label Mar 19, 2021
@SychO9 SychO9 removed the stale Issues that have had over 90 days of inactivity label Mar 20, 2021
@luceos
Copy link
Member

luceos commented Jun 8, 2021

This still applies to 1.0.2, see https://discuss.flarum.org/d/27629-test/3

image

However, after saving and then editing the username has been updated:

image

@davwheat
Copy link
Member Author

davwheat commented Jun 8, 2021

I opened this when I didn't understand the mentions system, but now I think it should be intended behaviour.

The only thing we have to base it off of is post ID, and nothing else. The display name is purely there to indicate to the user that they are, in fact, replying to the right user.

@SychO9
Copy link
Member

SychO9 commented Jun 22, 2021

This still applies to 1.0.2, see https://discuss.flarum.org/d/27629-test/3

However, after saving and then editing the username has been updated:

It's correct behaviour, the display name is only there for us to know who the author is, which is more readable, why would you write a wrong display name that's not the actual author ? the display name is always updated to the actual author's display name in case they update it, which also keeps everything consistent.

I opened this when I didn't understand the mentions system, but now I think it should be intended behaviour.

The only thing we have to base it off of is post ID, and nothing else. The display name is purely there to indicate to the user that they are, in fact, replying to the right user.

^this.

I would close this issue, I don't believe there is anything to fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants