Connect flag selector to flags filter on PR details page (#2343)

* feat: Update impacted files resolver for use pull, connect the flag selector to the api.

* update missing logic as requested + unknown flags message to be aligned with repo overview design

* Noticed the changing the pull query broke impacted files while smoke testing, dupicated the same compatibility work for the new resolver.

* Refactor to use a impacted files enum as requested.

src/pages/PullRequestPage/subroute/FilesChangedTab/FilesChanged/FilesChanged.jsx

@@ -2,6 +2,7 @@ import isNil from 'lodash/isNil'
 
 import { CommitStateEnum } from 'shared/utils/commit'
 import { ComparisonReturnType } from 'shared/utils/comparison'
+import { ImpactedFilesReturnType } from 'shared/utils/impactedFiles'
 import Spinner from 'ui/Spinner'
 
 import { useImpactedFilesTable } from './hooks'
@@ -13,10 +14,6 @@ const Loader = () => (
   </div>
 )
 
-function hasImpactedFiles(impactedFiles) {
-  return impactedFiles && impactedFiles?.length > 0
-}
-
 function hasReportWithoutChanges({
   pullHeadCoverage,
   pullBaseCoverage,
@@ -57,14 +54,29 @@ function FilesChangedTab() {
     )
   }
 
-  if (hasImpactedFiles(data?.impactedFiles)) {
+  if (data?.impactedFilesType === ImpactedFilesReturnType.UNKNOWN_FLAGS) {
+    return (
+      <div className="flex flex-col gap-2">
+        <p className="mt-4">
+          No coverage report uploaded for the selected flags in this pull
+          request&apos;s head commit.
+        </p>
+      </div>
+    )
+  }
+
+  if (
+    data?.impactedFilesType === ImpactedFilesReturnType.IMPACTED_FILES &&
+    data?.impactedFiles.length > 0
+  ) {
     return (
       <div className="flex flex-col gap-2">
         <Table />
       </div>
     )
   }
 
+  // TODO to be replaced by new comparison types
   if (
     hasReportWithoutChanges({
       pullHeadCoverage: data?.pullHeadCoverage,

src/pages/PullRequestPage/subroute/FilesChangedTab/FilesChanged/FilesChanged.spec.jsx

@@ -7,6 +7,7 @@ import { MemoryRouter, Route } from 'react-router-dom'
 
 import { CommitStateEnum } from 'shared/utils/commit'
 import { ComparisonReturnType } from 'shared/utils/comparison'
+import { ImpactedFilesReturnType } from 'shared/utils/impactedFiles'
 
 import FilesChanged from './FilesChanged'
 
@@ -67,7 +68,10 @@ const mockPull = {
             percentCovered: 27.35,
           },
           changeCoverage: 38.94,
-          impactedFiles: mockImpactedFiles,
+          impactedFiles: {
+            __typename: ImpactedFilesReturnType.IMPACTED_FILES,
+            results: mockImpactedFiles,
+          },
         },
       },
     },
@@ -145,7 +149,10 @@ describe('FilesChanged', () => {
                   percentCovered: 27.35,
                 },
                 changeCoverage: 38.94,
-                impactedFiles: [],
+                impactedFiles: {
+                  __typename: ImpactedFilesReturnType.IMPACTED_FILES,
+                  results: [],
+                },
               },
             },
           },
@@ -246,4 +253,47 @@ describe('FilesChanged', () => {
       expect(firstPullRequestCopy).toBeInTheDocument()
     })
   })
+
+  describe('unknown flag status', () => {
+    it('Displays server message + suggests carryforward flags', async () => {
+      const overrideData = {
+        owner: {
+          repository: {
+            pull: {
+              pullId: 14,
+              head: {
+                state: CommitStateEnum.COMPLETE,
+              },
+              compareWithBase: {
+                __typename: ComparisonReturnType.SUCCESSFUL_COMPARISON,
+                patchTotals: {
+                  percentCovered: 92.12,
+                },
+                headTotals: {
+                  percentCovered: 74.2,
+                },
+                baseTotals: {
+                  percentCovered: 27.35,
+                },
+                changeCoverage: 38.94,
+                impactedFiles: {
+                  __typename: 'UnknownFlags',
+                  message: 'Unkown flags detected',
+                },
+              },
+            },
+          },
+        },
+      }
+      setup({
+        overrideData,
+      })
+      render(<FilesChanged />, { wrapper })
+
+      const serverMessage = await screen.findByText(
+        /No coverage report uploaded for the selected flags in this pull request's head commit./
+      )
+      expect(serverMessage).toBeInTheDocument()
+    })
+  })
 })

