Skip to content

Commit

Permalink
fix: minor bugs in reCAPTCHA + Campaigns admin UIs (#3073)
Browse files Browse the repository at this point in the history 
* fix: minor bugs in reCAPTCHA + Campaigns admin UIs

* test: fix failing JS test
  • Loading branch information
@dkoo
dkoo authored Apr 19, 2024
1 parent e303a80 commit 876d0a9
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 102 deletions.
17 changes: 8 additions & 9 deletions assets/wizards/connections/views/main/recaptcha.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,13 @@ const Recaptcha = () => {
setError( null );
setIsLoading( true );
try {
setSettings(
await apiFetch( {
path: '/newspack/v1/recaptcha',
method: 'POST',
data,
} )
);
setSettingsToUpdate( {} );
const newSettings = await apiFetch( {
path: '/newspack/v1/recaptcha',
method: 'POST',
data,
} );
setSettings( newSettings );
setSettingsToUpdate( newSettings );
} catch ( e ) {
setError( e?.message || __( 'Error updating settings.', 'newspack-plugin' ) );
} finally {
Expand Down Expand Up @@ -129,7 +128,7 @@ const Recaptcha = () => {
step="0.05"
min="0"
max="1"
value={ settingsToUpdate?.threshold || '' }
value={ parseFloat( settingsToUpdate?.threshold || '' ) }
label={ __( 'Threshold', 'newspack-plugin' ) }
onChange={ value =>
setSettingsToUpdate( { ...settingsToUpdate, threshold: value } )
Expand Down
86 changes: 7 additions & 79 deletions assets/wizards/popups/components/segment-group/SegmentGroup.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,12 +307,12 @@ describe( 'A segment with conflicting prompts', () => {
};
} );

it( 'renders a conflict notice for uncategorized overlays in the same segment', async () => {
it( 'renders a conflict notice for mutliple overlays in the same segment', async () => {
SEGMENT.prompts = PROMPTS.overlaysUncategorized;

// Expected warning text.
const noticeText =
'If multiple uncategorized overlays share the same segment, only the most recent one will be displayed.';
'If multiple overlays are rendered on the same pageview, only the most recent one will be displayed.';
const props = {
campaignData: CAMPAIGN.campaignData,
campaign: CAMPAIGN.campaignId,
Expand All @@ -331,36 +331,12 @@ describe( 'A segment with conflicting prompts', () => {
);
} );

it( 'renders a conflict notice for categorized overlays in the same segment', async () => {
SEGMENT.prompts = PROMPTS.overlaysCategorized;

// Expected warning text.
const noticeText =
'If multiple overlays share the same segment and category filtering, only the most recent one will be displayed.';
const props = {
campaignData: CAMPAIGN.campaignData,
campaign: CAMPAIGN.campaignId,
segment: SEGMENT,
};
const { getByTestId } = render( <SegmentGroup { ...props } /> );
const notice1 = getByTestId( 'conflict-warning-3' );
const notice2 = getByTestId( 'conflict-warning-4' );

// Notice text has expected shape.
expect( notice1.textContent ).toEqual(
`${ PROMPTS.overlaysCategorized[ 1 ].title }: ${ noticeText }`
);
expect( notice2.textContent ).toEqual(
`${ PROMPTS.overlaysCategorized[ 0 ].title }: ${ noticeText }`
);
} );

it( 'renders a conflict notice for uncategorized above-header prompts in the same segment', async () => {
it( 'renders a conflict notice for multiple above-header prompts in the same segment', async () => {
SEGMENT.prompts = PROMPTS.aboveHeadersUncategorized;

// Expected warning text.
const noticeText =
'If multiple uncategorized above-header prompts share the same segment, only the most recent one will be displayed.';
'If multiple above-header prompts are rendered on the same pageview, only the most recent one will be displayed.';
const props = {
campaignData: CAMPAIGN.campaignData,
campaign: CAMPAIGN.campaignId,
Expand All @@ -379,36 +355,12 @@ describe( 'A segment with conflicting prompts', () => {
);
} );

it( 'renders a conflict notice for above-header prompts in the same segment', async () => {
SEGMENT.prompts = PROMPTS.aboveHeadersCategorized;

// Expected warning text.
const noticeText =
'If multiple above-header prompts share the same segment and category filtering, only the most recent one will be displayed.';
const props = {
campaignData: CAMPAIGN.campaignData,
campaign: CAMPAIGN.campaignId,
segment: SEGMENT,
};
const { getByTestId } = render( <SegmentGroup { ...props } /> );
const notice1 = getByTestId( 'conflict-warning-7' );
const notice2 = getByTestId( 'conflict-warning-8' );

// Notice text has expected shape.
expect( notice1.textContent ).toEqual(
`${ PROMPTS.aboveHeadersCategorized[ 1 ].title }: ${ noticeText }`
);
expect( notice2.textContent ).toEqual(
`${ PROMPTS.aboveHeadersCategorized[ 0 ].title }: ${ noticeText }`
);
} );

it( 'renders a conflict notice for uncategorized prompts in the same custom placement and segment', async () => {
it( 'renders a conflict notice for multiple prompts in the same custom placement and segment', async () => {
SEGMENT.prompts = PROMPTS.customPlacementsUncategorized;

// Expected warning text.
const noticeText =
'If multiple uncategorized prompts in the same custom placement share the same segment, only the most recent one will be displayed.';
'If multiple prompts are rendered in the same custom placement, only the most recent one will be displayed.';
const props = {
campaignData: CAMPAIGN.campaignData,
campaign: CAMPAIGN.campaignId,
Expand All @@ -427,30 +379,6 @@ describe( 'A segment with conflicting prompts', () => {
);
} );

it( 'renders a conflict notice for categorized prompts in the same custom placement and segment', async () => {
SEGMENT.prompts = PROMPTS.customPlacementsCategorized;

// Expected warning text.
const noticeText =
'If multiple prompts in the same custom placement share the same segment and category filtering, only the most recent one will be displayed.';
const props = {
campaignData: CAMPAIGN.campaignData,
campaign: CAMPAIGN.campaignId,
segment: SEGMENT,
};
const { getByTestId } = render( <SegmentGroup { ...props } /> );
const notice1 = getByTestId( 'conflict-warning-11' );
const notice2 = getByTestId( 'conflict-warning-12' );

// Notice text has expected shape.
expect( notice1.textContent ).toEqual(
`${ PROMPTS.customPlacementsCategorized[ 1 ].title }: ${ noticeText }`
);
expect( notice2.textContent ).toEqual(
`${ PROMPTS.customPlacementsCategorized[ 0 ].title }: ${ noticeText }`
);
} );

it( 'renders a conflict notice for conflicting prompts that have no segment', async () => {
const everyone = {
configuration: {},
Expand All @@ -464,7 +392,7 @@ describe( 'A segment with conflicting prompts', () => {

// Expected warning text.
const noticeText =
'If multiple uncategorized overlays share the same segment, only the most recent one will be displayed.';
'If multiple overlays are rendered on the same pageview, only the most recent one will be displayed.';
const props = {
campaignData: CAMPAIGN.campaignData,
campaign: CAMPAIGN.campaignId,
Expand Down
21 changes: 7 additions & 14 deletions assets/wizards/popups/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -380,31 +380,24 @@ const sharesSegments = ( segmentsA, segmentsB ) => {
);
};

export const buildWarning = ( prompt, promptCategories ) => {
export const buildWarning = prompt => {
if ( isOverlay( prompt ) || isAboveHeader( prompt ) ) {
return sprintf(
// Translators: %1: 'uncetegorized' if no categories. %2: 'above-header prompts' if above header, 'overlays' otherwise. %3: 'and category filtering' if categories.
// Translators: %s is prompt type (above-header or overlay).
__(
'If multiple%1$s %2$s share the same segment%3$s, only the most recent one will be displayed.',
'If multiple %s are rendered on the same pageview, only the most recent one will be displayed.',
'newspack-plugin'
),
0 === promptCategories.length ? __( ' uncategorized', 'newspack-plugin' ) : '',
isAboveHeader( prompt )
? __( 'above-header prompts', 'newspack-plugin' )
: __( 'overlays', 'newspack-plugin' ),
0 < promptCategories.length ? __( ' and category filtering', 'newspack-plugin' ) : ''
: __( 'overlays', 'newspack-plugin' )
);
}

if ( isCustomPlacement( prompt ) ) {
return sprintf(
// Translators: %1: 'uncetegorized' if no categories. %2: 'and category filtering' if categories.
__(
'If multiple%1$s prompts in the same custom placement share the same segment%2$s, only the most recent one will be displayed.',
'newspack-plugin'
),
0 === promptCategories.length ? __( ' uncategorized', 'newspack-plugin' ) : '',
0 < promptCategories.length ? __( ' and category filtering', 'newspack-plugin' ) : ''
return __(
'If multiple prompts are rendered in the same custom placement, only the most recent one will be displayed.',
'newspack-plugin'
);
}

Expand Down
5 changes: 5 additions & 0 deletions includes/class-recaptcha.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,10 +194,15 @@ public static function get_setting( $key ) {
return null;
}
$value = \get_option( self::OPTIONS_PREFIX . $key, $config[ $key ] );

// Use default value type for casting bool option value.
if ( is_bool( $config[ $key ] ) ) {
$value = (bool) $value;
} elseif ( empty( $value ) ) {
// If the stored value is empty, use the default value.
$value = $config[ $key ];
}

return $value;
}

Expand Down

0 comments on commit 876d0a9

Please sign in to comment.