From bcd76b18263a8cea3295fee8dd04e582a86fcded Mon Sep 17 00:00:00 2001 From: Nityanand Rai Date: Wed, 8 Feb 2023 12:40:45 +0530 Subject: [PATCH 01/10] [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 02/10] [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 03/10] [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 04/10] [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 05/10] [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) => { From 53d7770d40ded2a6a3fdd048c6ea59d98651b853 Mon Sep 17 00:00:00 2001 From: Nityanand Rai Date: Tue, 7 Mar 2023 10:45:51 +0530 Subject: [PATCH 06/10] [MI-2814]: Review fixes done of github PR #636 --- server/plugin/api.go | 102 +++--------------- server/plugin/plugin.go | 10 +- webapp/src/actions/index.js | 40 +------ webapp/src/client/client.js | 12 +-- webapp/src/components/github_issue/index.tsx | 9 +- .../attach_comment_to_issue.jsx | 16 +-- .../modals/close_reopen_issue/index.tsx | 6 +- .../create_update_issue.jsx | 59 ++++++---- .../modals/create_update_issue/index.js | 3 +- webapp/src/index.js | 6 +- webapp/src/websocket/index.js | 20 ---- 11 files changed, 82 insertions(+), 201 deletions(-) diff --git a/server/plugin/api.go b/server/plugin/api.go index babcfdfb8..3c96d6aef 100644 --- a/server/plugin/api.go +++ b/server/plugin/api.go @@ -97,9 +97,7 @@ func (p *Plugin) initializeAPI() { 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("/issue_info", p.checkAuth(p.attachUserContext(p.getIssueInfo), 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) @@ -959,72 +957,7 @@ func (p *Plugin) updateSettings(c *serializer.UserContext, w http.ResponseWriter p.writeJSON(w, info.Settings) } -func (p *Plugin) openAttachCommentIssueModal(c *serializer.UserContext, w http.ResponseWriter, r *http.Request) { - req := &serializer.OpenCreateCommentOrEditIssueModalRequestBody{} - if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - c.Log.WithError(err).Warnf("Error decoding the JSON body") - p.writeAPIError(w, &serializer.APIErrorResponse{ID: "", Message: "Please provide a valid JSON object.", StatusCode: http.StatusBadRequest}) - return - } - - userID := r.Header.Get(constants.HeaderMattermostUserID) - post, appErr := p.API.GetPost(req.PostID) - if appErr != nil { - p.writeAPIError(w, &serializer.APIErrorResponse{ID: "", Message: fmt.Sprintf("failed to load the post %s", req.PostID), StatusCode: http.StatusInternalServerError}) - return - } - if post == nil { - p.writeAPIError(w, &serializer.APIErrorResponse{ID: "", Message: fmt.Sprintf("failed to load the post %s : not found", req.PostID), StatusCode: http.StatusNotFound}) - return - } - - p.API.PublishWebSocketEvent( - wsEventAttachCommentToIssue, - map[string]interface{}{ - "postId": post.Id, - "owner": req.RepoOwner, - "repo": req.RepoName, - "number": req.IssueNumber, - }, - &model.WebsocketBroadcast{UserId: userID}, - ) -} - -func (p *Plugin) openCloseOrReopenIssueModal(c *serializer.UserContext, w http.ResponseWriter, r *http.Request) { - req := &serializer.OpenCreateCommentOrEditIssueModalRequestBody{} - if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - c.Log.WithError(err).Warnf("Error decoding the JSON body") - p.writeAPIError(w, &serializer.APIErrorResponse{ID: "", Message: "Please provide a valid JSON object.", StatusCode: http.StatusBadRequest}) - return - } - - userID := r.Header.Get(constants.HeaderMattermostUserID) - - post, appErr := p.API.GetPost(req.PostID) - if appErr != nil { - p.writeAPIError(w, &serializer.APIErrorResponse{ID: "", Message: fmt.Sprintf("failed to load the post %s", req.PostID), StatusCode: http.StatusInternalServerError}) - return - } - if post == nil { - p.writeAPIError(w, &serializer.APIErrorResponse{ID: "", Message: fmt.Sprintf("failed to load the post %s : not found", req.PostID), StatusCode: http.StatusNotFound}) - return - } - - p.API.PublishWebSocketEvent( - wsEventCloseOrReopenIssue, - map[string]interface{}{ - "channel_id": post.ChannelId, - "owner": req.RepoOwner, - "repo": req.RepoName, - "number": req.IssueNumber, - "status": req.Status, - "postId": req.PostID, - }, - &model.WebsocketBroadcast{UserId: userID}, - ) -} - -func (p *Plugin) openIssueEditModal(c *serializer.UserContext, w http.ResponseWriter, r *http.Request) { +func (p *Plugin) getIssueInfo(c *serializer.UserContext, w http.ResponseWriter, r *http.Request) { req := &serializer.OpenCreateCommentOrEditIssueModalRequestBody{} if err := json.NewDecoder(r.Body).Decode(&req); err != nil { c.Log.WithError(err).Warnf("Error decoding the JSON body") @@ -1080,7 +1013,6 @@ func (p *Plugin) openIssueEditModal(c *serializer.UserContext, w http.ResponseWr milestoneNumber = *issue.Milestone.Number } - userID := r.Header.Get(constants.HeaderMattermostUserID) post, appErr := p.API.GetPost(req.PostID) if appErr != nil { p.writeAPIError(w, &serializer.APIErrorResponse{ID: "", Message: fmt.Sprintf("failed to load the post %s", req.PostID), StatusCode: http.StatusInternalServerError}) @@ -1091,24 +1023,20 @@ func (p *Plugin) openIssueEditModal(c *serializer.UserContext, w http.ResponseWr return } - p.API.PublishWebSocketEvent( - wsEventCreateOrUpdateIssue, - map[string]interface{}{ - "title": *issue.Title, - "channel_id": post.ChannelId, - "postId": req.PostID, - "milestone_title": milestoneTitle, - "milestone_number": milestoneNumber, - "assignees": assignees, - "labels": labels, - "description": description, - "repo_full_name": fmt.Sprintf("%s/%s", req.RepoOwner, req.RepoName), - "issue_number": *issue.Number, - }, - &model.WebsocketBroadcast{UserId: userID}, - ) + issueInfo := map[string]interface{}{ + "title": *issue.Title, + "channel_id": post.ChannelId, + "postId": req.PostID, + "milestone_title": milestoneTitle, + "milestone_number": milestoneNumber, + "assignees": assignees, + "labels": labels, + "description": description, + "repo_full_name": fmt.Sprintf("%s/%s", req.RepoOwner, req.RepoName), + "issue_number": *issue.Number, + } - p.writeJSON(w, issue) + p.writeJSON(w, issueInfo) } func (p *Plugin) getIssueByNumber(c *serializer.UserContext, w http.ResponseWriter, r *http.Request) { diff --git a/server/plugin/plugin.go b/server/plugin/plugin.go index 0414be207..13d14a725 100644 --- a/server/plugin/plugin.go +++ b/server/plugin/plugin.go @@ -40,11 +40,9 @@ const ( wsEventConnect = "connect" wsEventDisconnect = "disconnect" // WSEventConfigUpdate is the WebSocket event to update the configurations on webapp. - WSEventConfigUpdate = "config_update" - wsEventRefresh = "refresh" - wsEventCreateOrUpdateIssue = "createOrUpdateIssue" - wsEventCloseOrReopenIssue = "closeOrReopenIssue" - wsEventAttachCommentToIssue = "attachCommentToIssue" + WSEventConfigUpdate = "config_update" + wsEventRefresh = "refresh" + wsEventCreateIssue = "createIssue" WSEventRefresh = "refresh" @@ -482,7 +480,7 @@ func (p *Plugin) disconnectGitHubAccount(userID string) { func (p *Plugin) openIssueCreateModal(userID string, channelID string, title string) { p.API.PublishWebSocketEvent( - wsEventCreateOrUpdateIssue, + wsEventCreateIssue, map[string]interface{}{ "title": title, "channel_id": channelID, diff --git a/webapp/src/actions/index.js b/webapp/src/actions/index.js index 17935a1e9..a82c60e82 100644 --- a/webapp/src/actions/index.js +++ b/webapp/src/actions/index.js @@ -211,47 +211,11 @@ export function getMilestoneOptions(repo) { }; } -export function attachCommentIssueModal(payload) { +export function issueInfo(payload) { return async (dispatch, getState) => { let data; try { - data = await Client.attachCommentIssueModal(payload); - } catch (error) { - return {error}; - } - - const connected = await checkAndHandleNotConnected(data)(dispatch, getState); - if (!connected) { - return {error: data}; - } - - return {data}; - }; -} - -export function editIssueModal(payload) { - return async (dispatch, getState) => { - let data; - try { - data = await Client.editIssueModal(payload); - } catch (error) { - return {error}; - } - - const connected = await checkAndHandleNotConnected(data)(dispatch, getState); - if (!connected) { - return {error: data}; - } - - return {data}; - }; -} - -export function closeOrReopenIssueModal(payload) { - return async (dispatch, getState) => { - let data; - try { - data = await Client.closeOrReopenIssueModal(payload); + data = await Client.issueInfo(payload); } catch (error) { return {error}; } diff --git a/webapp/src/client/client.js b/webapp/src/client/client.js index 9b26d88e6..8bc78c65c 100644 --- a/webapp/src/client/client.js +++ b/webapp/src/client/client.js @@ -7,16 +7,8 @@ import {ClientError} from 'mattermost-redux/client/client4'; import {id as pluginId} from '../manifest'; export default class Client { - editIssueModal = async (payload) => { - return this.doPost(`${this.url}/edit_issue_modal`, payload); - } - - closeOrReopenIssueModal = async (payload) => { - return this.doPost(`${this.url}/close_reopen_issue_modal`, payload); - } - - attachCommentIssueModal = async (payload) => { - return this.doPost(`${this.url}/attach_comment_issue_modal`, payload); + issueInfo = async (payload) => { + return this.doPost(`${this.url}/issue_info`, payload); } setServerRoute(url) { diff --git a/webapp/src/components/github_issue/index.tsx b/webapp/src/components/github_issue/index.tsx index 9edeee267..75eb03873 100644 --- a/webapp/src/components/github_issue/index.tsx +++ b/webapp/src/components/github_issue/index.tsx @@ -4,7 +4,7 @@ import {Theme} from 'mattermost-redux/types/preferences'; import {Post} from 'mattermost-redux/types/posts'; import {useDispatch} from 'react-redux'; -import {attachCommentIssueModal, editIssueModal, closeOrReopenIssueModal} from '../../actions'; +import {openCreateCommentOnIssueModal, openCreateOrUpdateIssueModal, openCloseOrReopenIssueModal} from '../../actions'; type GithubIssueProps = { theme: Theme, @@ -24,6 +24,7 @@ const GithubIssue = ({theme, post}: GithubIssueProps) => { issue_number: postProps.issue_number, postId: post.id, status: postProps.status, + channel_id: post.channel_id, }; const content = ( @@ -31,17 +32,17 @@ const GithubIssue = ({theme, post}: GithubIssueProps) => { ); diff --git a/webapp/src/components/modals/attach_comment_to_issue/attach_comment_to_issue.jsx b/webapp/src/components/modals/attach_comment_to_issue/attach_comment_to_issue.jsx index f959f884a..db9525042 100644 --- a/webapp/src/components/modals/attach_comment_to_issue/attach_comment_to_issue.jsx +++ b/webapp/src/components/modals/attach_comment_to_issue/attach_comment_to_issue.jsx @@ -40,11 +40,11 @@ export default class AttachIssueModal extends PureComponent { } if (!this.state.issueValue) { - const {owner, repo, number} = this.props.messageData ?? {}; + const {repo_owner, repo_name, issue_number} = this.props.messageData ?? {}; const issue = { - owner, - repo, - number, + owner: repo_owner, + repo: repo_name, + number: issue_number, comment: this.state.comment, post_id: this.props.post.id, show_attached_message: false, @@ -114,9 +114,9 @@ export default class AttachIssueModal extends PureComponent { return null; } - const {number} = messageData ?? {}; - const modalTitle = number ? 'Create a comment to GitHub Issue' : 'Attach Message to GitHub Issue'; - const component = number ? ( + const {issue_number} = messageData ?? {}; + const modalTitle = issue_number ? 'Create a comment to GitHub Issue' : 'Attach Message to GitHub Issue'; + const component = issue_number ? (
{ channel_id: messageData.channel_id, issue_comment: comment, status_reason: currentStatus, - repo: messageData.repo, - number: messageData.number, - owner: messageData.owner, + repo: messageData.repo_name, + number: messageData.issue_number, + owner: messageData.repo_owner, status: messageData.status, postId: messageData.postId, }; 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 2a8bbab83..af8aabd61 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 @@ -22,6 +22,7 @@ const initialState = { repo: null, issueTitle: '', issueDescription: '', + channelID: '', labels: [], assignees: [], milestone: null, @@ -34,6 +35,7 @@ export default class CreateOrUpdateIssueModal extends PureComponent { update: PropTypes.func.isRequired, close: PropTypes.func.isRequired, create: PropTypes.func.isRequired, + issueInfo: PropTypes.func.isRequired, post: PropTypes.object, theme: PropTypes.object.isRequired, visible: PropTypes.bool.isRequired, @@ -46,36 +48,53 @@ export default class CreateOrUpdateIssueModal extends PureComponent { this.validator = new Validator(); } + getIssueInfo = async () => { + const issueInfo = await this.props.issueInfo(this.props.messageData); + return issueInfo; + } + + updateState(issueInfo) { + var {channel_id, title, description, assignees, labels, milestone_title, milestone_number, repo_full_name} = issueInfo ?? {}; + if (!assignees) { + assignees = []; + } + if (!labels) { + labels = []; + } + + this.setState({assignees}); + this.setState({labels}); + this.setState({milestone: { + value: milestone_number, + label: milestone_title, + }, + repo: { + name: repo_full_name, + }, + channelID: channel_id, + issueDescription: description, + issueTitle: title.substring(0, MAX_TITLE_LENGTH)}); + } + /* eslint-disable react/no-did-update-set-state*/ componentDidUpdate(prevProps) { if (this.props.post && !this.props.messageData) { this.setState({issueDescription: this.props.post.message}); } - const {channel_id, title, description, assignees, labels, milestone_title, milestone_number, repo_full_name} = this.props.messageData ?? {}; - if (channel_id && (channel_id !== prevProps.messageData?.channel_id || title !== prevProps.messageData?.title || description !== prevProps.messageData?.description || assignees !== prevProps.messageData?.assignees || labels !== prevProps.messageData?.labels || milestone_title !== prevProps.messageData?.milestone_title || milestone_number !== prevProps.messageData?.milestone_number)) { - if (assignees) { - this.setState({assignees}); - } - if (labels) { - this.setState({labels}); - } - this.setState({milestone: { - value: milestone_number, - label: milestone_title, - }, - repo: { - name: repo_full_name, - }, - issueDescription: description, - issueTitle: title.substring(0, MAX_TITLE_LENGTH)}); + if (this.props.messageData && this.props.messageData.repo_owner && !prevProps.visible && this.props.visible) { + this.getIssueInfo().then((issueInfo) => { + this.updateState(issueInfo.data); + }); + } else if (this.props.messageData?.channel_id && (this.props.messageData?.channel_id !== prevProps.messageData?.channel_id || this.props.messageData?.title !== prevProps.messageData?.title)) { + this.updateState(this.props.messageData); } } /* eslint-enable */ // handle issue creation or updation after form is populated handleCreateOrUpdate = async (e) => { - const {channel_id, issue_number, repo_full_name} = this.props.messageData ?? {}; + const {issue_number} = this.props.messageData ?? {}; if (e && e.preventDefault) { e.preventDefault(); } @@ -99,7 +118,7 @@ export default class CreateOrUpdateIssueModal extends PureComponent { assignees: this.state.assignees, milestone: this.state.milestone && this.state.milestone.value, post_id: postId, - channel_id, + channel_id: this.state.channelID, issue_number, }; @@ -107,7 +126,7 @@ export default class CreateOrUpdateIssueModal extends PureComponent { issue.repo = this.state.repo; } this.setState({submitting: true}); - if (repo_full_name) { + if (issue_number) { const updated = await this.props.update(issue); if (updated?.error) { const errMessage = getErrorMessage(updated.error.message); diff --git a/webapp/src/components/modals/create_update_issue/index.js b/webapp/src/components/modals/create_update_issue/index.js index 39eaef65a..1bb5c8b34 100644 --- a/webapp/src/components/modals/create_update_issue/index.js +++ b/webapp/src/components/modals/create_update_issue/index.js @@ -6,7 +6,7 @@ import {bindActionCreators} from 'redux'; import {getPost} from 'mattermost-redux/selectors/entities/posts'; import {id as pluginId} from 'manifest'; -import {closeCreateOrUpdateIssueModal, createIssue, updateIssue} from 'actions'; +import {closeCreateOrUpdateIssueModal, createIssue, updateIssue, issueInfo} from 'actions'; import CreateOrUpdateIssueModal from './create_update_issue'; @@ -26,6 +26,7 @@ const mapDispatchToProps = (dispatch) => bindActionCreators({ close: closeCreateOrUpdateIssueModal, create: createIssue, update: updateIssue, + issueInfo, }, dispatch); export default connect(mapStateToProps, mapDispatchToProps)(CreateOrUpdateIssueModal); diff --git a/webapp/src/index.js b/webapp/src/index.js index 0a7a18a42..9b79a57da 100644 --- a/webapp/src/index.js +++ b/webapp/src/index.js @@ -15,7 +15,7 @@ import GithubIssue from './components/github_issue'; import Reducer from './reducers'; import Client from './client'; import {getConnected, setShowRHSAction} from './actions'; -import {handleConnect, handleDisconnect, handleConfigurationUpdate, handleOpenCreateOrUpdateIssueModal, handleOpenCreateCommentOnIssueModal, handleOpenCloseOrReopenIssueModal, handleReconnect, handleRefresh} from './websocket'; +import {handleConnect, handleDisconnect, handleConfigurationUpdate, handleOpenCreateOrUpdateIssueModal, handleReconnect, handleRefresh} from './websocket'; import {getServerRoute} from './selectors'; import {id as pluginId} from './manifest'; @@ -47,9 +47,7 @@ class PluginClass { registry.registerWebSocketEventHandler(`custom_${pluginId}_disconnect`, handleDisconnect(store)); registry.registerWebSocketEventHandler(`custom_${pluginId}_config_update`, handleConfigurationUpdate(store)); registry.registerWebSocketEventHandler(`custom_${pluginId}_refresh`, handleRefresh(store)); - registry.registerWebSocketEventHandler(`custom_${pluginId}_createOrUpdateIssue`, handleOpenCreateOrUpdateIssueModal(store)); - registry.registerWebSocketEventHandler(`custom_${pluginId}_attachCommentToIssue`, handleOpenCreateCommentOnIssueModal(store)); - registry.registerWebSocketEventHandler(`custom_${pluginId}_closeOrReopenIssue`, handleOpenCloseOrReopenIssueModal(store)); + registry.registerWebSocketEventHandler(`custom_${pluginId}_createIssue`, handleOpenCreateOrUpdateIssueModal(store)); registry.registerPostTypeComponent('custom_git_issue', GithubIssue); registry.registerReconnectHandler(handleReconnect(store)); diff --git a/webapp/src/websocket/index.js b/webapp/src/websocket/index.js index 88690dc55..3a91692cf 100644 --- a/webapp/src/websocket/index.js +++ b/webapp/src/websocket/index.js @@ -10,8 +10,6 @@ import { getYourAssignments, getYourPrs, openCreateOrUpdateIssueModal, - openCloseOrReopenIssueModal, - openCreateCommentOnIssueModal, } from '../actions'; import {id as pluginId} from '../manifest'; @@ -95,21 +93,3 @@ export function handleOpenCreateOrUpdateIssueModal(store) { store.dispatch(openCreateOrUpdateIssueModal(msg.data)); }; } - -export function handleOpenCloseOrReopenIssueModal(store) { - return (msg) => { - if (!msg.data) { - return; - } - store.dispatch(openCloseOrReopenIssueModal(msg.data)); - }; -} - -export function handleOpenCreateCommentOnIssueModal(store) { - return (msg) => { - if (!msg.data) { - return; - } - store.dispatch(openCreateCommentOnIssueModal(msg.data)); - }; -} From 44ac4e5eb3db2da856ade1f715c6dcefde167dcf Mon Sep 17 00:00:00 2001 From: Nityanand Rai Date: Fri, 10 Mar 2023 15:50:20 +0530 Subject: [PATCH 07/10] [MI-2814]: Review fixes done 1. Made comment field editable when trying to add a comment from message action. --- .../attach_comment_to_issue.jsx | 17 ++++++++++++----- .../create_update_issue/create_update_issue.jsx | 13 +++++++------ 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/webapp/src/components/modals/attach_comment_to_issue/attach_comment_to_issue.jsx b/webapp/src/components/modals/attach_comment_to_issue/attach_comment_to_issue.jsx index db9525042..d9803c9d7 100644 --- a/webapp/src/components/modals/attach_comment_to_issue/attach_comment_to_issue.jsx +++ b/webapp/src/components/modals/attach_comment_to_issue/attach_comment_to_issue.jsx @@ -72,7 +72,7 @@ export default class AttachIssueModal extends PureComponent { owner, repo, number, - comment: this.props.post.message, + comment: this.state.comment, post_id: this.props.post.id, show_attached_message: true, }; @@ -106,9 +106,17 @@ export default class AttachIssueModal extends PureComponent { }); }; + /* eslint-disable react/no-did-update-set-state*/ + componentDidUpdate(prevProps) { + if (this.props.post && !this.props.messageData && !prevProps.post) { + this.setState({comment: this.props.post.message}); + } + } + /* eslint-enable */ + render() { const {error, submitting, comment, issueValue} = this.state; - const {visible, theme, messageData, post} = this.props; + const {visible, theme, messageData} = this.props; const style = getStyle(theme); if (!visible) { return null; @@ -138,10 +146,9 @@ export default class AttachIssueModal extends PureComponent {
); 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 af8aabd61..2d5185c69 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 @@ -22,7 +22,7 @@ const initialState = { repo: null, issueTitle: '', issueDescription: '', - channelID: '', + channelId: '', labels: [], assignees: [], milestone: null, @@ -62,8 +62,6 @@ export default class CreateOrUpdateIssueModal extends PureComponent { labels = []; } - this.setState({assignees}); - this.setState({labels}); this.setState({milestone: { value: milestone_number, label: milestone_title, @@ -71,14 +69,16 @@ export default class CreateOrUpdateIssueModal extends PureComponent { repo: { name: repo_full_name, }, - channelID: channel_id, + assignees, + labels, + channelId: channel_id, issueDescription: description, issueTitle: title.substring(0, MAX_TITLE_LENGTH)}); } /* eslint-disable react/no-did-update-set-state*/ componentDidUpdate(prevProps) { - if (this.props.post && !this.props.messageData) { + if (this.props.post && !this.props.messageData && !prevProps.post) { this.setState({issueDescription: this.props.post.message}); } @@ -118,7 +118,7 @@ export default class CreateOrUpdateIssueModal extends PureComponent { assignees: this.state.assignees, milestone: this.state.milestone && this.state.milestone.value, post_id: postId, - channel_id: this.state.channelID, + channel_id: this.state.channelId, issue_number, }; @@ -252,6 +252,7 @@ export default class CreateOrUpdateIssueModal extends PureComponent { onChange={this.handleIssueTitleChange} /> {issueTitleValidationError} + {this.renderIssueAttributeSelectors()} Date: Fri, 10 Mar 2023 20:13:32 +0530 Subject: [PATCH 08/10] [MI-2814]: Review fixes done 1. Improved code readability --- .../attach_comment_to_issue/attach_comment_to_issue.jsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/webapp/src/components/modals/attach_comment_to_issue/attach_comment_to_issue.jsx b/webapp/src/components/modals/attach_comment_to_issue/attach_comment_to_issue.jsx index d9803c9d7..a674b6cac 100644 --- a/webapp/src/components/modals/attach_comment_to_issue/attach_comment_to_issue.jsx +++ b/webapp/src/components/modals/attach_comment_to_issue/attach_comment_to_issue.jsx @@ -106,13 +106,11 @@ export default class AttachIssueModal extends PureComponent { }); }; - /* eslint-disable react/no-did-update-set-state*/ componentDidUpdate(prevProps) { if (this.props.post && !this.props.messageData && !prevProps.post) { - this.setState({comment: this.props.post.message}); + this.setState({comment: this.props.post.message}); // eslint-disable-line react/no-did-update-set-state } } - /* eslint-enable */ render() { const {error, submitting, comment, issueValue} = this.state; From 91ae3abf6a57226c2dc9f13c16c8a1b8b3bfb104 Mon Sep 17 00:00:00 2001 From: Nityanand Rai Date: Mon, 13 Mar 2023 16:19:15 +0530 Subject: [PATCH 09/10] [MI-2814]: Review fixes done 1. Used GET method on getting the issue info 2. Improved code readability --- server/plugin/api.go | 38 ++++++++++--------- server/serializer/issue.go | 8 ---- webapp/src/actions/index.js | 4 +- webapp/src/client/client.js | 4 +- .../create_update_issue.jsx | 15 +++----- .../modals/create_update_issue/index.js | 4 +- 6 files changed, 33 insertions(+), 40 deletions(-) diff --git a/server/plugin/api.go b/server/plugin/api.go index 3c96d6aef..fd4381c13 100644 --- a/server/plugin/api.go +++ b/server/plugin/api.go @@ -97,7 +97,7 @@ func (p *Plugin) initializeAPI() { 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("/issue_info", p.checkAuth(p.attachUserContext(p.getIssueInfo), ResponseTypePlain)).Methods(http.MethodPost) + apiRouter.HandleFunc("/issue_info", p.checkAuth(p.attachUserContext(p.getIssueInfo), ResponseTypePlain)).Methods(http.MethodGet) 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) @@ -958,33 +958,37 @@ func (p *Plugin) updateSettings(c *serializer.UserContext, w http.ResponseWriter } func (p *Plugin) getIssueInfo(c *serializer.UserContext, w http.ResponseWriter, r *http.Request) { - req := &serializer.OpenCreateCommentOrEditIssueModalRequestBody{} - if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - c.Log.WithError(err).Warnf("Error decoding the JSON body") - p.writeAPIError(w, &serializer.APIErrorResponse{ID: "", Message: "Please provide a valid JSON object.", StatusCode: http.StatusBadRequest}) + owner := r.FormValue(constants.OwnerQueryParam) + repo := r.FormValue(constants.RepoQueryParam) + number := r.FormValue(constants.NumberQueryParam) + postID := r.FormValue(constants.PostIDQueryParam) + + issueNumber, err := strconv.Atoi(number) + if err != nil { + p.writeAPIError(w, &serializer.APIErrorResponse{Message: "Invalid param 'number'.", StatusCode: http.StatusBadRequest}) return } githubClient := p.githubConnectUser(c.Context.Ctx, c.GHInfo) - issue, _, err := githubClient.Issues.Get(c.Ctx, req.RepoOwner, req.RepoName, req.IssueNumber) + issue, _, err := githubClient.Issues.Get(c.Ctx, owner, repo, issueNumber) if err != nil { // If the issue is not found, it probably belongs to a private repo. // Return an empty response in that case. var gerr *github.ErrorResponse if errors.As(err, &gerr) && gerr.Response.StatusCode == http.StatusNotFound { c.Log.WithError(err).With(logger.LogContext{ - "owner": req.RepoOwner, - "repo": req.RepoName, - "number": req.IssueNumber, + "owner": owner, + "repo": repo, + "number": issueNumber, }).Debugf("Issue not found") p.writeJSON(w, nil) return } c.Log.WithError(err).With(logger.LogContext{ - "owner": req.RepoOwner, - "repo": req.RepoName, - "number": req.IssueNumber, + "owner": owner, + "repo": repo, + "number": issueNumber, }).Debugf("Could not get the issue") p.writeAPIError(w, &serializer.APIErrorResponse{Message: "Could not get the issue", StatusCode: http.StatusInternalServerError}) return @@ -1013,26 +1017,26 @@ func (p *Plugin) getIssueInfo(c *serializer.UserContext, w http.ResponseWriter, milestoneNumber = *issue.Milestone.Number } - post, appErr := p.API.GetPost(req.PostID) + post, appErr := p.API.GetPost(postID) if appErr != nil { - p.writeAPIError(w, &serializer.APIErrorResponse{ID: "", Message: fmt.Sprintf("failed to load the post %s", req.PostID), StatusCode: http.StatusInternalServerError}) + p.writeAPIError(w, &serializer.APIErrorResponse{ID: "", Message: fmt.Sprintf("failed to load the post %s", postID), StatusCode: http.StatusInternalServerError}) return } if post == nil { - p.writeAPIError(w, &serializer.APIErrorResponse{ID: "", Message: fmt.Sprintf("failed to load the post %s : not found", req.PostID), StatusCode: http.StatusNotFound}) + p.writeAPIError(w, &serializer.APIErrorResponse{ID: "", Message: fmt.Sprintf("failed to load the post %s : not found", postID), StatusCode: http.StatusNotFound}) return } issueInfo := map[string]interface{}{ "title": *issue.Title, "channel_id": post.ChannelId, - "postId": req.PostID, + "postId": postID, "milestone_title": milestoneTitle, "milestone_number": milestoneNumber, "assignees": assignees, "labels": labels, "description": description, - "repo_full_name": fmt.Sprintf("%s/%s", req.RepoOwner, req.RepoName), + "repo_full_name": fmt.Sprintf("%s/%s", owner, repo), "issue_number": *issue.Number, } diff --git a/server/serializer/issue.go b/server/serializer/issue.go index e21432423..a5d400a79 100644 --- a/server/serializer/issue.go +++ b/server/serializer/issue.go @@ -42,11 +42,3 @@ type CommentAndCloseRequest struct { Status string `json:"status"` PostID string `json:"postId"` } - -type OpenCreateCommentOrEditIssueModalRequestBody struct { - RepoOwner string `json:"repo_owner"` - RepoName string `json:"repo_name"` - IssueNumber int `json:"issue_number"` - PostID string `json:"postId"` - Status string `json:"status"` -} diff --git a/webapp/src/actions/index.js b/webapp/src/actions/index.js index a82c60e82..d6133bb2a 100644 --- a/webapp/src/actions/index.js +++ b/webapp/src/actions/index.js @@ -211,11 +211,11 @@ export function getMilestoneOptions(repo) { }; } -export function issueInfo(payload) { +export function getIssueInfo(owner, repo, issueNumber, postID) { return async (dispatch, getState) => { let data; try { - data = await Client.issueInfo(payload); + data = await Client.getIssueInfo(owner, repo, issueNumber, postID); } catch (error) { return {error}; } diff --git a/webapp/src/client/client.js b/webapp/src/client/client.js index 8bc78c65c..c20b18439 100644 --- a/webapp/src/client/client.js +++ b/webapp/src/client/client.js @@ -7,8 +7,8 @@ import {ClientError} from 'mattermost-redux/client/client4'; import {id as pluginId} from '../manifest'; export default class Client { - issueInfo = async (payload) => { - return this.doPost(`${this.url}/issue_info`, payload); + getIssueInfo = async (owner, repo, issueNumber, postID) => { + return this.doGet(`${this.url}/issue_info?owner=${owner}&repo=${repo}&number=${issueNumber}&postId=${postID}`); } setServerRoute(url) { 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 2d5185c69..8005c7a2a 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 @@ -49,18 +49,15 @@ export default class CreateOrUpdateIssueModal extends PureComponent { } getIssueInfo = async () => { - const issueInfo = await this.props.issueInfo(this.props.messageData); + const {repo_owner, repo_name, issue_number, postId} = this.props.messageData + const issueInfo = await this.props.getIssueInfo(repo_owner, repo_name, issue_number, postId); return issueInfo; } updateState(issueInfo) { - var {channel_id, title, description, assignees, labels, milestone_title, milestone_number, repo_full_name} = issueInfo ?? {}; - if (!assignees) { - assignees = []; - } - if (!labels) { - labels = []; - } + const {channel_id, title, description, milestone_title, milestone_number, repo_full_name} = issueInfo ?? {}; + const assignees = issueInfo?.assignees ?? []; + const labels = issueInfo?.labels ?? []; this.setState({milestone: { value: milestone_number, @@ -82,7 +79,7 @@ export default class CreateOrUpdateIssueModal extends PureComponent { this.setState({issueDescription: this.props.post.message}); } - if (this.props.messageData && this.props.messageData.repo_owner && !prevProps.visible && this.props.visible) { + if (this.props.messageData?.repo_owner && !prevProps.visible && this.props.visible) { this.getIssueInfo().then((issueInfo) => { this.updateState(issueInfo.data); }); diff --git a/webapp/src/components/modals/create_update_issue/index.js b/webapp/src/components/modals/create_update_issue/index.js index 1bb5c8b34..fef91d30f 100644 --- a/webapp/src/components/modals/create_update_issue/index.js +++ b/webapp/src/components/modals/create_update_issue/index.js @@ -6,7 +6,7 @@ import {bindActionCreators} from 'redux'; import {getPost} from 'mattermost-redux/selectors/entities/posts'; import {id as pluginId} from 'manifest'; -import {closeCreateOrUpdateIssueModal, createIssue, updateIssue, issueInfo} from 'actions'; +import {closeCreateOrUpdateIssueModal, createIssue, updateIssue, getIssueInfo} from 'actions'; import CreateOrUpdateIssueModal from './create_update_issue'; @@ -26,7 +26,7 @@ const mapDispatchToProps = (dispatch) => bindActionCreators({ close: closeCreateOrUpdateIssueModal, create: createIssue, update: updateIssue, - issueInfo, + getIssueInfo, }, dispatch); export default connect(mapStateToProps, mapDispatchToProps)(CreateOrUpdateIssueModal); From 0fbaf3fd5c9462a36dc87e0d24ff6beaf28c72ce Mon Sep 17 00:00:00 2001 From: Nityanand Rai Date: Mon, 13 Mar 2023 16:56:58 +0530 Subject: [PATCH 10/10] [MI-2814]: Improved linting error --- .../modals/create_update_issue/create_update_issue.jsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 8005c7a2a..7ad0983f1 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 @@ -35,7 +35,7 @@ export default class CreateOrUpdateIssueModal extends PureComponent { update: PropTypes.func.isRequired, close: PropTypes.func.isRequired, create: PropTypes.func.isRequired, - issueInfo: PropTypes.func.isRequired, + getIssueInfo: PropTypes.func.isRequired, post: PropTypes.object, theme: PropTypes.object.isRequired, visible: PropTypes.bool.isRequired, @@ -49,7 +49,7 @@ export default class CreateOrUpdateIssueModal extends PureComponent { } getIssueInfo = async () => { - const {repo_owner, repo_name, issue_number, postId} = this.props.messageData + const {repo_owner, repo_name, issue_number, postId} = this.props.messageData; const issueInfo = await this.props.getIssueInfo(repo_owner, repo_name, issue_number, postId); return issueInfo; }