Merge pull request #6333 from rxu/ticket/16904-master

[ticket/16904] Fix regression in MCP for topics selection
This commit is contained in:
Marc Alexander 2021-11-30 21:12:09 +01:00
commit 00f2b68f6d
No known key found for this signature in database
GPG key ID: 50E0D2423696F995
8 changed files with 457 additions and 71 deletions

View file

@ -35,7 +35,7 @@ function phpbb_module_notes_url($mode, $module_row)
}
global $user_id;
return ($user_id) ? "&u=$user_id" : '';
return phpbb_extra_url();
}
function phpbb_module_warn_url($mode, $module_row)
@ -43,34 +43,18 @@ function phpbb_module_warn_url($mode, $module_row)
if ($mode == 'front' || $mode == 'list')
{
global $forum_id;
return ($forum_id) ? "&f=$forum_id" : '';
return phpbb_extra_url();
}
if ($mode == 'warn_post')
{
global $forum_id, $post_id;
if ($post_id)
{
$url_extra = "&p=$post_id";
}
else if ($forum_id)
{
$url_extra = "&f=$forum_id";
}
else
{
$url_extra = '';
}
return $url_extra;
return phpbb_extra_url();
}
else
{
global $user_id;
return ($user_id) ? "&u=$user_id" : '';
return phpbb_extra_url();
}
}
@ -99,30 +83,34 @@ function phpbb_module_reports_url($mode, $module_row)
return phpbb_extra_url();
}
function phpbb_extra_url()
/**
* Generate URL parameters for MCP modules
*
* @param array $additional_parameters Array with additional parameters in format of ['key' => 'parameter_name']
*
* @return string String with URL parameters (empty string if not any)
*/
function phpbb_extra_url($additional_parameters = [])
{
global $forum_id, $topic_id, $post_id, $report_id, $user_id;
$url_extra = [];
$url_parameters = array_merge([
'f' => 'forum_id',
't' => 'topic_id',
'p' => 'post_id',
'r' => 'report_id',
'u' => 'user_id',
], $additional_parameters);
if ($post_id)
foreach ($url_parameters as $key => $value)
{
$url_extra = "&p=$post_id";
}
else if ($topic_id)
global $$value;
if (isset($$value) && $parameter = $$value)
{
$url_extra = "&t=$topic_id";
$url_extra[] = "$key=$parameter";
}
else if ($forum_id)
{
$url_extra = "&f=$forum_id";
}
else
{
$url_extra = '';
}
$url_extra .= ($user_id) ? "&u=$user_id" : '';
$url_extra .= ($report_id) ? "&r=$report_id" : '';
return $url_extra;
return implode('&', $url_extra);
}
/**

View file

@ -662,7 +662,7 @@ class p_master
// Add url_extra parameter to u_action url
if (!empty($this->module_ary) && $this->active_module !== false && $this->module_ary[$this->active_module_row_id]['url_extra'])
{
$this->module->u_action .= $this->module_ary[$this->active_module_row_id]['url_extra'];
$this->module->u_action .= '&' . $this->module_ary[$this->active_module_row_id]['url_extra'];
}
// Assign the module path for re-usage
@ -920,7 +920,7 @@ class p_master
}
// Was not allowed in categories before - /*!$item_ary['cat'] && */
$u_title .= (isset($item_ary['url_extra'])) ? $item_ary['url_extra'] : '';
$u_title .= (isset($item_ary['url_extra']) && $item_ary['url_extra']) ? '&' . $item_ary['url_extra'] : '';
// Only output a categories items if it's currently selected
if (!$depth || ($depth && (in_array($item_ary['parent'], array_values($this->module_cache['parents'])) || $item_ary['parent'] == $this->p_parent)))

View file

@ -0,0 +1,35 @@
<?php
/**
*
* This file is part of the phpBB Forum Software package.
*
* @copyright (c) phpBB Limited <https://www.phpbb.com>
* @license GNU General Public License, version 2 (GPL-2.0)
*
* For full copyright and license information, please see
* the docs/CREDITS.txt file.
*
*/
/**
* @group functional
*/
class phpbb_functional_mcp_logs_test extends phpbb_functional_test_case
{
public function test_delete_logs()
{
$this->add_lang(['mcp', 'common']);
$this->login();
$crawler = self::request('GET', "mcp.php?i=mcp_logs&mode=front&sid={$this->sid}");
$this->assertGreaterThanOrEqual(1, $crawler->filter('input[type=checkbox]')->count());
$form = $crawler->selectButton($this->lang('DELETE_ALL'))->form();
$crawler = self::submit($form);
$form = $crawler->selectButton($this->lang('YES'))->form();
$crawler = self::submit($form);
$this->assertCount(0, $crawler->filter('input[type=checkbox]'));
}
}

