Skip to content

Commit 7dfec2b

Browse files
GaryJonesclaude
andcommitted
fix: validate unique tag IDs for DFP Async provider
DFP Async uses the tag_id field as a div ID in the rendered HTML. When two ad codes share the same tag_id, Google's ad serving becomes confused because the same element ID appears multiple times on the page. This adds validation that prevents saving ad codes with duplicate tag_id values. The implementation uses a new 'acm_validate_ad_code' filter that providers can hook into for custom validation, keeping the solution extensible for other providers that may need similar checks. Validation errors are now displayed via a notice at the top of the page using a transient to pass the error message through the redirect. Fixes #69 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent c2f1ab7 commit 7dfec2b

File tree

3 files changed

+112
-6
lines changed

3 files changed

+112
-6
lines changed

src/Providers/class-doubleclick-for-publishers-async.php

Lines changed: 74 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ public function __construct() {
100100

101101
add_filter( 'acm_ad_code_args', array( $this, 'filter_ad_code_args' ) );
102102
add_filter( 'acm_output_html', array( $this, 'filter_output_html' ), 10, 2 );
103+
add_filter( 'acm_validate_ad_code', array( $this, 'validate_unique_tag_id' ), 10, 4 );
103104

104105
add_filter( 'acm_display_ad_codes_without_conditionals', '__return_true' );
105106

@@ -167,11 +168,7 @@ public function filter_output_html( $output_html, $tag_id ) {
167168
$tt = $tag['url_vars'];
168169
$matching_ad_code = $ad_code_manager->get_matching_ad_code( $tag['tag'] );
169170
if ( ! empty( $matching_ad_code ) ) {
170-
// @todo There might be a case when there are two tags registered with the same dimensions
171-
// and the same tag id ( which is just a div id ). This confuses DFP Async, so we need to make sure
172-
// that tags are unique
173-
174-
// Parse ad tags to output flexible unit dimensions
171+
// Parse ad tags to output flexible unit dimensions.
175172
$unit_sizes = $this->parse_ad_tag_sizes( $tt );
176173

177174
?>
@@ -235,6 +232,78 @@ public function parse_ad_tag_sizes( $url_vars ) {
235232
return $unit_sizes_output;
236233
}
237234

235+
/**
236+
* Validate that the tag_id is unique for DFP Async.
237+
*
238+
* DFP Async uses the tag_id as a div ID, so each tag_id must be unique
239+
* to prevent conflicts when multiple ads are rendered on the same page.
240+
*
241+
* @since 0.8.0
242+
*
243+
* @param true|WP_Error $valid Current validation state.
244+
* @param array $ad_code_vals The ad code values being saved.
245+
* @param int $id The ad code ID (0 for new ad codes).
246+
* @param string $method The method being performed ('add' or 'edit').
247+
* @return true|WP_Error True if valid, WP_Error if tag_id is not unique.
248+
*/
249+
public function validate_unique_tag_id( $valid, $ad_code_vals, $id, $method ) {
250+
// If already invalid, don't override the error.
251+
if ( is_wp_error( $valid ) ) {
252+
return $valid;
253+
}
254+
255+
// Check if tag_id is provided.
256+
if ( empty( $ad_code_vals['tag_id'] ) ) {
257+
return $valid;
258+
}
259+
260+
global $ad_code_manager;
261+
262+
$tag_id = $ad_code_vals['tag_id'];
263+
$existing_id = $this->find_ad_code_by_tag_id( $tag_id );
264+
265+
// If no existing ad code with this tag_id, it's valid.
266+
if ( ! $existing_id ) {
267+
return true;
268+
}
269+
270+
// If editing and the existing ad code is the same one we're editing, it's valid.
271+
if ( 'edit' === $method && $existing_id === $id ) {
272+
return true;
273+
}
274+
275+
return new WP_Error(
276+
'duplicate-tag-id',
277+
sprintf(
278+
/* translators: %s: the duplicate tag ID */
279+
__( 'The Tag ID "%s" is already in use. Each Tag ID must be unique to prevent conflicts with DFP Async.', 'ad-code-manager' ),
280+
esc_html( $tag_id )
281+
)
282+
);
283+
}
284+
285+
/**
286+
* Find an ad code by its tag_id.
287+
*
288+
* @since 0.8.0
289+
*
290+
* @param string $tag_id The tag_id to search for.
291+
* @return int|false The ad code post ID if found, false otherwise.
292+
*/
293+
protected function find_ad_code_by_tag_id( $tag_id ) {
294+
global $ad_code_manager;
295+
296+
$ad_codes = $ad_code_manager->get_ad_codes();
297+
298+
foreach ( $ad_codes as $ad_code ) {
299+
if ( isset( $ad_code['url_vars']['tag_id'] ) && $ad_code['url_vars']['tag_id'] === $tag_id ) {
300+
return $ad_code['post_id'];
301+
}
302+
}
303+
304+
return false;
305+
}
306+
238307
}
239308

240309
class Doubleclick_For_Publishers_Async_ACM_WP_List_Table extends ACM_WP_List_Table {

src/class-ad-code-manager.php

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,32 @@ function handle_admin_action() {
275275
foreach ( $this->current_provider->ad_code_args as $arg ) {
276276
$ad_code_vals[ $arg['key'] ] = sanitize_text_field( $_REQUEST['acm-column'][ $arg['key'] ] ?? '' );
277277
}
278+
279+
/**
280+
* Filter to validate ad code data before saving.
281+
*
282+
* Providers can hook into this filter to perform custom validation.
283+
* Return a WP_Error to prevent saving and display an error message.
284+
*
285+
* @since 0.8.0
286+
*
287+
* @param true|WP_Error $valid True if valid, WP_Error if validation fails.
288+
* @param array $ad_code_vals The ad code values being saved.
289+
* @param int $id The ad code ID (0 for new ad codes).
290+
* @param string $method The method being performed ('add' or 'edit').
291+
*/
292+
$validation_result = apply_filters( 'acm_validate_ad_code', true, $ad_code_vals, $id, $method );
293+
294+
if ( is_wp_error( $validation_result ) ) {
295+
if ( isset( $_REQUEST['doing_ajax'] ) && sanitize_text_field( $_REQUEST['doing_ajax'] ) ) {
296+
die( '<div class="error">' . esc_html( $validation_result->get_error_message() ) . '</div>' );
297+
}
298+
// Store the error message in a transient for display after redirect.
299+
set_transient( 'acm_validation_error_' . get_current_user_id(), $validation_result->get_error_message(), 30 );
300+
$message = 'validation-error';
301+
break;
302+
}
303+
278304
if ( 'add' === $method ) {
279305
$id = $this->create_ad_code( $ad_code_vals );
280306
} else {

views/ad-code-manager.tpl.php

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77
<h1><?php echo esc_html( get_admin_page_title() ); ?></h1>
88
<?php
99
if ( isset( $_REQUEST['message'] ) ) {
10+
$message_text = '';
11+
$message_class = 'updated';
12+
1013
switch ( $_REQUEST['message'] ) {
1114
case 'ad-code-added':
1215
$message_text = __( 'Ad code created.', 'ad-code-manager' );
@@ -20,12 +23,20 @@
2023
case 'options-saved':
2124
$message_text = __( 'Options saved.', 'ad-code-manager' );
2225
break;
26+
case 'validation-error':
27+
$transient_key = 'acm_validation_error_' . get_current_user_id();
28+
$message_text = get_transient( $transient_key );
29+
if ( $message_text ) {
30+
delete_transient( $transient_key );
31+
$message_class = 'error';
32+
}
33+
break;
2334
default:
2435
$message_text = '';
2536
break;
2637
}
2738
if ( '' !== $message_text ) {
28-
echo '<div class="message updated"><p>' . esc_html( $message_text ) . '</p></div>';
39+
echo '<div class="notice notice-' . esc_attr( $message_class ) . '"><p>' . esc_html( $message_text ) . '</p></div>';
2940
}
3041
}
3142
?>

0 commit comments

Comments
 (0)