MM-13967: Add support for more issue events #22

levb · 2019-02-14T01:29:48Z

Fixed MM-13967: Added support for various ticket update events. Did not add support for worklogs and issue links because these events have only limited data
Fixed MM-8650: removed enable/disable
Replaced SlackAttachments with regular markdown for display

jasonblais · 2019-02-14T02:23:28Z

Awesome!! 🎉

Did not add support for worklogs and issue links because these events have only limited data

is the data so limited, that any info we post back would be more-or-less useless?

Not a big deal if we don't support them (less important than the other issue-related events). Just curious so that when someone asks why we don't support it, I'll have the justification ready (we have at least one request for worklogs).

Replaced SlackAttachments with regular markdown for display

UX and I can work out the formatting after this is merged - thinking it might be easier when we see the data sent back.

Curious if there was a reason for replacing SlackAttachments with regular markdown?

hanzei

Added a few comments without testing it jet.

Once we are fine with the implementation, tests are needed.

server/configuration.go

server/plugin.go

server/webhook.go

hanzei · 2019-02-14T08:21:47Z

I propose to use https://ci-linux-mysql.mattermost.com for testing this. I can set up a test environment, once the initial review is done.

jwilander

Overall the changes look good, no major concerns from me just a couple small comments

server/webhook.go

levb · 2019-02-14T17:56:08Z

@jasonblais:
RE: "...the data so limited, that any info we post back would be more-or-less useless": there are neither issue keys nor the user data readily available in these messages. There are internal IDs, so with proper auth/API usage we can get the details later, but the webhooks themselves appear very limited. So, if we need it, will come in v2.

RE: "Reason for replacing SlackAttachments with regular markdown." Just seems a lot cleaner without them.Was there a reason that they were used to start with?

codecov-io · 2019-02-14T18:37:00Z

Codecov Report

Merging #22 into master will increase coverage by 7.54%.
The diff coverage is 91.02%.

@@            Coverage Diff             @@
##           master      #22      +/-   ##
==========================================
+ Coverage   79.05%   86.59%   +7.54%     
==========================================
  Files           4        5       +1     
  Lines         191      291     +100     
==========================================
+ Hits          151      252     +101     
- Misses         30       34       +4     
+ Partials       10        5       -5

Impacted Files	Coverage Δ
server/configuration.go	`34.78% <ø> (ø)`	⬆️
server/plugin.go	`100% <100%> (+11.68%)`	⬆️
server/format-slack-attachment.go	`83.78% <83.78%> (ø)`
server/format-markdown.go	`91.39% <91.39%> (ø)`

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

levb · 2019-02-14T18:46:07Z

@jasonblais @jwilander @hanzei @DHaussermann I updated the code with the PR feedback, you can see it in action at https://i-0cba36c59c7d65ed7.test.spinmint.com/ad-1/channels/autem-2 (sysadmin/sysadmin). It posts from https://levbrouk.atlassian.net/secure/RapidBoard.jspa?rapidView=1&view=planning.nodetail&selectedIssue=TES-39.

I'll add the tests for webhook once the UX has been reviewed since it'll just be a bunch of static comparisons.

jasonblais · 2019-02-15T03:39:11Z

@levb Sounds good for issue links and worklogs, not a top priority so let's cut. That explanation helps when going back to customer(s).

Re: attachments, that was a design decision in V1. However, Asaad and I are working on updating them for V2, and we'll compare how the messages look with just plain Markdown.

levb · 2019-02-16T02:52:19Z

@hanzei I think I got your change request covered, sorry for the initial misunderstanding.

@jasonblais please confirm with regards to whether to commit this with markdown messages once fully dev-approved, or hold until further UX review.

jasonblais · 2019-02-16T02:57:59Z

@levb Please hold for now, I got UI designs from Asaad (unrelated to changes in this PR), will see how the designs compare with the Markdown messages.

hanzei

LGTM 👍

hanzei · 2019-02-18T10:12:34Z

My 2 cents on attachments:

For me (and this is a personal opinion) attachments are the "bot way" to show information in Mattermost. When I search for e.g. a new JIRA issue in a channel, I just look out for the style of attachments. It help keeping bot messages separated from user messages.

I know of the technical flaws of slack attachments, but I think this we have decided to have them this way in our system and should stick (for now) with this.

