Add a settings to control URL substitution #57

gbonnefille · 2021-04-07T17:04:25Z

Summary

This substitution is known to break Linux experience.
While the raw URL is also known to work on Windows.

In some circumstance, it could be desirable to not substitute
URLs.

Ticket Link

Fixes #31

This substitution is known to break Linux experience. While the raw URL is also known to work on Windows. In some circumstance, it could be desirable to not subtitute URLs.

mattermod · 2021-04-07T17:04:29Z

Hello @gbonnefille,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

Per the Mattermost Contribution Guide, we need to add you to the list of approved contributors for the Mattermost project.

Please help complete the Mattermost contribution license agreement?
Once you have signed the CLA, please comment with /check-cla and confirm that the CLA check is green.

This is a standard procedure for many open source projects.

Please let us know if you have any questions.

We are very happy to have you join our growing community! If you're not yet a member, please consider joining our Contributors community channel to meet other contributors and discuss new opportunities with the core team.

codecov · 2021-04-07T17:05:44Z

Codecov Report

Merging #57 (da9f7bb) into master (67e9a53) will increase coverage by 1.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #57      +/-   ##
==========================================
+ Coverage   35.92%   36.97%   +1.05%     
==========================================
  Files           8        8              
  Lines         334      384      +50     
==========================================
+ Hits          120      142      +22     
- Misses        195      223      +28     
  Partials       19       19

Impacted Files	Coverage Δ
server/configuration.go	`34.78% <ø> (+4.01%)`	⬆️
server/meeting.go	`68.25% <100.00%> (+4.79%)`	⬆️
server/plugin.go	`42.85% <0.00%> (-3.81%)`	⬇️
server/command.go	`23.57% <0.00%> (-1.20%)`	⬇️
server/main.go	`0.00% <0.00%> (ø)`
server/http.go	`72.34% <0.00%> (+1.88%)`	⬆️
server/store.go	`4.54% <0.00%> (+1.91%)`	⬆️
server/user.go	`12.50% <0.00%> (+2.97%)`	⬆️

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 67e9a53...da9f7bb. Read the comment docs.

gbonnefille · 2021-04-07T17:06:47Z

Feel free to pick ideas from this proposed solution instead of merging it directly.

gbonnefille · 2021-04-15T13:33:21Z

/check-cla

mattermod · 2021-04-26T01:01:07Z

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

/cc @jasonblais @jfrerich @emilyacook

cpoile

Thanks @gbonnefille !

cpoile · 2021-05-07T20:10:54Z

@DHaussermann Are you able to test on windows, mac, and linux, and do we still have a Webex account to test with?

mattermod · 2021-05-18T01:01:05Z

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

/cc @jasonblais @jfrerich @emilyacook

DHaussermann · 2021-06-10T13:30:32Z

@gbonnefille I have tested this on Ubuntu, Mac and Windows Chrome.
The URL substitution is working. I know longer see the issue reported and the meeting link can be opened in the browser as expected.

That said I'm seeing a different issue. For all 3 OS when I join a meeting from the browser, there is no audio support.

This seems like an unrelated problem. I tested with the latest WebEx release before the PR and see the same issue.
0/5 if there is something we can do in the plugin to resolve this. I'm happy to open a new issue for investigation but, I'm curious of this is something you may also have come across?

Regardless - This PR works as intended and can be merged
LGTM!

Edit Created #61 to investigate audio support in browser.

mattermod · 2021-06-21T01:01:05Z

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

/cc @jasonblais @jfrerich @emilyacook

jasonblais · 2021-06-29T13:11:14Z

/update-branch

jasonblais · 2021-06-29T13:11:32Z

@cpoile looks like this would be ready to merge?

larkox · 2021-06-30T10:16:57Z

@jasonblais This is having some ci issues (some test are failing), so we shoulnd't merge yet.
@gbonnefille could you look at the tests and fix them?

gbonnefille · 2021-06-30T11:49:38Z

@jasonblais This is having some ci issues (some test are failing), so we shoulnd't merge yet.
@gbonnefille could you look at the tests and fix them?

I'm strictly unable to investigate on these failures.

cpoile · 2021-06-30T13:42:35Z

/update-branch

cpoile · 2021-06-30T14:15:22Z

@gbonnefille Could you click the Allow edits from maintainers button please? Then I'll fix the tests.

For more info, see: https://docs.github.com/en/github/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork

gbonnefille · 2021-06-30T14:34:23Z

