Merge pull request desktop#13306 from desktop/flatten-check-heirarchy

Flatten check run heirarchy

app/src/lib/api.ts

@@ -336,7 +336,6 @@ export interface IAPIRefCheckRun {
   readonly status: APICheckStatus
   readonly conclusion: APICheckConclusion | null
   readonly name: string
-  readonly output: IAPIRefCheckRunOutput
   readonly check_suite: IAPIRefCheckRunCheckSuite
   readonly app: IAPIRefCheckRunApp
   readonly completed_at: string

app/src/lib/ci-checks/ci-checks.ts

@@ -29,42 +29,16 @@ export interface IRefCheck {
   readonly status: APICheckStatus
   readonly conclusion: APICheckConclusion | null
   readonly appName: string
-  readonly checkSuiteId: number | null // API status don't have check suite id's
-  readonly output: IRefCheckOutput
   readonly htmlUrl: string | null
+
+  // Following are action check specific
+  readonly checkSuiteId: number | null
+  readonly actionJobSteps?: ReadonlyArray<IAPIWorkflowJobStep>
   readonly actionsWorkflowRunId?: number
+  readonly actionsWorkflowName?: string | null
   readonly logs_url?: string
 }
 
-/**
- * There are two types of check run outputs.
- *
- * 1. From GitHub Actions, which comes in steps with individual log texts,
- *    statuses, and duration info.
- * 2. From any other check run app, which comes with a generic string of
- *    whatever the check run app provides.
- */
-export type IRefCheckOutput =
-  | {
-      readonly title: string | null
-      readonly summary?: string | null
-      readonly type: RefCheckOutputType.Actions
-      readonly steps: ReadonlyArray<IAPIWorkflowJobStep>
-    }
-  | {
-      readonly title: string | null
-      readonly summary?: string | null
-      readonly type: RefCheckOutputType.Default
-      // This text is whatever a check run app decides to place in it.
-      // It may include html.
-      readonly text: string | null
-    }
-
-export enum RefCheckOutputType {
-  Actions = 'Actions',
-  Default = 'Default',
-}
-
 /**
  * A combined view of all legacy commit statuses as well as
  * check runs for a particular Git reference.
@@ -135,11 +109,6 @@ export function apiStatusToRefCheck(apiStatus: IAPIRefStatusItem): IRefCheck {
     conclusion,
     appName: '',
     checkSuiteId: null,
-    output: {
-      type: RefCheckOutputType.Default,
-      title: null,
-      text: null,
-    },
     htmlUrl: apiStatus.target_url,
   }
 }
@@ -261,10 +230,6 @@ export function apiCheckRunToRefCheck(checkRun: IAPIRefCheckRun): IRefCheck {
     conclusion: checkRun.conclusion,
     appName: checkRun.app.name,
     checkSuiteId: checkRun.check_suite.id,
-    output: {
-      ...checkRun.output,
-      type: RefCheckOutputType.Default,
-    },
     htmlUrl: checkRun.html_url,
   }
 }
@@ -427,11 +392,7 @@ export async function getLatestPRWorkflowRunsLogsForCheckRun(
       mappedCheckRuns.push({
         ...cr,
         htmlUrl: matchingJob.html_url,
-        output: {
-          ...cr.output,
-          type: RefCheckOutputType.Actions,
-          steps: matchingJob.steps,
-        },
+        actionJobSteps: matchingJob.steps,
       })
 
       continue
@@ -452,11 +413,7 @@ export async function getLatestPRWorkflowRunsLogsForCheckRun(
     mappedCheckRuns.push({
       ...cr,
       htmlUrl: matchingJob.html_url,
-      output: {
-        ...cr.output,
-        type: RefCheckOutputType.Actions,
-        steps: await parseJobStepLogs(logZip, matchingJob),
-      },
+      actionJobSteps: await parseJobStepLogs(logZip, matchingJob),
     })
   }
 
@@ -550,10 +507,11 @@ function getCheckRunWithActionsJobAndLogURLs(
       continue
     }
 
-    const { id, logs_url } = matchingWR
+    const { id, name, logs_url } = matchingWR
     mappedCheckRuns.push({
       ...cr,
       actionsWorkflowRunId: id,
+      actionsWorkflowName: name,
       logs_url,
     })
   }
@@ -572,3 +530,18 @@ export function getFormattedCheckRunDuration(
     .duration(getCheckDurationInSeconds(checkRun), 'seconds')
     .format('d[d] h[h] m[m] s[s]', { largest: 4 })
 }
+
+/** Get check run display name
+ *
+ * Goal: Action Workflow Name / workflow run name
+ * If no workflow run name (non-actions check), then just return the name.
+ */
+export function getCheckRunDisplayName(checkRun: IRefCheck): string {
+  const wfName =
+    checkRun.actionsWorkflowName !== undefined
+      ? checkRun.actionsWorkflowName
+      : checkRun.appName === 'GitHub Code Scanning'
+      ? 'Code scanning results' // seems this is hardcoded on dotcom too :/
+      : undefined
+  return wfName !== undefined ? `${wfName} / ${checkRun.name}` : checkRun.name
+}

app/src/ui/check-runs/ci-check-run-actions-logs.tsx

@@ -15,9 +15,7 @@ import {
 import { IAPIWorkflowJobStep } from '../../lib/api'
 import {
   getFormattedCheckRunDuration,
-  IRefCheckOutput,
   isFailure,
-  RefCheckOutputType,
 } from '../../lib/ci-checks/ci-checks'
 import { enableCICheckRunsLogs } from '../../lib/feature-flag'
 
@@ -26,8 +24,7 @@ const MAX_LOG_LINE_NUMBER_WIDTH = 100 // arbitrarily chosen
 const INDIVIDUAL_LOG_LINE_NUMBER_WIDTH_ALLOWED = 10
 
 interface ICICheckRunActionLogsProps {
-  /** The check run to display **/
-  readonly output: IRefCheckOutput
+  readonly actionSteps: ReadonlyArray<IAPIWorkflowJobStep>
 }
 
 interface ICICheckRunActionLogsState {
@@ -45,10 +42,7 @@ export class CICheckRunActionLogs extends React.PureComponent<
     super(props)
 
     const openSections = new Set<number>()
-    const firstFailedSectionIndex =
-      props.output.type === RefCheckOutputType.Actions
-        ? props.output.steps.findIndex(isFailure)
-        : -1
+    const firstFailedSectionIndex = this.props.actionSteps.findIndex(isFailure)
 
     openSections.add(firstFailedSectionIndex)
 
@@ -59,15 +53,8 @@ export class CICheckRunActionLogs extends React.PureComponent<
   }
 
   public componentDidUpdate(prevProps: ICICheckRunActionLogsProps) {
-    if (
-      prevProps.output.type === this.props.output.type ||
-      this.props.output.type !== RefCheckOutputType.Actions
-    ) {
-      return
-    }
-
     const openSections = new Set<number>()
-    const firstFailedSectionIndex = this.props.output.steps.findIndex(isFailure)
+    const firstFailedSectionIndex = this.props.actionSteps.findIndex(isFailure)
 
     openSections.add(firstFailedSectionIndex)
 
@@ -333,14 +320,9 @@ export class CICheckRunActionLogs extends React.PureComponent<
   }
 
   public render() {
-    const { output } = this.props
-
-    if (output.type !== RefCheckOutputType.Actions) {
-      // This shouldn't happen, should only be provided actions type
-      return <>Unable to load logs.</>
-    }
+    const { actionSteps } = this.props
 
-    return output.steps.map((step, i) => {
+    return actionSteps.map((step, i) => {
       const isSkipped = step.conclusion === 'skipped'
       const showLogs = this.state.openSections.has(i) && !isSkipped
 

app/src/ui/check-runs/ci-check-run-list-item.tsx

@@ -1,5 +1,8 @@
 import * as React from 'react'
-import { IRefCheck } from '../../lib/ci-checks/ci-checks'
+import {
+  getCheckRunDisplayName,
+  IRefCheck,
+} from '../../lib/ci-checks/ci-checks'
 import { Octicon } from '../octicons'
 import { getClassNameForCheck, getSymbolForCheck } from '../branches/ci-status'
 import classNames from 'classnames'
@@ -124,6 +127,7 @@ export class CICheckRunListItem extends React.PureComponent<
   public render() {
     const { checkRun, showLogs, baseHref } = this.props
 
+    const name = getCheckRunDisplayName(checkRun)
     return (
       <>
         <div
@@ -134,11 +138,11 @@ export class CICheckRunListItem extends React.PureComponent<
           <div className="ci-check-list-item-detail">
             <TooltippedContent
               className="ci-check-name"
-              tooltip={checkRun.name}
+              tooltip={name}
               onlyWhenOverflowed={true}
               tagName="div"
             >
-              {checkRun.name}
+              {name}
             </TooltippedContent>
 
             <div className="ci-check-description">{checkRun.description}</div>
@@ -155,7 +159,9 @@ export class CICheckRunListItem extends React.PureComponent<
             />
           </div>
         </div>
-        {showLogs && !this.isLoading() ? (
+        {showLogs &&
+        !this.isLoading() &&
+        checkRun.actionJobSteps !== undefined ? (
           <CICheckRunLogs
             checkRun={checkRun}
             baseHref={baseHref}