src/pages/PullRequestPage/subroute/FilesChangedTab/FilesChanged/Table/Table.spec.jsx

@@ -7,6 +7,8 @@ import { graphql } from 'msw'
 import { setupServer } from 'msw/node'
 import { MemoryRouter, Route } from 'react-router-dom'
 
+import { ImpactedFilesReturnType } from 'shared/utils/impactedFiles'
+
 import Table from './Table'
 
 jest.mock('../../shared/FileDiff', () => () => 'FileDiff Component')
@@ -37,6 +39,7 @@ const mockPull = {
           state: 'PROCESSED',
         },
         compareWithBase: {
+          __typename: 'Comparison',
           patchTotals: {
             percentCovered: 92.12,
           },
@@ -47,7 +50,10 @@ const mockPull = {
             percentCovered: 27.35,
           },
           changeCoverage: 38.94,
-          impactedFiles: mockTable,
+          impactedFiles: {
+            __typename: ImpactedFilesReturnType.IMPACTED_FILES,
+            results: mockTable,
+          },
         },
       },
     },
@@ -63,6 +69,7 @@ const mockNoTable = {
           state: 'PROCESSED',
         },
         compareWithBase: {
+          __typename: 'Comparison',
           patchTotals: {
             percentCovered: 92.12,
           },
@@ -73,7 +80,10 @@ const mockNoTable = {
             percentCovered: 27.35,
           },
           changeCoverage: 38.94,
-          impactedFiles: [],
+          impactedFiles: {
+            __typename: ImpactedFilesReturnType.IMPACTED_FILES,
+            results: [],
+          },
         },
       },
     },
@@ -89,6 +99,7 @@ const mockNoChange = {
           state: 'PROCESSED',
         },
         compareWithBase: {
+          __typename: 'Comparison',
           patchTotals: {
             percentCovered: 92.12,
           },
@@ -99,22 +110,25 @@ const mockNoChange = {
             percentCovered: 27.35,
           },
           changeCoverage: 38.94,
-          impactedFiles: [
-            {
-              isCriticalFile: true,
-              fileName: 'mafs.js',
-              headName: 'flag1/mafs.js',
-              baseCoverage: {
-                percentCovered: null,
-              },
-              headCoverage: {
-                percentCovered: null,
-              },
-              patchCoverage: {
-                percentCovered: null,
+          impactedFiles: {
+            __typename: ImpactedFilesReturnType.IMPACTED_FILES,
+            results: [
+              {
+                isCriticalFile: true,
+                fileName: 'mafs.js',
+                headName: 'flag1/mafs.js',
+                baseCoverage: {
+                  percentCovered: null,
+                },
+                headCoverage: {
+                  percentCovered: null,
+                },
+                patchCoverage: {
+                  percentCovered: null,
+                },
               },
-            },
-          ],
+            ],
+          },
         },
       },
     },
