From bcd76b18263a8cea3295fee8dd04e582a86fcded Mon Sep 17 00:00:00 2001 From: Nityanand Rai Date: Wed, 8 Feb 2023 12:40:45 +0530 Subject: [PATCH 1/5] [MI-2736]: Review fixes done 1. Improved code readability --- server/plugin/api.go | 2 +- webapp/src/client/client.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/plugin/api.go b/server/plugin/api.go index 913e81e16..d6f868422 100644 --- a/server/plugin/api.go +++ b/server/plugin/api.go @@ -97,7 +97,7 @@ func (p *Plugin) initializeAPI() { apiRouter.HandleFunc("/closeorreopenissue", p.checkAuth(p.attachUserContext(p.closeOrReopenIssue), ResponseTypePlain)).Methods(http.MethodPost) apiRouter.HandleFunc("/updateissue", p.checkAuth(p.attachUserContext(p.updateIssue), ResponseTypePlain)).Methods(http.MethodPost) apiRouter.HandleFunc("/editissuemodal", p.checkAuth(p.attachUserContext(p.openIssueEditModal), ResponseTypePlain)).Methods(http.MethodPost) - apiRouter.HandleFunc("/closereopenissuemodal", p.checkAuth(p.attachUserContext(p.openCloseOrReopenIssueModal), ResponseTypePlain)).Methods(http.MethodPost) + apiRouter.HandleFunc("/close_reopen_issue_modal", p.checkAuth(p.attachUserContext(p.openCloseOrReopenIssueModal), ResponseTypePlain)).Methods(http.MethodPost) apiRouter.HandleFunc("/attachcommentissuemodal", p.checkAuth(p.attachUserContext(p.openAttachCommentIssueModal), ResponseTypePlain)).Methods(http.MethodPost) apiRouter.HandleFunc("/createissuecomment", p.checkAuth(p.attachUserContext(p.createIssueComment), ResponseTypePlain)).Methods(http.MethodPost) apiRouter.HandleFunc("/mentions", p.checkAuth(p.attachUserContext(p.getMentions), ResponseTypePlain)).Methods(http.MethodGet) diff --git a/webapp/src/client/client.js b/webapp/src/client/client.js index 5ebcdb510..fb331f53c 100644 --- a/webapp/src/client/client.js +++ b/webapp/src/client/client.js @@ -12,7 +12,7 @@ export default class Client { } closeOrReopenIssueModal = async (payload) => { - return this.doPost(`${this.url}/closereopenissuemodal`, payload); + return this.doPost(`${this.url}/close_reopen_issue_modal`, payload); } attachCommentIssueModal = async (payload) => { From 048321a0fcad4e1e0bd5865d46562e5d6224d023 Mon Sep 17 00:00:00 2001 From: Nityanand Rai Date: Wed, 8 Feb 2023 15:31:32 +0530 Subject: [PATCH 2/5] [MI-2736]: Review fixes done 1. Fixed linting errors --- server/constants/constants.go | 6 +++--- server/plugin/api.go | 7 ++++--- server/plugin/command.go | 3 ++- server/plugin/mm_34646_token_refresh.go | 1 + server/plugin/plugin.go | 5 +++-- server/plugin/utils.go | 5 +++-- server/plugin/webhook.go | 5 +++-- 7 files changed, 19 insertions(+), 13 deletions(-) diff --git a/server/constants/constants.go b/server/constants/constants.go index 6e832b9be..c2e9ab504 100644 --- a/server/constants/constants.go +++ b/server/constants/constants.go @@ -13,7 +13,7 @@ const ( OwnerQueryParam = "owner" RepoQueryParam = "repo" NumberQueryParam = "number" - PostIdQueryParam = "postId" + PostIDQueryParam = "postId" IssueStatus = "status" AssigneesForProps = "assignees" @@ -21,7 +21,7 @@ const ( DescriptionForProps = "description" TitleForProps = "title" IssueNumberForProps = "issue_number" - IssueUrlForProps = "issue_url" + IssueURLForProps = "issue_url" RepoOwnerForProps = "repo_owner" RepoNameForProps = "repo_name" @@ -33,7 +33,7 @@ const ( IssueClose = "closed" IssueOpen = "open" - //Actions of webhook events + // Actions of webhook events ActionOpened = "opened" ActionClosed = "closed" ActionReopened = "reopened" diff --git a/server/plugin/api.go b/server/plugin/api.go index d6f868422..4ecbe06b7 100644 --- a/server/plugin/api.go +++ b/server/plugin/api.go @@ -16,10 +16,11 @@ import ( "github.com/pkg/errors" "golang.org/x/oauth2" - "github.com/mattermost/mattermost-plugin-api/experimental/bot/logger" - "github.com/mattermost/mattermost-plugin-api/experimental/flow" "github.com/mattermost/mattermost-plugin-github/server/constants" "github.com/mattermost/mattermost-plugin-github/server/serializer" + + "github.com/mattermost/mattermost-plugin-api/experimental/bot/logger" + "github.com/mattermost/mattermost-plugin-api/experimental/flow" "github.com/mattermost/mattermost-server/v6/model" "github.com/mattermost/mattermost-server/v6/plugin" ) @@ -1430,7 +1431,7 @@ func (p *Plugin) updateIssue(c *serializer.UserContext, w http.ResponseWriter, r return } - p.updatePost(post, issue, w) + p.updatePost(issue, w) p.writeJSON(w, result) } diff --git a/server/plugin/command.go b/server/plugin/command.go index e0e38de75..f95e925de 100644 --- a/server/plugin/command.go +++ b/server/plugin/command.go @@ -6,9 +6,10 @@ import ( "strings" "unicode" - "github.com/mattermost/mattermost-plugin-api/experimental/command" "github.com/mattermost/mattermost-plugin-github/server/constants" "github.com/mattermost/mattermost-plugin-github/server/serializer" + + "github.com/mattermost/mattermost-plugin-api/experimental/command" "github.com/mattermost/mattermost-server/v6/model" "github.com/mattermost/mattermost-server/v6/plugin" "github.com/pkg/errors" diff --git a/server/plugin/mm_34646_token_refresh.go b/server/plugin/mm_34646_token_refresh.go index ed931e710..adec22097 100644 --- a/server/plugin/mm_34646_token_refresh.go +++ b/server/plugin/mm_34646_token_refresh.go @@ -9,6 +9,7 @@ import ( "github.com/pkg/errors" "github.com/mattermost/mattermost-plugin-api/cluster" + "github.com/mattermost/mattermost-plugin-github/server/serializer" ) diff --git a/server/plugin/plugin.go b/server/plugin/plugin.go index b059c5a7a..0414be207 100644 --- a/server/plugin/plugin.go +++ b/server/plugin/plugin.go @@ -17,13 +17,14 @@ import ( pluginapi "github.com/mattermost/mattermost-plugin-api" "github.com/mattermost/mattermost-plugin-api/experimental/bot/poster" "github.com/mattermost/mattermost-plugin-api/experimental/telemetry" - "github.com/mattermost/mattermost-plugin-github/server/constants" - "github.com/mattermost/mattermost-plugin-github/server/serializer" "github.com/mattermost/mattermost-server/v6/model" "github.com/mattermost/mattermost-server/v6/plugin" "github.com/pkg/errors" "golang.org/x/oauth2" + "github.com/mattermost/mattermost-plugin-github/server/constants" + "github.com/mattermost/mattermost-plugin-github/server/serializer" + root "github.com/mattermost/mattermost-plugin-github" ) diff --git a/server/plugin/utils.go b/server/plugin/utils.go index a7628cb4b..8b5817827 100644 --- a/server/plugin/utils.go +++ b/server/plugin/utils.go @@ -16,9 +16,10 @@ import ( "strings" "unicode" - "github.com/google/go-github/v48/github" "github.com/mattermost/mattermost-plugin-github/server/constants" "github.com/mattermost/mattermost-plugin-github/server/serializer" + + "github.com/google/go-github/v48/github" "github.com/mattermost/mattermost-server/v6/model" "github.com/pkg/errors" ) @@ -370,7 +371,7 @@ func (p *Plugin) validateIssueRequestForUpdation(issue *serializer.UpdateIssueRe return true } -func (p *Plugin) updatePost(post *model.Post, issue *serializer.UpdateIssueRequest, w http.ResponseWriter) { +func (p *Plugin) updatePost(issue *serializer.UpdateIssueRequest, w http.ResponseWriter) { post, appErr := p.API.GetPost(issue.PostID) if appErr != nil { p.writeAPIError(w, &serializer.APIErrorResponse{ID: "", Message: fmt.Sprintf("failed to load the post %s", issue.PostID), StatusCode: http.StatusInternalServerError}) diff --git a/server/plugin/webhook.go b/server/plugin/webhook.go index ed9196556..c183d348f 100644 --- a/server/plugin/webhook.go +++ b/server/plugin/webhook.go @@ -13,8 +13,9 @@ import ( "sync" "time" - "github.com/google/go-github/v48/github" "github.com/mattermost/mattermost-plugin-github/server/constants" + + "github.com/google/go-github/v48/github" "github.com/mattermost/mattermost-server/v6/model" "github.com/microcosm-cc/bluemonday" ) @@ -556,7 +557,7 @@ func (p *Plugin) postIssueEvent(event *github.IssuesEvent) { post.Type = "custom_git_issue" post.Props = map[string]interface{}{ constants.TitleForProps: *issue.Title, - constants.IssueUrlForProps: *issue.HTMLURL, + constants.IssueURLForProps: *issue.HTMLURL, constants.IssueNumberForProps: *issue.Number, constants.DescriptionForProps: description, constants.AssigneesForProps: assignees, From 5c8d8f796c00003c8fa701e8a18742a7bcb3c75e Mon Sep 17 00:00:00 2001 From: Nityanand Rai Date: Thu, 9 Feb 2023 13:33:21 +0530 Subject: [PATCH 3/5] [MI-2736]: Review fixes done 1. Fixed linting error --- go.sum | 2 -- 1 file changed, 2 deletions(-) diff --git a/go.sum b/go.sum index 76bea24aa..372493be0 100644 --- a/go.sum +++ b/go.sum @@ -641,8 +641,6 @@ github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeN github.com/google/go-github v17.0.0+incompatible h1:N0LgJ1j65A7kfXrZnUDaYCs/Sf4rEjNlfyDHW9dolSY= github.com/google/go-github v17.0.0+incompatible/go.mod h1:zLgOLi98H3fifZn+44m+umXrS52loVEgC2AApnigrVQ= github.com/google/go-github/v35 v35.2.0/go.mod h1:s0515YVTI+IMrDoy9Y4pHt9ShGpzHvHO8rZ7L7acgvs= -github.com/google/go-github/v41 v41.0.0 h1:HseJrM2JFf2vfiZJ8anY2hqBjdfY1Vlj/K27ueww4gg= -github.com/google/go-github/v41 v41.0.0/go.mod h1:XgmCA5H323A9rtgExdTcnDkcqp6S30AVACCBDOonIxg= github.com/google/go-github/v48 v48.2.0 h1:68puzySE6WqUY9KWmpOsDEQfDZsso98rT6pZcz9HqcE= github.com/google/go-github/v48 v48.2.0/go.mod h1:dDlehKBDo850ZPvCTK0sEqTCVWcrGl2LcDiajkYi89Y= github.com/google/go-querystring v1.0.0/go.mod h1:odCYkC5MyYFN7vkCjXpyrEuKhc/BUO6wN/zVPAxq5ck= From 211089f0b271e7428e2ef48090c6d6f80f78c42b Mon Sep 17 00:00:00 2001 From: Nityanand Rai Date: Fri, 10 Feb 2023 14:39:38 +0530 Subject: [PATCH 4/5] [MI-2736]: Review fixes done 1. Improved code readability --- server/plugin/api.go | 8 ++++---- webapp/src/client/client.js | 8 ++++---- .../create_update_issue/create_update_issue.jsx | 13 +++++++------ 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/server/plugin/api.go b/server/plugin/api.go index 4ecbe06b7..f79955de9 100644 --- a/server/plugin/api.go +++ b/server/plugin/api.go @@ -95,11 +95,11 @@ func (p *Plugin) initializeAPI() { apiRouter.HandleFunc("/searchissues", p.checkAuth(p.attachUserContext(p.searchIssues), ResponseTypePlain)).Methods(http.MethodGet) apiRouter.HandleFunc("/yourassignments", p.checkAuth(p.attachUserContext(p.getYourAssignments), ResponseTypePlain)).Methods(http.MethodGet) apiRouter.HandleFunc("/createissue", p.checkAuth(p.attachUserContext(p.createIssue), ResponseTypePlain)).Methods(http.MethodPost) - apiRouter.HandleFunc("/closeorreopenissue", p.checkAuth(p.attachUserContext(p.closeOrReopenIssue), ResponseTypePlain)).Methods(http.MethodPost) - apiRouter.HandleFunc("/updateissue", p.checkAuth(p.attachUserContext(p.updateIssue), ResponseTypePlain)).Methods(http.MethodPost) - apiRouter.HandleFunc("/editissuemodal", p.checkAuth(p.attachUserContext(p.openIssueEditModal), ResponseTypePlain)).Methods(http.MethodPost) + apiRouter.HandleFunc("/close_or_reopen_issue", p.checkAuth(p.attachUserContext(p.closeOrReopenIssue), ResponseTypePlain)).Methods(http.MethodPost) + apiRouter.HandleFunc("/update_issue", p.checkAuth(p.attachUserContext(p.updateIssue), ResponseTypePlain)).Methods(http.MethodPost) + apiRouter.HandleFunc("/edit_issue_modal", p.checkAuth(p.attachUserContext(p.openIssueEditModal), ResponseTypePlain)).Methods(http.MethodPost) apiRouter.HandleFunc("/close_reopen_issue_modal", p.checkAuth(p.attachUserContext(p.openCloseOrReopenIssueModal), ResponseTypePlain)).Methods(http.MethodPost) - apiRouter.HandleFunc("/attachcommentissuemodal", p.checkAuth(p.attachUserContext(p.openAttachCommentIssueModal), ResponseTypePlain)).Methods(http.MethodPost) + apiRouter.HandleFunc("/attach_comment_issue_modal", p.checkAuth(p.attachUserContext(p.openAttachCommentIssueModal), ResponseTypePlain)).Methods(http.MethodPost) apiRouter.HandleFunc("/createissuecomment", p.checkAuth(p.attachUserContext(p.createIssueComment), ResponseTypePlain)).Methods(http.MethodPost) apiRouter.HandleFunc("/mentions", p.checkAuth(p.attachUserContext(p.getMentions), ResponseTypePlain)).Methods(http.MethodGet) apiRouter.HandleFunc("/unreads", p.checkAuth(p.attachUserContext(p.getUnreads), ResponseTypePlain)).Methods(http.MethodGet) diff --git a/webapp/src/client/client.js b/webapp/src/client/client.js index fb331f53c..e95d0f25e 100644 --- a/webapp/src/client/client.js +++ b/webapp/src/client/client.js @@ -8,7 +8,7 @@ import {id as pluginId} from '../manifest'; export default class Client { editIssueModal = async (payload) => { - return this.doPost(`${this.url}/editissuemodal`, payload); + return this.doPost(`${this.url}/edit_issue_modal`, payload); } closeOrReopenIssueModal = async (payload) => { @@ -16,7 +16,7 @@ export default class Client { } attachCommentIssueModal = async (payload) => { - return this.doPost(`${this.url}/attachcommentissuemodal`, payload); + return this.doPost(`${this.url}/attach_comment_issue_modal`, payload); } setServerRoute(url) { @@ -76,11 +76,11 @@ export default class Client { } closeOrReopenIssue = async (payload) => { - return this.doPost(`${this.url}/closeorreopenissue`, payload); + return this.doPost(`${this.url}/close_or_reopen_issue`, payload); } updateIssue = async (payload) => { - return this.doPost(`${this.url}/updateissue`, payload); + return this.doPost(`${this.url}/update_issue`, payload); } searchIssues = async (searchTerm) => { diff --git a/webapp/src/components/modals/create_update_issue/create_update_issue.jsx b/webapp/src/components/modals/create_update_issue/create_update_issue.jsx index b341a5fd5..2a8bbab83 100644 --- a/webapp/src/components/modals/create_update_issue/create_update_issue.jsx +++ b/webapp/src/components/modals/create_update_issue/create_update_issue.jsx @@ -64,8 +64,10 @@ export default class CreateOrUpdateIssueModal extends PureComponent { value: milestone_number, label: milestone_title, }, + repo: { + name: repo_full_name, + }, issueDescription: description, - repo: repo_full_name, issueTitle: title.substring(0, MAX_TITLE_LENGTH)}); } } @@ -151,29 +153,28 @@ export default class CreateOrUpdateIssueModal extends PureComponent { handleIssueDescriptionChange = (issueDescription) => this.setState({issueDescription}); renderIssueAttributeSelectors = () => { - if (!this.state.repo || (this.state.repo.permissions && !this.state.repo.permissions.push)) { + if (!this.state.repo || !this.state.repo.name || (this.state.repo.permissions && !this.state.repo.permissions.push)) { return null; } - const repoName = this.state.repo.name ?? this.state.repo; return ( <> Date: Fri, 10 Feb 2023 18:20:36 +0530 Subject: [PATCH 5/5] [MI-2736]: Review fixes done 1. Changed the case of few endpoints to snake case --- server/plugin/api.go | 12 ++++++------ webapp/src/client/client.js | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/server/plugin/api.go b/server/plugin/api.go index f79955de9..babcfdfb8 100644 --- a/server/plugin/api.go +++ b/server/plugin/api.go @@ -90,17 +90,17 @@ func (p *Plugin) initializeAPI() { apiRouter.HandleFunc("/user", p.checkAuth(p.attachContext(p.getGitHubUser), ResponseTypeJSON)).Methods(http.MethodPost) apiRouter.HandleFunc("/todo", p.checkAuth(p.attachUserContext(p.postToDo), ResponseTypeJSON)).Methods(http.MethodPost) apiRouter.HandleFunc("/reviews", p.checkAuth(p.attachUserContext(p.getReviews), ResponseTypePlain)).Methods(http.MethodGet) - apiRouter.HandleFunc("/yourprs", p.checkAuth(p.attachUserContext(p.getYourPrs), ResponseTypePlain)).Methods(http.MethodGet) - apiRouter.HandleFunc("/prsdetails", p.checkAuth(p.attachUserContext(p.getPrsDetails), ResponseTypePlain)).Methods(http.MethodPost) - apiRouter.HandleFunc("/searchissues", p.checkAuth(p.attachUserContext(p.searchIssues), ResponseTypePlain)).Methods(http.MethodGet) - apiRouter.HandleFunc("/yourassignments", p.checkAuth(p.attachUserContext(p.getYourAssignments), ResponseTypePlain)).Methods(http.MethodGet) - apiRouter.HandleFunc("/createissue", p.checkAuth(p.attachUserContext(p.createIssue), ResponseTypePlain)).Methods(http.MethodPost) + apiRouter.HandleFunc("/your_prs", p.checkAuth(p.attachUserContext(p.getYourPrs), ResponseTypePlain)).Methods(http.MethodGet) + apiRouter.HandleFunc("/prs_details", p.checkAuth(p.attachUserContext(p.getPrsDetails), ResponseTypePlain)).Methods(http.MethodPost) + apiRouter.HandleFunc("/search_issues", p.checkAuth(p.attachUserContext(p.searchIssues), ResponseTypePlain)).Methods(http.MethodGet) + apiRouter.HandleFunc("/your_assignments", p.checkAuth(p.attachUserContext(p.getYourAssignments), ResponseTypePlain)).Methods(http.MethodGet) + apiRouter.HandleFunc("/create_issue", p.checkAuth(p.attachUserContext(p.createIssue), ResponseTypePlain)).Methods(http.MethodPost) apiRouter.HandleFunc("/close_or_reopen_issue", p.checkAuth(p.attachUserContext(p.closeOrReopenIssue), ResponseTypePlain)).Methods(http.MethodPost) apiRouter.HandleFunc("/update_issue", p.checkAuth(p.attachUserContext(p.updateIssue), ResponseTypePlain)).Methods(http.MethodPost) apiRouter.HandleFunc("/edit_issue_modal", p.checkAuth(p.attachUserContext(p.openIssueEditModal), ResponseTypePlain)).Methods(http.MethodPost) apiRouter.HandleFunc("/close_reopen_issue_modal", p.checkAuth(p.attachUserContext(p.openCloseOrReopenIssueModal), ResponseTypePlain)).Methods(http.MethodPost) apiRouter.HandleFunc("/attach_comment_issue_modal", p.checkAuth(p.attachUserContext(p.openAttachCommentIssueModal), ResponseTypePlain)).Methods(http.MethodPost) - apiRouter.HandleFunc("/createissuecomment", p.checkAuth(p.attachUserContext(p.createIssueComment), ResponseTypePlain)).Methods(http.MethodPost) + apiRouter.HandleFunc("/create_issue_comment", p.checkAuth(p.attachUserContext(p.createIssueComment), ResponseTypePlain)).Methods(http.MethodPost) apiRouter.HandleFunc("/mentions", p.checkAuth(p.attachUserContext(p.getMentions), ResponseTypePlain)).Methods(http.MethodGet) apiRouter.HandleFunc("/unreads", p.checkAuth(p.attachUserContext(p.getUnreads), ResponseTypePlain)).Methods(http.MethodGet) apiRouter.HandleFunc("/labels", p.checkAuth(p.attachUserContext(p.getLabels), ResponseTypePlain)).Methods(http.MethodGet) diff --git a/webapp/src/client/client.js b/webapp/src/client/client.js index e95d0f25e..9b26d88e6 100644 --- a/webapp/src/client/client.js +++ b/webapp/src/client/client.js @@ -32,15 +32,15 @@ export default class Client { } getYourPrs = async () => { - return this.doGet(`${this.url}/yourprs`); + return this.doGet(`${this.url}/your_prs`); } getPrsDetails = async (prList) => { - return this.doPost(`${this.url}/prsdetails`, prList); + return this.doPost(`${this.url}/prs_details`, prList); } getYourAssignments = async () => { - return this.doGet(`${this.url}/yourassignments`); + return this.doGet(`${this.url}/your_assignments`); } getMentions = async () => { @@ -72,7 +72,7 @@ export default class Client { } createIssue = async (payload) => { - return this.doPost(`${this.url}/createissue`, payload); + return this.doPost(`${this.url}/create_issue`, payload); } closeOrReopenIssue = async (payload) => { @@ -84,11 +84,11 @@ export default class Client { } searchIssues = async (searchTerm) => { - return this.doGet(`${this.url}/searchissues?term=${searchTerm}`); + return this.doGet(`${this.url}/search_issues?term=${searchTerm}`); } attachCommentToIssue = async (payload) => { - return this.doPost(`${this.url}/createissuecomment`, payload); + return this.doPost(`${this.url}/create_issue_comment`, payload); } getIssue = async (owner, repo, issueNumber) => {