Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import { Badge, Box, Card, Checkbox, Flex, Paragraph, Text } from '@contentful/f
import tokens from '@contentful/f36-tokens';
import { cx } from '@emotion/css';
import type { EntryListRow as OverviewEntryListRow } from '../../../../utils/overviewEntryList';
import { ValidationFindingSeverity } from '@types';
import type { ValidationFinding } from '@types';
import {
noMappedContentBadge,
treeChildRowBase,
Expand All @@ -18,6 +20,8 @@ export interface OverviewEntryListProps {
onSelect: (entryIndex: number) => void;
onToggleEntrySelection: (entryKey: string, isSelected: boolean) => void;
areEntrySelectionsDisabled?: boolean;
/** Validation findings keyed by entry index, for badge rendering. */
findingsByEntryIndex?: ReadonlyMap<number, ValidationFinding[]>;
}

interface OverviewEntryRowCardProps {
Expand All @@ -29,6 +33,7 @@ interface OverviewEntryRowCardProps {
areEntrySelectionsDisabled: boolean;
showTreeLines: boolean;
isLastRow?: boolean;
findingsByEntryIndex?: ReadonlyMap<number, ValidationFinding[]>;
}

function OverviewEntryRowCard({
Expand All @@ -40,9 +45,15 @@ function OverviewEntryRowCard({
areEntrySelectionsDisabled,
showTreeLines,
isLastRow = true,
findingsByEntryIndex,
}: OverviewEntryRowCardProps) {
const isSelected = row.entryIndex === selectedEntryIndex;
const isEntrySelectedForCreation = selectedEntryKeys.has(row.id);
const entryFindings = findingsByEntryIndex?.get(row.entryIndex) ?? [];
const hasBlockFindings = entryFindings.some(
(f) => f.severity === ValidationFindingSeverity.Block
);
const hasWarnFindings = entryFindings.some((f) => f.severity === ValidationFindingSeverity.Warn);

const treeLineClass =
showTreeLines && cx(treeChildRowBase, isLastRow ? treeChildRowLast : treeChildRowNotLast);
Expand Down Expand Up @@ -95,6 +106,16 @@ function OverviewEntryRowCard({
No mapped content
</Badge>
)}
{hasBlockFindings && (
<Badge variant="negative" size="small" className={noMappedContentBadge}>
Needs attention
</Badge>
)}
{!hasBlockFindings && hasWarnFindings && (
<Badge variant="warning" size="small" className={noMappedContentBadge}>
Warning
</Badge>
)}
</Paragraph>
Comment on lines 104 to 119

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"No mapped content" and "Needs attention"/"Warning" are rendered as sibling <Badge> elements in a <Paragraph>. If an entry simultaneously has no mapped content type AND a block finding, both badges appear.

Perhaps this is an expected behavior but since the PR didn't mention about this, just wanted to double check it with you!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — this is intentional. An entry with no mapped content fields that also has a block finding (e.g. a required field is missing) would legitimately show both. The badges serve different purposes: "No mapped content" means none of the field mappings have any source refs, while "Needs attention" means the validate-payload step flagged a structural issue. They're not mutually exclusive so showing both makes sense here.

</button>
</Flex>
Expand All @@ -112,6 +133,7 @@ function OverviewEntryRowCard({
areEntrySelectionsDisabled={areEntrySelectionsDisabled}
showTreeLines
isLastRow={index === row.children.length - 1}
findingsByEntryIndex={findingsByEntryIndex}
/>
))}
</Box>
Expand All @@ -137,6 +159,7 @@ export function OverviewEntryList({
onSelect,
onToggleEntrySelection,
areEntrySelectionsDisabled = false,
findingsByEntryIndex,
}: OverviewEntryListProps) {
return (
<Flex flexDirection="column" gap="spacingS">
Expand All @@ -150,6 +173,7 @@ export function OverviewEntryList({
onToggleEntrySelection={onToggleEntrySelection}
areEntrySelectionsDisabled={areEntrySelectionsDisabled}
showTreeLines={false}
findingsByEntryIndex={findingsByEntryIndex}
/>
))}
</Flex>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { useMemo } from 'react';
import { Box, Button, Flex, Note, Paragraph, Text } from '@contentful/f36-components';
import { Box, Button, Checkbox, Flex, Note, Paragraph, Text } from '@contentful/f36-components';
import { LightbulbIcon } from '@contentful/f36-icons';
import type { MappingReviewSuspendPayload } from '@types';
import type { MappingReviewSuspendPayload, ValidationFinding } from '@types';
import { buildEntryListFromEntryBlockGraph } from '../../../../utils/overviewEntryList';
import { OverviewEntryList } from './OverviewEntryList';
import { overviewSectionBox, overviewSectionBoxScrollable } from './OverviewSection.styles';
Expand All @@ -18,6 +18,12 @@ interface OverviewProps {
isCtaLoading?: boolean;
isCtaDisabled?: boolean;
areEntrySelectionsDisabled?: boolean;
/** Whether any block-severity findings exist; drives the acknowledgement Note visibility. */
hasBlockFindings?: boolean;
/** Called with `true` when the user checks the block-findings acknowledgement. */
onBlockFindingsAcknowledged?: (acknowledged: boolean) => void;
/** Whether the user has acknowledged block findings. */
blockFindingsAcknowledged?: boolean;
}

const OverviewSection = ({
Expand All @@ -31,6 +37,9 @@ const OverviewSection = ({
isCtaLoading = false,
isCtaDisabled = false,
areEntrySelectionsDisabled = false,
hasBlockFindings = false,
onBlockFindingsAcknowledged,
blockFindingsAcknowledged = false,
}: OverviewProps) => {
const entryRows = useMemo(
() =>
Expand All @@ -42,6 +51,17 @@ const OverviewSection = ({
[payload.entryBlockGraph.entries, payload.contentTypes, payload.referenceGraph.edges]
);

const findingsByEntryIndex = useMemo((): ReadonlyMap<number, ValidationFinding[]> => {
const map = new Map<number, ValidationFinding[]>();
for (const finding of payload.validationFindings ?? []) {
if (finding.entryIndex === undefined) continue;
const list = map.get(finding.entryIndex) ?? [];
list.push(finding);
map.set(finding.entryIndex, list);
}
return map;
}, [payload.validationFindings]);

return (
<>
<Box padding="spacingL" className={overviewSectionBox}>
Expand All @@ -53,12 +73,28 @@ const OverviewSection = ({
</Flex>
<Paragraph marginBottom="none">
Review your content and associated entries below. Highlight text to make adjustments.
Select which entries youd like to create.
Select which entries you'd like to create.
</Paragraph>
</Flex>

<Splitter />

{hasBlockFindings && (
<Note variant="negative">
<Flex flexDirection="column" gap="spacingXs">
<Text>
Some entries have issues that may prevent the content from being created
correctly. Review the highlighted entries before proceeding.
</Text>
<Checkbox
isChecked={blockFindingsAcknowledged}
onChange={(event) => onBlockFindingsAcknowledged?.(event.target.checked)}>
I have reviewed the issues and want to proceed
</Checkbox>
</Flex>
</Note>
)}

<Flex justifyContent="space-between" alignItems="center" paddingBottom="none">
<Flex flexDirection="column" gap="spacingXs">
<Text fontWeight="fontWeightDemiBold" fontSize="fontSizeL">
Expand Down Expand Up @@ -91,6 +127,7 @@ const OverviewSection = ({
onSelect={onSelectEntryIndex}
onToggleEntrySelection={onToggleEntrySelection}
areEntrySelectionsDisabled={areEntrySelectionsDisabled}
findingsByEntryIndex={findingsByEntryIndex}
/>
</Box>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export const ReviewPage = ({
const [createdEntries, setCreatedEntries] = useState<EntryProps[] | null>(null);
const [isSummaryModalOpen, setIsSummaryModalOpen] = useState(false);
const [createError, setCreateError] = useState<string | null>(null);
const [blockFindingsAcknowledged, setBlockFindingsAcknowledged] = useState(false);
const [entryBlockGraph, setEntryBlockGraph] = useState<EntryBlockGraph>(() =>
structuredClone(payload.entryBlockGraph)
);
Expand All @@ -65,6 +66,7 @@ export const ReviewPage = ({
const nextEntryBlockGraph = structuredClone(payload.entryBlockGraph);
setEntryBlockGraph(nextEntryBlockGraph);
setSelectedEntryKeys(getAllEntrySelectionKeys(nextEntryBlockGraph.entries));
setBlockFindingsAcknowledged(false);
// eslint-disable-next-line react-hooks/exhaustive-deps -- only re-init on run identity
}, [runId, payload.documentId]);

Expand All @@ -84,6 +86,7 @@ export const ReviewPage = ({
}, [payload.contentTypes]);
const hasCreatedEntries = createdEntries !== null;
const isMappingDisabled = isCreatePending || hasCreatedEntries;
const hasBlockFindings = (payload.validationFindings ?? []).some((f) => f.severity === 'block');
const selectedEntryCount = useMemo(
() => countSelectedEntries(entryBlockGraph.entries, selectedEntryKeys),
[entryBlockGraph.entries, selectedEntryKeys]
Expand Down Expand Up @@ -263,8 +266,14 @@ export const ReviewPage = ({
ctaLabel={hasCreatedEntries ? 'View entries' : 'Create selected entries'}
onCtaClick={handleCreateOrViewEntries}
isCtaLoading={isCreatePending}
isCtaDisabled={!hasCreatedEntries && !hasSelectedEntries}
isCtaDisabled={
!hasCreatedEntries &&
(!hasSelectedEntries || (hasBlockFindings && !blockFindingsAcknowledged))
}
areEntrySelectionsDisabled={isMappingDisabled}
hasBlockFindings={hasBlockFindings}
blockFindingsAcknowledged={blockFindingsAcknowledged}
onBlockFindingsAcknowledged={setBlockFindingsAcknowledged}
/>
<MappingView
payload={reviewPayload}
Expand Down
20 changes: 20 additions & 0 deletions apps/drive-integration/src/types/workflow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,25 @@ export interface TabsImagesSuspendPayload {
tabs?: DocTabOption[];
}

export enum ValidationFindingSeverity {
Block = 'block',
Warn = 'warn',
}

/** A single validation finding from the validate-payload step. */
export interface ValidationFinding {
/** Machine-readable finding code. */
code: string;
/** Human-readable explanation. */
message: string;
/** `block` findings prevent advancing; `warn` findings are advisory. */
severity: ValidationFindingSeverity;
/** Zero-based entry index, when entry-scoped. */
entryIndex?: number;
/** Field ID, when field-scoped. */
fieldId?: string;
}

export interface MappingReviewSuspendPayload {
reason?: string;
suspendStepId: 'mapping-review';
Expand All @@ -134,6 +153,7 @@ export interface MappingReviewSuspendPayload {
entryBlockGraph: EntryBlockGraph;
referenceGraph: ReviewedReferenceGraph;
contentTypes: WorkflowContentType[];
validationFindings?: ValidationFinding[];

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⛏️Nit: I would remove the comment

}

export type WorkflowRunResult =
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
import { cleanup, render, screen } from '@testing-library/react';
import { afterEach, describe, expect, it, vi } from 'vitest';
import { ValidationFindingSeverity } from '@types';
import type { ValidationFinding } from '@types';
import type { EntryListRow } from '../../../../../src/utils/overviewEntryList';
import { OverviewEntryList } from '../../../../../src/locations/Page/components/overview/OverviewEntryList';

const makeRow = (entryIndex: number, label: string): EntryListRow => ({
id: `row-${entryIndex}`,
entryIndex,
contentTypeName: 'Article',
entryTitle: label,
children: [],
});

const renderList = (
rows: EntryListRow[],
findingsByEntryIndex?: ReadonlyMap<number, ValidationFinding[]>
) =>
render(
<OverviewEntryList
rows={rows}
selectedEntryIndex={null}
selectedEntryKeys={new Set()}
onSelect={vi.fn()}
onToggleEntrySelection={vi.fn()}
findingsByEntryIndex={findingsByEntryIndex}
/>
);

afterEach(() => cleanup());

describe('OverviewEntryList — validation finding badges (INTEG-4383)', () => {
it('renders a "Needs attention" badge for entries with block findings', () => {
const rows = [makeRow(0, 'Entry A')];
const findings: ValidationFinding[] = [
{
code: 'required-field-missing',
message: 'title missing',
severity: ValidationFindingSeverity.Block,
entryIndex: 0,
},
];
renderList(rows, new Map([[0, findings]]));

expect(screen.getByText('Needs attention')).toBeTruthy();
expect(screen.queryByText('Warning')).toBeNull();
});

it('renders a "Warning" badge for entries with only warn findings', () => {
const rows = [makeRow(0, 'Entry A')];
const findings: ValidationFinding[] = [
{
code: 'displayField-blank',
message: 'title blank',
severity: ValidationFindingSeverity.Warn,
entryIndex: 0,
},
];
renderList(rows, new Map([[0, findings]]));

expect(screen.getByText('Warning')).toBeTruthy();
expect(screen.queryByText('Needs attention')).toBeNull();
});

it('renders "Needs attention" (not Warning) when entry has both block and warn findings', () => {
const rows = [makeRow(0, 'Entry A')];
const findings: ValidationFinding[] = [
{
code: 'required-field-missing',
message: 'title missing',
severity: ValidationFindingSeverity.Block,
entryIndex: 0,
},
{
code: 'displayField-blank',
message: 'title blank',
severity: ValidationFindingSeverity.Warn,
entryIndex: 0,
},
];
renderList(rows, new Map([[0, findings]]));

expect(screen.getByText('Needs attention')).toBeTruthy();
expect(screen.queryByText('Warning')).toBeNull();
});

it('renders no finding badges when findingsByEntryIndex is undefined', () => {
const rows = [makeRow(0, 'Entry A')];
renderList(rows, undefined);

expect(screen.queryByText('Needs attention')).toBeNull();
expect(screen.queryByText('Warning')).toBeNull();
});

it('renders no finding badges for entries with no findings', () => {
const rows = [makeRow(0, 'Entry A'), makeRow(1, 'Entry B')];
const findings: ValidationFinding[] = [
{
code: 'required-field-missing',
message: 'title missing',
severity: ValidationFindingSeverity.Block,
entryIndex: 1,
},
];
renderList(rows, new Map([[1, findings]]));

// Only entry 1 should have the badge
expect(screen.getAllByText('Needs attention')).toHaveLength(1);
});
});
Loading
Loading