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

[FIX] reference discussion by its name in the popup #23710

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

Aman-Maheshwari
Copy link
Contributor

@Aman-Maheshwari Aman-Maheshwari commented Nov 13, 2021

  • I have read the Contributing Guide
  • I have signed the CLA - https://cla-assistant.io/RocketChat/Rocket.Chat
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • I have added necessary documentation (if applicable)
  • Any dependent changes have been merged and published in downstream modules

Proposed changes (including videos or screenshots)

In the popup, for discussion only the random discussion ID was visible, but now user can see the name of the discussion and also the name of the channel to which the discussion belongs to (in case of the same discussion name but different channel). Also using ID avoid the problem that could arise with the discussion having same name.

once send in the channel, corresponding discussion ID is converted to the link(as was before), but in the side the human readable name for the discussion is shown.

Before

before_discussion.mp4

After

after_discussion_fix.mp4

Issue(s)

#15369

Steps to test or reproduce

Mentioned in the issue

Further comments

@geekgonecrazy
Copy link
Member

IMO we have to figure out a better way to show the discussion in the message list too. Cause that is still super confusing if not

@paulopera
Copy link

paulopera commented Jul 29, 2022

It would be really nice if RoCket.Chat adopt this kind of approach to reference a discussion in a DM or Channel. My only concern is that IF I reference a discussion with a specific channel/person and that channel/person doesn't belong to the Parent channel or group could possibly create some "awkward situation" with all the people involved. Such as a new employe that doesn't have the clearance to access the discussion or someone that doesn't need to have access.
Some kind of check to verify if everyone in that particular Channel/DM has the rights to read the "to be referenced discussion" and then enable sending it.

Just after writing the text above I tested sending a url from the discussion to a person and if that person doesn't have the rights to access the discussion it shows a red alert on the screen.
So the App indeed make a check but I don't know how much it would impact in performance of the application validating it when trying to reference the discussion like described before.

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

Successfully merging this pull request may close these issues.

None yet

4 participants