Merge branch 'master' into dev/4.0

Author: iMattPro

config/location.yml

@@ -106,10 +106,6 @@ services:
     phpbb.ads.location.type.pop_up:
         class: phpbb\ads\location\type\pop_up
         parent: phpbb.ads.location.type.base
-        arguments:
-            - '@request'
-            - '@config'
-            - '@template'
         tags:
             - { name: phpbb.ads.location.type }
 

config/services.yml

@@ -47,8 +47,8 @@ services:
     phpbb.ads.listener:
         class: phpbb\ads\event\main_listener
         arguments:
+            - '@language'
             - '@template'
-            - '@template_context'
             - '@user'
             - '@config'
             - '@phpbb.ads.ad.manager'

event/main_listener.php

@@ -20,12 +20,12 @@
  */
 class main_listener implements EventSubscriberInterface
 {
+	/** @var \phpbb\language\language */
+	protected $language;
+
 	/** @var \phpbb\template\template */
 	protected $template;
 
-	/** @var \phpbb\template\context */
-	protected $template_context;
-
 	/** @var \phpbb\user */
 	protected $user;
 
@@ -58,7 +58,7 @@ public static function getSubscribedEvents()
 		return array(
 			'core.permissions'				=> 'set_permissions',
 			'core.user_setup'				=> 'load_language_on_setup',
-			'core.page_footer_after'		=> array(array('setup_ads'), array('visual_demo')),
+			'core.page_footer_after'		=> array(array('setup_ads'), array('visual_demo'), array('append_agreement')),
 			'core.page_header_after'		=> array(array('adblocker'), array('clicks')),
 			'core.delete_user_after'		=> 'remove_ad_owner',
 			'core.adm_page_header_after'	=> 'disable_xss_protection',
@@ -70,8 +70,8 @@ public static function getSubscribedEvents()
 	/**
 	 * Constructor
 	 *
+	 * @param \phpbb\language\language				$language			Language object
 	 * @param \phpbb\template\template				$template			Template object
-	 * @param \phpbb\template\context				$template_context	Template context object
 	 * @param \phpbb\user							$user				User object
 	 * @param \phpbb\config\config					$config				Config object
 	 * @param \phpbb\ads\ad\manager					$manager			Advertisement manager object
@@ -81,10 +81,10 @@ public static function getSubscribedEvents()
 	 * @param \phpbb\cache\driver\driver_interface	$cache				Cache driver object
 	 * @param string								$php_ext			PHP extension
 	 */
-	public function __construct(\phpbb\template\template $template, \phpbb\template\context $template_context, \phpbb\user $user, \phpbb\config\config $config, \phpbb\ads\ad\manager $manager, \phpbb\ads\location\manager $location_manager, \phpbb\controller\helper $controller_helper, \phpbb\request\request $request, \phpbb\cache\driver\driver_interface $cache, $php_ext)
+	public function __construct(\phpbb\language\language $language, \phpbb\template\template $template, \phpbb\user $user, \phpbb\config\config $config, \phpbb\ads\ad\manager $manager, \phpbb\ads\location\manager $location_manager, \phpbb\controller\helper $controller_helper, \phpbb\request\request $request, \phpbb\cache\driver\driver_interface $cache, $php_ext)
 	{
+		$this->language = $language;
 		$this->template = $template;
-		$this->template_context = $template_context;
 		$this->user = $user;
 		$this->config = $config;
 		$this->manager = $manager;
@@ -131,10 +131,8 @@ public function load_language_on_setup($event)
 	 */
 	public function setup_ads()
 	{
-		// Reason we access template's root ref is to check for existence
-		// of 'MESSAGE_TEXT', which signals error page.
-		$rootref = $this->template_context->get_root_ref();
-		$non_content_page = !empty($rootref['MESSAGE_TEXT']) || $this->is_non_content_page();
+		// check for the existence of 'MESSAGE_TEXT', which signals it's an error page.
+		$non_content_page = $this->template->retrieve_var('MESSAGE_TEXT') || $this->is_non_content_page();
 		$location_ids = $this->location_manager->get_all_location_ids();
 		$user_groups = $this->manager->load_memberships($this->user->data['user_id']);
 		$ad_ids = array();
@@ -278,4 +276,23 @@ protected function is_non_content_page()
 			'adm',
 		])) > 0;
 	}
