From 252b5fe4f717494adde6644f257a044a661d2a7b Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 9 Nov 2013 20:28:24 +0100 Subject: [PATCH 1/6] =?UTF-8?q?[=C5=A7icket/11896]=20Set=20form=5Ftime=20w?= =?UTF-8?q?ith=20time()=20when=20marking=20all=20notifications=20read?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit form_time is only set if is passed via the form. Since the mark notifications read link does not use the form, it will default to 0 causing the mark notifications logic to only mark notifications read if their time is smaller or equal to 0. This patch will change ucp_notifications to correctly set form_time for the confirm_box. PHPBB3-11896 --- phpBB/includes/ucp/ucp_notifications.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/includes/ucp/ucp_notifications.php b/phpBB/includes/ucp/ucp_notifications.php index 5a896c31b0..de9352a322 100644 --- a/phpBB/includes/ucp/ucp_notifications.php +++ b/phpBB/includes/ucp/ucp_notifications.php @@ -103,7 +103,7 @@ class ucp_notifications { confirm_box(false, 'NOTIFICATIONS_MARK_ALL_READ', build_hidden_fields(array( 'mark' => 'all', - 'form_time' => $form_time, + 'form_time' => time(), ))); } } From 29d3b08d5097ad90a5ed2d26bfc981898732dbab Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 9 Nov 2013 20:33:01 +0100 Subject: [PATCH 2/6] [ticket/11896] Add ability to define expected message after posting This change will add the ability to define the expected message after a topic or post has been submitted with the methods create_post() and create_topic(). If the expected message is not 'POST_STORED', we will not try to get the topic and post id. PHPBB3-11896 --- .../phpbb_functional_test_case.php | 36 +++++++++++++++---- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/tests/test_framework/phpbb_functional_test_case.php b/tests/test_framework/phpbb_functional_test_case.php index a0d186e0f2..64c8406adb 100644 --- a/tests/test_framework/phpbb_functional_test_case.php +++ b/tests/test_framework/phpbb_functional_test_case.php @@ -868,9 +868,10 @@ class phpbb_functional_test_case extends phpbb_test_case * @param string $subject * @param string $message * @param array $additional_form_data Any additional form data to be sent in the request + * @param string $expected Lang var of expected message after posting * @return array post_id, topic_id */ - public function create_topic($forum_id, $subject, $message, $additional_form_data = array()) + public function create_topic($forum_id, $subject, $message, $additional_form_data = array(), $expected = '') { $posting_url = "posting.php?mode=post&f={$forum_id}&sid={$this->sid}"; @@ -880,7 +881,14 @@ class phpbb_functional_test_case extends phpbb_test_case 'post' => true, ), $additional_form_data); - return self::submit_post($posting_url, 'POST_TOPIC', $form_data); + if ($expected !== '') + { + return self::submit_post($posting_url, 'POST_TOPIC', $form_data, $expected); + } + else + { + return self::submit_post($posting_url, 'POST_TOPIC', $form_data); + } } /** @@ -893,9 +901,10 @@ class phpbb_functional_test_case extends phpbb_test_case * @param string $subject * @param string $message * @param array $additional_form_data Any additional form data to be sent in the request + * @param string $expected Lang var of expected message after posting * @return array post_id, topic_id */ - public function create_post($forum_id, $topic_id, $subject, $message, $additional_form_data = array()) + public function create_post($forum_id, $topic_id, $subject, $message, $additional_form_data = array(), $expected = '') { $posting_url = "posting.php?mode=reply&f={$forum_id}&t={$topic_id}&sid={$this->sid}"; @@ -905,7 +914,14 @@ class phpbb_functional_test_case extends phpbb_test_case 'post' => true, ), $additional_form_data); - return self::submit_post($posting_url, 'POST_REPLY', $form_data); + if ($expected !== '') + { + return self::submit_post($posting_url, 'POST_REPLY', $form_data, $expected); + } + else + { + return self::submit_post($posting_url, 'POST_REPLY', $form_data); + } } /** @@ -914,9 +930,10 @@ class phpbb_functional_test_case extends phpbb_test_case * @param string $posting_url * @param string $posting_contains * @param array $form_data - * @return array post_id, topic_id + * @param string $expected Lang var of expected message after posting + * @return array|null post_id, topic_id if message is 'POST_STORED' */ - protected function submit_post($posting_url, $posting_contains, $form_data) + protected function submit_post($posting_url, $posting_contains, $form_data, $expected = 'POST_STORED') { $this->add_lang('posting'); @@ -945,7 +962,12 @@ class phpbb_functional_test_case extends phpbb_test_case // contained in one of the actual form fields that the browser sees (i.e. it ignores "hidden" inputs) // Instead, I send it as a request with the submit button "post" set to true. $crawler = self::request('POST', $posting_url, $form_data); - $this->assertContains($this->lang('POST_STORED'), $crawler->filter('html')->text()); + $this->assertContains($this->lang($expected), $crawler->filter('html')->text()); + + if ($expected !== 'POST_STORED') + { + return; + } $url = $crawler->selectLink($this->lang('VIEW_MESSAGE', '', ''))->link()->getUri(); return array( From 95c2b12bf7be4e6ad1070d7dd11a9afd7fc1efa8 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 9 Nov 2013 20:35:37 +0100 Subject: [PATCH 3/6] [ticket/11896] Add functional tests for marking all notifications read PHPBB3-11896 --- tests/functional/notification_test.php | 33 ++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/functional/notification_test.php b/tests/functional/notification_test.php index 7f33ad1859..dd1b8ec981 100644 --- a/tests/functional/notification_test.php +++ b/tests/functional/notification_test.php @@ -52,4 +52,37 @@ class phpbb_functional_notification_test extends phpbb_functional_test_case $this->assert_checkbox_is_unchecked($cplist, $checkbox_name); } } + + public function test_mark_notifications_read() + { + // Create a new standard user + $this->create_user('notificationtestuser'); + $this->add_user_group('NEWLY_REGISTERED', array('notificationtestuser')); + $this->login('notificationtestuser'); + $crawler = self::request('GET', 'index.php'); + $this->assertContains('notificationtestuser', $crawler->filter('.icon-logout')->text()); + + // Post a new post that needs approval + $this->create_post(2, 1, 'Re: Welcome to phpBB3', 'This is a test [b]post[/b] posted by notificationtestuser.', array(), 'POST_STORED_MOD'); + $crawler = self::request('GET', "viewtopic.php?t=1&sid={$this->sid}"); + $this->assertNotContains('This is a test post posted by notificationtestuser.', $crawler->filter('html')->text()); + + // logout + $crawler = self::request('GET', 'ucp.php?sid=' . $this->sid . '&mode=logout'); + + // admin login + $this->login(); + $this->add_lang('ucp'); + $crawler = self::request('GET', 'ucp.php?i=ucp_notifications'); + + // At least one notification should exist + $this->assertGreaterThan(0, $crawler->filter('#notification_list_button strong')->text()); + + // Get form token + $link = $crawler->selectLink($this->lang('NOTIFICATIONS_MARK_ALL_READ'))->link()->getUri(); + $crawler = self::request('GET', substr($link, strpos($link, 'ucp.'))); + $form = $crawler->selectButton($this->lang('YES'))->form(); + $crawler = self::submit($form); + $this->assertEquals(0, $crawler->filter('#notification_list_button strong')->text()); + } } From 308329b5479d16e104c9eb7372222fee67ccdbf5 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 14 Nov 2013 15:09:49 +0100 Subject: [PATCH 4/6] [ticket/11896] Minor code improvements in phpbb_functional_test_case Use assertContainsLang() and get rid of unnecessary logic in create_post() and create_topic(). The docblocks were also slightly improved. PHPBB3-11896 --- .../phpbb_functional_test_case.php | 28 +++++-------------- 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/tests/test_framework/phpbb_functional_test_case.php b/tests/test_framework/phpbb_functional_test_case.php index 64c8406adb..876e42a4c2 100644 --- a/tests/test_framework/phpbb_functional_test_case.php +++ b/tests/test_framework/phpbb_functional_test_case.php @@ -868,10 +868,10 @@ class phpbb_functional_test_case extends phpbb_test_case * @param string $subject * @param string $message * @param array $additional_form_data Any additional form data to be sent in the request - * @param string $expected Lang var of expected message after posting + * @param string $expected Lang var of expected message after posting or null * @return array post_id, topic_id */ - public function create_topic($forum_id, $subject, $message, $additional_form_data = array(), $expected = '') + public function create_topic($forum_id, $subject, $message, $additional_form_data = array(), $expected = 'POST_STORED') { $posting_url = "posting.php?mode=post&f={$forum_id}&sid={$this->sid}"; @@ -881,14 +881,7 @@ class phpbb_functional_test_case extends phpbb_test_case 'post' => true, ), $additional_form_data); - if ($expected !== '') - { - return self::submit_post($posting_url, 'POST_TOPIC', $form_data, $expected); - } - else - { - return self::submit_post($posting_url, 'POST_TOPIC', $form_data); - } + return self::submit_post($posting_url, 'POST_TOPIC', $form_data, $expected); } /** @@ -901,10 +894,10 @@ class phpbb_functional_test_case extends phpbb_test_case * @param string $subject * @param string $message * @param array $additional_form_data Any additional form data to be sent in the request - * @param string $expected Lang var of expected message after posting + * @param string $expected Lang var of expected message after posting or null * @return array post_id, topic_id */ - public function create_post($forum_id, $topic_id, $subject, $message, $additional_form_data = array(), $expected = '') + public function create_post($forum_id, $topic_id, $subject, $message, $additional_form_data = array(), $expected = 'POST_STORED') { $posting_url = "posting.php?mode=reply&f={$forum_id}&t={$topic_id}&sid={$this->sid}"; @@ -914,14 +907,7 @@ class phpbb_functional_test_case extends phpbb_test_case 'post' => true, ), $additional_form_data); - if ($expected !== '') - { - return self::submit_post($posting_url, 'POST_REPLY', $form_data, $expected); - } - else - { - return self::submit_post($posting_url, 'POST_REPLY', $form_data); - } + return self::submit_post($posting_url, 'POST_REPLY', $form_data, $expected); } /** @@ -962,7 +948,7 @@ class phpbb_functional_test_case extends phpbb_test_case // contained in one of the actual form fields that the browser sees (i.e. it ignores "hidden" inputs) // Instead, I send it as a request with the submit button "post" set to true. $crawler = self::request('POST', $posting_url, $form_data); - $this->assertContains($this->lang($expected), $crawler->filter('html')->text()); + $this->assertContainsLang($expected, $crawler->filter('html')->text()); if ($expected !== 'POST_STORED') { From 7f10312bf21dba335a863fb1222db8b9ed64ef0e Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 14 Nov 2013 15:17:25 +0100 Subject: [PATCH 5/6] [ticket/11896] Correctly document return of null in docblocks Also got rid of previous incorrect comment in docblocks. PHPBB3-11896 --- tests/test_framework/phpbb_functional_test_case.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_framework/phpbb_functional_test_case.php b/tests/test_framework/phpbb_functional_test_case.php index 876e42a4c2..eba5a2dfdf 100644 --- a/tests/test_framework/phpbb_functional_test_case.php +++ b/tests/test_framework/phpbb_functional_test_case.php @@ -868,8 +868,8 @@ class phpbb_functional_test_case extends phpbb_test_case * @param string $subject * @param string $message * @param array $additional_form_data Any additional form data to be sent in the request - * @param string $expected Lang var of expected message after posting or null - * @return array post_id, topic_id + * @param string $expected Lang var of expected message after posting + * @return array|null post_id, topic_id if message is 'POST_STORED' */ public function create_topic($forum_id, $subject, $message, $additional_form_data = array(), $expected = 'POST_STORED') { @@ -894,8 +894,8 @@ class phpbb_functional_test_case extends phpbb_test_case * @param string $subject * @param string $message * @param array $additional_form_data Any additional form data to be sent in the request - * @param string $expected Lang var of expected message after posting or null - * @return array post_id, topic_id + * @param string $expected Lang var of expected message after posting + * @return array|null post_id, topic_id if message is 'POST_STORED' */ public function create_post($forum_id, $topic_id, $subject, $message, $additional_form_data = array(), $expected = 'POST_STORED') { From 0d3396487e58091053778c9eb40ebcb6fa86536f Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 14 Nov 2013 18:16:29 +0100 Subject: [PATCH 6/6] [ticket/11896] Use $form_time and fix out of bounds $form_time PHPBB3-11896 --- phpBB/includes/ucp/ucp_notifications.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/phpBB/includes/ucp/ucp_notifications.php b/phpBB/includes/ucp/ucp_notifications.php index de9352a322..63dbe79666 100644 --- a/phpBB/includes/ucp/ucp_notifications.php +++ b/phpBB/includes/ucp/ucp_notifications.php @@ -27,7 +27,8 @@ class ucp_notifications add_form_key('ucp_notification'); $start = $request->variable('start', 0); - $form_time = min($request->variable('form_time', 0), time()); + $form_time = $request->variable('form_time', 0); + $form_time = ($form_time <= 0 || $form_time > time()) ? time() : $form_time; $phpbb_notifications = $phpbb_container->get('notification_manager'); @@ -103,7 +104,7 @@ class ucp_notifications { confirm_box(false, 'NOTIFICATIONS_MARK_ALL_READ', build_hidden_fields(array( 'mark' => 'all', - 'form_time' => time(), + 'form_time' => $form_time, ))); } }