own: support removing teams from manually assigned owners (#53199)

Adds team support to the remove owner button. ## Test plan ![CleanShot 2023-06-08 at 15 14 26](https://github.com/sourcegraph/sourcegraph/assets/5090588/b6fd6c13-69ef-45bb-906e-a4d6400a1bb2)
sourcegraph · Jun 22, 2023 · 586aef0 · 586aef0
1 parent ecbf306
commit 586aef0
Show file tree

Hide file tree

Showing 6 changed files with 77 additions and 52 deletions.
diff --git a/client/web/src/repo/blob/own/FileOwnershipEntry.tsx b/client/web/src/repo/blob/own/FileOwnershipEntry.tsx
@@ -26,7 +26,6 @@ interface Props {
     makeOwnerButton?: React.ReactElement
     repoID: string
     filePath: string
-    userID?: string
     setRemoveOwnerError: any
     refetch: any
 }
@@ -43,7 +42,6 @@ export const FileOwnershipEntry: React.FunctionComponent<Props> = ({
     makeOwnerButton,
     repoID,
     filePath,
-    userID,
     refetch,
     setRemoveOwnerError,
 }) => {
@@ -57,6 +55,9 @@ export const FileOwnershipEntry: React.FunctionComponent<Props> = ({
         return owner.email
     }
 
+    const userID = owner.__typename === 'Person' && owner.user?.__typename === 'User' ? owner.user.id : undefined
+    const teamID = owner.__typename === 'Team' ? owner.id : undefined
+
     const email = findEmail()
 
     const assignedOwnerReasons: AssignedOwnerFields[] = reasons
@@ -119,7 +120,8 @@ export const FileOwnershipEntry: React.FunctionComponent<Props> = ({
                                 onError={setRemoveOwnerError}
                                 repoId={repoID}
                                 path={filePath}
-                                userId={userID}
+                                userID={userID}
+                                teamID={teamID}
                                 isDirectAssigned={isDirectAssigned}
                             />
                         ))}

diff --git a/client/web/src/repo/blob/own/FileOwnershipPanel.story.tsx b/client/web/src/repo/blob/own/FileOwnershipPanel.story.tsx
@@ -81,6 +81,7 @@ const response: FetchOwnershipResult = {
                             __typename: 'Ownership',
                             owner: {
                                 __typename: 'Team',
+                                id: 'asdf',
                                 avatarURL: null,
                                 teamDisplayName: 'Delta Team',
                                 name: 'delta',

diff --git a/client/web/src/repo/blob/own/OwnerList.tsx b/client/web/src/repo/blob/own/OwnerList.tsx
@@ -148,29 +148,20 @@ export const OwnerList: React.FunctionComponent<OwnerListProps> = ({
                                         reason.__typename === 'AssignedOwner'
                                 )
                             )
-                            .map((ownership, index) => {
-                                const userId =
-                                    ownership.owner.__typename === 'Person' &&
-                                    ownership.owner.user?.__typename === 'User'
-                                        ? ownership.owner.user.id
-                                        : undefined
+                            .map((ownership, index) => (
                                 // This list is not expected to change, so it's safe to use the index as a key.
-
-                                return (
-                                    <React.Fragment key={index}>
-                                        {index > 0 && <tr className={styles.bordered} />}
-                                        <FileOwnershipEntry
-                                            refetch={refetch}
-                                            owner={ownership.owner}
-                                            userID={userId}
-                                            repoID={repoID}
-                                            filePath={filePath}
-                                            reasons={ownership.reasons}
-                                            setRemoveOwnerError={setRemoveOwnerError}
-                                        />
-                                    </React.Fragment>
-                                )
-                            })}
+                                <React.Fragment key={index}>
+                                    {index > 0 && <tr className={styles.bordered} />}
+                                    <FileOwnershipEntry
+                                        refetch={refetch}
+                                        owner={ownership.owner}
+                                        repoID={repoID}
+                                        filePath={filePath}
+                                        reasons={ownership.reasons}
+                                        setRemoveOwnerError={setRemoveOwnerError}
+                                    />
+                                </React.Fragment>
+                            ))}
                         {
                             /* Visually separate two sets with a horizontal rule (like subsequent owners are)
                              * if there is data in both owners and signals.
@@ -211,7 +202,6 @@ export const OwnerList: React.FunctionComponent<OwnerListProps> = ({
                                             owner={ownership.owner}
                                             reasons={ownership.reasons}
                                             makeOwnerButton={makeOwnerButton?.(userId)}
-                                            userID={userId}
                                             repoID={repoID}
                                             filePath={filePath}
                                             refetch={refetch}

diff --git a/client/web/src/repo/blob/own/RemoveOwnerButton.tsx b/client/web/src/repo/blob/own/RemoveOwnerButton.tsx
@@ -6,16 +6,22 @@ import { ErrorLike, asError } from '@sourcegraph/common'
 import { useMutation } from '@sourcegraph/http-client'
 import { Button, Icon, Tooltip } from '@sourcegraph/wildcard'
 
-import { RemoveAssignedOwnerResult, RemoveAssignedOwnerVariables } from '../../../graphql-operations'
+import {
+    RemoveAssignedOwnerResult,
+    RemoveAssignedOwnerVariables,
+    RemoveAssignedTeamResult,
+    RemoveAssignedTeamVariables,
+} from '../../../graphql-operations'
 
-import { REMOVE_ASSIGNED_OWNER } from './grapqlQueries'
+import { REMOVE_ASSIGNED_OWNER, REMOVE_ASSIGNED_TEAM } from './grapqlQueries'
 
 export interface RemoveOwnerButtonProps {
     onSuccess: () => Promise<any>
     onError: (e: Error) => void
     repoId: string
     path: string
-    userId?: string
+    userID?: string
+    teamID?: string
     isDirectAssigned: boolean
 }
 
@@ -24,35 +30,44 @@ export const RemoveOwnerButton: React.FC<RemoveOwnerButtonProps> = ({
     onError,
     repoId,
     path,
-    userId,
+    userID,
+    teamID,
     isDirectAssigned,
 }) => {
     const tooltipContent = !isDirectAssigned
         ? 'Ownership can only be modified at the same direct path as it was assigned.'
         : 'Remove ownership'
 
-    const [removeAssignedOwner, { loading }] = useMutation<RemoveAssignedOwnerResult, RemoveAssignedOwnerVariables>(
-        REMOVE_ASSIGNED_OWNER,
-        {}
-    )
+    const [removeAssignedOwner, { loading: removeLoading }] = useMutation<
+        RemoveAssignedOwnerResult,
+        RemoveAssignedOwnerVariables
+    >(REMOVE_ASSIGNED_OWNER, {})
+    const [removeAssignedTeam, { loading: removeTeamLoading }] = useMutation<
+        RemoveAssignedTeamResult,
+        RemoveAssignedTeamVariables
+    >(REMOVE_ASSIGNED_TEAM, {})
+
+    const createInputObject = (id: string): any => ({
+        variables: {
+            input: {
+                absolutePath: path,
+                assignedOwnerID: id,
+                repoID: repoId,
+            },
+        },
+        onCompleted: async () => {
+            await onSuccess()
+        },
+        onError: (errors: ErrorLike) => {
+            onError(asError(errors))
+        },
+    })
 
     const removeOwner: () => Promise<void> = async () => {
-        if (userId) {
-            await removeAssignedOwner({
-                variables: {
-                    input: {
-                        absolutePath: path,
-                        assignedOwnerID: userId,
-                        repoID: repoId,
-                    },
-                },
-                onCompleted: async () => {
-                    await onSuccess()
-                },
-                onError: (errors: ErrorLike) => {
-                    onError(asError(errors))
-                },
-            })
+        if (userID) {
+            await removeAssignedOwner(createInputObject(userID))
+        } else if (teamID) {
+            await removeAssignedTeam(createInputObject(teamID))
         }
     }
 
@@ -67,7 +82,11 @@ export const RemoveOwnerButton: React.FC<RemoveOwnerButtonProps> = ({
                 size="sm"
                 disabled={!isDirectAssigned}
             >
-                <Icon color="white" aria-hidden={true} svgPath={loading ? mdiLoading : mdiDelete} />
+                <Icon
+                    color="white"
+                    aria-hidden={true}
+                    svgPath={removeLoading || removeTeamLoading ? mdiLoading : mdiDelete}
+                />
                 Remove owner
             </Button>
         </Tooltip>

diff --git a/client/web/src/repo/blob/own/grapqlQueries.ts b/client/web/src/repo/blob/own/grapqlQueries.ts
@@ -20,6 +20,7 @@ export const OWNER_FIELDS = gql`
             }
         }
         ... on Team {
+            id
             name
             teamDisplayName: displayName
             avatarURL
@@ -199,3 +200,11 @@ export const REMOVE_ASSIGNED_OWNER = gql`
         }
     }
 `
+
+export const REMOVE_ASSIGNED_TEAM = gql`
+    mutation RemoveAssignedTeam($input: AssignOwnerOrTeamInput!) {
+        removeAssignedTeam(input: $input) {
+            alwaysNil
+        }
+    }
+`
diff --git a/enterprise/cmd/frontend/internal/own/resolvers/resolvers.go b/enterprise/cmd/frontend/internal/own/resolvers/resolvers.go
@@ -794,6 +794,7 @@ func (r *ownResolver) computeAssignedTeams(ctx context.Context, db edb.Enterpris
 
 	fetchedTeams := make(map[int32]*types.Team)
 
+	isDirectMatch := false
 	for _, summary := range assignedTeamSummaries {
 		var team *types.Team
 		teamID := summary.OwnerTeamID
@@ -807,14 +808,17 @@ func (r *ownResolver) computeAssignedTeams(ctx context.Context, db edb.Enterpris
 			team = teamFromDB
 			fetchedTeams[teamID] = teamFromDB
 		}
+		if blob.Path() == summary.FilePath {
+			isDirectMatch = true
+		}
 		res := ownershipResolver{
 			db: db,
 			resolvedOwner: &codeowners.Team{
 				Team: team,
 			},
 			reasons: []*ownershipReasonResolver{
 				{
-					&assignedOwner{},
+					&assignedOwner{directMatch: isDirectMatch},
 				},
 			},
 		}