Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Refactoring tbl_change.php #552

Merged
merged 15 commits into from

2 participants

@scnakandala

Hi Marc,
I have refactored some parts of the tbl_change.php using extract method in this pull request.

@lem9 lem9 was assigned
@lem9
Owner

Supun,
I'm not sure that this is not used. There are some libraries that are using global $analyzed_sql (which is not a good idea but it's the current state of the code base).

But for the moment the tests are fine so it might be acceptable to remove it,

@lem9
Owner

If the UploadDir directive is defined in config.inc.php, and your table contains a BLOB column, this library is used to list the files present in the upload directory, to be able to upload one of them inside the BLOB.

@lem9

You explained only $disp_message and lost the $disp_query question.

the $disp_query is not used in the bellow code. So I thought it is not required to mention about it here.

Owner

OK

@lem9
Owner

Supun, I found a bug:

  • click on Insert for a table
  • click on the Function header: you should stay on the insert page, but remove the Function fields, but a different page is displayed
  • same problem for the Type header
@lem9
Owner

Nice work!

@lem9 lem9 merged commit 52a9edf into phpmyadmin:master

1 check passed

Details default The Travis CI build passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 144 additions and 97 deletions.
  1. +132 −1 libraries/insert_edit.lib.php
  2. +12 −96 tbl_change.php
