mattermost-community / mattermost-plugin-jitsi

Jitsi plugin for Mattermost :electric_plug:

Apache License 2.0

195 stars 90 forks source link

[mm-141] - Uses the siteURL for all calls from the web app #197

Closed maisnamrajusingh closed 2 years ago

maisnamrajusingh commented 3 years ago

Summary

Adds the site url to the calls in the frontend. Fixes #141

Ticket Link

https://github.com/mattermost/mattermost-plugin-jitsi/issues/141

codecov-commenter commented 3 years ago

Codecov Report

Merging #197 (77b4981) into master (f0771d3) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #197   +/-   ##
=======================================
  Coverage   45.47%   45.47%           
=======================================
  Files           7        7           
  Lines         873      873           
=======================================
  Hits          397      397           
  Misses        451      451           
  Partials       25       25

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f0771d3...77b4981. Read the comment docs.

maisnamraju commented 3 years ago

@larkox can you take a look and see if its good to go.

maisnamrajusingh commented 3 years ago

@sanjaydemansol can you update the changes Michael asked for ?

hanzei commented 2 years ago

@dipak-demansol Could you please review this PR so we can wrap up the v2.1.0 release?

DHaussermann commented 2 years ago

@larkox or @maisnamraju, @dipak-demansol and I have reviewed this change and found no regression. However, we noticed that using the plugin to launch and join a meeting on a subpath server also seems to work as expected on the master branch.

Can you please confirm the issue that was solved in https://github.com/mattermost/mattermost-plugin-jitsi/issues/141 We may need guidance or test steps on this PR to validate that the issue observed has been resolved.

hanzei commented 2 years ago

@DHaussermann I sill think this PR is a code improvement. Do you have objections on merging it?

DHaussermann commented 2 years ago

Hi @maisnamrajusingh and @hanzei We have regression tested this PR and found no issues with the change.

Jitsi working on a subpath server can be added to release testing.

No further functional testing is needed for this change.

LGTM!

Note that I am unable to modify label in this Repo.