View file

@ -0,0 +1,323 @@
<?php
/**
*
* This file is part of the phpBB Forum Software package.
*
* @copyright (c) phpBB Limited <https://www.phpbb.com>
* @license GNU General Public License, version 2 (GPL-2.0)
*
* For full copyright and license information, please see
* the docs/CREDITS.txt file.
*
*/
/**
* @group functional
*/
class phpbb_functional_mcp_main_test extends phpbb_functional_test_case
{
public function test_create_topics()
{
$this->add_lang(['acp/common', 'acp/forums', 'common']);
$this->login();
$this->admin_login();
// Disable flood interval to post >1 of topics
$crawler = self::request('GET', "adm/index.php?i=acp_board&mode=post&sid={$this->sid}");
$form = $crawler->selectButton($this->lang('SUBMIT'))->form([
'config[flood_interval]' => 0,
]);
$crawler = self::submit($form);
$this->assertContainsLang('CONFIG_UPDATED', $crawler->text());
// Create a forum to move topics around
$forum_name = 'MCP Test #1';
$crawler = self::request('GET', "adm/index.php?i=acp_forums&mode=manage&sid={$this->sid}");
$form = $crawler->selectButton($this->lang('CREATE_FORUM'))->form([
'forum_name' => $forum_name,
]);
$crawler = self::submit($form);
$form = $crawler->selectButton($this->lang('SUBMIT'))->form([
'forum_parent_id' => 1,
'forum_perm_from' => 2,
]);
$crawler = self::submit($form);
$this->assertContainsLang('FORUM_CREATED', $crawler->text());
// Create topics to test with
$post = [];
$post[] = $this->create_topic(2, 'Test Topic 3', 'Testing forum moderation actions from MCP/View forum page.');
$crawler = self::request('GET', "viewtopic.php?t={$post[0]['topic_id']}&sid={$this->sid}");
$this->assertStringContainsString('Testing forum moderation actions from MCP/View forum page.', $crawler->filter('html')->text());
$post[] = $this->create_topic(2, 'Topic to merge with', 'Testing merge topics moderation actions from MCP/View forum page.');
$crawler = self::request('GET', "viewtopic.php?t={$post[1]['topic_id']}&sid={$this->sid}");
$this->assertStringContainsString('Testing merge topics moderation actions from MCP/View forum page.', $crawler->filter('html')->text());
return $post;
}
/**
* @depends test_create_topics
*/
public function test_mcp_view_forum($post)
{
$this->add_lang(['common']);
$this->login();
// Browse MCP main page from forum view (gives &f=2)
$crawler = self::request('GET', "viewforum.php?f=2&sid={$this->sid}");
$crawler = self::$client->click($crawler->selectLink($this->lang('MCP_SHORT'))->link());
// Test forum moderation page has a list of topics to select
$this->assertGreaterThanOrEqual(3, $crawler->filter('input[type=checkbox]')->count());
return $post;
}
public function mcp_view_forum_actions_data()
{
// action, success message, require_confirmation
return [
['delete_topic', 'TOPIC_DELETED_SUCCESS', true],
['restore_topic', 'TOPIC_RESTORED_SUCCESS', true],
['fork', 'TOPIC_FORKED_SUCCESS', true],
['lock', 'TOPIC_LOCKED_SUCCESS', true],
['unlock', 'TOPIC_UNLOCKED_SUCCESS', true],
['resync', 'TOPIC_RESYNC_SUCCESS', false],
['make_global', 'TOPIC_TYPE_CHANGED', true],
['make_announce', 'TOPIC_TYPE_CHANGED', true],
['make_sticky', 'TOPIC_TYPE_CHANGED', true],
['make_normal', 'TOPIC_TYPE_CHANGED', true],
['merge_topics', 'POSTS_MERGED_SUCCESS', true],
['move', 'TOPIC_MOVED_SUCCESS', true],
];
}
/**
* @depends test_mcp_view_forum
* @dataProvider mcp_view_forum_actions_data
*/
public function test_mcp_view_forum_actions($action, $message, $require_confirmation, $post)
{
$topic_id_1 = $post[0]['topic_id'];
$topic_id_2 = $post[1]['topic_id'];
$this->add_lang(['common', 'mcp']);
$this->login();
$crawler = self::request('GET', "viewforum.php?f=2&sid={$this->sid}");
$crawler = self::$client->click($crawler->selectLink($this->lang('MCP_SHORT'))->link());
// Test actions
$form = $crawler->selectButton($this->lang('SUBMIT'))->form()->disableValidation()->setValues([
'action' => $action,
'topic_id_list' => [$action == 'move' ? $topic_id_2 : $topic_id_1], // while moving, topic_id_1 has been already merged into topic_id_2
]);
$crawler = self::submit($form);
if ($require_confirmation)
{
if ($action == 'merge_topics')
{
// Merge topic_id_1 into topic_id_2
$select_for_merge_link = $crawler->selectLink($this->lang('SELECT_MERGE'))->reduce(
function ($node, $i) use ($topic_id_2)
{
return (bool) strpos($node->attr('href'), "to_topic_id=$topic_id_2");
}
)->link();
$crawler = self::$client->click($select_for_merge_link);
}
$form = $crawler->selectButton($this->lang('YES'))->form();
if (in_array($action, ['fork', 'move']))
{
// Fork or move the topic to the 'MCP Test #1'
$forum_id = $crawler->filter('select > option')->reduce(
function ($node, $i)
{
return (bool) strpos($node->text(), 'MCP Test #1');
}
)->attr('value');
$form['to_forum_id']->select($forum_id);
}
$crawler = self::submit($form);
}
$this->assertStringContainsString($this->lang($message), $crawler->filter('#message p')->text());
}
/**
* @depends test_mcp_view_forum_actions
*/
public function test_mcp_view_forum_permanently_delete_topic()
{
$this->add_lang(['common', 'mcp']);
$this->login();
// Get to the forum 'MCP Test #1' where the topic has been moved to in previous test
$crawler = self::request('GET', "index.php?sid={$this->sid}");
$crawler = self::$client->click($crawler->selectLink('MCP Test #1')->link());
$crawler = self::$client->click($crawler->selectLink($this->lang('MCP_SHORT'))->link());
// Get topic ids to delete (forked and moved topics in the previous test)
$topic_link_1 = $crawler->selectLink('Test Topic 3')->attr('href');
$topic_link_2 = $crawler->selectLink('Topic to merge with')->attr('href');
$topic_ids = [
(int) $this->get_parameter_from_link($topic_link_1, 't'),
(int) $this->get_parameter_from_link($topic_link_2, 't'),
];
$form = $crawler->selectButton($this->lang('SUBMIT'))->form()->disableValidation()->setValues([
'action' => 'delete_topic',
'topic_id_list' => $topic_ids, // tick both topics in the list
]);
$crawler = self::submit($form);
$form = $crawler->selectButton($this->lang('YES'))->form();
$form['delete_permanent']->tick();
$crawler = self::submit($form);
$this->assertStringContainsString($this->lang('TOPICS_DELETED_SUCCESS'), $crawler->filter('#message p')->text());
}
public function mcp_view_topic_actions_data()
{
// action, success message, require_confirmation
return [
['lock_post', 'POSTS_LOCKED_SUCCESS', true],
['unlock_post', 'POSTS_UNLOCKED_SUCCESS', true],
['resync', 'TOPIC_RESYNC_SUCCESS', false],
['split_all', 'TOPIC_SPLIT_SUCCESS', true],
['split_beyond', 'TOPIC_SPLIT_SUCCESS', true],
['merge_posts', 'POSTS_MERGED_SUCCESS', true],
['delete_post', 'POSTS_DELETED_SUCCESS', true],
];
}
public function test_create_topic_with_replies()
{
$this->login();
// Create topic and replies to test with
$post = [];
$post[] = $this->create_topic(2, 'Test Topic 4', 'Testing topic moderation actions from MCP/View topic page.');
$crawler = self::request('GET', "viewtopic.php?t={$post[0]['topic_id']}&sid={$this->sid}");
$this->assertStringContainsString('Testing topic moderation actions from MCP/View topic page.', $crawler->filter('html')->text());
// Create replies. Flood control was disabled above
for ($i = 1; $i <= 15; $i++)
{
sleep(1);
$post_text = "This is reply number $i to the Test Topic 4 to test moderation actions from MCP/View topic page.";
$post[$i] = $this->create_post(2, $post[0]['topic_id'], 'Re: Test Topic 4', $post_text);
$crawler = self::request('GET', "viewtopic.php?p={$post[$i]['post_id']}&sid={$this->sid}#p{$post[$i]['post_id']}");
$this->assertStringContainsString($post_text, $crawler->filter('html')->text());
}
return $post;
}
/**
* @depends test_create_topic_with_replies
* @dataProvider mcp_view_topic_actions_data
*/
public function test_mcp_view_topic_actions($action, $message, $require_confirmation, $post)
{
$this->add_lang(['common', 'mcp']);
$this->login();
$crawler = self::request('GET', "viewtopic.php?t={$post[0]['topic_id']}&sid={$this->sid}");
$crawler = self::$client->click($crawler->selectLink($this->lang('MCP_SHORT'))->link());
$this->assertLessThanOrEqual(count($post), $crawler->filter('input[type=checkbox]')->count());
// Test actions
$form = $crawler->selectButton($this->lang('SUBMIT'))->form();
// Set posts to select for actions
$post_id_list = [];
switch ($action)
{
case 'lock_post':
case 'unlock_post':
$post_id_list = [$post[1]['post_id'], $post[2]['post_id']];
break;
case 'split_all':
$post_id_list = [$post[13]['post_id'], $post[14]['post_id'], $post[15]['post_id']]; // Split last 3 replies
$subject = '[Split] Topic 1';
break;
case 'split_beyond':
$post_id_list = [$post[10]['post_id']]; // Split from 10th reply
$subject = '[Split] Topic 2';
break;
case 'merge_posts':
$post_id_list = [$post[7]['post_id'], $post[8]['post_id'], $post[9]['post_id']]; // Split replies 7, 8, 9
break;
case 'delete_post':
$post_id_list = [$post[4]['post_id'], $post[5]['post_id'], $post[6]['post_id']]; // Delete posts 4, 5, 6
break;
default:
break;
}
$form->disableValidation()->setValues([
'action' => $action,
'post_id_list' => $post_id_list, // tick post ids
]);
$crawler = self::submit($form);
if ($require_confirmation)
{
if ($action == 'merge_posts')
{
// Merge posts into '[Split] Topic 1'
// Get topics list to select from
$crawler = self::$client->click($crawler->selectLink($this->lang('SELECT_TOPIC'))->link());
// Get '[Split] Topic 1' topic_id
$to_topic_link = $crawler->selectLink('[Split] Topic 1')->attr('href');
$to_topic_id = (int) $this->get_parameter_from_link($to_topic_link, 't');
// Select '[Split] Topic 1'
$select_for_merge_link = $crawler->selectLink($this->lang('SELECT_MERGE'))->reduce(
function ($node, $i) use ($to_topic_id)
{
return (bool) strpos($node->attr('href'), "to_topic_id=$to_topic_id");
}
)->link();
$crawler = self::$client->click($select_for_merge_link);
$this->assertEquals($to_topic_id, (int) $crawler->filter('#to_topic_id')->attr('value'));
// Reselect post ids to move
$form = $crawler->selectButton($this->lang('SUBMIT'))->form()->disableValidation()->setValues(['post_id_list' => $post_id_list]);
$crawler = self::submit($form);
}
if (in_array($action, ['split_all', 'split_beyond']))
{
$form = $crawler->selectButton($this->lang('SUBMIT'))->form()->disableValidation()->setValues([
'subject' => $subject,
'post_id_list' => $post_id_list, // tick post ids
'to_forum_id' => 2,
]);
$crawler = self::submit($form);
}
$form = $crawler->selectButton($this->lang('YES'))->form();
$crawler = self::submit($form);
}
$this->assertStringContainsString($this->lang($message), $crawler->filter('#message p')->text());
}
}

