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.

Sign up for GitHub

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

Jump to bottom

fix(chat-features): Add messageId and change id to participantId #14945

Merged
merged 13 commits into from
Aug 6, 2024
Merged

fix(chat-features): Add messageId and change id to participantId #14945

merged 13 commits into from
Aug 6, 2024

Conversation

he-patrick
Copy link
Contributor

@he-patrick he-patrick commented Jul 29, 2024 •
edited by saghul
Loading

  • messageId needs to be received so that reactions can be attached to specific messages
  • participantId indicates that it is the participant's id rather than the message's id

Depends on: jitsi/lib-jitsi-meet#2548

@jitsi-jenkins
Copy link

Hi, thanks for your contribution!
If you haven't already done so, could you please make sure you sign our CLA (https://jitsi.org/icla for individuals and https://jitsi.org/ccla for corporations)? We would unfortunately be unable to merge your patch unless we have that piece :(.

saghul
saghul previously approved these changes Jul 30, 2024
View reviewed changes
Copy link
Member

@saghul saghul left a comment

Choose a reason for hiding this comment

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

LGTM, we need to land the LJM change first though. @damencho can you PTAL too?

@mihhu mihhu added the gsoc Appropriate for GSoC students to tackle label Jul 30, 2024
@saghul
Copy link
Member

saghul commented Jul 31, 2024

Code looks good, can you please fix the linting errors?

@he-patrick
Copy link
Contributor Author

Fixed.

@he-patrick
Copy link
Contributor Author

Ah I see there's a couple more — working on them now.

@he-patrick
Copy link
Contributor Author

Let me know if that works 🙂

@saghul
Copy link
Member

saghul commented Aug 2, 2024

Jenkins please test this please.

@damencho
Copy link
Member

damencho commented Aug 6, 2024

Nope, it is related

@damencho
Copy link
Member

damencho commented Aug 6, 2024

IFrameAPICommandsTest. testCommandSendChatMessage

@damencho
Copy link
Member

damencho commented Aug 6, 2024

We should figure it out what is the problem as it may break all tests, till now 2 out of 2 are failing.

saghul
saghul requested changes Aug 6, 2024
View reviewed changes
react/features/chat/middleware.ts Outdated Show resolved Hide resolved
saghul
saghul reviewed Aug 6, 2024
View reviewed changes
modules/API/API.js Outdated
@@ -1814,9 +1814,9 @@ class API {
* Notify external application of a participant, remote or local, being
* removed from the conference by another participant.
*
* @param {string} kicked - The ID of the participant removed from the
Copy link
Member

Choose a reason for hiding this comment

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

This looks like a merge conflict.

@saghul
Copy link
Member

saghul commented Aug 6, 2024

Jenkins please test this please.

@saghul saghul merged commit 8bfa659 into jitsi:master Aug 6, 2024
9 checks passed
@saghul
Copy link
Member

saghul commented Aug 6, 2024

🚀 🚀 🚀 🚀

@he-patrick he-patrick deleted the add-message-id branch August 13, 2024 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@saghul saghul saghul approved these changes

Assignees
No one assigned
Labels
gsoc Appropriate for GSoC students to tackle
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@he-patrick @jitsi-jenkins @saghul @damencho @mihhu