2/5 on sticking with attachments, but not blocking.

hanzei · 2019-02-25T09:27:45Z

@levb Is this ready for a final round of review?

levb · 2019-02-28T02:06:04Z

@hanzei Yes it is. I rewrote much of it to simplify dual-mode md/slack attachment, but stopped short of making it configurable.

Also, if you have a test server where you can install it for testing, it'd be great; otherwise LMK and either I can bring up a spinmint server, or we can install this on community-daily? I'd like @jasonblais and @DHaussermann to have a pass at it.

@jasonblais I brought back the Slack attachments, and I believe preserved the formatting as it was. IMO this can be merged now, as is.

jasonblais · 2019-02-28T03:15:49Z

Thanks @levb. It would be good to see the formatting of the new events before merging.

hanzei

LGTM for a code/dev standpoint. I will leave the review of the design to other people 😉

jasonblais · 2019-03-04T14:09:37Z

@levb As discussed last week, we can merge and test on community-daily. We have designs from Asaad, but they can be done on a separate PR.

jasonblais

Approving as per above comment.

* restrict plugin activation based on license * run go mod tidy -v * wip * add enterprise package * rename license naming to enterprise if user does not have enterprise license, check number of instances installed before allowed to install another cloud or server instance * remove function * fix review feedback * Add license checking tests * wip * wip by Lev * wip * wip * remove comments * remove function to return pointer to true bool value * getMockInstanceStoreKV(0) should return initialized empty store was returning instance store with one instance correct all occurences call of getMockInstanceStoreKV(0) to call with 1 instanance. This fixes many failing tests Add tests for InstallInstance function with extensive license checking * fix linting Co-authored-by: Lev Brouk <[email protected]>

* iterate through instances when printing out the channel subscriptions update tests for new printing output format don't require --intance for subscribelist * Update tests * shorten field name * several PR feedback fixes * remove unused variable rename map variable * Disconnect users on instance uninstall (#41) * [GH-4] add EE license check (#22) * restrict plugin activation based on license * run go mod tidy -v * wip * add enterprise package * rename license naming to enterprise if user does not have enterprise license, check number of instances installed before allowed to install another cloud or server instance * remove function * fix review feedback * Add license checking tests * wip * wip by Lev * wip * wip * remove comments * remove function to return pointer to true bool value * getMockInstanceStoreKV(0) should return initialized empty store was returning instance store with one instance correct all occurences call of getMockInstanceStoreKV(0) to call with 1 instanance. This fixes many failing tests Add tests for InstallInstance function with extensive license checking * fix linting Co-authored-by: Lev Brouk <[email protected]> * Fixed #31, #33 (#54) - Use plugin URL as the "home" for app links - Added extra logging for suspicious callbacks - Use templates for `/jira instance install` output * Fix test (#64) * GH-21 Fixed websocket update on user disconnect, instance uninstall (#61) * Fixed websocket update on user disconnect * Revert "Fixed websocket update on user disconnect" This reverts commit 84ca4b27ca2a3ed55408bf359786b3162dc1b29a. * Fixed empty set refresh, defaulting in commands * fixed instance status update * GH-49: Fixed webhook, transition commands (#67) * Use connectInstances array for userConnected redux selector (#71) * Use connectInstances array for userConnected redux selector * add null check * [GH-45] Make sure frontend has up-to-date default instance data (#65) * fetch instances on modal open * Fix errors regarding redux update and missing channelId prop * Get instances when subscribe modal opens * fix tests * Handle case where default instance value in frontend is stale (uninstalled) * Make if statement more safe * Make if statement more safe * fix test * lint * GH-60: Fixed multi-work command alias parsing in webapp (#72) * Fixed --instance Co-authored-by: Lev <[email protected]> Co-authored-by: Lev Brouk <[email protected]> Co-authored-by: Michael Kochell <[email protected]>

Lev Brouk added 2 commits February 13, 2019 17:11

Refactored the JIRA plugin to support more events

99bff04

Removed Enabled config setting

81572b7

levb requested review from jwilander, jasonblais and hanzei February 14, 2019 01:29

hanzei requested changes Feb 14, 2019

View reviewed changes

server/configuration.go Show resolved Hide resolved

server/plugin.go Outdated Show resolved Hide resolved

server/webhook.go Outdated Show resolved Hide resolved