+
+	/**
+	 * Append additional agreement details to the privacy agreement.
+	 *
+	 * @return void
+	 */
+	public function append_agreement()
+	{
+		if ((strpos($this->user->page['page_name'], 'ucp') !== 0)
+			|| !$this->template->retrieve_var('S_AGREEMENT')
+			|| ($this->template->retrieve_var('AGREEMENT_TITLE') !== $this->language->lang('PRIVACY')))
+		{
+			return;
+		}
+
+		$this->language->add_lang('ucp', 'phpbb/ads');
+
+		$this->template->append_var('AGREEMENT_TEXT', $this->language->lang('PHPBB_ADS_PRIVACY_POLICY', $this->config['sitename']));
+	}
 }

language/en/ucp.php

@@ -29,4 +29,12 @@
 	'ACTIVE_ADS'	=> 'Active ads',
 	'EXPIRED_ADS'	=> 'Expired ads',
 	'NO_ADS'		=> '<strong>You do not have any advertisements displayed on this board.</strong>',
+
+	'PHPBB_ADS_PRIVACY_POLICY' => '
+		<br><br>
+		<h3>Advertising</h3>
+		“%1$s” may display advertising provided by third-party networks or services. These advertisements may use cookies, tracking pixels, or similar technologies to collect information about your browsing activities on this site and, in some cases, across other websites. This information may be used to deliver relevant advertisements, measure the effectiveness of advertising campaigns, and tailor content to your interests.
+		<br><br>
+		Any data collected through such third-party advertising services is subject to the privacy and cookie policies of the respective providers. “%1$s” does not control the cookies or data collection practices of these third parties. We encourage you to review the policies of the relevant advertising providers for more details on how your information is processed.
+	',
 ));

location/type/pop_up.php

@@ -12,31 +12,6 @@
 
 class pop_up extends base
 {
-	/** @var \phpbb\request\request */
-	protected $request;
-	/** @var \phpbb\config\config */
-	protected $config;
-	/** @var \phpbb\template\template */
-	protected $template;
-
-	/**
-	 * pop_up location constructor.
-	 *
-	 * @param \phpbb\user              $user     User object
-	 * @param \phpbb\language\language $language Language object
-	 * @param \phpbb\request\request   $request  Request object
-	 * @param \phpbb\config\config     $config   Config object
-	 * @param \phpbb\template\template $template Template object
-	 */
-	public function __construct(\phpbb\user $user, \phpbb\language\language $language, \phpbb\request\request $request, \phpbb\config\config $config, \phpbb\template\template $template)
-	{
-		parent::__construct($user, $language);
-
-		$this->request = $request;
-		$this->config = $config;
-		$this->template = $template;
-	}
-
 	/**
 	 * {@inheritDoc}
 	 */
@@ -52,22 +27,4 @@ public function get_category()
 	{
 		return self::CAT_INTERACTIVE;
 	}
-
-	/**
-	 * {@inheritDoc}
-	 */
-	public function will_display()
-	{
-		if ($this->request->is_set($this->config['cookie_name'] . '_pop_up', \phpbb\request\request_interface::COOKIE))
-		{
-			return false;
-		}
-
-		$this->template->assign_vars(array(
-			'POP_UP_COOKIE_NAME'	=> $this->config['cookie_name'] . '_pop_up',
-			'POP_UP_COOKIE_EXPIRES'	=> gmdate('D, d M Y H:i:s T', strtotime('+1 day')),
-			'POP_UP_COOKIE_PATH'	=> $this->config['cookie_path'],
-		));
-		return true;
-	}
 }

styles/all/template/includes/phpbb_ads_pop_up.html

@@ -1,11 +1,20 @@
 {% import '@phpbb_ads/phpbb_ads_macro.html' as ad %}
 <script>
