Skip to content

Commit 8cefa2e

Browse files
GaryJonesGaryJones
authored
Merge pull request #186 from Automattic/fix/72-no-ad-code-empty-output
2 parents 008f061 + d6a941d commit 8cefa2e

File tree

4 files changed

+402
-21
lines changed

4 files changed

+402
-21
lines changed

‎.phpcs.xml.dist‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@
5353
<element value="Automattic\AdCodeManager"/>
5454
</property>
5555
</properties>
56+
<!-- Allow 3-character prefix "acm" - this is an established plugin prefix. -->
57+
<exclude name="WordPress.NamingConventions.PrefixAllGlobals.ShortPrefixPassed"/>
5658
</rule>
5759

5860
<rule ref="WordPress.WP.I18n">

‎src/class-acm-widget.php‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,13 +60,35 @@ function update( $new_instance, $old_instance ) {
6060

6161
// Display the widget
6262
function widget( $args, $instance ) {
63+
// Capture the ad content to check if we have anything to display.
64+
// This fixes issue #72: don't output empty widget wrapper when no ad code found.
65+
ob_start();
66+
do_action( 'acm_tag', $instance['ad_zone'] );
67+
$ad_content = ob_get_clean();
68+
69+
/**
70+
* Filters whether to display the widget when no ad content is found.
71+
*
72+
* @since 0.8.0
73+
*
74+
* @param bool $display Whether to display the widget. Default false.
75+
* @param string $ad_zone The ad zone ID.
76+
* @param array $args Widget display arguments.
77+
* @param array $instance Widget instance settings.
78+
*/
79+
if ( empty( $ad_content ) && ! apply_filters( 'acm_display_empty_widget', false, $instance['ad_zone'], $args, $instance ) ) {
80+
return;
81+
}
82+
6383
echo $args['before_widget'];
6484
$title = apply_filters( 'widget_title', $instance['title'] );
6585

6686
if ( ! empty( $title ) ) {
6787
echo $args['before_title'] . esc_html( $title ) . $args['after_title'];
6888
}
69-
do_action( 'acm_tag', $instance['ad_zone'] );
89+
90+
// phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped -- Ad content is escaped during token replacement in get_acm_tag().
91+
echo $ad_content;
7092
echo $args['after_widget'];
7193
}
7294
}

‎tests/Integration/WidgetTest.php‎