@gbonnefille Could you click the Allow edits from maintainers button please? Then I'll fix the tests.

For more info, see: https://docs.github.com/en/github/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork

As far as I understand Github, it is already checked.

mattermod · 2021-07-11T01:01:03Z

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

/cc @jasonblais @jfrerich @emilyacook

…setting

hanzei · 2021-07-14T13:38:23Z

@gbonnefille I've fixed the test for you so we can merge this PR. Thanks for the contribution! 🎉

gbonnefille · 2021-07-15T06:43:46Z

@gbonnefille I've fixed the test for you so we can merge this PR. Thanks for the contribution! tada

Many thanks!

Add a settings to control URL substitution

f2e9fd4

This substitution is known to break Linux experience. While the raw URL is also known to work on Windows. In some circumstance, it could be desirable to not subtitute URLs.

gbonnefille requested a review from cpoile as a code owner April 7, 2021 17:04

hanzei added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester labels Apr 7, 2021

mattermod added the Lifecycle/1:stale label Apr 26, 2021

cpoile approved these changes May 7, 2021

View reviewed changes

cpoile removed the Lifecycle/1:stale label May 7, 2021

mattermod added the Lifecycle/1:stale label May 18, 2021

hanzei requested a review from DHaussermann May 27, 2021 12:01

Fix linter issues

e42ec9d

DHaussermann approved these changes Jun 10, 2021

View reviewed changes

DHaussermann added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester Lifecycle/1:stale labels Jun 10, 2021

mattermod added the Lifecycle/1:stale label Jun 21, 2021

Merge branch 'master' into add-url-conversion-setting

3e5f8ff

jasonblais removed the Lifecycle/1:stale label Jun 29, 2021

Merge branch 'master' into add-url-conversion-setting

568a64e

mattermod added the Lifecycle/1:stale label Jul 11, 2021

hanzei added 2 commits July 14, 2021 15:22

Merge remote-tracking branch 'origin/master' into add-url-conversion-…

cb60f7c

…setting

Fix test

da9f7bb

hanzei added this to the v1.2.0 milestone Jul 14, 2021

hanzei removed the Lifecycle/1:stale label Jul 14, 2021

hanzei merged commit 928f1bf into mattermost-community:master Jul 14, 2021

mickmister mentioned this pull request Jul 15, 2021

Release 1.2.0 #66

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a settings to control URL substitution #57

Add a settings to control URL substitution #57

gbonnefille commented Apr 7, 2021

mattermod commented Apr 7, 2021

codecov bot commented Apr 7, 2021 •

edited

Loading

gbonnefille commented Apr 7, 2021

gbonnefille commented Apr 15, 2021

mattermod commented Apr 26, 2021

cpoile left a comment

cpoile commented May 7, 2021

mattermod commented May 18, 2021

DHaussermann commented Jun 10, 2021 •

edited

Loading

mattermod commented Jun 21, 2021

jasonblais commented Jun 29, 2021

jasonblais commented Jun 29, 2021

larkox commented Jun 30, 2021

gbonnefille commented Jun 30, 2021

cpoile commented Jun 30, 2021

cpoile commented Jun 30, 2021

gbonnefille commented Jun 30, 2021

mattermod commented Jul 11, 2021

hanzei commented Jul 14, 2021

gbonnefille commented Jul 15, 2021

Add a settings to control URL substitution #57

Add a settings to control URL substitution #57

Conversation

gbonnefille commented Apr 7, 2021

Summary

Ticket Link

mattermod commented Apr 7, 2021

codecov bot commented Apr 7, 2021 • edited Loading

Codecov Report

gbonnefille commented Apr 7, 2021

gbonnefille commented Apr 15, 2021

mattermod commented Apr 26, 2021

cpoile left a comment

Choose a reason for hiding this comment

cpoile commented May 7, 2021

mattermod commented May 18, 2021

DHaussermann commented Jun 10, 2021 • edited Loading

mattermod commented Jun 21, 2021

jasonblais commented Jun 29, 2021

jasonblais commented Jun 29, 2021

larkox commented Jun 30, 2021

gbonnefille commented Jun 30, 2021

cpoile commented Jun 30, 2021

cpoile commented Jun 30, 2021

gbonnefille commented Jun 30, 2021

mattermod commented Jul 11, 2021

hanzei commented Jul 14, 2021

gbonnefille commented Jul 15, 2021

codecov bot commented Apr 7, 2021 •

edited

Loading

DHaussermann commented Jun 10, 2021 •

edited

Loading