server/webhook.go Outdated Show resolved Hide resolved

server/webhook.go Outdated Show resolved Hide resolved

hanzei added 1: PM Review Requires review by a product manager 2: Dev Review Requires review by a core committer labels Feb 14, 2019

jwilander reviewed Feb 14, 2019

View reviewed changes

server/webhook.go Outdated Show resolved Hide resolved

server/webhook.go Outdated Show resolved Hide resolved

PR feedback

7332066

jwilander approved these changes Feb 14, 2019

View reviewed changes

PR feedback from @hanzei

1a857ba

Lev Brouk added 2 commits February 16, 2019 13:55

Added back webhook tests, updated for MD

7948be0

coverage

486716c

hanzei mentioned this pull request Feb 18, 2019

Add Support Additional Events #6

Closed

hanzei approved these changes Feb 18, 2019

View reviewed changes

hanzei removed the 2: Dev Review Requires review by a core committer label Feb 18, 2019

hanzei added this to the v2.0.0 milestone Feb 18, 2019

hanzei mentioned this pull request Feb 22, 2019

Plugin 1.1.0 not working anymore. #20

Closed

Added back formatting as Slack attachments

eb2c567

hanzei self-requested a review February 25, 2019 07:11

hanzei mentioned this pull request Feb 25, 2019

MM-13964: Format JIRA message to MM markdown #24

Closed

2 tasks

hanzei approved these changes Mar 4, 2019

View reviewed changes

jasonblais approved these changes Mar 4, 2019

View reviewed changes

jasonblais added 4: Reviews Complete All reviewers have approved the pull request and removed 1: PM Review Requires review by a product manager labels Mar 4, 2019

levb merged commit f882dad into mattermost:master Mar 5, 2019

levb mentioned this pull request Mar 12, 2019

Fixed a nil pointer panic on empty Fields.Priority in new issues #31

Merged

jasonblais mentioned this pull request Apr 1, 2019

compatibility issue with mattermost 5.8.0 #32

Closed

levb deleted the MM-13967-all-JIRA-events branch April 26, 2019 03:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MM-13967: Add support for more issue events #22

MM-13967: Add support for more issue events #22

levb commented Feb 14, 2019

jasonblais commented Feb 14, 2019

hanzei left a comment

hanzei commented Feb 14, 2019

jwilander left a comment

levb commented Feb 14, 2019 •

edited

Loading

codecov-io commented Feb 14, 2019 •

edited

Loading

levb commented Feb 14, 2019 •

edited

Loading

jasonblais commented Feb 15, 2019

levb commented Feb 16, 2019

jasonblais commented Feb 16, 2019

hanzei left a comment

hanzei commented Feb 18, 2019 •

edited

Loading

hanzei commented Feb 25, 2019

levb commented Feb 28, 2019 •

edited

Loading

jasonblais commented Feb 28, 2019

hanzei left a comment

jasonblais commented Mar 4, 2019

jasonblais left a comment

MM-13967: Add support for more issue events #22

MM-13967: Add support for more issue events #22

Conversation

levb commented Feb 14, 2019

jasonblais commented Feb 14, 2019

hanzei left a comment

Choose a reason for hiding this comment

hanzei commented Feb 14, 2019

jwilander left a comment

Choose a reason for hiding this comment

levb commented Feb 14, 2019 • edited Loading

codecov-io commented Feb 14, 2019 • edited Loading

Codecov Report

levb commented Feb 14, 2019 • edited Loading

jasonblais commented Feb 15, 2019

levb commented Feb 16, 2019

jasonblais commented Feb 16, 2019

hanzei left a comment

Choose a reason for hiding this comment

hanzei commented Feb 18, 2019 • edited Loading

hanzei commented Feb 25, 2019

levb commented Feb 28, 2019 • edited Loading

jasonblais commented Feb 28, 2019

hanzei left a comment

Choose a reason for hiding this comment

jasonblais commented Mar 4, 2019

jasonblais left a comment

Choose a reason for hiding this comment

levb commented Feb 14, 2019 •

edited

Loading

codecov-io commented Feb 14, 2019 •

edited

Loading

levb commented Feb 14, 2019 •

edited

Loading

hanzei commented Feb 18, 2019 •

edited

Loading

levb commented Feb 28, 2019 •

edited

Loading