Create slash commands to allow each MM user to connect to their twitter account. by chetanyakan · Pull Request #9 · mattermost-community/mattermost-plugin-twitter

chetanyakan · 2020-10-18T13:05:15Z

Summary

Create Slash commands to connect and disconnect.
Use twitter OAuth1.0a to allow each user to connect their twitter accounts.

Ticket Link

#6

…er account.

chetanyakan · 2020-10-18T13:10:15Z

Screenshots

Slash command responses.

OAuth success page.

Slash command autocomplete.

levb · 2020-10-19T13:19:12Z

@chetanyakan @larkox This seems like a perfect application for https://github.com/mattermost/mattermost-plugin-api/tree/master/experimental/oauther. Thoughts?

aaronrothschild

Thanks for your contribution @chetanyakan ! I'll defer to @larkox and @levb on technical implementation but UI so far looks good!

chetanyakan · 2020-10-19T14:42:22Z

@levb The implementation in experminetal/oather seems to be for OAuth 2.

Twitter uses OAuth 1.0a to allow access to private account information or perform a Twitter action on behalf of a Twitter account and OAuth 2 to access information publicly available on Twitter.

Refer: https://developer.twitter.com/en/docs/authentication/overview

My implementation here is for OAuth 1.0a and is inspired from the of Jira-server OAuth1 implementation.

levb · 2020-10-19T18:47:36Z

Thx for clarifying, I commented before I read the code in detail. Let me review now.

chetanyakan · 2020-10-28T16:31:09Z

@levb @larkox Let me know if you need any help to understand/test this.

larkox

Awesome work! There are a few small comments, but apart of that, it looks fantastic. I may even take some ideas for other plugins 😄 Thanks!

larkox · 2020-10-29T09:18:29Z

+	if err != nil {
+		c.api.LogError("twitterLoginCallback: Failed to verify twitter credentials for connected user.", "Error", err.Error())
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+		return
+	}
+	if resp != nil {
+		defer resp.Body.Close()
+	}


Should we check the StatusCode of the response, or that is handled somewhere else?

larkox · 2020-10-29T09:38:29Z