View
133 libraries/insert_edit.lib.php
@@ -2365,11 +2365,142 @@ function PMA_verifyWhetherValueCanBeTruncatedAndAppendExtraData(
. ' WHERE ' . $_REQUEST['where_clause'][0];
if ($GLOBALS['dbi']->fetchValue($sql_for_real_value) !== false) {
- $extra_data['truncatableFieldValue'] = $GLOBALS['dbi']->fetchValue($sql_for_real_value);
+ $extra_data['truncatableFieldValue']
+ = $GLOBALS['dbi']->fetchValue($sql_for_real_value);
} else {
$extra_data['isNeedToRecheck'] = false;
}
}
+/**
+ * Function to get the fields of a table
+ *
+ * @param string $db current db
+ * @param string $table current table
+ *
+ * @return array
+ */
+function PMA_getTableFields($db, $table)
+{
+ $GLOBALS['dbi']->selectDb($db);
+ return array_values($GLOBALS['dbi']->getColumns($db, $table));
+
+}
+
+/**
+ * Function to determine Insert/Edit rows
+ *
+ * @param string $where_clause where clause
+ * @param string $db current database
+ * @param string $table current table
+ *
+ * @return mixed
+ */
+function PMA_determineInsertOrEdit($where_clause, $db, $table)
+{
+ if (isset($_REQUEST['where_clause'])) {
+ $where_clause = $_REQUEST['where_clause'];
+ }
+ if (isset($_SESSION['edit_next'])) {
+ $where_clause = $_SESSION['edit_next'];
+ unset($_SESSION['edit_next']);
+ $after_insert = 'edit_next';
+ }
+ if (isset($_REQUEST['ShowFunctionFields'])) {
+ $GLOBALS['cfg']['ShowFunctionFields'] = $_REQUEST['ShowFunctionFields'];
+ }
+ if (isset($_REQUEST['ShowFieldTypesInDataEditView'])) {
+ $GLOBALS['cfg']['ShowFieldTypesInDataEditView']
+ = $_REQUEST['ShowFieldTypesInDataEditView'];
+ }
+ if (isset($_REQUEST['after_insert'])) {
+ $after_insert = $_REQUEST['after_insert'];
+ }
+
+ if (isset($where_clause)) {
+ // we are editing
+ $insert_mode = false;
+ $where_clause_array = PMA_getWhereClauseArray($where_clause);
+ list($where_clauses, $result, $rows, $found_unique_key)
+ = PMA_analyzeWhereClauses(
+ $where_clause_array, $table, $db
+ );
+ } else {
+ // we are inserting
+ $insert_mode = true;
+ $where_clause = null;
+ list($result, $rows) = PMA_loadFirstRow($table, $db);
+ $where_clauses = null;
+ $where_clause_array = null;
+ $found_unique_key = false;
+ }
+
+ // Copying a row - fetched data will be inserted as a new row,
+ // therefore the where clause is needless.
+ if (isset($_REQUEST['default_action'])
+ && $_REQUEST['default_action'] === 'insert'
+ ) {
+ $where_clause = $where_clauses = null;
+ }
+
+ return array(
+ $insert_mode, $where_clause, $where_clause_array, $where_clauses,
+ $result, $rows, $found_unique_key,
+ isset($after_insert) ? $after_insert : null
+ );
+}
+
+/**
+ * Function to get comments for the table columns
+ *
+ * @param string $db current database
+ * @param string $table current table
+ *
+ * @return array $comments_map comments for columns
+ */
+function PMA_getCommentsMap($db, $table)
+{
+ /**
+ * get table information
+ * @todo should be done by a Table object
+ */
+ include 'libraries/tbl_info.inc.php';
+
+ /**
+ * Get comments for table fileds/columns
+ */
+ $comments_map = array();
+
+ if ($GLOBALS['cfg']['ShowPropertyComments']) {
+ $comments_map = PMA_getComments($db, $table);
+ }
+
+ return $comments_map;
+}
+
+/**
+ * Function to get URL parameters
+ *
+ * @param string $db current database
+ * @param string $table current table
+ *
+ * @return array $url_params url parameters
+ */
+function PMA_getUrlParameters($db, $table)
+{
+ /**
+ * @todo check if we could replace by "db_|tbl_" - please clarify!?
+ */
+ $url_params = array(
+ 'db' => $db,
+ 'sql_query' => $_REQUEST['sql_query']
+ );
+
+ if (preg_match('@^tbl_@', $GLOBALS['goto'])) {
+ $url_params['table'] = $table;
+ }
+
+ return $url_params;
+}
?>
View
108 tbl_change.php
@@ -24,34 +24,18 @@
require_once 'libraries/insert_edit.lib.php';
/**
- * Sets global variables.
- * Here it's better to use a if, instead of the '?' operator
- * to avoid setting a variable to '' when it's not present in $_REQUEST
+ * Determine whether Insert or Edit and set global variables
*/
+list(
+ $insert_mode, $where_clause, $where_clause_array, $where_clauses,
+ $result, $rows, $found_unique_key, $after_insert
+) = PMA_determineInsertOrEdit($where_clause, $db, $table);
-if (isset($_REQUEST['where_clause'])) {
- $where_clause = $_REQUEST['where_clause'];
-}
-if (isset($_SESSION['edit_next'])) {
- $where_clause = $_SESSION['edit_next'];
- unset($_SESSION['edit_next']);
- $after_insert = 'edit_next';
-}
-if (isset($_REQUEST['ShowFunctionFields'])) {
- $cfg['ShowFunctionFields'] = $_REQUEST['ShowFunctionFields'];
-}
-if (isset($_REQUEST['ShowFieldTypesInDataEditView'])) {
- $cfg['ShowFieldTypesInDataEditView'] = $_REQUEST['ShowFieldTypesInDataEditView'];
-}
-if (isset($_REQUEST['after_insert'])) {
- $after_insert = $_REQUEST['after_insert'];
-}
/**
* file listing
- */
+*/
require_once 'libraries/file_listing.lib.php';
-
/**
* Defines the url to return to in case of error in a sql statement
* (at this point, $GLOBALS['goto'] will be set but could be empty)
@@ -64,43 +48,13 @@
$GLOBALS['goto'] = 'db_sql.php';
}
}
-/**
- * @todo check if we could replace by "db_|tbl_" - please clarify!?
- */
-$_url_params = array(
- 'db' => $db,
- 'sql_query' => $_REQUEST['sql_query']
-);
-if (preg_match('@^tbl_@', $GLOBALS['goto'])) {
- $_url_params['table'] = $table;
-}
+$_url_params = PMA_getUrlParameters($db, $table);
$err_url = $GLOBALS['goto'] . PMA_generate_common_url($_url_params);
unset($_url_params);
-
-/**
- * Sets parameters for links
- * where is this variable used?
- * replace by PMA_generate_common_url($url_params);
- */
-$url_query = PMA_generate_common_url($url_params, 'html', '');
-
-/**
- * get table information
- * @todo should be done by a Table object
- */
-require_once 'libraries/tbl_info.inc.php';
-
-/**
- * Get comments for table fileds/columns
- */
-$comments_map = array();
-
-if ($GLOBALS['cfg']['ShowPropertyComments']) {
- $comments_map = PMA_getComments($db, $table);
-}
+$comments_map = PMA_getCommentsMap($db, $table);
/**
* START REGULAR OUTPUT
@@ -120,52 +74,14 @@
/**
* Displays the query submitted and its result
*
- * @todo where does $disp_message and $disp_query come from???
+ * $disp_message come from tbl_replace.php
*/
if (! empty($disp_message)) {
- if (! isset($disp_query)) {
- $disp_query = null;
- }
- $response->addHTML(PMA_Util::getMessage($disp_message, $disp_query));
+ $response->addHTML(PMA_Util::getMessage($disp_message, null));
}
-/**
- * Get the analysis of SHOW CREATE TABLE for this table
- */
-$analyzed_sql = PMA_Table::analyzeStructure($db, $table);
-/**
- * Get the list of the fields of the current table
- */
-$GLOBALS['dbi']->selectDb($db);
-$table_fields = array_values($GLOBALS['dbi']->getColumns($db, $table));
-
-$paramTableDbArray = array($table, $db);
-
-/**
- * Determine what to do, edit or insert?
- */
-if (isset($where_clause)) {
- // we are editing
- $insert_mode = false;
- $where_clause_array = PMA_getWhereClauseArray($where_clause);
- list($where_clauses, $result, $rows, $found_unique_key)
- = PMA_analyzeWhereClauses($where_clause_array, $table, $db);
-} else {
- // we are inserting
- $insert_mode = true;
- $where_clause = null;
- list($result, $rows) = PMA_loadFirstRow($table, $db);
- $where_clauses = null;
- $where_clause_array = null;
- $found_unique_key = false;
-}
-
-// Copying a row - fetched data will be inserted as a new row,
-// therefore the where clause is needless.
-if (isset($_REQUEST['default_action']) && $_REQUEST['default_action'] === 'insert') {
- $where_clause = $where_clauses = null;
-}
+$table_fields = PMA_getTableFields($db, $table);
// retrieve keys into foreign fields, if any
$foreigners = PMA_getForeigners($db, $table);
@@ -387,7 +303,7 @@
$html_output .= PMA_getValueColumn(
$column, $backup_field, $column_name_appendix, $unnullify_trigger,
$tabindex, $tabindex_for_value, $idindex, $data, $special_chars,
- $foreignData, $odd_row, $paramTableDbArray, $rownumber_param, $titles,
+ $foreignData, $odd_row, array($table, $db), $rownumber_param, $titles,
$text_dir, $special_chars_encoded, $vkey, $is_upload,
$biggest_max_file_size, $default_char_editing,
$no_support_types, $gis_data_types, $extracted_columnspec
Something went wrong with that request. Please try again.