feat(sprint-poker): Can vote on GitLab issue #6398

nickoferrall · 2022-04-13T15:37:56Z

Fixes #6322 & fixes #6287

This PR adds:

a GitLab label to the Estimate phase
the user can select "As Comment" or "Do not update"
if they select "As Comment", voting on a GitLab issue in Parabol adds a comment to the GitLab issue
the GitLab comment includes the "Powered by Parabol" link

Note: when selecting the "As Comment" or "Do not update" label, the Update/Edit Score button can flicker. This bug exists in GitHub in prod, so I created an issue for it: #6397

…rabolInc/parabol into feat/6088/gitlab-issues-functionality

packages/server/graphql/mutations/helpers/pushEstimateToGitLab.ts

mattkrick

looking good! My only advice would be to consider filling up the GitLabServerManager with all the helper functions you can to set a good pattern moving forward

mattkrick · 2022-04-21T22:42:42Z

packages/server/graphql/mutations/helpers/pushEstimateToGitLab.ts

+      meetingName,
+      discussionURL
+    )
+    const [, commentError] = await gitlabRequest(createNote, {


+1 I know we pushed it off on the last PR. Is this PR the right time to have GitLabServerManager implement TaskIntegrationManager? Having another integration using that abstraction could do some good in how they shape up & remove the need for a large refactoring PR at the end of this milestone

I've implemented the new pattern including createNote in this PR: #6427

mattkrick · 2022-04-21T22:44:49Z

packages/server/graphql/mutations/updateGitLabDimensionField.ts

+import {GQLContext} from '../graphql'
+import UpdateGitLabDimensionFieldPayload from '../types/UpdateGitLabDimensionFieldPayload'
+
+interface Args {


-1 could you write this as a resolver using the new SDL pattern? that way these args will be provided for you

Nice, it was cool to see the new pattern! The type safety is so good 🙌 Update pushed!

packages/server/postgres/migrations/1649750150687_GitLabDimensionFieldMap.ts

Dschoordsch · 2022-04-25T10:49:06Z

packages/server/postgres/migrations/1649750150687_GitLabDimensionFieldMap.ts

+    "dimensionName" VARCHAR(120) NOT NULL,
+    "gid" VARCHAR(140) NOT NULL,
+    "labelTemplate" VARCHAR(100) NOT NULL,
+    PRIMARY KEY ("teamId", "dimensionName", "gid")


-1 Why have an id field if it's not the primary key? It does not even have a unique constraint.

I followed the GitHub example here: https://github.com/ParabolInc/parabol/blob/cf55ed4d0877b1756ffc7213062dde5056ed662b/packages/server/postgres/migrations/1631822263196_GitHubDimensionFieldMap.ts

I'm not sure why. Perhaps @mattkrick could help clarify?

every node needs an ID in graphql/relay so i just gave it a one so we didn't have to concat 3 fields together ("teamId", "dimensionName", "nameWithOwner"). safe to remove if you'd rather concat em

I'm also fine with keeping it, but it must have a unique constraint since you can just insert/update serial fields.

Dschoordsch · 2022-04-26T14:14:23Z

packages/server/postgres/migrations/1649750150687_GitLabDimensionFieldMap.ts

+    "id" SERIAL,
+    "teamId" VARCHAR(100) NOT NULL,
+    "dimensionName" VARCHAR(120) NOT NULL,
+    "gid" VARCHAR(140) NOT NULL,


-2 I don't understand why we store the voting dimension by issue id. I think we should have the providerId, projectId instead, so it is set for the project and all upcoming voting issues.

For the [Integration]DimensionFieldMaps, my understanding is we want to store the dimensionName, e.g. Story Points, and labelTemplate, e.g. __comment, associated with a particular project/repo.

We do this so that the user's preferences are stored so they don't need to keep changing between "As a comment" and "Do not update", for example.

Previously, I thought it was associated with an issue, so I was storing the gid. But, I now see that when I update the "As a comment" label to "As a label", in the GitHub integration, it updates the label for all other issues in that repo.

So, I agree that I should change gid to projectId so that we store the preferences for all issues within a project in GitLab.

What I'm unclear on is why we should have providerId? Are you thinking we should have this instead of teamId? The same issue could exist in multiple teams. By including the teamId, we only update the label preference on that team.

I would like to tie all integration settings to their provider. As these are potentially self-hosted a client could switch between two instances where both use the same ids belonging to different projects. Or teams move orgs, there are some scenarios where this could happen. We should keep the teamId as well.

Storing the providerId makes sense for that self-hosted switching edge case. So you're happy for us to store teamId, projectId, dimensionName, and providerId here?

As the key/unique constraint to lookup the setting, yes. And then "labelTemplate" to store the actual setting.

Sounds good. Update pushed!

nickoferrall · 2022-04-27T15:08:18Z

packages/server/graphql/mutations/helpers/pushEstimateToGitLab.ts

+  const manager = new GitLabServerManager(accessToken!, provider.serverBaseUrl!)
+  const gitlabRequest = manager.getGitLabRequest(info, context)
+
+  const [data] = await gitlabRequest<GetIssueQuery>(getIssue, {gid})


In this PR #6427 this is refactored to use the TaskIntegrationManager pattern

packages/server/graphql/mutations/helpers/pushEstimateToGitLab.ts

packages/server/graphql/public/typeDefs/updateGitLabDimensionField.graphql

Dschoordsch · 2022-05-02T09:46:58Z

packages/server/graphql/types/EstimateStage.ts

+          const gitlabAuth = await dataLoader
+            .get('teamMemberIntegrationAuths')
+            .load({service: 'gitlab', teamId, userId: viewerId})
+          if (!gitlabAuth?.accessToken) return null


-2 This is failing for meeting participants without gitlab integration. They will not be able to join a Poker Meeting with GitLab issues at all.

GraphQLError: Cannot return null for non-nullable field EstimateStage.(serviceField,serviceField,serviceField,serviceField,serviceField,serviceField,serviceField,serviceField).

Reproduce: Ingo has GitLab integration, creates a Poker Meeting, adds GitLab tasks and moves to estimate stage. Paul has no integration and tries to join the meeting.

Dschoordsch · 2022-05-04T07:14:18Z

Since Matt had no -2 you are free to merge per our policy

nickoferrall and others added 30 commits February 21, 2022 17:14

render list of gitlab issues

3d7e51f

use usePaginationFragment

779662a

increase default value for usePaginationFragment

e8423fe

implement GitLabScopingSearchResultItem

857ba8b

add GitLabScopingSelectAllIssues

49f3377

can add and remove gitlab issues in scope phase

f563fcc

add UpdatePokerScopeMutation gitlab optimistic updater

87ab072

able to select all issues

37284a6

clean up type errors

e98ebc9

merge with master

0a9bba9

add projects name alias to rootSchema

5f27a7d

include projectIds in search query

6b0819a

return iid from GitLabIssueId

2f910f3

fix merge conflict

d52e82f

sort projects by lastActivityAt

fec37a1

implement fetchGitLabProjects

86fd627

implement gitlab issue menu

730d0f7

implement NewGitLabIssueMenuRoot instead of using useAllIntegrations

434ab02

load next if projects dont have any issues

4721123

update comment

c18debf

remove gitlab menu root and use defaultProjects query to populate menu

2b23794

increase projectsFirst from 10 to 20

790a144

query all gitlab projects

248ebad

merge with master

1e87555

chore(comment): how to extend BaseTaskIntegration

77d371c

able to create a new gitlab issue

b56dd56

use GitLabServerManager and implement parseWebPath

70283f8

fix undefined baseUri

67363cf

Merge branch 'feat/6088/gitlab-issues-functionality' of github.com:Pa…

6783b66

…rabolInc/parabol into feat/6088/gitlab-issues-functionality

changed taskIntegrationGitLab and GitLabId to use providerId

5170941

nickoferrall requested a review from igorlesnenko April 20, 2022 09:22

igorlesnenko reviewed Apr 20, 2022

View reviewed changes

packages/server/graphql/mutations/helpers/pushEstimateToGitLab.ts Outdated Show resolved Hide resolved

update pushEstimateToGitLab checks

bfa9ec2

Base automatically changed from feat/6303/track-gitlab to master April 20, 2022 12:30

fix merge conflicts

aa691d8

igorlesnenko approved these changes Apr 20, 2022

View reviewed changes

igorlesnenko requested a review from Dschoordsch April 20, 2022 13:09

mattkrick requested changes Apr 21, 2022

View reviewed changes

use new sdl pattern in updateGitLabDimensionField

30b30a4

nickoferrall requested a review from mattkrick April 22, 2022 10:15

Dschoordsch requested changes Apr 25, 2022

View reviewed changes

change teamId to varchar(100)

2511dcf

nickoferrall requested a review from Dschoordsch April 25, 2022 14:26

Dschoordsch requested changes Apr 26, 2022

View reviewed changes

use projectId and providerId in gitlab dimension queries

07c3d42

nickoferrall requested a review from Dschoordsch April 27, 2022 14:19

use GetIssueQuery type

c1ff4d5

nickoferrall commented Apr 27, 2022

View reviewed changes

nickoferrall mentioned this pull request Apr 28, 2022

feat(sprint-poker): Push task to GitLab #6427

Merged

Dschoordsch requested changes May 2, 2022

View reviewed changes

nickoferrall added 4 commits May 3, 2022 11:37

Merge branch 'master' into feat/6322/can-vote-on-gitlab-issue

fcecb09

return success payload for updateGitlabDimensionField mutation

22108df

use accessUserId instead of viewerId for the gitlabAuth

f3efc7d

fix merge conflicts

80737ec

nickoferrall requested a review from Dschoordsch May 3, 2022 14:24

Dschoordsch approved these changes May 4, 2022

View reviewed changes

nickoferrall merged commit 54c309e into master May 4, 2022

nickoferrall deleted the feat/6322/can-vote-on-gitlab-issue branch May 4, 2022 08:26

github-actions bot mentioned this pull request Sep 26, 2022

chore: release v6.75.1 #7229

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(sprint-poker): Can vote on GitLab issue #6398

feat(sprint-poker): Can vote on GitLab issue #6398

nickoferrall commented Apr 13, 2022 •

edited

Loading

mattkrick left a comment

mattkrick Apr 21, 2022

nickoferrall Apr 22, 2022

mattkrick Apr 21, 2022

nickoferrall Apr 22, 2022

Dschoordsch Apr 25, 2022

nickoferrall Apr 25, 2022

mattkrick Apr 26, 2022

Dschoordsch Apr 27, 2022

Dschoordsch Apr 27, 2022

Dschoordsch Apr 26, 2022

nickoferrall Apr 26, 2022

Dschoordsch Apr 27, 2022

nickoferrall Apr 27, 2022

Dschoordsch Apr 27, 2022

nickoferrall Apr 27, 2022

nickoferrall Apr 27, 2022

Dschoordsch May 2, 2022

Dschoordsch commented May 4, 2022

feat(sprint-poker): Can vote on GitLab issue #6398

feat(sprint-poker): Can vote on GitLab issue #6398

Conversation

nickoferrall commented Apr 13, 2022 • edited Loading

mattkrick left a 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

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

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

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

Dschoordsch commented May 4, 2022

nickoferrall commented Apr 13, 2022 •

edited

Loading