+		ChannelId: channelID,
+		Message:   message,
+	}
+	post.SetProps(model.StringInterface{


As far as I know, this is not needed. You can just put the UserID in the post as the bot id, and it will work the same way, right? Or is there any reason to do it this way?

larkox · 2020-10-29T09:39:45Z

+        "default": "LwAgBVckO0EQlI2zka55ZFAN16xMRY8T0zvzVI4KbVlCK6tBN1"
+      },
+      {
+        "key": "EncryptionKey",


Are we using this? In theory, there is no need to encrypt the tokens, so you can get rid of this and the logic around the encryption.

I agree with @larkox. There doesn't seem to be a need to encrypt them.

hanzei

Thanks for the great work 👍

Looking forward on what can be done with this plugin.

hanzei · 2020-11-04T12:44:04Z

 # Jetbrains
 .idea/
+
+vendor


When using this repo outside $GOPATH, we need to vendor the dependencies using go mod vendor for IDE auto-complete.

Should I add this to my local .gitignore file?

Yes, please.

This seems unresolved

hanzei · 2020-11-04T12:44:52Z

 require (
+	github.com/dghubble/go-twitter v0.0.0-20201011215211-4b180d0cc78d
+	github.com/dghubble/oauth1 v0.6.0
+	github.com/google/go-cmp v0.5.1 // indirect


Nit pick: Please remove the indirect dependencies.

hanzei · 2020-11-04T12:45:43Z

+    "description": "A Matermost plugin to connect to Twitter.",
    "version": "0.1.0",
-    "min_server_version": "5.12.0",
+    "min_server_version": "5.27.0",


What 5.27.0 feature does this plugin use?

hanzei · 2020-11-04T12:46:26Z

+func GetCommand(iconData string) *model.Command {
+	return &model.Command{
+		Trigger:              "twitter",
+		DisplayName:          "Twitter",


Plugin don't need a DisplayName. Mind removing this?

hanzei · 2020-11-04T12:47:01Z

+
+func twitterConnect(ctx *Context, args ...string) (*model.CommandResponse, *model.AppError) {
+	// If the user is already connected to twitter.
+	if twUser, err := ctx.store.GetTwitterUser(ctx.UserId); err == nil && twUser != nil {


Should we log the error?

Added a log in the store method before return.

hanzei · 2020-11-04T12:57:01Z

+	return nil
+}
+
+func (s Store) StoreTTL(key string, data []byte, ttlSeconds int64) error {


Does this need to get exported?

hanzei · 2020-11-04T12:57:10Z

+	return nil
+}
+
+func (s Store) StoreWithOptions(key string, value []byte, opts model.PluginKVSetOptions) (bool, error) {


Does this need to get exported?

hanzei · 2020-11-04T12:57:13Z

+	return success, nil
+}
+
+func (s Store) Delete(key string) error {


Does this need to get exported?

hanzei · 2020-11-04T12:57:36Z

+	if len(data) == 0 {
+		return ErrNotFound
+
+		// TODO: Remove this


Should that todo get resolved?

hanzei · 2020-11-04T12:59:59Z

+
+// Ensure makes sure the initial value for a key is set to the value provided, if it does not already exists
+// Returns the value set for the key in kv-store and error
+func (s *Store) Ensure(key string, newValue []byte) ([]byte, error) {


Is this used somewhere?

hanzei · 2020-11-10T15:35:32Z

/update-branch

hanzei · 2020-11-10T15:46:09Z

@chetanyakan Sorry for tinkering with your branch. CI is working now: https://app.circleci.com/pipelines/github/mattermost/mattermost-plugin-twitter/2/workflows/1b7a6e98-36df-414d-98e2-19f50e4345b1/jobs/6

chetanyakan · 2020-11-10T16:00:10Z

Thanks, @hanzei! I'll pull these changes.

hanzei

Thanks for updating the PR! A few things still seem unresolved .

hanzei · 2021-07-15T12:18:44Z

 # Jetbrains
 .idea/
+
+vendor


This seems unresolved

hanzei · 2021-07-15T12:19:42Z

+	return data, nil
+}
+
+func (s Store) Store(key string, data []byte) error {


This also seems unresolved.

Create slash commands to allow each MM user to connect to their twitt…

a60dc6a

…er account.

chetanyakan force-pushed the GH-6 branch from 43eb52f to a60dc6a Compare October 18, 2020 13:07

hanzei requested a review from aaronrothschild October 19, 2020 11:55

hanzei added the 1: PM Review Requires review by a product manager label Oct 19, 2020

hanzei requested a review from levb October 19, 2020 11:55

hanzei added the 2: Dev Review Requires review by a core committer label Oct 19, 2020

levb requested a review from larkox October 19, 2020 13:19

aaronrothschild requested review from aaronrothschild and removed request for aaronrothschild October 19, 2020 13:46

aaronrothschild approved these changes Oct 19, 2020

View reviewed changes

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

Clean up assets and update logs

9d2e325

larkox reviewed Oct 29, 2020

View reviewed changes

hanzei requested changes Nov 4, 2020

View reviewed changes

hanzei removed the request for review from levb November 4, 2020 13:04

hanzei added 3 commits November 10, 2020 16:36

go mod tidy

b83c175

Trigger CI

1ec099d

For real now

8168845

Feedback changes

8458f46

hanzei self-requested a review December 16, 2020 14:54

hanzei requested changes Jul 15, 2021

View reviewed changes

Update .gitignore

7d9bfcc

Update plugin.json

d9e22c4

hanzei added the Awaiting Submitter Action Blocked on the author label Aug 19, 2021

hanzei mentioned this pull request Oct 24, 2022

Use ReactDOM webpack external #10

Merged

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

Conversation

chetanyakan commented Oct 18, 2020

Summary

Ticket Link

Uh oh!

chetanyakan commented Oct 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Screenshots

Uh oh!

levb commented Oct 19, 2020

Uh oh!

aaronrothschild left a comment

Choose a reason for hiding this comment

Uh oh!

chetanyakan commented Oct 19, 2020

Uh oh!

levb commented Oct 19, 2020

Uh oh!

chetanyakan commented Oct 28, 2020

Uh oh!

larkox left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hanzei left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hanzei commented Nov 10, 2020

Uh oh!

hanzei commented Nov 10, 2020

Uh oh!

chetanyakan commented Nov 10, 2020

Uh oh!

hanzei left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

chetanyakan commented Oct 18, 2020 •

edited

Loading