Merge FEATURE-create-issue-modal into master #239

jfrerich · 2020-04-14T15:42:35Z

Summary

This PR is a request to merge the FEATURE-create-issue-modal into the master branch.

NOTE TO PR REVIEWERS:

All of the code has been approved in previous PRs that were incrementally merged into the feature branch. Please review with this in mind (ie. you've likely already given in-depth reviews)

QA: To expedite the turnaround time to Feature Complete, we omitted QA from the incremental PRs, once I took over the code from the original author. @DHaussermann, you should see fixes for all items you found in the original PR for this feature.

Here is the comment from that original PR. #123 (comment)

* Implemented modal to create GitHub issue through the posts' dropdown options * Implemented some feedback from the PR * api.go - add missing bracket and remove getting repositories from inside err block - replace hard coded 410 with http.StatusGone gitub_repo_selector.jsx - remove unused isValid() function create_issue.jsx - remove assignment for close variable because using this.props.close directly create_issue.jsx - use GitHubIcon component in place of fa* class * remove pre-declare for githubClient * isolate errors in create_issue modal to it's own component and don't pass to github_repo_selector component and have it handle it's parents errors Co-authored-by: Lev <1187448+levb@users.noreply.github.com> Co-authored-by: Jason Frerich <jason.frerich@mattermost.com> Co-authored-by: mattermod <mattermod@users.noreply.github.com> Co-authored-by: Ben Schumacher <ben.schumacher@mattermost.com>

Co-authored-by: mattermod <mattermod@users.noreply.github.com>

* GitHubRepoSelector component - now uses ReactSelectSetting component - made into its own component so it can be shared with the attach comment to issue modal, which needs a repo selector - uses the new validator component for validation in the ReactSelectSetting component - getReps() - called only once when componentDidMount is called - performs the client call to get the repos - pulled this.state variables initialization into its own initialState variable CreateIssueModal component - holds selected repo state - calls validator component ReactSelectSetting component - handles validation - handles reducing values when typing text in the search box - handles multiple select types automatically Setting component - simple component to render the select and validation components in ReactSelectSetting Validator component - contains addComponent function to attach an "isvalid" function to a ReactSelectSetting components. Passed down from parent CreateIssueModal - contains validate() function for computing if all registers componsents are valid * Fix a couple of PR change requests * Add action getRepos Add yourRepos Prop * Review feedback fixes

codecov-io · 2020-04-14T15:42:40Z

Codecov Report

Merging #239 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #239   +/-   ##
=======================================
  Coverage   18.72%   18.72%           
=======================================
  Files          10       10           
  Lines        2302     2302           
=======================================
  Hits          431      431           
  Misses       1847     1847           
  Partials       24       24

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 ef63090...ef63090. Read the comment docs.

hanzei

Thanks for adding this great feature 👍

server/api.go

webapp/src/components/modals/create_issue/create_issue.jsx

server/api.go

with the post.Message

larkox

LGTM. Mostly nitpicks and questions.

server/api.go

webapp/src/components/react_select_setting.jsx

webapp/src/components/modals/create_issue/index.js

Mattermost

server/api.go

in the server code to correctly set the owner/repo of the issue getting created if Issues.Create() call returns an error, handle it instead of continuing

hanzei · 2020-05-09T00:11:24Z

@jfrerich I did deffer some of the webapp comments towards you given that I don't understand the code well.

@mickmister Please let me know which of the webapp comments are blocking.

mickmister · 2020-05-09T00:30:52Z

@hanzei After looking through my requested changes, there are none that are blocking. I'm making a ticket for the option.label vs option.value situation, and investigating why we need to do the checkAndHandleNotConnected call.

DHaussermann · 2020-05-14T15:22:40Z

There is a bit of an issue remaining with create functionality. The business logic is not consistent between Create and Attach when the plugin is not locked to any Organization.

Attach will allow attaching to any public repo where you're able to add an issue comment.

Create will allow you to create an issue in any repo where you are a member or contributor. Public repos that allow any users to create an issue are not returned in seaach

@aaronrothschild - I don't feel too strongly that the attach functionality is more correct (it's a bit awkward to use). It's not necessarily an issue that I can do something from the Web UI but not via the plugin. But, having inconsistent business logic between the 2 features is more of an issue. I would suggest that the logic of which repos you can create in match the Attach functionality Thoughts?

All other issues (listed in the linked item by Jason) have been resolved!

mickmister · 2020-05-14T15:52:19Z

Attach will allow attaching to any public repo where you're able to add an issue comment.

@DHaussermann I always thought this was a bit overkill. My opinion is to change the attach functionality to match create, and only include repos that you are a member/contributor of. If I want to create an issue or comment on a repo that I am not a member of, I wouldn't think this plugin would help me do that. It should be a more tailored experience for the user imo.

DHaussermann · 2020-05-14T16:05:51Z

My opinion is to change the attach functionality to match create, and only include repos that you are a member/contributor of.

I'm inclined to agree with @mickmister I don't think being able to comment in any public repo adds too much value and it makes the usability worse. One thing to conciser is that we already have attach out in production. We would need to clearly release note that we're making this change.

…e-modal

mickmister · 2020-05-16T06:13:48Z

It seems to me that this is ready to merge. I've created a ticket to change the "Attach to Issue" functionality as it seems obvious that the UX should be tailored to the user. I'd like to get this merged so we can address the remaining issues in separate PRs.

@DHaussermann I think we can merge once we have your blessing on this. Regarding the followup change for "Attach to Issue", I agree with "We would need to clearly release note that we're making this change."

