Skip to content

Fix: Mentioned users removed after editing message [CRNS - 493] - #1050

Merged
vishalnarkhede merged 2 commits into
v4.0.0-beta.2from
khushal87-crns-493
Dec 10, 2021
Merged

vishalnarkhede merged 2 commits into
v4.0.0-beta.2from
khushal87-crns-493

Conversation

@khushal87

@khushal87 khushal87 commented Dec 10, 2021 •

Copy link
Copy Markdown
Contributor

🎯 Goal

This PR fixes the empty mentioned users array issue when a message with a mentioned user is edited.
Fixes: #1023
Reference ticket: https://stream-io.atlassian.net/browse/CRNS-493

🛠 Implementation details

The solution is fixed in useEffect hook.

📹 Video

Screen.Recording.2021-12-10.at.13.24.42.mov

🎨 UI Changes

iOS
Before After
Android
Before After

🧪 Testing

☑️ Checklist

  • I have signed the Stream CLA (required)
  • PR targets the develop branch
  • Commits follow the conventional commits spec
  • New code is covered by tests
  • Screenshots added for visual changes
  • Documentation is updated

@khushal87
khushal87 changed the base branch from develop to v4.0.0-beta.2 December 10, 2021 08:43
@madsroskar

Copy link
Copy Markdown
Contributor

Thanks for adding the recording 🙌🏻 And nice catch! I'll drop a link to this PR in a card on our trello for when we get e2e tests set up again so we can consider this case there too :)

const mentionedUsers = message.mentioned_users.map((user) => user.id);
setMentionedUsers(mentionedUsers);
}
}, [message]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
}, [message]);
}, [messageValue]);

Will this work as well? Generally its better to avoid objects as hook dependency

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I will have to check. Give me some time. 🤞

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@vishalnarkhede Done, it works :)

@vishalnarkhede
vishalnarkhede merged commit 10efd85 into v4.0.0-beta.2 Dec 10, 2021
@vishalnarkhede
vishalnarkhede deleted the khushal87-crns-493 branch December 10, 2021 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Array mentioned_users empty after message edited

3 participants