mattermost-plugin-jira
mattermost-plugin-jira copied to clipboard
mattermost
mm-768 - prepends the siteURL in the redirectConnect method
Summary
using the getState().entities.general.config.SiteURL to fetch the site url and prepend it to the path as mentioned in the issue
comments.
Fixes #768
Ticket Link
https://github.com/mattermost/mattermost-plugin-jira/issues/768
Hello @maisnamrajusingh,
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.
Codecov Report
Merging #785 (87a0e3e) into master (f986c15) will decrease coverage by
0.01%. The diff coverage is0.00%.
:exclamation: Current head 87a0e3e differs from pull request most recent head 4acda12. Consider uploading reports for the commit 4acda12 to get more accurate results
@@ Coverage Diff @@
## master #785 +/- ##
==========================================
- Coverage 31.46% 31.45% -0.02%
==========================================
Files 49 49
Lines 5982 5984 +2
==========================================
Hits 1882 1882
- Misses 3911 3913 +2
Partials 189 189
| Impacted Files | Coverage Δ | |
|---|---|---|
| server/plugin.go | 10.98% <0.00%> (-0.09%) |
:arrow_down: |
| server/user_cloud.go | 0.00% <0.00%> (ø) |
|
| server/user_server.go | 8.60% <0.00%> (ø) |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update f986c15...4acda12. Read the comment docs.
@mickmister using string templates makes sense. I have committed it.
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
@mickmister totally forgot about this. Updated this to move the code into selector.
@maisnamrajusingh Can you check the rest of the repo's webapp folder to see if there other spots where we are not using the site URL when we should?
It looks like this repo already has a function to use the siteURL getPluginServerRoute. Can we use this selector instead of creating a new one?
https://github.com/mattermost/mattermost-plugin-jira/blob/d4f87fe8a91c1ef4b62228f0b8aeb4a14c6b1ca9/webapp/src/selectors/index.ts#L14
Also @maisnamrajusingh @sibasankarnayak can you please verify whether there are other spots that need to be updated that may need to use the site URL?
It looks like this repo already has a function to use the siteURL
getPluginServerRoute. Can we use this selector instead of creating a new one?https://github.com/mattermost/mattermost-plugin-jira/blob/d4f87fe8a91c1ef4b62228f0b8aeb4a14c6b1ca9/webapp/src/selectors/index.ts#L14
Also @maisnamrajusingh @sibasankarnayak can you please verify whether there are other spots that need to be updated that may need to use the site URL?
@mickmister these two are different one returns basePath + '/plugins/' + PluginId and other one baseUrl
we can make a common function which will return basePath so we can use in common
@sibasankarnayak The plugin never needs to calculate the base URL without /plugins/' + PluginId, so 1/5 we can get rid of the getBaseUrl function and just use the existing one.
@sibasankarnayak The plugin never needs to calculate the base URL without
/plugins/' + PluginId, so 1/5 we can get rid of thegetBaseUrlfunction and just use the existing one.
@mickmister have removed the new code and used the existing one , but can see there is a high severity security vulnerability can you check this ?
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
@mickmister have removed the new code and used the existing one , but can see there is a high severity security vulnerability can you check this ?
The vulnerability found is unrelated to this PR, and is not a case where we will change the code in this project
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
i need a matter-most that working on sub URL, already discussed this with dylan so he will provide me the access of that and after that i will test this.
Looks like you don't have permissions to trigger this command. Only available for the PR submitter and org members
We don't have permissions to update this PR, please contact the submitter to apply the update.
@dipak-demansol i didn't face this issue so far.
i shared you a server URL and My ID password, pls check with that.
@maisnamrajusingh when i use this Command as /jira instance connect URL then i redirected on the New Tab/Page of jira but i got the error, Developer sir, are you able to re-generate this issue?
@maisnamrajusingh Gentle reminder to Reply On this issue.
@dipak-demansol just confirming that this is resolved and that this pr is ready for final review
@maisnamrajusingh when i use this Command as /jira instance connect URL then i redirected on the New Tab/Page of jira but i got the error, Developer sir, are you able to re-generate this issue?
@maisnamrajusingh Gentle reminder to Reply On this issue.
@maisnamrajusingh i'm still facing this issue. let me know if is there any info or help you want from me to regenerate this issue.
We don't have permissions to update this PR, please contact the submitter to apply the update.
@maisnamraju I am also facing this issue. I see the nginx issue when trying to connect. If possible, can you see if this is reproducible on the subpath test server here https://subpath.test.mattermost.com/mattermost
One more though would be to pull master up into your PR branch to see if it resolves anything.
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 @aspleenic
The file .circleci/config.yml is in the blocklist for external contributors. Hence, these changes are not tested by the CI pipeline active until the build is re-triggered by a core committer or the PR is merged. Please be careful when reviewing it.
/cc @mattermost/core-security @mattermost/core-build-engineers