fix: [CODE-2493] codeowners status in PR overview page (#2833)

* fix: [CODE-2493] resolve comments
* fix: [CODE-2493] fix codeowners status in PR overview page

web/src/framework/strings/stringTypes.ts

@@ -37,6 +37,7 @@ export interface StringsMap {
   applyChanges: string
   approve: string
   approved: string
+  approvedBy: string
   artifacts: string
   ascending: string
   assignPeople: string
@@ -156,6 +157,7 @@ export interface StringsMap {
   changeRole: string
   changedSinceLastView: string
   changes: string
+  changesRequested: string
   changesRequestedBy: string
   'changesSection.approvalPending': string
   'changesSection.changesAppByRev': string
@@ -665,6 +667,8 @@ export interface StringsMap {
   optionalExtendedDescription: string
   overview: string
   owner: string
+  owners: string
+  ownersHeading: string
   pageLoading: string
   pageNotFound: string
   'pageTitle.accessControl': string

web/src/i18n/strings.en.yaml

@@ -1167,11 +1167,15 @@ upload: Upload
 unorderedList: Unordered list
 checklist: Check list
 code: Code
+approvedBy: Approved By
+ownersHeading: OWNERS
+changesRequestedBy: Changes Requested By
+changesRequested: changesRequested
+owners: Owners
 showMoreText: Show More
 addOptionalCommitMessage: Add (optional) commit message
 confirmStrat: 'Confirm {{strat}}'
 waiting: Waiting
-changesRequestedBy: CHANGES REQUESTED BY
 mergeBranchTitle: Merge branch {{branchName}} of {{repoPath}} (#{{prNum}})
 http: HTTP
 ssh: SSH

web/src/pages/PullRequest/CodeOwners/CodeOwnersOverview.tsx

@@ -39,10 +39,12 @@ import { useShowRequestError } from 'hooks/useShowRequestError'
 import type { TypesCodeOwnerEvaluation, TypesCodeOwnerEvaluationEntry } from 'services/code'
 import type { PRChecksDecisionResult } from 'hooks/usePRChecksDecision'
 import { CodeOwnerReqDecision, findChangeReqDecisions, findWaitingDecisions } from 'utils/Utils'
+import { PullReqReviewDecision } from '../PullRequestUtils'
 import css from './CodeOwnersOverview.module.scss'
 
 interface ChecksOverviewProps extends Pick<GitInfoProps, 'repoMetadata' | 'pullReqMetadata'> {
   prChecksDecisionResult: PRChecksDecisionResult
+  reqCodeOwnerLatestApproval: boolean
   codeOwners?: TypesCodeOwnerEvaluation
   standalone: boolean
 }
@@ -52,6 +54,7 @@ export function CodeOwnersOverview({
   repoMetadata,
   pullReqMetadata,
   prChecksDecisionResult,
+  reqCodeOwnerLatestApproval,
   standalone
 }: ChecksOverviewProps) {
   const { getString } = useStrings()
@@ -61,7 +64,11 @@ export function CodeOwnersOverview({
   useShowRequestError(error)
 
   const changeReqEntries = findChangeReqDecisions(codeOwners?.evaluation_entries, CodeOwnerReqDecision.CHANGEREQ)
-  const waitingEntries = findWaitingDecisions(codeOwners?.evaluation_entries)
+  const waitingEntries = findWaitingDecisions(
+    codeOwners?.evaluation_entries,
+    pullReqMetadata,
+    reqCodeOwnerLatestApproval
+  )
 
   const approvalEntries = findChangeReqDecisions(codeOwners?.evaluation_entries, CodeOwnerReqDecision.APPROVED)
 
@@ -133,6 +140,7 @@ export function CodeOwnersOverview({
 
 interface CodeOwnerSectionsProps extends Pick<GitInfoProps, 'repoMetadata' | 'pullReqMetadata'> {
   data: TypesCodeOwnerEvaluation
+  reqCodeOwnerLatestApproval?: boolean
 }
 
 const CodeOwnerSections: React.FC<CodeOwnerSectionsProps> = ({ repoMetadata, pullReqMetadata, data }) => {
@@ -145,7 +153,11 @@ const CodeOwnerSections: React.FC<CodeOwnerSectionsProps> = ({ repoMetadata, pul
   )
 }
 
-export const CodeOwnerSection: React.FC<CodeOwnerSectionsProps> = ({ data }) => {
+export const CodeOwnerSection: React.FC<CodeOwnerSectionsProps> = ({
+  data,
+  pullReqMetadata,
+  reqCodeOwnerLatestApproval
+}) => {
   const { getString } = useStrings()
 
   const columns = useMemo(
@@ -155,11 +167,15 @@ export const CodeOwnerSection: React.FC<CodeOwnerSectionsProps> = ({ data }) =>
           id: 'CODE',
           width: '45%',
           sort: true,
-          Header: 'CODE',
+          Header: getString('code'),
           accessor: 'CODE',
           Cell: ({ row }: CellProps<TypesCodeOwnerEvaluationEntry>) => {
             return (
-              <Text lineClamp={1} padding={{ left: 'small', right: 'small' }} color={Color.BLACK}>
+              <Text
+                lineClamp={1}
+                padding={{ left: 'small', right: 'small' }}
+                color={Color.BLACK}
+                flex={{ justifyContent: 'space-between' }}>
                 {row.original.pattern}
               </Text>
             )
@@ -169,7 +185,7 @@ export const CodeOwnerSection: React.FC<CodeOwnerSectionsProps> = ({ data }) =>
           id: 'Owners',
           width: '13%',
           sort: true,
-          Header: 'OWNERS',
+          Header: getString('ownersHeading'),
           accessor: 'OWNERS',
           Cell: ({ row }: CellProps<TypesCodeOwnerEvaluationEntry>) => {
             return (
@@ -233,7 +249,7 @@ export const CodeOwnerSection: React.FC<CodeOwnerSectionsProps> = ({ data }) =>
           accessor: 'ChangesRequested',
           Cell: ({ row }: CellProps<TypesCodeOwnerEvaluationEntry>) => {
             const changeReqEvaluations = row?.original?.owner_evaluations?.filter(
-              evaluation => evaluation.review_decision === 'changereq'
+              evaluation => evaluation.review_decision === PullReqReviewDecision.CHANGEREQ
             )
             return (
               <Layout.Horizontal className={css.ownerContainer} spacing="tiny">
@@ -279,13 +295,15 @@ export const CodeOwnerSection: React.FC<CodeOwnerSectionsProps> = ({ data }) =>
         },
         {
           id: 'approvedBy',
-          Header: 'APPROVED BY',
+          Header: getString('approvedBy'),
           sort: true,
           width: '15%',
           accessor: 'APPROVED BY',
           Cell: ({ row }: CellProps<TypesCodeOwnerEvaluationEntry>) => {
             const approvedEvaluations = row?.original?.owner_evaluations?.filter(
-              evaluation => evaluation.review_decision === 'approved'
+              evaluation =>
+                evaluation.review_decision === PullReqReviewDecision.APPROVED &&
+                (reqCodeOwnerLatestApproval ? evaluation.review_sha === pullReqMetadata?.source_sha : true)
             )
             return (
               <Layout.Horizontal className={css.ownerContainer} spacing="tiny">

web/src/pages/PullRequest/Conversation/PullRequestOverviewPanel/sections/ChangesSection.tsx

@@ -122,7 +122,11 @@ const ChangesSection = (props: ChangesSectionProps) => {
     }) // eslint-disable-next-line @typescript-eslint/no-explicit-any
     .filter((entry: any) => entry !== null && entry !== undefined) // Filter out the null entries
 
-  const codeOwnerPendingEntries = findWaitingDecisions(codeOwners?.evaluation_entries)
+  const codeOwnerPendingEntries = findWaitingDecisions(
+    codeOwners?.evaluation_entries,
+    pullReqMetadata,
+    reqCodeOwnerLatestApproval
+  )
   const approvedEvaluations = reviewers?.filter(evaluation => evaluation.review_decision === 'approved')
   const latestApprovalArr = approvedEvaluations?.filter(
     approved => !checkIfOutdatedSha(approved.sha, pullReqMetadata?.source_sha as string)
@@ -610,7 +614,12 @@ const ChangesSection = (props: ChangesSectionProps) => {
           <Container
             className={css.codeOwnerContainer}
             padding={{ top: 'small', bottom: 'small', left: 'xxxlarge', right: 'small' }}>
-            <CodeOwnerSection data={codeOwners} pullReqMetadata={pullReqMetadata} repoMetadata={repoMetadata} />
+            <CodeOwnerSection
+              data={codeOwners}
+              pullReqMetadata={pullReqMetadata}
+              repoMetadata={repoMetadata}
+              reqCodeOwnerLatestApproval={reqCodeOwnerLatestApproval}
+            />
           </Container>
         )}
       </Render>

web/src/pages/PullRequest/Conversation/PullRequestSideBar/PullRequestSideBar.tsx

@@ -55,7 +55,7 @@ const PullRequestSideBar = (props: PullRequestSideBarProps) => {
   const { getString } = useStrings()
   const { showError, showSuccess } = useToaster()
   const generateReviewDecisionInfo = (
-    reviewDecision: EnumPullReqReviewDecision | PullReqReviewDecision.outdated
+    reviewDecision: EnumPullReqReviewDecision | PullReqReviewDecision.OUTDATED
   ): {
     name: IconName
     color?: Color
@@ -76,7 +76,7 @@ const PullRequestSideBar = (props: PullRequestSideBarProps) => {
     }
 
     switch (reviewDecision) {
-      case PullReqReviewDecision.changeReq:
+      case PullReqReviewDecision.CHANGEREQ:
         info = {
           name: 'error-transparent-no-outline',
           color: Color.RED_700,
@@ -87,7 +87,7 @@ const PullRequestSideBar = (props: PullRequestSideBarProps) => {
           message: 'requested changes'
         }
         break
-      case PullReqReviewDecision.approved:
+      case PullReqReviewDecision.APPROVED:
         info = {
           name: 'tick-circle',
           color: Color.GREEN_700,
@@ -97,7 +97,7 @@ const PullRequestSideBar = (props: PullRequestSideBarProps) => {
           message: 'approved changes'
         }
         break
-      case PullReqReviewDecision.pending:
+      case PullReqReviewDecision.PENDING:
         info = {
           name: 'waiting',
           color: Color.GREY_700,
@@ -107,7 +107,7 @@ const PullRequestSideBar = (props: PullRequestSideBarProps) => {
           message: 'pending review'
         }
         break
-      case PullReqReviewDecision.outdated:
+      case PullReqReviewDecision.OUTDATED:
         info = {
           name: 'dot',
           color: Color.GREY_100,
@@ -273,7 +273,7 @@ const PullRequestSideBar = (props: PullRequestSideBarProps) => {
                           </Text>
                         }
                         tooltipProps={{ isDark: true, interactionKind: PopoverInteractionKind.HOVER }}>
-                        {updatedReviewDecision === PullReqReviewDecision.outdated ? (
+                        {updatedReviewDecision === PullReqReviewDecision.OUTDATED ? (
                           <img className={css.svgOutdated} src={ignoreFailed} width={20} height={20} />
                         ) : (
                           <Icon {...omit(reviewerInfo, 'iconProps')} />
@@ -281,7 +281,7 @@ const PullRequestSideBar = (props: PullRequestSideBarProps) => {
                       </Utils.WrapOptionalTooltip>
                       <Avatar
                         className={cx(css.reviewerAvatar, {
-                          [css.iconPadding]: updatedReviewDecision !== PullReqReviewDecision.changeReq
+                          [css.iconPadding]: updatedReviewDecision !== PullReqReviewDecision.CHANGEREQ
                         })}
                         name={reviewer.reviewer.display_name}
                         size="small"

web/src/pages/PullRequest/PullRequestUtils.tsx

@@ -49,10 +49,10 @@ export function isSystemComment(commentItems: CommentItem<TypesPullReqActivity>[
 }
 
 export enum PullReqReviewDecision {
-  approved = 'approved',
+  APPROVED = 'approved',
-  changeReq = 'changereq',
+  CHANGEREQ = 'changereq',
-  pending = 'pending',
+  PENDING = 'pending',
-  outdated = 'outdated'
+  OUTDATED = 'outdated'
 }
 
 export const processReviewDecision = (
@@ -60,8 +60,8 @@ export const processReviewDecision = (
   reviewedSHA?: string,
   sourceSHA?: string
 ) =>
-  review_decision === PullReqReviewDecision.approved && reviewedSHA !== sourceSHA
+  review_decision === PullReqReviewDecision.APPROVED && reviewedSHA !== sourceSHA
-    ? PullReqReviewDecision.outdated
+    ? PullReqReviewDecision.OUTDATED
     : review_decision
 
 export function getActivePullReqPageSection(): PullRequestSection | undefined {

web/src/utils/Utils.ts

@@ -28,8 +28,11 @@ import type {
   TypesCodeOwnerEvaluationEntry,
   RepoRepositoryOutput,
   TypesLabel,
-  TypesLabelValue
+  TypesLabelValue,
+  TypesPullReq,
+  TypesOwnerEvaluation
 } from 'services/code'
+import { PullReqReviewDecision } from 'pages/PullRequest/PullRequestUtils'
 
 export enum ACCESS_MODES {
   VIEW,
@@ -290,18 +293,28 @@ export const findChangeReqDecisions = (
   }
 }
 
-export const findWaitingDecisions = (entries: TypesCodeOwnerEvaluationEntry[] | null | undefined) => {
+export const findWaitingDecisions = (
+  entries: TypesCodeOwnerEvaluationEntry[] | null | undefined,
+  pullReqMetadata: TypesPullReq,
+  reqCodeOwnerLatestApproval: boolean
+) => {
   if (entries === null || entries === undefined) {
     return []
   } else {
     return entries.filter((entry: TypesCodeOwnerEvaluationEntry) => {
-      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      // skip entry if all the owners have not given any review_decision yet
-      const hasEmptyDecision = entry?.owner_evaluations?.some((evaluation: any) => evaluation?.review_decision === '')
+      const hasNoReview = entry?.owner_evaluations?.every(
-      const hasNoApprovedDecision = !entry?.owner_evaluations?.some(
+        (evaluation: TypesOwnerEvaluation | { review_decision: string }) => evaluation.review_decision === ''
-        evaluation => evaluation.review_decision === 'approved'
       )
-
+      if (hasNoReview) return false
-      return hasEmptyDecision && hasNoApprovedDecision
+      // add entry to waiting decision array if approved changes are outdated or no approvals are found for the given entry
+      // ( this will hence include pending or change requested entries)
+      const hasApprovedDecision = entry?.owner_evaluations?.some(
+        evaluation =>
+          evaluation.review_decision === PullReqReviewDecision.APPROVED &&
+          (reqCodeOwnerLatestApproval ? evaluation.review_sha === pullReqMetadata.source_sha : true)
+      )
+      return !hasApprovedDecision
     })
   }
 }