-	(function($, window, document, phpbb) {
+	(() => {
 		'use strict';
 
-		$(window).on('load', function() {
-			document.cookie = '{{ POP_UP_COOKIE_NAME }}=true; expires={{ POP_UP_COOKIE_EXPIRES }}; path={{ POP_UP_COOKIE_PATH }}';
-			phpbb.alert('{{ lang('ADVERTISEMENT')|upper|e('js') }}', `{{ ad.renderAds(AD_POP_UP, AD_POP_UP_ID, AD_POP_UP_CENTER, 'margin: 0;') }}`);
-		});
-	})(jQuery, window, document, phpbb);
+		const key = 'phpbb_ads_pop_up';
+		const interval_days = 1;
+		const lastShown = localStorage.getItem(key);
+
+		if (!lastShown || Date.now() - +lastShown >= interval_days * 86400000) {
+			localStorage.setItem(key, Date.now());
+			window.addEventListener('load', () => {
+				phpbb.alert(
+					'{{ lang('ADVERTISEMENT')|upper|e('js') }}',
+					`{{ ad.renderAds(AD_POP_UP, AD_POP_UP_ID, AD_POP_UP_CENTER, 'margin: 0;') }}`
+				);
+			});
+		}
+	})();
 </script>

tests/event/amend_agreement_test.php

@@ -0,0 +1,62 @@
+<?php
+/**
+ *
+ * Pages extension for the phpBB Forum Software package.
+ *
+ * @copyright (c) 2014 phpBB Limited <https://www.phpbb.com>
+ * @license GNU General Public License, version 2 (GPL-2.0)
+ *
+ */
+
+namespace phpbb\ads\tests\event;
+
+class amend_agreement_test extends main_listener_base
+{
+	/**
+	 * Data for test_append_agreement
+	 *
+	 * @return array
+	 */
+	public function append_agreement_data()
+	{
+		return [
+			[false, 'PRIVACY', 0], // No agreement
+			[true, 'TERMS', 0], // Wrong title
+			[true, 'PRIVACY', 1], // Correct conditions
+		];
+	}
+
+	/**
+	 * Test the append_agreement method
+	 *
+	 * @dataProvider append_agreement_data
+	 * @param mixed $s_agreement S_AGREEMENT template variable value
+	 * @param mixed $agreement_title AGREEMENT_TITLE template variable value
+	 * @param int $expected_append_calls Expected append_var calls
+	 */
+	public function test_append_agreement($s_agreement, $agreement_title, $expected_append_calls)
+	{
+		$this->config['sitename'] = 'Test Forum';
+		$this->user->page['page_name'] = 'ucp.php';
+
+		$this->template->expects(self::atMost(2))
+			->method('retrieve_var')
+			->withConsecutive(['S_AGREEMENT'], ['AGREEMENT_TITLE'])
+			->willReturnOnConsecutiveCalls($s_agreement, $this->language->lang($agreement_title));
+
+		if ($expected_append_calls > 0)
+		{
+			$this->template->expects(self::once())
+				->method('append_var')
+				->with('AGREEMENT_TEXT', $this->language->lang('PHPBB_ADS_PRIVACY_POLICY', 'Test Forum'));
+		}
+		else
+		{
+			$this->template->expects(self::never())
+				->method('append_var');
+		}
+
+		$listener = $this->get_listener();
+		$listener->append_agreement();
+	}
+}

tests/event/main_listener_base.php

@@ -38,12 +38,12 @@ class main_listener_base extends phpbb_database_test_case
 	/** @var template|MockObject */
 	protected template|MockObject $template;
 
-	/** @var context|MockObject */
-	protected context|MockObject $template_context;
-
 	/** @var user|MockObject */
 	protected user|MockObject $user;
 
+	/** @var \phpbb\language\language */
+	protected $language;
+
 	/** @var string ads_table */
 	protected string $ads_table;
 
@@ -107,8 +107,8 @@ protected function setUp(): void
 			->getMock();
 		$phpbb_dispatcher = new phpbb_mock_event_dispatcher();
 		$lang_loader = new language_file_loader($phpbb_root_path, $phpEx);
-		$lang = new language($lang_loader);
-		$user = new user($lang, datetime::class);
+		$this->language = new language($lang_loader);
+		$user = new user($this->language, datetime::class);
 		$request = $this->getMockBuilder(request::class)
 			->disableOriginalConstructor()
 			->getMock();
