[MI-2820]: Modified API endpoint to require a project to be provided #144

avas27JTG · 2023-02-24T13:04:01Z

Modified API endpoint to require a project to be provided

1. Updated API to support creating pipeline subscriptions. 2. Updated the UI for subscription modal to display pipeline event types. 3. Updated the UI of subscription card for for displaying pipeline subscriptions.

…ure-devops into MI-2315

…Created" event type

…eployment started" and "Release deployment completed"

…Run state changed" event type

…king

… preview.

…ure-devops into MI-2343

…in-azure-devops into MI-2304

…ure-devops into MI-2315

…pipeline job request from requested user via MM post.

…ure-devops into MI-2339

…d/rejected Pipeline release request.

…e central login into it, formatted code (#32) * [MI-2669]: Created a hook for intercepting redux changes and moved the central login into it, formatted code * [MI-2669]: Modified comment * [MI-2669]: Review fixes --------- Co-authored-by: Abhishek Verma <abhishek.verma@brightscout.com>

Co-authored-by: Abhishek Verma <abhishek.verma@brightscout.com>

…ser for oAuth. (#35) * [MI-2748]: Implemented 1:1 mapping between MM user and Azure DevOps user for oAuth. * [MI-2748]: Added 1 test case and http response code * [MI-2748]: Fixed condition * [MI-2748]: Fixed token refresh logic * [MI-2748]: Fixed CI * [MI-2748]: Fixed CI * [MI-2748]: Review fixes * [MI-2748]: Fixed CI * [MI-2748]: Review fixes * [MI-2748]: Review fixes --------- Co-authored-by: Abhishek Verma <abhishek.verma@brightscout.com>

…om plugin's server. (#36) * [MI-2750]: Implemented fetching channel list on webapp and remove from plugin's server. * [MI-2750]: Review fixes * [MI-2750]: Changed comment message --------- Co-authored-by: Abhishek Verma <abhishek.verma@brightscout.com>

…er (#37) * [MI-2765]: Used @mattermost/client for making webapp calls to MM server * [MI-2765]: Review fix * [MI-2765]: Review fix * [MI-2765]: Removed Mattermost API call RTK service * [MI-2765]: Fixed CI --------- Co-authored-by: Abhishek Verma <abhishek.verma@brightscout.com>

mickmister · 2023-02-24T14:19:29Z

server/plugin/api.go

+
+	organization := pathParams[constants.PathParamOrganization]
+	project := pathParams[constants.PathParamProject]
+	if _, isProjectLinked := p.IsProjectLinked(projectList, serializers.ProjectDetails{OrganizationName: strings.ToLower(organization), ProjectName: cases.Title(language.Und).String(project)}); !isProjectLinked {


There's a lot going on in this line. Maybe break this into local variables first?

mickmister · 2023-02-24T14:20:07Z

server/plugin/api.go

+	organization := pathParams[constants.PathParamOrganization]
+	project := pathParams[constants.PathParamProject]
+	if _, isProjectLinked := p.IsProjectLinked(projectList, serializers.ProjectDetails{OrganizationName: strings.ToLower(organization), ProjectName: cases.Title(language.Und).String(project)}); !isProjectLinked {
+		p.API.LogError(fmt.Sprintf("Project %s is not linked", project))


I think we should use LogWarn since there isn't an actionable step for the admin to fix this. Please keep this is mind for other features/fixes implemented.

Sure, fixed

mickmister · 2023-02-24T14:21:43Z

server/plugin/api.go

+	}
+
+	organization := pathParams[constants.PathParamOrganization]
+	project := pathParams[constants.PathParamProject]


We could "return early" if project is blank right? Before we interact with the store

We have made organization and project as path params here so, if they are empty this API will not be called and we will not reach here and get 404

hanzei · 2023-02-26T21:59:47Z

@avas27JTG Could you please rebase against master and change the target branch?

…e-devops into MI-2820

mickmister · 2023-02-27T15:03:39Z

server/plugin/api.go

@@ -413,6 +413,26 @@ func (p *Plugin) handleGetSubscriptions(w http.ResponseWriter, r *http.Request)
 		return
 	}

+	projectList, err := p.Store.GetAllProjects(mattermostUserID)
+	if err != nil {
+		p.API.LogError(constants.ErrorFetchProjectList, "Error", err.Error())


Same comment about using LogWarn instead here

mickmister · 2023-02-27T15:04:27Z

server/plugin/api.go

@@ -432,7 +452,6 @@ func (p *Plugin) handleGetSubscriptions(w http.ResponseWriter, r *http.Request)
 	channelID := r.URL.Query().Get(constants.QueryParamChannelID)
 	serviceType := r.URL.Query().Get(constants.QueryParamServiceType)
 	eventType := r.URL.Query().Get(constants.QueryParamEventType)
-	project := r.URL.Query().Get(constants.QueryParamProject)
 	if project != "" {


What's the intended functionality when project is an empty string? The code reads a bit weird because there is a large block for handling when it's not empty, but then the function ends after that block

The code will reach here only when the project is not empty and it's valid i.e. linked/exists on the plugin
and in that case, we are returning the array of subscriptions

Removed this check as it's no longer needed now as we have made it required now

raghavaggarwal2308 and others added 30 commits November 1, 2022 14:11

[MI-2304] Pipeline subscription

85a669a

1. Updated API to support creating pipeline subscriptions. 2. Updated the UI for subscription modal to display pipeline event types. 3. Updated the UI of subscription card for for displaying pipeline subscriptions.

[MI-2315] Added slash command to add/list pipeline subscriptions

21c0365

[MI-2304] Fixed casing of event type description

1f0947e

Merge branch 'MI-2304' of github.com:Brightscout/mattermost-plugin-az…

b73575d

…ure-devops into MI-2315

[MI-2321] Created notification post for "Release Created" and "Build …

b201a3d

…Created" event type

[MI-2321] Added notification post for "Release Abandoned", "Release d…

0798808

…eployment started" and "Release deployment completed"

[MI-2321] Added notification post for "Run stage state changed" and "…

b2dd84f

…Run state changed" event type

[MI-2321] Refactored code

a46984f

[MI-2321] Removed commented code

5839e9a

[MI-2321] Removed unused struct

53058ac

[MI-2321] Fixed bug: Pull reqest commented event notification not wor…

1f222ea

…king

[MI-2339][server]: Added support to show pipeline build link preview.

e878063

[MI-2339][server]: Added mock method for pipeline build

23f45e4

[MI-2321] Fixed author icon css on notification post

6b9b96e

[MI-2339][server]: Updated error message

8963751

[MI-2343][server+webapp]: Added support to show pipeline release link…

76d49bb

… preview.

Merge branch 'MI-2339' of github.com:Brightscout/mattermost-plugin-az…

731f2ee

…ure-devops into MI-2343

[MI-2304] Review fixes

32b6a67

Merge branch 'epic_phase_3' of github.com:Brightscout/mattermost-plug…

4130bbf

…in-azure-devops into MI-2304

Merge branch 'MI-2304' of github.com:Brightscout/mattermost-plugin-az…

bd96af0

…ure-devops into MI-2315

[MI-2315] Review fixes

b65ea80

[MI-2304] Review fixes

1c7ca9a

[MI-2345][server+webapp]: Added feature for user to Approve/Reject a …

c800d02

…pipeline job request from requested user via MM post.

Merge branch 'MI-2315' of github.com:Brightscout/mattermost-plugin-az…

05caef6

…ure-devops into MI-2339

[MI-2339][server][review_fix]: Minor refactoring

6e870e8

[MI-2345][server]: Added mock method for UpdatePipelineApprovalRequest

d2c2ab9

[MI-2345][server]: Fixed variable name for failing CI

7ae186e

[MI-2350][server]: Add feature to update status of an already approve…

b92f752

…d/rejected Pipeline release request.

[MI-2350][server]: Changed variable names to prevent shadow declarations

1dd3333

[MI-2304] Review fixes

ef552c0

avas27JTG and others added 9 commits February 7, 2023 12:27

[Refactor]: Removed unused util

8c5b806

[MI-2669]: Created hook to fetch and load filters (#34)

1e0fc5f

Co-authored-by: Abhishek Verma <abhishek.verma@brightscout.com>

[Fix]: Moved subscriptionID field at the top in struct

6d00878

[MI-2820]: Modified API endpoint to require a project to be provided

e5950fc

[MI-2820]: Removed extra line

6b20c52

mickmister reviewed Feb 24, 2023

View reviewed changes

hanzei added the 2: Dev Review Requires review by a core committer label Feb 26, 2023

hanzei changed the base branch from epic_phase_3 to master February 26, 2023 21:58

hanzei changed the base branch from master to epic_phase_3 February 26, 2023 21:59

avas27JTG added 2 commits February 27, 2023 13:26

[MI-2820]: Review fixes

633b076

Merge branch 'master' of github.com:mattermost/mattermost-plugin-azur…

8af9234

…e-devops into MI-2820

avas27JTG changed the base branch from epic_phase_3 to master February 27, 2023 08:06

avas27JTG requested a review from mickmister February 27, 2023 08:10

mickmister reviewed Feb 27, 2023

View reviewed changes

[MI-2820]: Added LogWarn and refactored code

3e5a793

avas27JTG requested a review from mickmister February 28, 2023 15:52

mickmister approved these changes Feb 28, 2023

View reviewed changes

hanzei requested a review from m1lt0n March 28, 2023 19:00

m1lt0n approved these changes Mar 29, 2023

View reviewed changes

hanzei added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core committer labels Mar 29, 2023

hanzei merged commit 4f8e0b0 into master Mar 29, 2023

hanzei deleted the MI-2820 branch March 29, 2023 11:15

avas27JTG mentioned this pull request Apr 10, 2023

[MOI-39]: Created plugin API to send images as attachments in a post. Brightscout/mattermost-plugin-openai#25

Merged

mickmister mentioned this pull request Apr 11, 2023

handleGetSubscriptions API endpoint should require a project to be provided #143

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MI-2820]: Modified API endpoint to require a project to be provided #144

[MI-2820]: Modified API endpoint to require a project to be provided #144

avas27JTG commented Feb 24, 2023

mickmister Feb 24, 2023

avas27JTG Feb 27, 2023

mickmister Feb 24, 2023

avas27JTG Feb 27, 2023

mickmister Feb 24, 2023

avas27JTG Feb 27, 2023 •

edited

Loading

hanzei commented Feb 26, 2023

mickmister Feb 27, 2023

mickmister Feb 27, 2023

avas27JTG Feb 28, 2023

[MI-2820]: Modified API endpoint to require a project to be provided #144

[MI-2820]: Modified API endpoint to require a project to be provided #144

Conversation

avas27JTG commented Feb 24, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

avas27JTG Feb 27, 2023 • edited Loading

Choose a reason for hiding this comment

hanzei commented Feb 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

avas27JTG Feb 27, 2023 •

edited

Loading