fix: name the lfx insights project in the review card and shorten the hand-off reason (lfx-cdp-insights-ops#822)

backend/src/services/slack/answerRequestCommand.test.ts

@@ -49,6 +49,7 @@ const row = {
   sourceUrl: 'https://acme.slack.com/archives/C1/p1',
   action: 'ask',
   requestedBy: 'U-REQUESTER',
+  projectName: 'Acme Widgets',
   requestQuestion: question,
 } as any
 
@@ -110,7 +111,7 @@ describe('runAnswerRequestCommand', () => {
       CATALOG_ID,
       expect.objectContaining({
         action: 'review',
-        reviewReason: 'the requester couldn\'t answer "Did you mean Acme?" (No, ask the CDP team)',
+        reviewReason: "the requester said Acme Widgets isn't Acme",
         proposedAction: 'human_review',
       }),
     )

backend/src/services/slack/answerRequestCommand.ts

@@ -1,14 +1,12 @@
 import { answerProjectCatalogQuestion, findProjectCatalogById } from '@crowd/data-access-layer'
-import {
-  IDbProjectCatalog,
-  IRequestQuestionOption,
-} from '@crowd/data-access-layer/src/project-catalog/types'
+import { IDbProjectCatalog } from '@crowd/data-access-layer/src/project-catalog/types'
 import {
   applyRequestStatus,
   buildBotAnsweredContent,
   decodeAnswerValue,
   describeAnsweredQuestion,
   findQuestionOption,
+  handOffReason,
   refreshRequestStatusMessage,
 } from '@crowd/project-onboarding'
 import { getSlackBotConfig } from '@crowd/slack'
@@ -46,10 +44,6 @@ function canAnswer(row: IDbProjectCatalog, actorId: string): boolean {
   return row.requestedBy === actorId || getSlackBotConfig().reviewerUserIds.includes(actorId)
 }
 
-function handOffReason(question: string, option: IRequestQuestionOption): string {
-  return `the requester couldn't answer "${question}" (${option.label})`
-}
-
 async function startAnswerEvaluation(
   temporal: OnboardingRequestTemporal | null,
   catalogId: string,
@@ -109,7 +103,7 @@ export async function runAnswerRequestCommand({
     answeredBy: actorId,
     hints: option.hints ?? null,
     action: option.handOff ? 'review' : 'evaluate',
-    reviewReason: option.handOff ? handOffReason(row.requestQuestion.text, option) : null,
+    reviewReason: option.handOff ? handOffReason(row, row.requestQuestion) : null,
     proposedAction: option.handOff ? 'human_review' : null,
   })
   if (!answered) {

services/libs/data-access-layer/src/project-catalog/types.ts

@@ -71,6 +71,7 @@ export interface IRequestQuestionOption {
   label: string
   hints?: IRequestHints
   handOff?: boolean
+  suggested?: boolean
 }
 
 export interface IRequestQuestion {

services/libs/project-onboarding/src/botThreadReply.test.ts

@@ -124,10 +124,10 @@ describe('botThreadReply', () => {
         ...project,
         classificationKind: 'ambiguous',
         proposedAction: 'human_review',
-        reviewReason: 'the requester couldn\'t answer "Did you mean Acme?" (No, ask the CDP team)',
+        reviewReason: "the requester said Acme Widgets isn't Acme",
       }),
     ).toBe(
-      `👀 ${REPO_LABEL}: the requester couldn't answer "Did you mean Acme?" (No, ask the CDP team). I've asked the CDP team to pick the right project and add it by hand; they'll confirm here.`,
+      `👀 ${REPO_LABEL}: the requester said Acme Widgets isn't Acme. I've asked the CDP team to pick the right project and add it by hand; they'll confirm here.`,
     )
     expect(
       buildBotReviewReply({

services/libs/project-onboarding/src/classifyCatalogRow.test.ts

@@ -278,6 +278,21 @@ describe('classifyCatalogRow', () => {
     })
   })
 
+  it('names the LFX Insights project when it differs from the PCC node that owns it', async () => {
+    const parent = { ...pccProject, name: 'Software Supply Chain Security Project', isLeaf: false }
+    const result = await classifyCatalogRow(
+      row({ projectName: 'Uptane' }),
+      deps({ candidates: [parent], cdpSegment: { ...segment, name: 'Uptane' } }),
+    )
+
+    expect(result.classification).toMatchObject({
+      classificationKind: 'lf_in_cdp',
+      pccProjectId: 'pcc-1',
+      matchedProjectName: 'Uptane',
+      matchedSegmentId: 'seg-1',
+    })
+  })
+
   it('routes an LF project in CDP to review when auto apply is off', async () => {
     const result = await classifyCatalogRow(
       row(),

services/libs/project-onboarding/src/classifyCatalogRow.ts

@@ -118,7 +118,7 @@ function toProposal(resolution: OnboardingResolution): IProposal {
         classificationKind: resolution.kind,
         proposedAction: resolution.action,
         pccProjectId: resolution.pccProject.projectId,
-        matchedProjectName: resolution.pccProject.name,
+        matchedProjectName: resolution.segment.name,
         matchedSegmentId: resolution.segment.segmentId,
       }
     case 'lf_owner_in_cdp':

services/libs/project-onboarding/src/requestQuestion.test.ts

@@ -7,6 +7,7 @@ import {
   describeAnsweredQuestion,
   encodeAnswerValue,
   findQuestionOption,
+  handOffReason,
 } from './requestQuestion'
 
 const question = {
@@ -17,18 +18,41 @@ const question = {
       id: 'yes',
       label: 'Yes, Acme',
       hints: { pccProject: { projectId: 'p', name: 'Acme', slug: 'a', isLeaf: true } },
+      suggested: true,
     },
     { id: 'no', label: 'No, ask the CDP team', handOff: true },
   ],
 }
+const pickQuestion = {
+  kind: 'pick_project' as const,
+  text: 'There are 2 Acme projects in LFX Insights. Which one should this repo go in?',
+  options: [
+    { id: 'seg-1', label: 'Acme · 28 repos', hints: { segmentId: 'seg-1' } },
+    { id: 'seg-2', label: 'Acme · no repos yet', hints: { segmentId: 'seg-2' } },
+    { id: 'not_sure', label: 'Not sure, ask the CDP team', handOff: true },
+  ],
+}
+const conflictQuestion = {
+  kind: 'lf_conflict' as const,
+  text: "You said Acme isn't an LF project, but I found an LF project called Acme Labs. Which is right?",
+  options: [
+    {
+      id: 'use_lf',
+      label: 'Use the LF project Acme Labs',
+      hints: { pccProject: { projectId: 'p', name: 'Acme Labs', slug: 'a', isLeaf: true } },
+    },
+    { id: 'not_lf', label: "It's not LF, treat it as new", hints: { treatAsNew: true } },
+    { id: 'not_sure', label: 'Not sure, ask the CDP team', handOff: true },
+  ],
+}
 const row = {
   id: '8f1c2a44-5b7e-4d2f-9c1a-3e6f7a8b9c0d',
   repoUrl: 'https://github.com/acme/widget',
   requestQuestion: question,
 }
 
 describe('buildBotQuestionContent', () => {
-  it('posts the question with one button per option, the first one primary', () => {
+  it('posts the question with one button per option, the suggested one primary', () => {
     const content = buildBotQuestionContent(row)
 
     expect(content?.text).toBe(
@@ -56,6 +80,14 @@ describe('buildBotQuestionContent', () => {
     expect(buildBotQuestionContent({ ...row, requestQuestion: null })).toBeNull()
   })
 
+  it('leaves every button plain when the bot has no suggestion', () => {
+    const content = buildBotQuestionContent({ ...row, requestQuestion: pickQuestion })
+
+    const buttons = content?.blocks?.[1] as { elements: Array<{ style?: string }> }
+    expect(buttons.elements).toHaveLength(3)
+    expect(buttons.elements.every((button) => button.style === undefined)).toBe(true)
+  })
+
   it('replaces the question with the answer and what happens next, buttons gone', () => {
     const answered = buildBotAnsweredContent(row, question, question.options[0], 'U1')
     expect(answered.text).toContain(
@@ -78,6 +110,23 @@ describe('answer values', () => {
     expect(decodeAnswerValue('abc|')).toBeNull()
   })
 
+  it('describes a hand-off in a few words per question kind', () => {
+    const named = { projectName: 'Acme Widgets' }
+    expect(handOffReason(named, question)).toBe("the requester said Acme Widgets isn't Acme")
+    expect(handOffReason({ projectName: null }, question)).toBe(
+      "the requester didn't recognise the project I suggested",
+    )
+    expect(handOffReason(named, pickQuestion)).toBe(
+      "the requester couldn't tell which Acme Widgets project this repo belongs to",
+    )
+    expect(handOffReason({ projectName: null }, pickQuestion)).toBe(
+      "the requester couldn't tell which project this repo belongs to",
+    )
+    expect(handOffReason(named, conflictQuestion)).toBe(
+      "the requester isn't sure whether Acme Widgets is the LF project Acme Labs",
+    )
+  })
+
   it('finds options and describes an answered question', () => {
     expect(findQuestionOption(question, 'no')?.handOff).toBe(true)
     expect(findQuestionOption(question, 'maybe')).toBeNull()

services/libs/project-onboarding/src/requestQuestion.ts

@@ -45,7 +45,7 @@ function optionButtons(catalogId: string, options: IRequestQuestionOption[]): Kn
         action_id: answerActionId(start + offset),
         value: encodeAnswerValue(catalogId, option.id),
         text: { type: 'plain_text', text: buttonText(option.label) },
-        ...(start + offset === 0 && !option.handOff ? { style: 'primary' as const } : {}),
+        ...(option.suggested ? { style: 'primary' as const } : {}),
       })),
     })
   }
@@ -85,6 +85,32 @@ export function buildBotAnsweredContent(
   return { text, blocks: mrkdwnSections(text) }
 }
 
+function suggestedProjectName(question: IRequestQuestion): string | null {
+  return (
+    question.options.find((option) => option.hints?.pccProject)?.hints?.pccProject?.name ?? null
+  )
+}
+
+export function handOffReason(
+  row: Pick<IDbProjectCatalog, 'projectName'>,
+  question: IRequestQuestion,
+): string {
+  const name = row.projectName
+  const suggested = suggestedProjectName(question)
+  switch (question.kind) {
+    case 'did_you_mean':
+      return name && suggested
+        ? `the requester said ${name} isn't ${suggested}`
+        : "the requester didn't recognise the project I suggested"
+    case 'pick_project':
+      return `the requester couldn't tell which ${name ? `${name} ` : ''}project this repo belongs to`
+    case 'lf_conflict':
+      return suggested
+        ? `the requester isn't sure whether ${name ?? 'this repo'} is the LF project ${suggested}`
+        : `the requester isn't sure whether ${name ?? 'this repo'} is an LF project`
+  }
+}
+
 export function describeAnsweredQuestion(question: IRequestQuestion): string | null {
   if (!question.answeredBy || !question.answer) {
     return null

services/libs/project-onboarding/src/requestResolver.test.ts

@@ -131,6 +131,7 @@ describe('resolveOnboardingRequest', () => {
                 isLeaf: true,
               },
             },
+            suggested: true,
           },
           { id: 'no', label: 'No, ask the CDP team', handOff: true },
         ],

services/libs/project-onboarding/src/requestResolver.ts

@@ -215,7 +215,12 @@ function didYouMean(projectName: string, best: IPccCandidate): OnboardingResolut
       kind: 'did_you_mean',
       text: `I couldn't find a project called "${projectName}". Did you mean ${best.name}?`,
       options: [
-        { id: 'yes', label: `Yes, ${best.name}`, hints: { pccProject: toHintProject(best) } },
+        {
+          id: 'yes',
+          label: `Yes, ${best.name}`,
+          hints: { pccProject: toHintProject(best) },
+          suggested: true,
+        },
         { id: 'no', label: 'No, ask the CDP team', handOff: true },
       ],
     },