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(office): fixes user invitation to external meeting opening jitsi instead #331

Merged

Conversation

phiter
Copy link
Contributor

@phiter phiter commented May 24, 2020

Description

This fixes a bug where when inviting users to join an external meeting, a jitsi room was created instead.

closes #222

How to test?

  1. Open matrix in two different accounts
  2. Join an external meeting in one account
  3. Invite the other user to join the same meeting
  4. Accept the invitation as the other user

Expected behavior

The user should join the external meeting and no jitsi call should be created for that room.

@phiter
Copy link
Contributor Author

phiter commented May 24, 2020

I decided to take the shortest path to fix this one since it is something my company needs to use. I extracted the onEnterRoom function and exported it, then imported it to be used where the accept invitation event happens.

I think that function should be extracted to another file instead of OfficePage.js but I don't know where exactly, so please review it and let me know where is the best place to put it.

@angeliski
Copy link
Contributor

Nice @phiter
Maybe can we move that functions to the store?
What do you think @megatroom? React isn't my strongest point

@angeliski angeliski requested a review from megatroom May 25, 2020 14:24
Copy link
Contributor

@megatroom megatroom left a comment

Choose a reason for hiding this comment

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

Excellent refactoring! This division by functions was very good. 💯 🚀

Would you mind doing some tests to ensure this behavior?

I left a comment regarding the signature of a function.

frontend/src/morpheus/containers/OfficePage.js Outdated Show resolved Hide resolved
Copy link
Contributor

@juliemar juliemar left a comment

Choose a reason for hiding this comment

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

Hi @phiter I just merged your previous PR and now there is a conflict in this branch. Could you fix them for me to continue merge?

@phiter
Copy link
Contributor Author

phiter commented May 30, 2020

@juliemar done!

@juliemar juliemar requested a review from megatroom June 1, 2020 11:24
Copy link
Contributor

@megatroom megatroom left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

@juliemar juliemar merged commit e1643e4 into ResultadosDigitais:develop Jun 8, 2020
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.

4 participants