diff --git a/web/src/framework/strings/stringTypes.ts b/web/src/framework/strings/stringTypes.ts index a30bc9bd5..39760e794 100644 --- a/web/src/framework/strings/stringTypes.ts +++ b/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 diff --git a/web/src/i18n/strings.en.yaml b/web/src/i18n/strings.en.yaml index 72e52364f..1fb9b968a 100644 --- a/web/src/i18n/strings.en.yaml +++ b/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 diff --git a/web/src/pages/PullRequest/CodeOwners/CodeOwnersOverview.tsx b/web/src/pages/PullRequest/CodeOwners/CodeOwnersOverview.tsx index ad0e679d8..c35628cbb 100644 --- a/web/src/pages/PullRequest/CodeOwners/CodeOwnersOverview.tsx +++ b/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 { 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 { data: TypesCodeOwnerEvaluation + reqCodeOwnerLatestApproval?: boolean } const CodeOwnerSections: React.FC = ({ repoMetadata, pullReqMetadata, data }) => { @@ -145,7 +153,11 @@ const CodeOwnerSections: React.FC = ({ repoMetadata, pul ) } -export const CodeOwnerSection: React.FC = ({ data }) => { +export const CodeOwnerSection: React.FC = ({ + data, + pullReqMetadata, + reqCodeOwnerLatestApproval +}) => { const { getString } = useStrings() const columns = useMemo( @@ -155,11 +167,15 @@ export const CodeOwnerSection: React.FC = ({ data }) => id: 'CODE', width: '45%', sort: true, - Header: 'CODE', + Header: getString('code'), accessor: 'CODE', Cell: ({ row }: CellProps) => { return ( - + {row.original.pattern} ) @@ -169,7 +185,7 @@ export const CodeOwnerSection: React.FC = ({ data }) => id: 'Owners', width: '13%', sort: true, - Header: 'OWNERS', + Header: getString('ownersHeading'), accessor: 'OWNERS', Cell: ({ row }: CellProps) => { return ( @@ -233,7 +249,7 @@ export const CodeOwnerSection: React.FC = ({ data }) => accessor: 'ChangesRequested', Cell: ({ row }: CellProps) => { const changeReqEvaluations = row?.original?.owner_evaluations?.filter( - evaluation => evaluation.review_decision === 'changereq' + evaluation => evaluation.review_decision === PullReqReviewDecision.CHANGEREQ ) return ( @@ -279,13 +295,15 @@ export const CodeOwnerSection: React.FC = ({ data }) => }, { id: 'approvedBy', - Header: 'APPROVED BY', + Header: getString('approvedBy'), sort: true, width: '15%', accessor: 'APPROVED BY', Cell: ({ row }: CellProps) => { 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 ( diff --git a/web/src/pages/PullRequest/Conversation/PullRequestOverviewPanel/sections/ChangesSection.tsx b/web/src/pages/PullRequest/Conversation/PullRequestOverviewPanel/sections/ChangesSection.tsx index ad8878b69..df9b9abe8 100644 --- a/web/src/pages/PullRequest/Conversation/PullRequestOverviewPanel/sections/ChangesSection.tsx +++ b/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) => { - + )} diff --git a/web/src/pages/PullRequest/Conversation/PullRequestSideBar/PullRequestSideBar.tsx b/web/src/pages/PullRequest/Conversation/PullRequestSideBar/PullRequestSideBar.tsx index 1964c5410..25b680c0a 100644 --- a/web/src/pages/PullRequest/Conversation/PullRequestSideBar/PullRequestSideBar.tsx +++ b/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) => { } tooltipProps={{ isDark: true, interactionKind: PopoverInteractionKind.HOVER }}> - {updatedReviewDecision === PullReqReviewDecision.outdated ? ( + {updatedReviewDecision === PullReqReviewDecision.OUTDATED ? ( ) : ( @@ -281,7 +281,7 @@ const PullRequestSideBar = (props: PullRequestSideBarProps) => { [ } export enum PullReqReviewDecision { - approved = 'approved', - changeReq = 'changereq', - pending = 'pending', - outdated = 'outdated' + APPROVED = 'approved', + CHANGEREQ = 'changereq', + PENDING = 'pending', + OUTDATED = 'outdated' } export const processReviewDecision = ( @@ -60,8 +60,8 @@ export const processReviewDecision = ( reviewedSHA?: string, sourceSHA?: string ) => - review_decision === PullReqReviewDecision.approved && reviewedSHA !== sourceSHA - ? PullReqReviewDecision.outdated + review_decision === PullReqReviewDecision.APPROVED && reviewedSHA !== sourceSHA + ? PullReqReviewDecision.OUTDATED : review_decision export function getActivePullReqPageSection(): PullRequestSection | undefined { diff --git a/web/src/utils/Utils.ts b/web/src/utils/Utils.ts index a4727c61c..3f386851d 100644 --- a/web/src/utils/Utils.ts +++ b/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 - const hasEmptyDecision = entry?.owner_evaluations?.some((evaluation: any) => evaluation?.review_decision === '') - const hasNoApprovedDecision = !entry?.owner_evaluations?.some( - evaluation => evaluation.review_decision === 'approved' + // skip entry if all the owners have not given any review_decision yet + const hasNoReview = entry?.owner_evaluations?.every( + (evaluation: TypesOwnerEvaluation | { review_decision: string }) => evaluation.review_decision === '' ) - - return hasEmptyDecision && hasNoApprovedDecision + if (hasNoReview) return false + // 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 }) } }