hanzei · 2020-05-16T07:38:38Z

👍 for continuing the discussion about "Attach to Issue" in #273

aaronrothschild

LGTM

DHaussermann

Tested and passed

Functional testing completed
All issues identified previously with create functionality have been resolved.
Follow-up issue created to make the business logic consistent for the pre-exiting attach functionality

LGTM!

Updated release testing to cover this feature.

dhadiseputro and others added 6 commits March 20, 2020 13:08

fix linting error (#221)

763816b

[GH-219] After creating issue, reply to post in MM with the Gith… (#222)

e7c21fa

Co-authored-by: mattermod <mattermod@users.noreply.github.com>

Add user validation for repo and title fields (#228)

f6c2182

Add attach and create post actions (#229)

e81835b

jfrerich added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester labels Apr 14, 2020

jfrerich requested review from levb, mickmister and DHaussermann April 14, 2020 15:42

jfrerich requested review from hanzei and larkox as code owners April 14, 2020 15:42

jfrerich self-assigned this Apr 14, 2020

Merge branch 'master' into FEATURE-create-issue-modal

1aa6563

levb approved these changes Apr 15, 2020

View reviewed changes

hanzei previously requested changes Apr 16, 2020

View reviewed changes

jfrerich added 3 commits April 16, 2020 10:36

PR Review changes

1194c2d

Allow user to edit the Description of the ticket. Continue to initialze

fc19f20

with the post.Message

fix the spinner in the form_button.

4e7e89e

larkox requested changes Apr 20, 2020

View reviewed changes

Append created issue message with a mention that issue was created from

8b6718b

Mattermost

hanzei reviewed Apr 25, 2020

View reviewed changes

server/api.go Outdated Show resolved Hide resolved

hanzei added this to the v1.0.0 milestone Apr 25, 2020

jfrerich added 5 commits April 27, 2020 11:29

Remove team from permalink function. Not needed with the redirect link

65788bd

s/createIssueModalVisible/isCreateIssueModalVisible/

722c7ac

send the repo value from the client to be of "owner/repo". Split this

d2a2076

in the server code to correctly set the owner/repo of the issue getting created if Issues.Create() call returns an error, handle it instead of continuing

check issue.PostId not empty

ce678a8

use lowercase for string comparison

f8f4fdd

Return early if modal is not visible

eab01f5

hanzei requested a review from mickmister May 9, 2020 00:10

mickmister approved these changes May 9, 2020

View reviewed changes

mickmister removed the 2: Dev Review Requires review by a core committer label May 9, 2020

mickmister requested a review from DHaussermann May 9, 2020 00:34

hanzei unassigned jfrerich May 9, 2020

mickmister mentioned this pull request May 9, 2020

Fix meeting ended webhook mattermost/mattermost-plugin-zoom#111

Merged

Check SiteURL on startup

85b7fcb

DHaussermann requested a review from aaronrothschild May 14, 2020 15:23

DHaussermann added the 1: PM Review Requires review by a product manager label May 14, 2020

Merge remote-tracking branch 'origin/master' into FEATURE-create-issu…

ef63090

…e-modal

mickmister mentioned this pull request May 16, 2020

For the "Attach to Issue" feature, only show repos the user is a member or collaborator of #273

Open

aaronrothschild removed the 1: PM Review Requires review by a product manager label May 18, 2020

aaronrothschild approved these changes May 18, 2020

View reviewed changes

DHaussermann approved these changes May 18, 2020

View reviewed changes

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 May 18, 2020

hanzei merged commit f3ed305 into master May 18, 2020

hanzei deleted the FEATURE-create-issue-modal branch May 18, 2020 17:19

hanzei added this to Submitted in Integrations Team via automation May 18, 2020

hanzei moved this from Submitted to Done in Integrations Team May 18, 2020

hanzei mentioned this pull request Jul 1, 2021

Re-queue an item for next meeting mattermost-community/mattermost-plugin-agenda#12

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Merge FEATURE-create-issue-modal into master #239

Merge FEATURE-create-issue-modal into master #239

jfrerich commented Apr 14, 2020

codecov-io commented Apr 14, 2020 •

edited

Loading

hanzei left a comment

larkox left a comment

hanzei commented May 9, 2020

mickmister commented May 9, 2020

DHaussermann commented May 14, 2020 •

edited

Loading

mickmister commented May 14, 2020

DHaussermann commented May 14, 2020

mickmister commented May 16, 2020

hanzei commented May 16, 2020

aaronrothschild left a comment

DHaussermann left a comment •

edited by hanzei

Loading

Merge FEATURE-create-issue-modal into master #239

Merge FEATURE-create-issue-modal into master #239

Conversation

jfrerich commented Apr 14, 2020

Summary

codecov-io commented Apr 14, 2020 • edited Loading

Codecov Report

hanzei left a comment

Choose a reason for hiding this comment

larkox left a comment

Choose a reason for hiding this comment

hanzei commented May 9, 2020

mickmister commented May 9, 2020

DHaussermann commented May 14, 2020 • edited Loading

mickmister commented May 14, 2020

DHaussermann commented May 14, 2020

mickmister commented May 16, 2020

hanzei commented May 16, 2020

aaronrothschild left a comment

Choose a reason for hiding this comment

DHaussermann left a comment • edited by hanzei Loading

Choose a reason for hiding this comment

codecov-io commented Apr 14, 2020 •

edited

Loading

DHaussermann commented May 14, 2020 •

edited

Loading

DHaussermann left a comment •

edited by hanzei

Loading