@@ -127,6 +141,7 @@ const mockSingularTableData = {
       pull: {
         pullId: 14,
         compareWithBase: {
+          __typename: 'Comparison',
           impactedFile: {
             headName: 'file A',
             isNewFile: true,

src/pages/PullRequestPage/subroute/FilesChangedTab/FilesChanged/hooks/useImpactedFilesTable.js

@@ -1,9 +1,11 @@
 import isEqual from 'lodash/isEqual'
 import isNumber from 'lodash/isNumber'
+import qs from 'qs'
 import { useCallback, useState } from 'react'
-import { useParams } from 'react-router-dom'
+import { useLocation, useParams } from 'react-router-dom'
 
 import { usePull } from 'services/pull'
+import { ImpactedFilesReturnType } from 'shared/utils/impactedFiles'
 
 const orderingDirection = Object.freeze({
   desc: 'DESC',
@@ -18,56 +20,74 @@ export const orderingParameter = Object.freeze({
   missesCount: 'MISSES_COUNT',
 })
 
-function getFilters({ sortBy }) {
+function getFilters({ sortBy, flags }) {
   return {
     ordering: {
       direction: sortBy?.desc ? orderingDirection.desc : orderingDirection.asc,
       parameter: orderingParameter[sortBy?.id],
     },
     hasUnintendedChanges: false,
+    ...(flags ? { flags } : {}),
   }
 }
 
 function transformImpactedFilesData({ pull }) {
   const compareWithBase = pull?.compareWithBase
-  const impactedFiles = compareWithBase?.impactedFiles?.map((impactedFile) => {
-    const headCoverage = impactedFile?.headCoverage?.percentCovered
-    const missesCount = impactedFile?.missesCount || 0
-    const patchCoverage = impactedFile?.patchCoverage?.percentCovered
-    const baseCoverage = impactedFile?.baseCoverage?.percentCovered
-    const changeCoverage =
-      isNumber(headCoverage) && isNumber(baseCoverage)
-        ? headCoverage - baseCoverage
-        : Number.NaN
-    const hasHeadOrPatchCoverage =
-      isNumber(headCoverage) || isNumber(patchCoverage)
 
-    return {
-      missesCount,
-      headCoverage,
-      patchCoverage,
-      changeCoverage,
-      hasHeadOrPatchCoverage,
-      headName: impactedFile?.headName,
-      fileName: impactedFile?.fileName,
-      isCriticalFile: impactedFile?.isCriticalFile,
-      pullId: pull?.pullId,
+  const mutatedImpactedFiles = compareWithBase?.impactedFiles?.results?.map(
+    (impactedFile) => {
+      const headCoverage = impactedFile?.headCoverage?.percentCovered
+      const missesCount = impactedFile?.missesCount
+      const patchCoverage = impactedFile?.patchCoverage?.percentCovered
+      const baseCoverage = impactedFile?.baseCoverage?.percentCovered
+      const changeCoverage =
+        isNumber(headCoverage) && isNumber(baseCoverage)
+          ? headCoverage - baseCoverage
+          : Number.NaN
+      const hasHeadOrPatchCoverage =
+        isNumber(headCoverage) || isNumber(patchCoverage)
+
+      return {
+        missesCount,
+        headCoverage,
+        patchCoverage,
+        changeCoverage,
+        hasHeadOrPatchCoverage,
+        headName: impactedFile?.headName,
+        fileName: impactedFile?.fileName,
+        isCriticalFile: impactedFile?.isCriticalFile,
+        pullId: pull?.pullId,
+      }
     }
-  })
+  )
+  // Keep old way but just pass the plain impactedFiles if the status is not ImpactedFile
+  const impactedFiles =
+    compareWithBase?.impactedFiles?.__typename ===
+    ImpactedFilesReturnType.IMPACTED_FILES
+      ? mutatedImpactedFiles
+      : compareWithBase?.impactedFiles
+
   return {
     headState: pull?.head?.state,
     impactedFiles,
     pullHeadCoverage: compareWithBase?.headTotals?.percentCovered,
     pullPatchCoverage: compareWithBase?.patchTotals?.percentCovered,
     pullBaseCoverage: compareWithBase?.baseTotals?.percentCovered,
     compareWithBaseType: compareWithBase?.__typename,
+    impactedFilesType: compareWithBase?.impactedFiles?.__typename,
   }
 }
 
 export function useImpactedFilesTable() {
   const { provider, owner, repo, pullId } = useParams()
   const [sortBy, setSortBy] = useState([{ id: 'missesCount', desc: true }])
-  const filters = getFilters({ sortBy: sortBy[0] })
+  const location = useLocation()
+  const queryParams = qs.parse(location.search, {
+    ignoreQueryPrefix: true,
+    depth: 1,
+  })
+  const flags = queryParams?.flags
+  const filters = getFilters({ sortBy: sortBy[0], flags })
 
   const { data: pullData, isLoading } = usePull({
     provider,