Lines changed: 297 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,297 @@
1+
<?php
2+
/**
3+
* Integration tests for the ACM_Ad_Zones widget.
4+
*
5+
* Tests for issue #72: Ad code tag still rendered if no valid ad codes found.
6+
*
7+
* @package Automattic\AdCodeManager\Tests\Integration
8+
*/
9+
10+
declare( strict_types=1 );
11+
12+
namespace Automattic\AdCodeManager\Tests\Integration;
13+
14+
use ACM_Ad_Zones;
15+
16+
/**
17+
* Test case for ACM_Ad_Zones widget.
18+
*/
19+
class WidgetTest extends TestCase {
20+
21+
/**
22+
* The widget instance.
23+
*
24+
* @var ACM_Ad_Zones
25+
*/
26+
private ACM_Ad_Zones $widget;
27+
28+
/**
29+
* Set up test fixtures.
30+
*/
31+
public function set_up(): void {
32+
parent::set_up();
33+
$this->widget = new ACM_Ad_Zones();
34+
}
35+
36+
/**
37+
* Test widget outputs nothing when no ad code is found.
38+
*
39+
* This is the main fix for issue #72 - the widget should not output
40+
* empty wrapper HTML when there's no ad content to display.
41+
*
42+
* @covers ACM_Ad_Zones::widget
43+
*/
44+
public function test_widget_outputs_nothing_when_no_ad_code_found(): void {
45+
$args = array(
46+
'before_widget' => '<div class="widget">',
47+
'after_widget' => '</div>',
48+
'before_title' => '<h2>',
49+
'after_title' => '</h2>',
50+
);
51+
$instance = array(
52+
'title' => 'Test Ad',
53+
'ad_zone' => 'nonexistent_zone',
54+
);
55+
56+
ob_start();
57+
$this->widget->widget( $args, $instance );
58+
$output = ob_get_clean();
59+
60+
$this->assertEmpty( $output, 'Widget should output nothing when no ad code is found.' );
61+
}
62+
63+
/**
64+
* Test widget outputs nothing for empty ad zone.
65+
*
66+
* @covers ACM_Ad_Zones::widget
67+
*/
68+
public function test_widget_outputs_nothing_for_empty_ad_zone(): void {
69+
$args = array(
70+
'before_widget' => '<div class="widget">',
71+
'after_widget' => '</div>',
72+
'before_title' => '<h2>',
73+
'after_title' => '</h2>',
74+
);
75+
$instance = array(
76+
'title' => '',
77+
'ad_zone' => '',
78+
);
79+
80+
ob_start();
81+
$this->widget->widget( $args, $instance );
82+
$output = ob_get_clean();
83+
84+
$this->assertEmpty( $output, 'Widget should output nothing for empty ad zone.' );
85+
}
86+
87+
/**
88+
* Test widget outputs wrapper when filter returns true for empty content.
89+
*
90+
* The acm_display_empty_widget filter allows themes to force display
91+
* of the widget wrapper even when no ad content is found.
92+
*
93+
* @covers ACM_Ad_Zones::widget
94+
*/
95+
public function test_widget_outputs_wrapper_when_filter_returns_true(): void {
96+
add_filter( 'acm_display_empty_widget', '__return_true' );
97+
98+
$args = array(
99+
'before_widget' => '<div class="widget">',
100+
'after_widget' => '</div>',
101+
'before_title' => '<h2>',
102+
'after_title' => '</h2>',
103+
);
104+
$instance = array(
105+
'title' => '',
106+
'ad_zone' => 'nonexistent_zone',
107+
);
108+
109+
ob_start();
110+
$this->widget->widget( $args, $instance );
111+
$output = ob_get_clean();
112+
113+
remove_filter( 'acm_display_empty_widget', '__return_true' );
114+
115+
$this->assertStringContainsString( '<div class="widget">', $output, 'Widget should output wrapper when filter returns true.' );
116+
$this->assertStringContainsString( '</div>', $output, 'Widget should output closing wrapper when filter returns true.' );
117+
}
118+
119+
/**
120+
* Test widget outputs title when filter allows empty content.
121+
*
122+
* @covers ACM_Ad_Zones::widget
123+
*/
124+
public function test_widget_outputs_title_when_filter_allows_empty_content(): void {
125+
add_filter( 'acm_display_empty_widget', '__return_true' );
126+
127+
$args = array(
128+
'before_widget' => '<div class="widget">',
129+
'after_widget' => '</div>',
130+
'before_title' => '<h2>',
131+
'after_title' => '</h2>',
132+
);
133+
$instance = array(
134+
'title' => 'Advertisement',
135+
'ad_zone' => 'nonexistent_zone',
136+
);
137+
138+
ob_start();
139+
$this->widget->widget( $args, $instance );
140+
$output = ob_get_clean();
141+
142+
remove_filter( 'acm_display_empty_widget', '__return_true' );
143+
144+
$this->assertStringContainsString( '<h2>Advertisement</h2>', $output, 'Widget should output title when filter allows.' );
145+
}
146+
147+
/**
148+
* Test acm_display_empty_widget filter receives correct arguments.
149+
*
150+
* @covers ACM_Ad_Zones::widget
151+
*/
152+
public function test_filter_receives_correct_arguments(): void {
153+
$received_args = array();
154+
155+
$filter_callback = function ( $display, $ad_zone, $args, $instance ) use ( &$received_args ) {
156+
$received_args = array(
157+
'display' => $display,
158+
'ad_zone' => $ad_zone,
159+
'args' => $args,
160+
'instance' => $instance,
161+
);
162+
return false;
163+
};
164+
165+
add_filter( 'acm_display_empty_widget', $filter_callback, 10, 4 );
166+
167+
$args = array(
168+
'before_widget' => '<div class="widget">',
169+
'after_widget' => '</div>',
170+
'before_title' => '<h2>',
171+
'after_title' => '</h2>',
172+
);
173+
$instance = array(
174+
'title' => 'Test',
175+
'ad_zone' => 'test_zone',
176+
);
177+
178+
ob_start();
179+
$this->widget->widget( $args, $instance );
180+
ob_get_clean();
181+
182+
remove_filter( 'acm_display_empty_widget', $filter_callback, 10 );
183+
184+
$this->assertFalse( $received_args['display'], 'First argument should be false.' );
185+
$this->assertSame( 'test_zone', $received_args['ad_zone'], 'Second argument should be the ad zone.' );
186+
$this->assertSame( $args, $received_args['args'], 'Third argument should be widget args.' );
187+
$this->assertSame( $instance, $received_args['instance'], 'Fourth argument should be widget instance.' );
188+
}
189+
190+
/**
191+
* Test widget with valid ad code outputs content.
192+
*
193+
* @covers ACM_Ad_Zones::widget
194+
*/
195+
public function test_widget_outputs_content_with_valid_ad_code(): void {
196+
// Create a valid ad code.
197+
$ad_code_data = array();
198+
foreach ( $this->acm->current_provider->ad_code_args as $arg ) {
199+
$ad_code_data[ $arg['key'] ] = 'test_value_' . $arg['key'];
200+
}
201+
$ad_code_data['priority'] = 10;
202+
$ad_code_data['operator'] = 'AND';
203+
204+
// Need to set up a tag that's registered.
205+
// First, let's check what tags are available.
206+
$ad_tag_ids = $this->acm->ad_tag_ids;
207+
if ( empty( $ad_tag_ids ) ) {
208+
$this->markTestSkipped( 'No ad tag IDs available for testing.' );
209+
}
210+
211+
// Get the first tag with enable_ui_mapping.
212+
$test_tag = null;
213+
foreach ( $ad_tag_ids as $tag ) {
214+
if ( isset( $tag['enable_ui_mapping'] ) && $tag['enable_ui_mapping'] ) {
215+
$test_tag = $tag['tag'];
216+
break;
217+
}
218+
}
219+
220+
if ( ! $test_tag ) {
221+
$this->markTestSkipped( 'No tags with enable_ui_mapping available for testing.' );
222+
}
223+
224+
// Set the tag in ad code data.
225+
$ad_code_data['tag'] = $test_tag;
226+
227+
// Create the ad code.
228+
$ad_code_id = $this->acm->create_ad_code( $ad_code_data );
229+
$this->assertIsInt( $ad_code_id, 'Ad code should be created successfully.' );
230+
231+
// Register ad codes to make them available.
232+
$this->acm->register_ad_codes( $this->acm->get_ad_codes() );
233+
234+
// Enable display of ad codes without conditionals.
235+
add_filter( 'acm_display_ad_codes_without_conditionals', '__return_true' );
236+
237+
$args = array(
238+
'before_widget' => '<div class="widget">',
239+
'after_widget' => '</div>',
240+
'before_title' => '<h2>',
241+
'after_title' => '</h2>',
242+
);
243+
$instance = array(
244+
'title' => 'Ad Widget',
245+
'ad_zone' => $test_tag,
246+
);
247+
248+
ob_start();
249+
$this->widget->widget( $args, $instance );
250+
$output = ob_get_clean();
251+
252+
remove_filter( 'acm_display_ad_codes_without_conditionals', '__return_true' );
253+
254+
// Clean up.
255+
$this->acm->delete_ad_code( $ad_code_id );
256+
257+
$this->assertStringContainsString( '<div class="widget">', $output, 'Widget should output wrapper with valid ad code.' );
258+
$this->assertStringContainsString( '</div>', $output, 'Widget should output closing wrapper.' );
259+
}
260+
261+
/**
262+
* Test that acm_tag action is still fired (backwards compatibility).
263+
*
264+
* @covers ACM_Ad_Zones::widget
265+
*/
266+
public function test_acm_tag_action_is_fired(): void {
267+
$action_fired = false;
268+
$action_tag = null;
269+
270+
$action_callback = function ( $tag_id ) use ( &$action_fired, &$action_tag ) {
271+
$action_fired = true;
272+
$action_tag = $tag_id;
273+
};
274+
275+
add_action( 'acm_tag', $action_callback, 5 ); // Priority 5 to run before the default handler.
276+
277+
$args = array(
278+
'before_widget' => '<div class="widget">',
279+
'after_widget' => '</div>',
280+
'before_title' => '<h2>',
281+
'after_title' => '</h2>',
282+
);
283+
$instance = array(
284+
'title' => '',
285+
'ad_zone' => 'test_action_zone',
286+
);
287+
288+
ob_start();
289+
$this->widget->widget( $args, $instance );
290+
ob_get_clean();
291+
292+
remove_action( 'acm_tag', $action_callback, 5 );
293+
294+
$this->assertTrue( $action_fired, 'acm_tag action should be fired for backwards compatibility.' );
295+
$this->assertSame( 'test_action_zone', $action_tag, 'acm_tag action should receive the correct tag ID.' );
296+
}
297+
}

0 commit comments

Comments
 (0)