matrix-react-sdk
matrix-react-sdk copied to clipboard
End jitsi call when member is banned
Jitsi call needs to be ended when member is banned.
Currently if user A bans user B, then A don't see B in the call anymore, but B stays in the call (call window dialog is minimized) and B can hear/see what is going on in the call.
This PR invokes Jitsi call ending logic when user is banned.
Here's what your changelog entry will look like:
🐛 Bug Fixes
- End jitsi call when member is banned (#8879). Contributed by @maheichyk.
Signed-off-by: Mikhail Aheichyk [email protected]
It only partially solves the "Jitsi" problem, but at least it will fix that any always-on-screen widget will be closed when the user is banned from a room. It is not more inconsistent than the current flow where the leave
state closes the call but also don't actually block the user from joining it again. Both with leave
or ban
, the home server will not accept any requests from a widget anymore.
It would still improve the sitation for some use-cases, we have examples of closed environments where this change would be very helpful.
@turt2live what changes could be applied to this PR to make it acceptable?
Hi @turt2live could you approve this MR or provide some more feedback?
@turt2live thanks for feedback. Since the change is fine, we would like it to be approved and merged to be used. I currently don't have enough experience and time to create cypress test for this issue unfortunately, but an internal ticket has been created and will be addressed later. Hope this approach works.
The test should be relatively easy as it won't need to test Jitsi. If you run into issues, visiting #element-dev:matrix.org on Matrix would be best so the team can help out.
At the moment, we require tests when the change can reasonably have tests - this PR should be on the easier side to test.
@turt2live e2e tests are added, please review
Hi @turt2live, could you please approve?
Hi @turt2live branch was updated to fix an issue in the spec connected to matrix-js-sdk version, could you please rerun workflows?
widgets/widget-pip-close.spec.ts fails. Maybe somebody (@turt2live) could help how to fix that? Error:
Oops...we found an error preparing this test file:
> cypress/e2e/widgets/widget-pip-close.spec.ts
The error was:
Error: Webpack Compilation Error
./node_modules/matrix-js-sdk/src/matrix.ts 53:9
Module parse failed: Unexpected token (53:9)
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
| export * from './@types/search';
| export * from './models/room-summary';
> export * as ContentHelpers from "./content-helpers";
| export {
| createNewMatrixCall,
@ ./cypress/e2e/widgets/widget-pip-close.spec.ts 28:17-52
at handle (/home/runner/.cache/Cypress/10.4.0/Cypress/resources/app/node_modules/@packages/server/node_modules/@cypress/webpack-preprocessor/dist/index.js:180:23)
....
This occurred while Cypress was compiling and bundling your test code. This is usually caused by:
- A missing file or dependency
- A syntax error in the file or one of its dependencies
That looks like something went wrong in the js-sdk - if you have a js-sdk branch name which matches this PR's branch name then it might need updating/fixing, or possibly just merge latest react-sdk develop
into this PR (again).
Hi @t3chguy, @kerryarchibald could you please merge these changes?
This PR was approved by Travis but he cannot merge it right now as far as I know.
I did additional changes to the widget-pip-close.spec.ts
spec that were necessary due to webpack compilation error and updates to 'matrix-js-sdk'.
@t3chguy thanks for review, please approve :) Could you please trigger workflows to see that all fine? (since I am first-time contributor the workflows are not started automatically with my commits)
timeline.spec.ts failed twice in a row, looks like an issue to me, but not related to this PR
Looks sane to me otherwise, and once the CI passes, thanks for fixing!!
@t3chguy Thanks a lot for checking that!
I fixed the comment issue and with the next update of develop branch could restart the pipelines. If my test is fine but some other one not, could you merge PR then?