From 7b86a5bc6008653b7ecd0ae091089bc524ab36f4 Mon Sep 17 00:00:00 2001 From: Cesar G Date: Thu, 5 Dec 2013 22:31:27 -0800 Subject: [PATCH 1/9] [ticket/12038] Check that the move action succeeded before moving the rows. PHPBB3-12038 --- phpBB/adm/style/ajax.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/phpBB/adm/style/ajax.js b/phpBB/adm/style/ajax.js index 78fcbd88fd..959580d6c2 100644 --- a/phpBB/adm/style/ajax.js +++ b/phpBB/adm/style/ajax.js @@ -8,7 +8,11 @@ * an item is moved up. It moves the row up or down, and deactivates / * activates any up / down icons that require it (the ones at the top or bottom). */ -phpbb.addAjaxCallback('row_down', function() { +phpbb.addAjaxCallback('row_down', function(res) { + if (typeof res.success === 'undefined' || !res.success) { + return; + } + var el = $(this), tr = el.parents('tr'), trSwap = tr.next(); @@ -16,7 +20,11 @@ phpbb.addAjaxCallback('row_down', function() { tr.insertAfter(trSwap); }); -phpbb.addAjaxCallback('row_up', function() { +phpbb.addAjaxCallback('row_up', function(res) { + if (typeof res.success === 'undefined' || !res.success) { + return; + } + var el = $(this), tr = el.parents('tr'), trSwap = tr.prev(); From 8d9cc63c19325f4782e7de8f72abd38b7af2657d Mon Sep 17 00:00:00 2001 From: Cesar G Date: Thu, 5 Dec 2013 22:33:03 -0800 Subject: [PATCH 2/9] [ticket/12038] AJAXify move up/down buttons in the smilies/topic icons page. PHPBB3-12038 --- phpBB/adm/style/acp_icons.html | 10 ++++++---- phpBB/includes/acp/acp_icons.php | 9 +++++++++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/phpBB/adm/style/acp_icons.html b/phpBB/adm/style/acp_icons.html index bfe4878ba5..b2676c54ef 100644 --- a/phpBB/adm/style/acp_icons.html +++ b/phpBB/adm/style/acp_icons.html @@ -242,10 +242,12 @@ {items.CODE} {items.EMOTION} - - {ICON_MOVE_UP_DISABLED}{ICON_MOVE_UP}  - {ICON_MOVE_DOWN_DISABLED}{ICON_MOVE_DOWN} -  {ICON_EDIT} {ICON_DELETE} + + + {ICON_MOVE_UP} + + {ICON_MOVE_DOWN} + {ICON_EDIT} {ICON_DELETE} diff --git a/phpBB/includes/acp/acp_icons.php b/phpBB/includes/acp/acp_icons.php index 658be4cc6b..2d90d42992 100644 --- a/phpBB/includes/acp/acp_icons.php +++ b/phpBB/includes/acp/acp_icons.php @@ -832,6 +832,7 @@ class acp_icons WHERE {$fields}_order = $switch_order_id AND {$fields}_id <> $icon_id"; $db->sql_query($sql); + $move_executed = (bool) $db->sql_affectedrows(); // Only update the other entry too if the previous entry got updated if ($db->sql_affectedrows()) @@ -846,6 +847,14 @@ class acp_icons $cache->destroy('_icons'); $cache->destroy('sql', $table); + if ($request->is_ajax()) + { + $json_response = new \phpbb\json_response; + $json_response->send(array( + 'success' => $move_executed, + )); + } + break; } From d399d255b69ad02e7b8d5f043637ffe948bae7e2 Mon Sep 17 00:00:00 2001 From: Cesar G Date: Thu, 5 Dec 2013 22:35:06 -0800 Subject: [PATCH 3/9] [ticket/12038] AJAXify move up/down buttons in the module management pages. PHPBB3-12038 --- phpBB/adm/style/acp_modules.html | 4 ++-- phpBB/includes/acp/acp_modules.php | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/phpBB/adm/style/acp_modules.html b/phpBB/adm/style/acp_modules.html index 5024a950b9..c7688a610c 100644 --- a/phpBB/adm/style/acp_modules.html +++ b/phpBB/adm/style/acp_modules.html @@ -151,9 +151,9 @@  {L_DISABLE}{L_ENABLE}  - {ICON_MOVE_UP} + {ICON_MOVE_UP} - {ICON_MOVE_DOWN} + {ICON_MOVE_DOWN} {ICON_EDIT} {ICON_DELETE} diff --git a/phpBB/includes/acp/acp_modules.php b/phpBB/includes/acp/acp_modules.php index 100e33044b..c124377ba9 100644 --- a/phpBB/includes/acp/acp_modules.php +++ b/phpBB/includes/acp/acp_modules.php @@ -170,6 +170,14 @@ class acp_modules $this->remove_cache_file(); } + if ($request->is_ajax()) + { + $json_response = new \phpbb\json_response; + $json_response->send(array( + 'success' => ($move_module_name !== false), + )); + } + break; case 'quickadd': From 97558e5fd489f1cb4d4b7bae025842ff467d97d9 Mon Sep 17 00:00:00 2001 From: Cesar G Date: Thu, 5 Dec 2013 22:35:47 -0800 Subject: [PATCH 4/9] [ticket/12038] AJAXify move up/down buttons in the permission roles page. PHPBB3-12038 --- phpBB/adm/style/acp_permission_roles.html | 19 +++++-------------- phpBB/includes/acp/acp_permission_roles.php | 9 +++++++++ 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/phpBB/adm/style/acp_permission_roles.html b/phpBB/adm/style/acp_permission_roles.html index b8fdaeb837..b3137f134c 100644 --- a/phpBB/adm/style/acp_permission_roles.html +++ b/phpBB/adm/style/acp_permission_roles.html @@ -157,20 +157,11 @@
{roles.ROLE_DESCRIPTION} {L_VIEW_ASSIGNED_ITEMS}{L_VIEW_ASSIGNED_ITEMS} - - - {ICON_MOVE_UP_DISABLED} - {ICON_MOVE_DOWN} - - {ICON_MOVE_UP} - {ICON_MOVE_DOWN} - - {ICON_MOVE_UP} - {ICON_MOVE_DOWN_DISABLED} - - {ICON_MOVE_UP_DISABLED} - {ICON_MOVE_DOWN_DISABLED} - + + + {ICON_MOVE_UP} + + {ICON_MOVE_DOWN} {ICON_EDIT} {ICON_DELETE} diff --git a/phpBB/includes/acp/acp_permission_roles.php b/phpBB/includes/acp/acp_permission_roles.php index 17e48d6576..21729df4f1 100644 --- a/phpBB/includes/acp/acp_permission_roles.php +++ b/phpBB/includes/acp/acp_permission_roles.php @@ -27,6 +27,7 @@ class acp_permission_roles { global $db, $user, $auth, $template, $cache, $phpbb_container; global $config, $phpbb_root_path, $phpbb_admin_path, $phpEx; + global $request; include_once($phpbb_root_path . 'includes/functions_user.' . $phpEx); include_once($phpbb_root_path . 'includes/acp/auth.' . $phpEx); @@ -375,6 +376,14 @@ class acp_permission_roles AND role_order IN ($order, " . (($action == 'move_up') ? $order - 1 : $order + 1) . ')'; $db->sql_query($sql); + if ($request->is_ajax()) + { + $json_response = new \phpbb\json_response; + $json_response->send(array( + 'success' => (bool) $db->sql_affectedrows(), + )); + } + break; } From 58a07647595a617c7c0c73b4dbde3b5858b29227 Mon Sep 17 00:00:00 2001 From: Cesar G Date: Thu, 5 Dec 2013 22:36:21 -0800 Subject: [PATCH 5/9] [ticket/12038] AJAXify move up/down buttons in custom profile fields page. PHPBB3-12038 --- phpBB/adm/style/acp_profile.html | 16 +++++----------- phpBB/includes/acp/acp_profile.php | 8 ++++++++ 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/phpBB/adm/style/acp_profile.html b/phpBB/adm/style/acp_profile.html index 3546873f6c..25120fe71c 100644 --- a/phpBB/adm/style/acp_profile.html +++ b/phpBB/adm/style/acp_profile.html @@ -201,17 +201,11 @@ {fields.FIELD_TYPE} {fields.L_ACTIVATE_DEACTIVATE} | {L_TRANSLATE} - - - {ICON_MOVE_UP_DISABLED} - {ICON_MOVE_DOWN} - - {ICON_MOVE_UP} - {ICON_MOVE_DOWN} - - {ICON_MOVE_UP} - {ICON_MOVE_DOWN_DISABLED} - + + + {ICON_MOVE_UP} + + {ICON_MOVE_DOWN} {ICON_EDIT} diff --git a/phpBB/includes/acp/acp_profile.php b/phpBB/includes/acp/acp_profile.php index 4e8145009f..570bde1ac7 100644 --- a/phpBB/includes/acp/acp_profile.php +++ b/phpBB/includes/acp/acp_profile.php @@ -299,6 +299,14 @@ class acp_profile WHERE field_order IN ($field_order, " . (($action == 'move_up') ? $field_order - 1 : $field_order + 1) . ')'; $db->sql_query($sql); + if ($request->is_ajax()) + { + $json_response = new \phpbb\json_response; + $json_response->send(array( + 'success' => (bool) $db->sql_affectedrows(), + )); + } + break; case 'create': From 9e8ee56404c40cfb0ffbc435a83aa726dc41a8c0 Mon Sep 17 00:00:00 2001 From: Cesar G Date: Thu, 5 Dec 2013 22:36:55 -0800 Subject: [PATCH 6/9] [ticket/12038] AJAXify move up/down buttons in report/denial reasons page. PHPBB3-12038 --- phpBB/adm/style/acp_reasons.html | 16 +++++----------- phpBB/includes/acp/acp_reasons.php | 8 ++++++++ 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/phpBB/adm/style/acp_reasons.html b/phpBB/adm/style/acp_reasons.html index ca3aebb338..d629a9553f 100644 --- a/phpBB/adm/style/acp_reasons.html +++ b/phpBB/adm/style/acp_reasons.html @@ -86,17 +86,11 @@
{reasons.REASON_DESCRIPTION} {reasons.REASON_COUNT} - - - {ICON_MOVE_UP_DISABLED} - {ICON_MOVE_DOWN} - - {ICON_MOVE_UP} - {ICON_MOVE_DOWN} - - {ICON_MOVE_UP} - {ICON_MOVE_DOWN_DISABLED} - + + + {ICON_MOVE_UP} + + {ICON_MOVE_DOWN} {ICON_EDIT} {ICON_DELETE} diff --git a/phpBB/includes/acp/acp_reasons.php b/phpBB/includes/acp/acp_reasons.php index 71e9108c2c..26599571d2 100644 --- a/phpBB/includes/acp/acp_reasons.php +++ b/phpBB/includes/acp/acp_reasons.php @@ -26,6 +26,7 @@ class acp_reasons { global $db, $user, $auth, $template, $cache; global $config, $phpbb_root_path, $phpbb_admin_path, $phpEx; + global $request; $user->add_lang(array('mcp', 'acp/posting')); @@ -288,6 +289,13 @@ class acp_reasons WHERE reason_order IN (' . $order . ', ' . (($action == 'move_up') ? $order - 1 : $order + 1) . ')'; $db->sql_query($sql); + if ($request->is_ajax()) + { + $json_response = new \phpbb\json_response; + $json_response->send(array( + 'success' => (bool) $db->sql_affectedrows(), + )); + } break; } From 823d2b697a9bcec96f4ef841a77bfe900ce530f4 Mon Sep 17 00:00:00 2001 From: Cesar G Date: Fri, 6 Dec 2013 00:09:36 -0800 Subject: [PATCH 7/9] [ticket/12038] Use $move_executed in place of $db->sql_affectedrows(). PHPBB3-12038 --- phpBB/includes/acp/acp_icons.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/includes/acp/acp_icons.php b/phpBB/includes/acp/acp_icons.php index 2d90d42992..327b510299 100644 --- a/phpBB/includes/acp/acp_icons.php +++ b/phpBB/includes/acp/acp_icons.php @@ -835,7 +835,7 @@ class acp_icons $move_executed = (bool) $db->sql_affectedrows(); // Only update the other entry too if the previous entry got updated - if ($db->sql_affectedrows()) + if ($move_executed) { $sql = "UPDATE $table SET {$fields}_order = $switch_order_id From 3ccc8add10b4a6d915d3edbb3075351301277aab Mon Sep 17 00:00:00 2001 From: Cesar G Date: Fri, 6 Dec 2013 12:50:16 -0800 Subject: [PATCH 8/9] [ticket/12038] Do not rely on stale order value to move items. This makes it possible to move the items more than once with AJAX. PHPBB3-12038 --- phpBB/includes/acp/acp_permission_roles.php | 37 ++++++++-------- phpBB/includes/acp/acp_profile.php | 48 +++++++++------------ phpBB/includes/acp/acp_reasons.php | 16 +++++-- 3 files changed, 53 insertions(+), 48 deletions(-) diff --git a/phpBB/includes/acp/acp_permission_roles.php b/phpBB/includes/acp/acp_permission_roles.php index 21729df4f1..b5b838faad 100644 --- a/phpBB/includes/acp/acp_permission_roles.php +++ b/phpBB/includes/acp/acp_permission_roles.php @@ -47,6 +47,11 @@ class acp_permission_roles $form_name = 'acp_permissions'; add_form_key($form_name); + if (!$role_id && in_array($action, array('remove', 'edit', 'move_up', 'move_down'))) + { + trigger_error($user->lang['NO_ROLE_SELECTED'] . adm_back_link($this->u_action), E_USER_WARNING); + } + switch ($mode) { case 'admin_roles': @@ -86,11 +91,6 @@ class acp_permission_roles { case 'remove': - if (!$role_id) - { - trigger_error($user->lang['NO_ROLE_SELECTED'] . adm_back_link($this->u_action), E_USER_WARNING); - } - $sql = 'SELECT * FROM ' . ACL_ROLES_TABLE . ' WHERE role_id = ' . $role_id; @@ -124,10 +124,6 @@ class acp_permission_roles break; case 'edit': - if (!$role_id) - { - trigger_error($user->lang['NO_ROLE_SELECTED'] . adm_back_link($this->u_action), E_USER_WARNING); - } // Get role we edit $sql = 'SELECT * @@ -274,12 +270,7 @@ class acp_permission_roles case 'edit': if ($action == 'edit') - { - if (!$role_id) - { - trigger_error($user->lang['NO_ROLE_SELECTED'] . adm_back_link($this->u_action), E_USER_WARNING); - } - + { $sql = 'SELECT * FROM ' . ACL_ROLES_TABLE . ' WHERE role_id = ' . $role_id; @@ -367,7 +358,17 @@ class acp_permission_roles case 'move_up': case 'move_down': - $order = request_var('order', 0); + $sql = 'SELECT role_order + FROM ' . ACL_ROLES_TABLE . " + WHERE role_id = $role_id"; + $result = $db->sql_query($sql); + $order = $db->sql_fetchfield('role_order'); + + if ($order === false || ($order == 0 && $action == 'move_up')) + { + break; + } + $order = (int) $order; $order_total = $order * 2 + (($action == 'move_up') ? -1 : 1); $sql = 'UPDATE ' . ACL_ROLES_TABLE . ' @@ -430,8 +431,8 @@ class acp_permission_roles 'U_EDIT' => $this->u_action . '&action=edit&role_id=' . $row['role_id'], 'U_REMOVE' => $this->u_action . '&action=remove&role_id=' . $row['role_id'], - 'U_MOVE_UP' => $this->u_action . '&action=move_up&order=' . $row['role_order'], - 'U_MOVE_DOWN' => $this->u_action . '&action=move_down&order=' . $row['role_order'], + 'U_MOVE_UP' => $this->u_action . '&action=move_up&role_id=' . $row['role_id'], + 'U_MOVE_DOWN' => $this->u_action . '&action=move_down&role_id=' . $row['role_id'], 'U_DISPLAY_ITEMS' => ($row['role_id'] == $display_item) ? '' : $this->u_action . '&display_item=' . $row['role_id'] . '#assigned_to') ); diff --git a/phpBB/includes/acp/acp_profile.php b/phpBB/includes/acp/acp_profile.php index 570bde1ac7..6efd778b12 100644 --- a/phpBB/includes/acp/acp_profile.php +++ b/phpBB/includes/acp/acp_profile.php @@ -39,11 +39,17 @@ class acp_profile $this->tpl_name = 'acp_profile'; $this->page_title = 'ACP_CUSTOM_PROFILE_FIELDS'; + $field_id = $request->variable('field_id', 0); $action = (isset($_POST['create'])) ? 'create' : request_var('action', ''); $error = array(); $s_hidden_fields = ''; + if (!$field_id && in_array($action, array('delete','activate', 'deactivate', 'move_up', 'move_down', 'edit'))) + { + trigger_error($user->lang['NO_FIELD_ID'] . adm_back_link($this->u_action), E_USER_WARNING); + } + // Define some default values for each field type $default_values = array( FIELD_STRING => array('field_length' => 10, 'field_minlen' => 0, 'field_maxlen' => 20, 'field_validation' => '.*', 'field_novalue' => '', 'field_default_value' => ''), @@ -98,12 +104,6 @@ class acp_profile switch ($action) { case 'delete': - $field_id = request_var('field_id', 0); - - if (!$field_id) - { - trigger_error($user->lang['NO_FIELD_ID'] . adm_back_link($this->u_action), E_USER_WARNING); - } if (confirm_box(true)) { @@ -210,12 +210,6 @@ class acp_profile break; case 'activate': - $field_id = request_var('field_id', 0); - - if (!$field_id) - { - trigger_error($user->lang['NO_FIELD_ID'] . adm_back_link($this->u_action), E_USER_WARNING); - } $sql = 'SELECT lang_id FROM ' . LANG_TABLE . " @@ -256,12 +250,6 @@ class acp_profile break; case 'deactivate': - $field_id = request_var('field_id', 0); - - if (!$field_id) - { - trigger_error($user->lang['NO_FIELD_ID'] . adm_back_link($this->u_action), E_USER_WARNING); - } $sql = 'UPDATE ' . PROFILE_FIELDS_TABLE . " SET field_active = 0 @@ -291,7 +279,19 @@ class acp_profile case 'move_up': case 'move_down': - $field_order = request_var('order', 0); + + $sql = 'SELECT field_order + FROM ' . PROFILE_FIELDS_TABLE . " + WHERE field_id = $field_id"; + $result = $db->sql_query($sql); + $field_order = $db->sql_fetchfield('field_order'); + $db->sql_freeresult($result); + + if ($field_order === false || ($field_order == 0 && $action == 'move_up')) + { + break; + } + $field_order = (int) $field_order; $order_total = $field_order * 2 + (($action == 'move_up') ? -1 : 1); $sql = 'UPDATE ' . PROFILE_FIELDS_TABLE . " @@ -312,7 +312,6 @@ class acp_profile case 'create': case 'edit': - $field_id = request_var('field_id', 0); $step = request_var('step', 1); $submit = (isset($_REQUEST['next']) || isset($_REQUEST['prev'])) ? true : false; @@ -324,11 +323,6 @@ class acp_profile // We are editing... we need to grab basic things if ($action == 'edit') { - if (!$field_id) - { - trigger_error($user->lang['NO_FIELD_ID'] . adm_back_link($this->u_action), E_USER_WARNING); - } - $sql = 'SELECT l.*, f.* FROM ' . PROFILE_LANG_TABLE . ' l, ' . PROFILE_FIELDS_TABLE . ' f WHERE l.lang_id = ' . $this->edit_lang_id . " @@ -927,8 +921,8 @@ class acp_profile 'U_EDIT' => $this->u_action . "&action=edit&field_id=$id", 'U_TRANSLATE' => $this->u_action . "&action=edit&field_id=$id&step=3", 'U_DELETE' => $this->u_action . "&action=delete&field_id=$id", - 'U_MOVE_UP' => $this->u_action . "&action=move_up&order={$row['field_order']}", - 'U_MOVE_DOWN' => $this->u_action . "&action=move_down&order={$row['field_order']}", + 'U_MOVE_UP' => $this->u_action . "&action=move_up&field_id=$id", + 'U_MOVE_DOWN' => $this->u_action . "&action=move_down&field_id=$id", 'S_NEED_EDIT' => $s_need_edit) ); diff --git a/phpBB/includes/acp/acp_reasons.php b/phpBB/includes/acp/acp_reasons.php index 26599571d2..26ff5aa0a4 100644 --- a/phpBB/includes/acp/acp_reasons.php +++ b/phpBB/includes/acp/acp_reasons.php @@ -281,7 +281,17 @@ class acp_reasons case 'move_up': case 'move_down': - $order = request_var('order', 0); + $sql = 'SELECT reason_order + FROM ' . REPORTS_REASONS_TABLE . " + WHERE reason_id = $reason_id"; + $result = $db->sql_query($sql); + $order = $db->sql_fetchfield('reason_order'); + + if ($order === false || ($order == 0 && $action == 'move_up')) + { + break; + } + $order = (int) $order; $order_total = $order * 2 + (($action == 'move_up') ? -1 : 1); $sql = 'UPDATE ' . REPORTS_REASONS_TABLE . ' @@ -371,8 +381,8 @@ class acp_reasons 'U_EDIT' => $this->u_action . '&action=edit&id=' . $row['reason_id'], 'U_DELETE' => (!$other_reason) ? $this->u_action . '&action=delete&id=' . $row['reason_id'] : '', - 'U_MOVE_UP' => $this->u_action . '&action=move_up&order=' . $row['reason_order'], - 'U_MOVE_DOWN' => $this->u_action . '&action=move_down&order=' . $row['reason_order']) + 'U_MOVE_UP' => $this->u_action . '&action=move_up&id=' . $row['reason_id'], + 'U_MOVE_DOWN' => $this->u_action . '&action=move_down&id=' . $row['reason_id']) ); } $db->sql_freeresult($result); From 06262aca541691fd28f02d9b6983b57429f4a671 Mon Sep 17 00:00:00 2001 From: Cesar G Date: Mon, 30 Dec 2013 11:08:02 -0800 Subject: [PATCH 9/9] [ticket/12038] Free query results. PHPBB3-12038 --- phpBB/includes/acp/acp_permission_roles.php | 1 + phpBB/includes/acp/acp_reasons.php | 1 + 2 files changed, 2 insertions(+) diff --git a/phpBB/includes/acp/acp_permission_roles.php b/phpBB/includes/acp/acp_permission_roles.php index b5b838faad..aca45575d3 100644 --- a/phpBB/includes/acp/acp_permission_roles.php +++ b/phpBB/includes/acp/acp_permission_roles.php @@ -363,6 +363,7 @@ class acp_permission_roles WHERE role_id = $role_id"; $result = $db->sql_query($sql); $order = $db->sql_fetchfield('role_order'); + $db->sql_freeresult($result); if ($order === false || ($order == 0 && $action == 'move_up')) { diff --git a/phpBB/includes/acp/acp_reasons.php b/phpBB/includes/acp/acp_reasons.php index 26ff5aa0a4..569bb73ab0 100644 --- a/phpBB/includes/acp/acp_reasons.php +++ b/phpBB/includes/acp/acp_reasons.php @@ -286,6 +286,7 @@ class acp_reasons WHERE reason_id = $reason_id"; $result = $db->sql_query($sql); $order = $db->sql_fetchfield('reason_order'); + $db->sql_freeresult($result); if ($order === false || ($order == 0 && $action == 'move_up')) {