From 4f9628ed436de53b1ab1a06b123755a86a8f2288 Mon Sep 17 00:00:00 2001 From: Surinder Kumar Date: Fri, 12 May 2017 10:53:57 +0100 Subject: [PATCH] Improve handling of nulls and default values in the data editor. Fixes #2257 --- .../templates/column/sql/9.2_plus/nodes.sql | 3 +- .../templates/column/sql/default/nodes.sql | 3 +- web/pgadmin/static/css/pgadmin.css | 2 +- .../js/slickgrid/slick.pgadmin.editors.js | 39 ++++++++++++++-- .../js/slickgrid/slick.pgadmin.formatters.js | 46 ++++++++++++++----- web/pgadmin/tools/sqleditor/__init__.py | 25 +++++++++- web/pgadmin/tools/sqleditor/command.py | 30 +++++++++++- .../tools/sqleditor/static/css/sqleditor.css | 12 ++++- .../templates/sqleditor/js/sqleditor.js | 28 ++++++++++- 9 files changed, 163 insertions(+), 25 deletions(-) diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/column/sql/9.2_plus/nodes.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/column/sql/9.2_plus/nodes.sql index 759e65764..f3353d642 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/column/sql/9.2_plus/nodes.sql +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/column/sql/9.2_plus/nodes.sql @@ -1,4 +1,5 @@ -SELECT att.attname as name, att.attnum as OID, format_type(ty.oid,NULL) AS datatype +SELECT att.attname as name, att.attnum as OID, format_type(ty.oid,NULL) AS datatype, +att.attnotnull as not_null, att.atthasdef as has_default_val FROM pg_attribute att JOIN pg_type ty ON ty.oid=atttypid JOIN pg_namespace tn ON tn.oid=ty.typnamespace diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/column/sql/default/nodes.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/column/sql/default/nodes.sql index 7536a9cf1..4f1de2ade 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/column/sql/default/nodes.sql +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/column/sql/default/nodes.sql @@ -1,4 +1,5 @@ -SELECT att.attname as name, att.attnum as OID, format_type(ty.oid,NULL) AS datatype +SELECT att.attname as name, att.attnum as OID, format_type(ty.oid,NULL) AS datatype, +att.attnotnull as not_null, att.atthasdef as has_default_val FROM pg_attribute att JOIN pg_type ty ON ty.oid=atttypid JOIN pg_namespace tn ON tn.oid=ty.typnamespace diff --git a/web/pgadmin/static/css/pgadmin.css b/web/pgadmin/static/css/pgadmin.css index 6508feb35..8422983cf 100644 --- a/web/pgadmin/static/css/pgadmin.css +++ b/web/pgadmin/static/css/pgadmin.css @@ -780,4 +780,4 @@ lgg-el-container[el=md] .pg-el-lg-8, } .user-language select{ height: 25px !important; -} \ No newline at end of file +} diff --git a/web/pgadmin/static/js/slickgrid/slick.pgadmin.editors.js b/web/pgadmin/static/js/slickgrid/slick.pgadmin.editors.js index cdfba4d25..faad73126 100644 --- a/web/pgadmin/static/js/slickgrid/slick.pgadmin.editors.js +++ b/web/pgadmin/static/js/slickgrid/slick.pgadmin.editors.js @@ -110,7 +110,12 @@ // When text editor opens this.loadValue = function (item) { - if (item[args.column.pos] === "") { + var col = args.column; + + if (_.isUndefined(item[args.column.pos]) && col.has_default_val) { + $input.val(""); + } + else if (item[args.column.pos] === "") { $input.val("''"); } else { @@ -145,7 +150,14 @@ }; this.isValueChanged = function () { - return (!($input.val() == "" && defaultValue == null)) && ($input.val() != defaultValue); + // Use _.isNull(value) for comparison for null instead of + // defaultValue == null, because it returns true for undefined value. + if ($input.val() == "" && _.isUndefined(defaultValue)) { + return false; + } else { + return (!($input.val() == "" && _.isNull(defaultValue))) && + ($input.val() != defaultValue); + } }; this.validate = function () { @@ -253,7 +265,7 @@ this.loadValue = function (item) { var data = defaultValue = item[args.column.pos]; - if (typeof data === "object" && !Array.isArray(data)) { + if (data && typeof data === "object" && !Array.isArray(data)) { data = JSON.stringify(data); } else if (Array.isArray(data)) { var temp = []; @@ -282,7 +294,11 @@ }; this.isValueChanged = function () { - return (!($input.val() == "" && defaultValue == null)) && ($input.val() != defaultValue); + if ($input.val() == "" && _.isUndefined(defaultValue)) { + return false; + } else { + return (!($input.val() == "" && _.isNull(defaultValue))) && ($input.val() != defaultValue); + } }; this.validate = function () { @@ -498,6 +514,12 @@ }; this.validate = function () { + if (args.column.validator) { + var validationResults = args.column.validator(this.serializeValue()); + if (!validationResults.valid) { + return validationResults; + } + } return { valid: true, msg: null @@ -837,7 +859,14 @@ }; this.isValueChanged = function () { - return (!($input.val() == "" && defaultValue == null)) && ($input.val() != defaultValue); + if ($input.val() == "" && _.isUndefined(defaultValue)) { + return false; + } else if ($input.val() == "" && defaultValue == "") { + return true; + } else { + return (!($input.val() == "" && _.isNull(defaultValue ))) && + ($input.val() != defaultValue); + } }; this.validate = function () { diff --git a/web/pgadmin/static/js/slickgrid/slick.pgadmin.formatters.js b/web/pgadmin/static/js/slickgrid/slick.pgadmin.formatters.js index 290bdddac..d14b18173 100644 --- a/web/pgadmin/static/js/slickgrid/slick.pgadmin.formatters.js +++ b/web/pgadmin/static/js/slickgrid/slick.pgadmin.formatters.js @@ -19,8 +19,15 @@ }); function JsonFormatter(row, cell, value, columnDef, dataContext) { - if (value == null || value === "") { - return ""; + // If column has default value, set placeholder + if (_.isUndefined(value) && columnDef.has_default_val) { + return "[default]"; + } + else if ( + (_.isUndefined(value) && columnDef.not_null) || + (_.isUndefined(value) || value === null) + ) { + return "[null]"; } else { // Stringify only if it's json object if (typeof value === "object" && !Array.isArray(value)) { @@ -42,11 +49,15 @@ } function NumbersFormatter(row, cell, value, columnDef, dataContext) { - if (_.isUndefined(value) || value === null) { - return "[null]"; + // If column has default value, set placeholder + if (_.isUndefined(value) && columnDef.has_default_val) { + return "[default]"; } - else if (value === "") { - return ''; + else if ( + (_.isUndefined(value) || value === null || value === "") || + (_.isUndefined(value) && columnDef.not_null) + ) { + return "[null]"; } else { return "" + _.escape(value) + ""; @@ -57,17 +68,30 @@ /* Checkbox has 3 states * 1) checked=true * 2) unchecked=false - * 3) indeterminate=null/'' + * 3) indeterminate=null */ - if (value == null || value === "") { - return "[null]"; + if (_.isUndefined(value) && columnDef.has_default_val) { + return "[default]"; + } + else if ( + (_.isUndefined(value) && columnDef.not_null) || + (value == null || value === "") + ) { + return "[null]"; } return value ? "true" : "false"; } function TextFormatter(row, cell, value, columnDef, dataContext) { - if (_.isUndefined(value) || value === null) { - return "[null]"; + // If column has default value, set placeholder + if (_.isUndefined(value) && columnDef.has_default_val) { + return "[default]"; + } + else if ( + (_.isUndefined(value) && columnDef.not_null) || + (_.isUndefined(value) || _.isNull(value)) + ) { + return "[null]"; } else { return _.escape(value); diff --git a/web/pgadmin/tools/sqleditor/__init__.py b/web/pgadmin/tools/sqleditor/__init__.py index 6431b3a31..828cb998c 100644 --- a/web/pgadmin/tools/sqleditor/__init__.py +++ b/web/pgadmin/tools/sqleditor/__init__.py @@ -441,8 +441,23 @@ def get_columns(trans_id): columns = dict() columns_info = None primary_keys = None + rset = None status, error_msg, conn, trans_obj, session_obj = check_transaction_status(trans_id) if status and conn is not None and session_obj is not None: + + ver = conn.manager.version + # Get the template path for the column + template_path = 'column/sql/#{0}#'.format(ver) + command_obj = pickle.loads(session_obj['command_obj']) + if hasattr(command_obj, 'obj_id'): + SQL = render_template("/".join([template_path, + 'nodes.sql']), + tid=command_obj.obj_id) + # rows with attribute not_null + status, rset = conn.execute_2darray(SQL) + if not status: + return internal_server_error(errormsg=rset) + # Check PK column info is available or not if 'primary_keys' in session_obj: primary_keys = session_obj['primary_keys'] @@ -450,10 +465,17 @@ def get_columns(trans_id): # Fetch column information columns_info = conn.get_column_info() if columns_info is not None: - for col in columns_info: + for key, col in enumerate(columns_info): col_type = dict() col_type['type_code'] = col['type_code'] col_type['type_name'] = None + if rset: + col_type['not_null'] = col['not_null'] = \ + rset['rows'][key]['not_null'] + + col_type['has_default_val'] = col['has_default_val'] = \ + rset['rows'][key]['has_default_val'] + columns[col['name']] = col_type # As we changed the transaction object we need to @@ -603,6 +625,7 @@ def save(trans_id): status, error_msg, conn, trans_obj, session_obj = check_transaction_status(trans_id) if status and conn is not None \ and trans_obj is not None and session_obj is not None: + setattr(trans_obj, 'columns_info', session_obj['columns_info']) # If there is no primary key found then return from the function. if len(session_obj['primary_keys']) <= 0 or len(changed_data) <= 0: diff --git a/web/pgadmin/tools/sqleditor/command.py b/web/pgadmin/tools/sqleditor/command.py index 1795155f1..960146762 100644 --- a/web/pgadmin/tools/sqleditor/command.py +++ b/web/pgadmin/tools/sqleditor/command.py @@ -442,6 +442,25 @@ class TableCommand(GridCommand): # For newly added rows if of_type == 'added': + + # When new rows are added, only changed columns data is + # sent from client side. But if column is not_null and has + # no_default_value, set column to blank, instead + # of not null which is set by default. + column_data = {} + column_type = {} + pk_names, primary_keys = self.get_primary_keys() + + for each_col in self.columns_info: + if ( + self.columns_info[each_col]['not_null'] and + not self.columns_info[each_col][ + 'has_default_val'] + ): + column_data[each_col] = None + column_type[each_col] =\ + self.columns_info[each_col]['type_name'] + for each_row in changed_data[of_type]: data = changed_data[of_type][each_row]['data'] # Remove our unique tracking key @@ -450,12 +469,19 @@ class TableCommand(GridCommand): data_type = set_column_names(changed_data[of_type][each_row]['data_type']) list_of_rowid.append(data.get('__temp_PK')) + # Update columns value and data type + # with columns having not_null=False and has + # no default value + column_data.update(data) + column_type.update(data_type) + sql = render_template("/".join([self.sql_path, 'insert.sql']), - data_to_be_saved=data, + data_to_be_saved=column_data, primary_keys=None, object_name=self.object_name, nsp_name=self.nsp_name, - data_type=data_type) + data_type=column_type, + pk_names=pk_names) list_of_sql.append(sql) # For updated rows diff --git a/web/pgadmin/tools/sqleditor/static/css/sqleditor.css b/web/pgadmin/tools/sqleditor/static/css/sqleditor.css index 574ef53ac..564471816 100644 --- a/web/pgadmin/tools/sqleditor/static/css/sqleditor.css +++ b/web/pgadmin/tools/sqleditor/static/css/sqleditor.css @@ -420,6 +420,16 @@ input.editor-checkbox:focus { background: #e46b6b; } +/* Disabled row */ +.grid-canvas .disabled_row { + background: #eeeeee; +} + +/* Disabled cell */ +.grid-canvas .disabled_cell { + color: #999999; +} + /* color the first column */ .sr .sc:first-child { background-color: #2c76b4; @@ -445,4 +455,4 @@ input.editor-checkbox:focus { .sr.ui-widget-content { border-top: 1px solid silver; -} +} \ No newline at end of file diff --git a/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js b/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js index 2062aa2bb..ba9dd4339 100644 --- a/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js +++ b/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js @@ -557,7 +557,9 @@ define( id: c.name, pos: c.pos, field: c.name, - name: c.label + name: c.label, + not_null: c.not_null, + has_default_val: c.has_default_val }; // Get the columns width based on data type @@ -625,6 +627,12 @@ define( } } } + // Disable rows having default values + if (!_.isUndefined(self.handler.rows_to_disable) && + _.indexOf(self.handler.rows_to_disable, i) !== -1 + ) { + cssClass += ' disabled_row'; + } return {'cssClasses': cssClass}; } @@ -702,6 +710,14 @@ define( // This will be used to collect primary key for that row grid.onBeforeEditCell.subscribe(function (e, args) { var before_data = args.item; + + // If newly added row is saved but grid is not refreshed, + // then disable cell editing for that row + if(self.handler.rows_to_disable && + _.contains(self.handler.rows_to_disable, args.row)) { + return false; + } + if(self.handler.can_edit && before_data && '__temp_PK' in before_data) { var _pk = before_data.__temp_PK, _keys = self.handler.primary_keys, @@ -1629,6 +1645,8 @@ define( self.query_start_time = new Date(); self.rows_affected = 0; self._init_polling_flags(); + // keep track of newly added rows + self.rows_to_disable = new Array(); self.trigger( 'pgadmin-sqleditor:loading-icon:show', @@ -2077,7 +2095,9 @@ define( 'label': column_label, 'cell': col_cell, 'can_edit': self.can_edit, - 'type': type + 'type': type, + 'not_null': c.not_null, + 'has_default_val': c.has_default_val }; columns.push(col); }); @@ -2320,6 +2340,10 @@ define( grid.setSelectedRows([]); } + // Add last row(new row) to keep track of it + if (is_added) { + self.rows_to_disable.push(grid.getDataLength()-1); + } // Reset data store self.data_store = { 'added': {},