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(messagesStore) - Mentions in forwarded messages #10640

Merged
merged 2 commits into from
Oct 9, 2023

Conversation

DorraJaouad
Copy link
Contributor

☑️ Resolves

🖌️ UI Checklist

🖼️ Screenshots / Screencasts

Before 😞 🏚️ After: The actual message 🏡 After : The forwarded message
image image image

🚧 Tasks

  • visual review
  • code review

🏁 Checklist

@DorraJaouad DorraJaouad added bug feature: chat 💬 Chat and system messages feature: api 🛠️ OCS API for conversations, chats and participants labels Oct 5, 2023
@DorraJaouad DorraJaouad added this to the 💙 Next Major (28) milestone Oct 5, 2023
@DorraJaouad DorraJaouad self-assigned this Oct 5, 2023
Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

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

Tested with displayName.contains(id), displayName.contains(' '), displayName === id, and @all

image

Person will be notified with the forwarded message again, but should be expected (silent: false) and intended (if user forwards some tasks to internal chat, for example)

@@ -1306,7 +1306,8 @@ const actions = {
for (const key in message.messageParameters) {
if (key.startsWith('mention')) {
const mention = message.messageParameters[key]
message.message = message.message.replace(`{${key}}`, `@${mention.name}`)
const mentionString = key.includes('mention-call') ? `**${mention.name}**` : `@"${mention.id}"`
Copy link
Member

Choose a reason for hiding this comment

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

Not sure it causes issues somewhere, but usually we only quote when a space or slash is contained in the id

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It also works for ids that do not contain spaces of slashes, maybe a specific use case I am not aware of?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Would be safe to copypaste that method with reference to the upstream component

@DorraJaouad DorraJaouad enabled auto-merge October 6, 2023 12:47
…ntion to bold display name.

Signed-off-by: DorraJaouad <dorra.jaoued7@gmail.com>
Signed-off-by: DorraJaouad <dorra.jaoued7@gmail.com>
@DorraJaouad DorraJaouad force-pushed the fix/10631/fix-forwarded-mentions branch from c0a693f to edc7878 Compare October 9, 2023 21:17
@DorraJaouad DorraJaouad merged commit 1a3d347 into master Oct 9, 2023
26 checks passed
@DorraJaouad DorraJaouad deleted the fix/10631/fix-forwarded-mentions branch October 9, 2023 21:43
@nickvergessen
Copy link
Member

nickvergessen commented Oct 10, 2023

Backport to 27, now that we know it's only 2 lines of production code?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug feature: api 🛠️ OCS API for conversations, chats and participants feature: chat 💬 Chat and system messages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RFC: Mentions in forwarded messages
3 participants