Include extra metadata when clicking an interactive button #12697

rfoyard · 2019-10-09T13:23:00Z

Summary

Includes user_name, team_domain and channel_name when clicking an interactive button.

Ticket Link

JIRA: MM-10164
Fixes #12377

…ractive button

aaronrothschild

LGTM, Thanks @rfoyard for your contribution. This is certainly helpful when developing interactive apps!

hanzei

Thanks for the PR @rfoyard. I think we can improve a bit on the performance side of the code. In general it looks good 👍

model/integration_action.go

hanzei · 2019-10-17T20:26:54Z

app/integration_action.go

@@ -69,13 +69,6 @@ func (a *App) DoPostActionWithCookie(postId, actionId, userId, selectedOption st
 		close(pchan)
 	}()

-	cchan := make(chan store.StoreResult, 1)


This code was here to make the db requests asynchronous and improve performance. I would like to keep it here, or is there a reason not to?

This requests the channel from the postId but when the post does not exist, it's useless.
It was not a problem before, but as I need the channel to get the teamId, I would have to make another synchronous request when the post does not exist (to get the channel from the cookie's channelId line 85).
With this change I'm doing only one request to get the channel but if you prefer, I can change it and do another request just when using the cookie.
Just let me know.

The usual case is the the cookie is not set and we jump into the else case of L94. Hence, It don't see a problem with just making the request even though we might not need it.

Ok, I didn't know this was the usual case but with this in mind, I'll rework the PR. Thanks

I changed the requests flow in 1b85d42, just let me know what you think.

app/integration_action.go

Renamed TeamDomain to TeamName.

saturninoabril

Tested and looks good to me.

hanzei

LGTM besides by request at https://github.com/mattermost/mattermost-server/pull/12697/files#r336210163

levb

Just one small nit, otherwise LGTM. Thanks for parsing through that complex function!

@hanzei @crspeller This PR reminds me of my earlier proposal to eliminate the dependency on the database, and always use the "cookie" for the action metadata. Thoughts?

app/integration_action.go

hanzei · 2019-10-30T12:58:59Z

@hanzei @crspeller This PR reminds me of my earlier proposal to eliminate the dependency on the database, and always use the "cookie" for the action metadata. Thoughts?

3/5 that we want this. Performance is key with post actions.

Does this need client side changes?

levb · 2019-10-30T13:34:35Z

@hanzei I believe that all of the client code is in place, we should check what minimum client versions support cookies (including the RN fixes). Let's take this conversation (switching to cookies 100%) out of this ticket though, I'll log a Jira issue for Toolkit to triage.

hanzei

Thanks for digging into this 👍

app/integration_action.go

levb · 2019-10-30T16:39:58Z

(waiting for the tests)

rfoyard · 2019-10-30T17:47:06Z

Thanks everybody

hanzei · 2019-11-01T12:05:09Z

Hey @rfoyard,

Would you be interested in adding an example usage of this functionality to the demo plugin? This would greatly help with testing this.

rfoyard · 2019-11-01T13:25:05Z

Hey @hanzei, sure, I'll take a look at it next week.

* master: (70 commits) Run unused against codebase (mattermost#12968) [MM-12623] Create CLI command "config reset" (mattermost#10296) Migrate tests from store/storetest/status_store.go to use test… (mattermost#12873) Fix golangci-lint target (mattermost#12985) [MM-16437] Plugin crashes the server when calling WriteHeader with an invalid http code (mattermost#11276) MM-18060: Include deleted posts in compliance export. (mattermost#12957) [MM-18331] When patching a bot send websocket notification (mattermost#12373) [MM-19473] Fix data race on user login (mattermost#12870) license, openGraph tests: convert to testify (mattermost#12919) oauth_test: use testify (mattermost#12949) [MM-18830] Unhelpful error message when adding bot to a channel before adding to team (mattermost#12844) emoji_test: update to use testify (mattermost#12932) MM-19310 - Wrong validation message when Bot name ends in a . (mattermost#12905) Migrate tests from store/storetest/oauth_store.go to use testify (mattermost#12875) Include extra metadata when clicking an interactive button (mattermost#12697) MM-19553: Generate valid passwords on bulk import. (mattermost#12871) Convert api4/webhook_test.go t.Fatal calls into require/assert calls (mattermost#12904) Fix golangci-lint target for non GOPATH installations (mattermost#12934) MM-17888 Check plugin Helpers minimum server version comments (mattermost#12663) [MM-18898] Stringify plugin.Log* parameters (mattermost#12700) ...

rfoyard · 2019-11-04T13:18:26Z

@hanzei I created a draft PR in the demo plugin.
It creates a new /interactive command with an Interactive Button.
When clicked, a new post is created displaying the JSON generated by the request (user_id, user_name, channel_id, channel_name, team_id, team_domain, ...).
Do you need something more specific ?

hanzei · 2019-11-04T13:30:38Z

That's totally fine. 👍 I will review your PR soon.

Include user_name, team_domain and channel_name when clicking an inte…

50e74f0

…ractive button

rfoyard marked this pull request as ready for review October 9, 2019 13:28

Fixed api4's TestPostActionCookies test

e07d2c2

jasonblais added the Hacktoberfest label Oct 10, 2019

jasonblais requested a review from aaronrothschild October 10, 2019 18:24

jasonblais added the 1: PM Review Requires review by a product manager label Oct 10, 2019

jasonblais assigned aaronrothschild Oct 10, 2019

aaronrothschild approved these changes Oct 10, 2019

View reviewed changes

aaronrothschild removed the 1: PM Review Requires review by a product manager label Oct 10, 2019

hanzei added 2: Dev Review Requires review by a developer 3: QA Review Requires review by a QA tester. May occur at the same time as Dev Review labels Oct 15, 2019

hanzei requested review from lieut-data and hanzei and removed request for lieut-data October 15, 2019 09:49

hanzei unassigned aaronrothschild Oct 15, 2019

hanzei added this to the v5.18.0 milestone Oct 15, 2019

hanzei requested changes Oct 17, 2019

View reviewed changes

Moved team database request to improve concurrency.

30e8548

Renamed TeamDomain to TeamName.

lindy65 requested a review from saturninoabril October 28, 2019 07:24

saturninoabril approved these changes Oct 29, 2019

View reviewed changes

saturninoabril added QA/Review Done QA review is completed but other reviews are outstanding (exception to usual process) and removed 3: QA Review Requires review by a QA tester. May occur at the same time as Dev Review labels Oct 29, 2019

hanzei self-requested a review October 29, 2019 21:49

Merge remote-tracking branch 'origin/master' into MM-10164

1e68699

hanzei reviewed Oct 30, 2019

View reviewed changes

hanzei requested a review from levb October 30, 2019 12:35

levb suggested changes Oct 30, 2019

View reviewed changes

app/integration_action.go Outdated Show resolved Hide resolved

app/integration_action.go Outdated Show resolved Hide resolved

Further optimizations to database requests

1b85d42

hanzei requested review from hanzei and levb October 30, 2019 16:16

hanzei approved these changes Oct 30, 2019

View reviewed changes

levb suggested changes Oct 30, 2019

View reviewed changes

app/integration_action.go Outdated Show resolved Hide resolved

Removed useless goroutine when fetching channel

a51912b

levb approved these changes Oct 30, 2019

View reviewed changes

levb added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a developer labels Oct 30, 2019

levb merged commit 4111ffd into mattermost:master Oct 30, 2019

rfoyard deleted the MM-10164 branch October 30, 2019 17:46

rfoyard mentioned this pull request Nov 4, 2019

Added /interactive command to demo interactive button functionality mattermost/mattermost-plugin-demo#73

Merged

hanzei added the Demo Plugin Changes/Done Required changes to the demo plugin have been submitted label Nov 14, 2019

amyblais added Changelog/Done Required changelog entry has been written Docs/Needed Requires documentation labels Nov 18, 2019

amyblais assigned aaronrothschild Nov 21, 2019

aaronrothschild removed their assignment Feb 13, 2020

aaronrothschild removed the Docs/Needed Requires documentation label Feb 13, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Include extra metadata when clicking an interactive button #12697

Include extra metadata when clicking an interactive button #12697

rfoyard commented Oct 9, 2019

aaronrothschild left a comment

hanzei left a comment

hanzei Oct 17, 2019

rfoyard Oct 18, 2019

hanzei Oct 30, 2019

rfoyard Oct 30, 2019

rfoyard Oct 30, 2019

saturninoabril left a comment

hanzei left a comment

levb left a comment

hanzei commented Oct 30, 2019

levb commented Oct 30, 2019

hanzei left a comment

levb commented Oct 30, 2019

rfoyard commented Oct 30, 2019

hanzei commented Nov 1, 2019

rfoyard commented Nov 1, 2019

rfoyard commented Nov 4, 2019

hanzei commented Nov 4, 2019

Include extra metadata when clicking an interactive button #12697

Include extra metadata when clicking an interactive button #12697

Conversation

rfoyard commented Oct 9, 2019

Summary

Ticket Link

aaronrothschild left a comment

Choose a reason for hiding this comment

hanzei left a comment

Choose a reason for hiding this comment

hanzei Oct 17, 2019

Choose a reason for hiding this comment

rfoyard Oct 18, 2019

Choose a reason for hiding this comment

hanzei Oct 30, 2019

Choose a reason for hiding this comment

rfoyard Oct 30, 2019

Choose a reason for hiding this comment

rfoyard Oct 30, 2019

Choose a reason for hiding this comment

saturninoabril left a comment

Choose a reason for hiding this comment

hanzei left a comment

Choose a reason for hiding this comment

levb left a comment

Choose a reason for hiding this comment

hanzei commented Oct 30, 2019

levb commented Oct 30, 2019

hanzei left a comment

Choose a reason for hiding this comment

levb commented Oct 30, 2019

rfoyard commented Oct 30, 2019

hanzei commented Nov 1, 2019

rfoyard commented Nov 1, 2019

rfoyard commented Nov 4, 2019

hanzei commented Nov 4, 2019