View file

@ -14,7 +14,7 @@
/**
* @group functional
*/
class phpbb_functional_mcp_test extends phpbb_functional_test_case
class phpbb_functional_mcp_quickmod_tools_test extends phpbb_functional_test_case
{
public function test_post_new_topic()
{
@ -46,7 +46,7 @@ class phpbb_functional_mcp_test extends phpbb_functional_test_case
public function test_move_post_to_topic($crawler)
{
$this->login();
$this->add_lang('mcp');
$this->add_lang(['common', 'mcp']);
// Select the post in MCP
$form = $crawler->selectButton($this->lang('SUBMIT'))->form(array(
@ -54,28 +54,10 @@ class phpbb_functional_mcp_test extends phpbb_functional_test_case
));
$form['post_id_list'][0]->tick();
$crawler = self::submit($form);
$this->assertStringContainsString($this->lang('MERGE_POSTS'), $crawler->filter('html')->text());
$this->assertStringContainsString($this->lang('MERGE_POSTS_CONFIRM'), $crawler->filter('html')->text());
$form = $crawler->selectButton('Yes')->form();
$form = $crawler->selectButton($this->lang('YES'))->form();
$crawler = self::submit($form);
$this->assertStringContainsString($this->lang('POSTS_MERGED_SUCCESS'), $crawler->text());
return $crawler;
}
public function test_delete_logs()
{
$this->login();
$crawler = self::request('GET', "mcp.php?i=mcp_logs&mode=front&sid={$this->sid}");
$this->assertGreaterThanOrEqual(1, $crawler->filter('input[type=checkbox]')->count());
$this->add_lang('mcp');
$form = $crawler->selectButton($this->lang('DELETE_ALL'))->form();
$crawler = self::submit($form);
$form = $crawler->selectButton('Yes')->form();
$crawler = self::submit($form);
$this->assertCount(0, $crawler->filter('input[type=checkbox]'));
}
}

View file

@ -0,0 +1,55 @@
<?php
/**
*
* This file is part of the phpBB Forum Software package.
*
* @copyright (c) phpBB Limited <https://www.phpbb.com>
* @license GNU General Public License, version 2 (GPL-2.0)
*
* For full copyright and license information, please see
* the docs/CREDITS.txt file.
*
*/
/**
* @group functional
*/
class phpbb_functional_mcp_test extends phpbb_functional_test_case
{
public function test_all_mcp_module_links()
{
$this->login();
$this->add_lang(['common', 'mcp']);
// Browse MCP main page
$crawler = self::request('GET', 'index.php');
$crawler = self::$client->click($crawler->selectLink($this->lang('MCP_SHORT'))->link());
// Get all MCP module URLs array
$mcp_modules = $crawler->filter('.tabs a')->each(
function ($node, $i)
{
return $node->link();
}
);
// Browse all MCP modules and get their mode URLs array
$mcp_submodules = [];
foreach ($mcp_modules as $module)
{
$crawler = self::$client->click($module);
$mcp_submodules = array_merge($mcp_submodules, $crawler->filter('.cp-menu a')->each(
function ($node, $i)
{
return $node->link();
}
));
}
// Browse all MCP submodules' modes
foreach ($mcp_submodules as $mcp_submodule)
{
self::$client->click($mcp_submodule);
}
}
}

View file

@ -1190,9 +1190,12 @@ class phpbb_functional_test_case extends phpbb_test_case
return null;
}
$url = $crawler->selectLink($form_data['subject'])->link()->getUri();
$post_link = $crawler->filter('.postbody a[title="Post"]')->last()->attr('href');
$topic_link = $crawler->filter('h2[class="topic-title"] > a')->attr('href');
$post_id = $this->get_parameter_from_link($post_link, 'p');
$topic_id = $this->get_parameter_from_link($topic_link, 't');
$topic_id = $this->get_parameter_from_link($url, 't');
if (!$topic_id)
{
$topic_id = $form_data['topic_id'];
@ -1200,7 +1203,7 @@ class phpbb_functional_test_case extends phpbb_test_case
return array(
'topic_id' => $topic_id,
'post_id' => $this->get_parameter_from_link($url, 'p'),
'post_id' => $post_id,
);
}

View file

@ -118,6 +118,6 @@ class mention_helper_test extends phpbb_database_test_case
*/
public function test_get_mentioned_user_ids($incoming_xml, $expected_result)
{
$this->assertSame($expected_result, $this->mention_helper->get_mentioned_user_ids($incoming_xml));
$this->assertEqualsCanonicalizing($expected_result, $this->mention_helper->get_mentioned_user_ids($incoming_xml));
}
}