@@ -144,21 +144,18 @@ protected function setUp(): void
 			$class = "\\phpbb\\ads\\location\\type\\$type";
 			if ($type === 'pop_up')
 			{
-				$location_types['phpbb.ads.location.type.' . $type] = new $class($user, $lang, $request, $config, $template);
+				$location_types['phpbb.ads.location.type.' . $type] = new $class($user, $this->language, $request, $config, $template);
 			}
 			else
 			{
-				$location_types['phpbb.ads.location.type.' . $type] = new $class($user, $lang);
+				$location_types['phpbb.ads.location.type.' . $type] = new $class($user, $this->language);
 			}
 		}
 
 		// Load/Mock classes required by the listener class
 		$this->template = $this->getMockBuilder(template::class)
 			->disableOriginalConstructor()
 			->getMock();
-		$this->template_context = $this->getMockBuilder(context::class)
-			->disableOriginalConstructor()
-			->getMock();
 		$this->user = $user;
 		$this->config = new config(array('phpbb_ads_adblocker_message' => '0'));
 		$this->manager = new ad_manager($this->new_dbal(), $this->config, $this->ads_table, $this->ad_locations_table, $this->ad_group_table);
@@ -181,8 +178,8 @@ protected function setUp(): void
 	protected function get_listener(): main_listener
 	{
 		return new main_listener(
+			$this->language,
 			$this->template,
-			$this->template_context,
 			$this->user,
 			$this->config,
 			$this->manager,

tests/functional/ucp_test.php

@@ -56,6 +56,12 @@ public function test_ucp_module()
 		$this->assertContainsLang('AD_NAME', $crawler->filter('.table1')->text());
 	}
 
+	public function test_ucp_agreement()
+	{
+		$crawler = self::request('GET', 'ucp.php?mode=privacy');
+		$this->assertStringContainsString($this->lang('PHPBB_ADS_PRIVACY_POLICY', 'yourdomain.com'), $crawler->filter('.agreement')->html());
+	}
+
 	protected function get_ucp_module($assert_response_html = true): Crawler
 	{
 		return self::request('GET', "ucp.php?i=-phpbb-ads-ucp-main_module&mode=stats&sid=$this->sid", array(), $assert_response_html);

tests/location/get_all_location_ids_test.php

@@ -20,7 +20,7 @@ class get_all_location_ids_test extends location_base
 	public static function get_all_location_ids_data(): array
 	{
 		return array(
-			array('index', '', false, array(
+			array('index', '', array(
 				'above_header',
 				'after_header_navbar',
 				'below_header',
@@ -31,7 +31,7 @@ public static function get_all_location_ids_data(): array
 				'slide_up',
 				'scripts',
 			)),
-			array('viewtopic', '', false, array(
+			array('viewtopic', '', array(
 				'above_header',
 				'after_header_navbar',
 				'below_header',
@@ -48,7 +48,7 @@ public static function get_all_location_ids_data(): array
 				'slide_up',
 				'scripts',
 			)),
-			array('memberlist', 'viewprofile', false, array(
+			array('memberlist', 'viewprofile', array(
 				'above_header',
 				'after_header_navbar',
 				'below_header',
@@ -61,13 +61,14 @@ public static function get_all_location_ids_data(): array
 				'slide_up',
 				'scripts',
 			)),
-			array('index', '', true, array(
+			array('index', '', array(
 				'above_header',
 				'after_header_navbar',
 				'below_header',
 				'above_footer',
 				'after_footer_navbar',
 				'below_footer',
+				'pop_up',
 				'slide_up',
 				'scripts',
 			)),
@@ -79,16 +80,11 @@ public static function get_all_location_ids_data(): array
 	 *
 	 * @dataProvider get_all_location_ids_data
 	 */
-	public function test_get_all_location_ids($page_name, $query_string, $cookie, $expected)
+	public function test_get_all_location_ids($page_name, $query_string, $expected)
 	{
 		$this->user->page['page_name'] = $page_name;
 		$this->user->page['query_string'] = $query_string;
 
-		$this->request
-			->method('is_set')
-			->with('_pop_up')
-			->willReturn($cookie);
-
 		$manager = $this->get_manager();
 
 		$location_ids = $manager->get_all_location_ids();