Skip to content

Commit af7b6cb

Browse files
thomasguillotclaude
andcommitted
fix(audience): take the sample cap and truncation bound from the engine
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent d18287b commit af7b6cb

10 files changed

Lines changed: 68 additions & 24 deletions

File tree

plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/catalog-impact.test.jsx

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,30 @@ describe( 'CatalogImpact', () => {
235235
} );
236236
} );
237237

238+
// The engine echoes the cap it applied; the client's constant is only a fallback.
239+
it( 'says the table is a sample against the cap the engine reports', async () => {
240+
apiFetch.mockResolvedValue( detail( { preview_limited: true, sample_count: 3, sample_limit: 3 } ) );
241+
render( <CatalogImpact stats={ stats() } /> );
242+
243+
await act( async () => {
244+
openModal();
245+
} );
246+
247+
expect( screen.getByText( 'Showing a sample of 3 products.' ) ).toBeInTheDocument();
248+
} );
249+
250+
// The engine flags a preview as limited when it merely skipped an unpriceable product.
251+
it( 'says nothing about sampling when the table never reached the cap', async () => {
252+
apiFetch.mockResolvedValue( detail( { preview_limited: true, sample_count: 3, sample_limit: 50 } ) );
253+
render( <CatalogImpact stats={ stats() } /> );
254+
255+
await act( async () => {
256+
openModal();
257+
} );
258+
259+
expect( screen.queryByText( /Showing a sample of/ ) ).not.toBeInTheDocument();
260+
} );
261+
238262
it( 'withholds the table button and explains itself when nothing is affected', () => {
239263
render( <CatalogImpact stats={ stats( { total_matching: 0 } ) } /> );
240264

plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/catalog-impact.tsx

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ export default function CatalogImpact( { stats }: CatalogImpactProps ) {
4444
const open = useCallback( () => {
4545
setIsOpen( true );
4646
setHasError( false );
47-
// A landed sample is kept; a failure is retried.
47+
// A failure is retried; only a landed sample short-circuits.
4848
if ( detail || inFlight.current ) {
4949
return;
5050
}
@@ -77,6 +77,7 @@ export default function CatalogImpact( { stats }: CatalogImpactProps ) {
7777
emptyReason = 'no-products';
7878
}
7979
}
80+
const note = detail ? sampleNote( detail ) : null;
8081

8182
return (
8283
<Card.Root className="newspack-pricing-rules__impact">
@@ -108,9 +109,7 @@ export default function CatalogImpact( { stats }: CatalogImpactProps ) {
108109
{ ! hasError && emptyReason && <ImpactEmpty reason={ emptyReason } /> }
109110
{ ! hasError && detail && ! emptyReason && (
110111
<>
111-
{ detail.preview_limited && detail.sample_count >= IMPACT_SAMPLE_LIMIT && (
112-
<p className="newspack-pricing-rules__muted">{ sampleNote( detail.sample_count ) }</p>
113-
) }
112+
{ note && <p className="newspack-pricing-rules__muted">{ note }</p> }
114113
<ImpactTable
115114
baseline={ detail.sample }
116115
segmentGroups={ detail.segment_groups ?? [] }

plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/constants.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ export const RULE_PREVIEW_API_PATH = '/wc-dynamic-pricing/v1/rules/preview';
99
export const IMPACT_PREVIEW_API_PATH = '/wc-dynamic-pricing/v1/impact-preview';
1010

1111
/**
12-
* The engine's route maximum. Compared against sample_count because the engine
13-
* flags a preview limited even when it merely skipped a product it could not price.
12+
* The engine's route maximum, which the catalog read asks for in full. Also the
13+
* fallback when a payload omits the cap it applied.
1414
*/
1515
export const IMPACT_SAMPLE_LIMIT = 50;

plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/impact-format.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,10 @@ import { __, _n, sprintf } from '@wordpress/i18n';
1313
* Internal dependencies
1414
*/
1515
import { formatCount } from '../../../../../packages/components/src/breadcrumbs/format-count';
16+
import { IMPACT_SAMPLE_LIMIT } from './constants';
1617

1718
// Re-exported, not reimplemented: only this helper normalises the locales
18-
// WordPress ships that Intl rejects, and the header count beside these figures
19-
// already uses it.
19+
// WordPress ships that Intl rejects.
2020
export { formatCount };
2121

2222
export function formatPrice( amount: number, currency: PricingRulesCurrency ): string {
@@ -45,12 +45,18 @@ export function cycleMarkerNote(): string {
4545
}
4646

4747
/**
48-
* Shared so the modal and the editor preview cannot drift into two msgids.
48+
* The caption for a table the engine capped, or null when it did not. The cap comes
49+
* from the payload because the engine's two entry points default differently, and
50+
* `preview_limited` alone cannot tell a capped sample from one that skipped a
51+
* product it could not price.
4952
*/
50-
export function sampleNote( sampleCount: number ): string {
53+
export function sampleNote( payload: CatalogImpactResponse ): string | null {
54+
if ( ! payload.preview_limited || payload.sample_count < ( payload.sample_limit ?? IMPACT_SAMPLE_LIMIT ) ) {
55+
return null;
56+
}
5157
return sprintf(
5258
/* translators: %s: how many products the table lists. */
53-
_n( 'Showing a sample of %s product.', 'Showing a sample of %s products.', sampleCount, 'newspack-plugin' ),
54-
formatCount( sampleCount )
59+
_n( 'Showing a sample of %s product.', 'Showing a sample of %s products.', payload.sample_count, 'newspack-plugin' ),
60+
formatCount( payload.sample_count )
5561
);
5662
}

plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/impact-stats.test.jsx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,18 @@ describe( 'ImpactStats', () => {
6666
expect( screen.getByText( /new sign-ups only/ ) ).toBeInTheDocument();
6767
} );
6868

69-
it( 'marks a capped count as a lower bound, and only that count', () => {
69+
it( 'leaves the product count exact when only the audience is capped', () => {
7070
render( <ImpactStats totalMatching={ 500 } countLimited={ false } audience={ audience( { count_limited: true } ) } /> );
7171
expect( screen.getByText( '500' ) ).toBeInTheDocument();
7272
expect( screen.getByText( '12+' ) ).toBeInTheDocument();
7373
} );
7474

75+
it( 'bounds the renewal split when the audience is capped', () => {
76+
render( <ImpactStats totalMatching={ 500 } countLimited={ false } audience={ audience( { count_limited: true } ) } /> );
77+
expect( screen.getByText( '8+' ) ).toBeInTheDocument();
78+
expect( screen.getByText( '4+ protected' ) ).toBeInTheDocument();
79+
} );
80+
7581
it( 'marks both counts as lower bounds when both are capped', () => {
7682
render( <ImpactStats totalMatching={ 500 } countLimited audience={ audience( { count_limited: true } ) } /> );
7783
expect( screen.getByText( '500+' ) ).toBeInTheDocument();

plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/impact-stats.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,15 @@ export default function ImpactStats( { totalMatching, countLimited, audience }:
5151
<Stat value={ bounded( totalMatching, countLimited ) } label={ __( 'Products affected', 'newspack-plugin' ) } />
5252
{ scope && <Stat value={ bounded( scope.total, scope.count_limited ) } label={ __( 'Subscribers in scope', 'newspack-plugin' ) } /> }
5353
{ scope && ! isLocked && (
54+
// The engine truncates oldest-first and the oldest are the ones a cohort
55+
// gate protects, so a capped split under-reports who is repriced.
5456
<Stat
55-
value={ formatCount( scope.caught ) }
57+
value={ bounded( scope.caught, scope.count_limited ) }
5658
label={ __( 'Eligible at renewal', 'newspack-plugin' ) }
5759
note={ sprintf(
5860
/* translators: %s: how many subscribers in scope keep their current price. */
5961
_n( '%s protected', '%s protected', scope.protected, 'newspack-plugin' ),
60-
formatCount( scope.protected )
62+
bounded( scope.protected, scope.count_limited )
6163
) }
6264
/>
6365
) }

plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/list.tsx

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -127,10 +127,8 @@ export default function PricingRulesList() {
127127
fetchData();
128128
}, [ fetchData ] );
129129

130-
// One row is enough: total_matching and count_limited do not vary with the
131-
// limit, and pricing the whole sample costs several times as much. `audience`
132-
// renders from this payload too, so it must stay limit-invariant when the
133-
// engine starts sending it.
130+
// One row is enough: total_matching, count_limited and audience are all computed
131+
// before the limit applies, and pricing the whole sample costs several times as much.
134132
const statsRequest = useRef( 0 );
135133
const gateTimer = useRef< ReturnType< typeof setTimeout > | undefined >( undefined );
136134

plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/rule-preview.test.jsx

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,14 @@ describe( 'RulePreview', () => {
140140
expect( screen.queryByText( /Showing a sample of/ ) ).not.toBeInTheDocument();
141141
} );
142142

143+
// The per-rule route takes no limit at all, so only the payload knows the cap.
144+
it( 'measures the sample against the cap the engine reports', async () => {
145+
apiFetch.mockResolvedValue( response( { preview_limited: true, sample_count: 10, sample_limit: 10, total_matching: 120 } ) );
146+
render( <RulePreview body={ {} } /> );
147+
await settle();
148+
expect( screen.getByText( 'Showing a sample of 10 products.' ) ).toBeInTheDocument();
149+
} );
150+
143151
// The engine flags a preview as limited when it merely skipped an unpriceable product.
144152
it( 'says nothing about sampling when the table never reached the cap', async () => {
145153
apiFetch.mockResolvedValue( response( { preview_limited: true, sample_count: 33, total_matching: 36 } ) );

plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/rule-preview.tsx

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import ImpactEmpty, { type ImpactEmptyReason } from './impact-empty';
2222
import ImpactStats from './impact-stats';
2323
import ImpactTable from './impact-table';
2424
import { sampleNote } from './impact-format';
25-
import { RULE_PREVIEW_API_PATH as PREVIEW_PATH, IMPACT_SAMPLE_LIMIT } from './constants';
25+
import { RULE_PREVIEW_API_PATH as PREVIEW_PATH } from './constants';
2626

2727
const DEBOUNCE_MS = 500;
2828

@@ -95,6 +95,7 @@ export default function RulePreview( { body, showCycleNote }: RulePreviewProps )
9595
}
9696

9797
const preview = data as RulePreviewResponse;
98+
const note = sampleNote( preview );
9899

99100
return (
100101
<div className={ `newspack-pricing-rules__preview${ isLoading ? ' is-loading' : '' }` }>
@@ -105,9 +106,7 @@ export default function RulePreview( { body, showCycleNote }: RulePreviewProps )
105106
currency={ preview.currency }
106107
showCycleNote={ showCycleNote }
107108
/>
108-
{ preview.preview_limited && preview.sample_count >= IMPACT_SAMPLE_LIMIT && (
109-
<p className="newspack-pricing-rules__muted">{ sampleNote( preview.sample_count ) }</p>
110-
) }
109+
{ note && <p className="newspack-pricing-rules__muted">{ note }</p> }
111110
</div>
112111
);
113112
}

plugins/newspack-plugin/src/wizards/audience/views/pricing-rules/types.d.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,10 +126,12 @@ interface CatalogImpactResponse {
126126
count_limited: boolean;
127127
preview_limited: boolean;
128128
sample_count: number;
129+
// The cap the engine applied; omitted rather than guessed when it had none.
130+
sample_limit?: number;
129131
currency: PricingRulesCurrency;
130132
sample: CatalogImpactRow[];
131133
segment_groups?: SegmentImpactGroup[];
132-
// Absent on the catalogue route until the engine's subscriptions layer sends it.
134+
// Absent unless the engine's subscriptions layer is present.
133135
audience?: RuleAudienceData;
134136
}
135137

0 commit comments

Comments
 (0)