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.

Sign up for GitHub

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

Jump to bottom

Created/Attached to a Jira issue message should include a reference to the Jira plugin #464

Merged
merged 7 commits into from
Feb 26, 2020
Merged

Created/Attached to a Jira issue message should include a reference to the Jira plugin #464

merged 7 commits into from
Feb 26, 2020

Conversation

AGMETEOR
Copy link
Contributor

@AGMETEOR AGMETEOR commented Feb 5, 2020

Summary

Created/Attached to a Jira issue message should include a reference to the Jira plugin

Fixes #438

mickmister
mickmister suggested changes Feb 5, 2020
View reviewed changes
Copy link
Contributor

@mickmister mickmister left a comment

Choose a reason for hiding this comment

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

Thanks @AGMETEOR, just one request and a comment

server/http.go Outdated Show resolved Hide resolved
server/user.go Outdated Show resolved Hide resolved
@mickmister mickmister added 1: PM Review Requires review by a product manager 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester labels Feb 5, 2020
@AGMETEOR @mickmister
Updated routeToDocsConnect value
e9d1e79 
Co-Authored-By: Michael Kochell <mjkochell@gmail.com>
levb
levb suggested changes Feb 6, 2020
View reviewed changes
server/http.go Outdated Show resolved Hide resolved
server/issue.go Outdated Show resolved Hide resolved
server/user.go Outdated Show resolved Hide resolved
levb
levb suggested changes Feb 7, 2020
View reviewed changes
Copy link
Contributor

@levb levb left a comment

Choose a reason for hiding this comment

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

Please make the naming consistent.

server/http.go Outdated Show resolved Hide resolved
server/issue.go Outdated Show resolved Hide resolved
server/plugin.go Show resolved Hide resolved
server/user.go Outdated Show resolved Hide resolved
@AGMETEOR
Add tests for routeToDocsOrConnect
883186d 
This commit adds tests to routeToDocsOrConnect
There's also need to edit GetPlugin on mocked JIRA Instance to return configured plugin
with the API and mocked stores. Consequently, that change also meant that one test in issue_test.go
that depends on those mocks also had to be fixed.
@AGMETEOR
Change routeToDocsOrConnect to routeStart and make consistent
ed51615
levb
levb approved these changes Feb 11, 2020
View reviewed changes
server/plugin.go Show resolved Hide resolved
@levb levb removed the 2: Dev Review Requires review by a core committer label Feb 11, 2020
@aaronrothschild aaronrothschild removed the 1: PM Review Requires review by a product manager label Feb 13, 2020
@DHaussermann
Copy link

Hi @AGMETEOR ,

Can I get some clarification on the scope of this PR? When creating an issue from a post I can see your change.

  1. The post created now contain a link with the text of the issue key that points to the issue rather than the full path.
  2. The post created now contains by mattermost-jira-plugin

I don't see any change when attaching to an issue where the post references the Jira plugin. Should I be seeing this as well?

@AGMETEOR
Copy link
Contributor Author

Hi @AGMETEOR ,

Can I get some clarification on the scope of this PR? When creating an issue from a post I can see your change.

  1. The post created now contain a link with the text of the issue key that points to the issue rather than the full path.
  2. The post created now contains by mattermost-jira-plugin

I don't see any change when attaching to an issue where the post references the Jira plugin. Should I be seeing this as well?

Hello @DHaussermann , thanks for the review :)
I think for the issue that's created using the plugin, the github issue description wanted a way for mattermost users who might not be using the plugin to become more aware of it. The suggestion was that when they click by mattermost-jira-plugin, we take them to plugin docs if they're already connected, otherwise we start a connect process for them.

Clicking on the issue key link takes them to jira to view the ticket.

I hope I was able to answer properly.

@DHaussermann
Copy link

@AGMETEOR Sorry for being unclear. I was explaining the changes that I'm seeing implemented. Yes, all of that is clear to me :) .

But, my remaining question is... Should there also be a change in behavior to mention the plugin in the post created when the Attach to Jira functionality is used? There is mention of changing this as well but, it look the same to me.

@AGMETEOR
Copy link
Contributor Author

@AGMETEOR Sorry for being unclear. I was explaining the changes that I'm seeing implemented. Yes, all of that is clear to me :) .

But, my remaining question is... Should there also be a change in behavior to mention the plugin in the post created when the Attach to Jira functionality is used? There is mention of changing this as well but, it look the same to me.

@DHaussermann I see what you mean there. I totally forgot about the scenario when the attach route is used. I have made the necessary changes.

DHaussermann
DHaussermann approved these changes Feb 18, 2020
View reviewed changes
Copy link

@DHaussermann DHaussermann left a comment

Choose a reason for hiding this comment

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

Tested and passed

  • The post for create and attach now contain a link with the text of the issue key that points to the issue rather than the full path.
  • The post created now contains "... by mattermost-jira-plugin" which links to the repo
  • Tested Create and Attach cases
  • Regression tested create and attach from child and root posts
    No issues found. LGTM!
    Thanks @AGMETEOR for this enhancement.

@DHaussermann DHaussermann added 4: Reviews Complete All reviewers have approved the pull request QA Review Done PR has been approved by QA and removed 3: QA Review Requires review by a QA tester labels Feb 18, 2020
@jfrerich
Copy link
Contributor

Thanks @AGMETEOR! Merging this PR now.

@jfrerich jfrerich merged commit 3254af6 into mattermost:master Feb 26, 2020
@jfrerich jfrerich mentioned this pull request Mar 31, 2020
Closed
11 tasks
@jfrerich jfrerich mentioned this pull request Apr 28, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aaronrothschild aaronrothschild aaronrothschild approved these changes

@levb levb levb approved these changes

@mickmister mickmister mickmister approved these changes

@DHaussermann DHaussermann DHaussermann approved these changes

Assignees
No one assigned
Labels
4: Reviews Complete All reviewers have approved the pull request QA Review Done PR has been approved by QA
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Created/Attached to a Jira issue message should include a reference to the Jira plugin
6 participants
@AGMETEOR @DHaussermann @jfrerich @aaronrothschild @levb @mickmister