public inbox for [email protected]
help / color / mirror / Atom feedFrom: Surinder Kumar <[email protected]>
To: Dave Page <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: [pgAdmin4][Patch][RM2257]: Query tool - Insert row doesn't use default values
Date: Fri, 5 May 2017 17:22:42 +0530
Message-ID: <CAM5-9D-kZEWrqgxr6-PyB_YVCdovJm8PpXGDpdfcpo65t_Sakg@mail.gmail.com> (raw)
In-Reply-To: <CAM5-9D_ZEysH0ewuCoOoon18mim_G9r-2es=+MbyYXnsLqrTEw@mail.gmail.com>
References: <CAM5-9D_SSL81uT4AqsRr8WPABWA6S-iE34OxLb8YqfVN3myeJg@mail.gmail.com>
<CA+OCxoySjV87N+YEkhXRzWaaGNCX4o+KuKnVGrYhLuWYeDx+SA@mail.gmail.com>
<CAM5-9D8MdAqvix74_K+eMkeGkP7r4Aost1Uqz4hupMRPxwqzAw@mail.gmail.com>
<CA+OCxoxkfbwJaWgCTTZ7eqrkv7Y49jU2EUmDsH_5raZqSFwZsA@mail.gmail.com>
<CAM5-9D_ZEysH0ewuCoOoon18mim_G9r-2es=+MbyYXnsLqrTEw@mail.gmail.com>
List-Unsubscribe: <mailto:[email protected]?body=unsub%20pgadmin-hackers>
Hi Dave,
The support to handle [null] and [default] values is added for following
formatters:
- JsonFormatter
- NumbersFormatter
- CheckmarkFormatter
- TextFormatter
Previously when a new row is added, it was not validating each and every
cell for columns with attribute not_null = true.
Introduced a new function validateRow(...) which is called on adding a new
row, it validates each cell with column having attribute(not_null=true).
the corresponding cell will highlighted and save button will be disabled if
value is [null].
Now I will add more feature test cases for remaining formatters. Will send
separate patch for feature test cases once completed.
Please review updated patch.
On Tue, May 2, 2017 at 5:57 PM, Surinder Kumar <
[email protected]> wrote:
> Hi Dave,
>
> On Tue, May 2, 2017 at 5:21 PM, Dave Page <[email protected]> wrote:
>
>> Hi,
>>
>> This is looking much better now :-). Couple of thoughts and a bug:
>>
>> - Only the TextFormatter seems to handle both [null] and [default]
>> values. Shouldn't all formatters do so (including Json and Checkmark)?
>>
> Yes, I will apply the same changes for other formatters too.
>
>> For example, "serial" columns currently get displayed as [null] when left
>> blank, but I would expect to see [default].
>>
>> - I would suggest we put [null] and [default] in a lighter colour -
>> #999999.
>>
>> - With the feature test patch added, I seem to be consistently getting
>> the following failure (immediately after your new tests run):
>>
>> ======================================================================
>> ERROR: runTest (pgadmin.feature_tests.xss_che
>> cks_panels_and_query_tool_test.CheckForXssFeatureTest)
>> Test XSS check for panels and query tool
>> ----------------------------------------------------------------------
>> Traceback (most recent call last):
>> File "/Users/dpage/git/pgadmin4/web/regression/feature_utils/base_feature_test.py",
>> line 42, in setUp
>> self._screenshot()
>> File "/Users/dpage/git/pgadmin4/web/regression/feature_utils/base_feature_test.py",
>> line 92, in _screenshot
>> python_version))
>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa
>> ges/selenium/webdriver/remote/webdriver.py", line 802, in
>> get_screenshot_as_file
>> png = self.get_screenshot_as_png()
>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa
>> ges/selenium/webdriver/remote/webdriver.py", line 821, in
>> get_screenshot_as_png
>> return base64.b64decode(self.get_screenshot_as_base64().encode('
>> ascii'))
>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa
>> ges/selenium/webdriver/remote/webdriver.py", line 831, in
>> get_screenshot_as_base64
>> return self.execute(Command.SCREENSHOT)['value']
>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa
>> ges/selenium/webdriver/remote/webdriver.py", line 238, in execute
>> self.error_handler.check_response(response)
>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa
>> ges/selenium/webdriver/remote/errorhandler.py", line 193, in
>> check_response
>> raise exception_class(message, screen, stacktrace)
>> UnexpectedAlertPresentException: Alert Text: None
>> Message: unexpected alert open: {Alert text : Are you sure you wish to
>> close the pgAdmin 4 browser?}
>> (Session info: chrome=58.0.3029.81)
>> (Driver info: chromedriver=2.29.461585 (0be2cd95f834e9ee7c46bcc7cf405b483f5ae83b),platform=Mac
>> OS X 10.12.3 x86_64)
>>
>>
>> ======================================================================
>> ERROR: runTest (pgadmin.feature_tests.xss_che
>> cks_pgadmin_debugger_test.CheckDebuggerForXssFeatureTest)
>> Test table DDL generation
>> ----------------------------------------------------------------------
>> Traceback (most recent call last):
>> File "/Users/dpage/git/pgadmin4/web/regression/feature_utils/base_feature_test.py",
>> line 42, in setUp
>> self._screenshot()
>> File "/Users/dpage/git/pgadmin4/web/regression/feature_utils/base_feature_test.py",
>> line 92, in _screenshot
>> python_version))
>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa
>> ges/selenium/webdriver/remote/webdriver.py", line 802, in
>> get_screenshot_as_file
>> png = self.get_screenshot_as_png()
>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa
>> ges/selenium/webdriver/remote/webdriver.py", line 821, in
>> get_screenshot_as_png
>> return base64.b64decode(self.get_screenshot_as_base64().encode('
>> ascii'))
>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa
>> ges/selenium/webdriver/remote/webdriver.py", line 831, in
>> get_screenshot_as_base64
>> return self.execute(Command.SCREENSHOT)['value']
>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa
>> ges/selenium/webdriver/remote/webdriver.py", line 238, in execute
>> self.error_handler.check_response(response)
>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa
>> ges/selenium/webdriver/remote/errorhandler.py", line 193, in
>> check_response
>> raise exception_class(message, screen, stacktrace)
>> UnexpectedAlertPresentException: Alert Text: None
>> Message: unexpected alert open: {Alert text : Are you sure you wish to
>> close the pgAdmin 4 browser?}
>> (Session info: chrome=58.0.3029.81)
>> (Driver info: chromedriver=2.29.461585 (0be2cd95f834e9ee7c46bcc7cf405b483f5ae83b),platform=Mac
>> OS X 10.12.3 x86_64)
>>
> Sure. I will fix this.
>
>>
>> Thanks!
>>
>>
>> On Fri, Apr 28, 2017 at 10:19 AM, Surinder Kumar <
>> [email protected]> wrote:
>>
>>> Hi Dave,
>>>
>>> Please find updated patch for RM case and a separate patch for Feature
>>> tests.
>>>
>>> *Python:*
>>>
>>> - Added [default] label for cells with default values while inserting a
>>> new row.
>>>
>>> - Introduced a FieldValidator function for cells that don't allow null
>>> values. If user tries to insert null value, field with be highlighted with
>>> red borders around.
>>>
>>> -
>>> If a cell contains blank string('') and when we set it to null, the
>>> change into the cell is not detected. It was because the comparison
>>> for (defaultValue == null) return true if defaultValue is undefined.
>>> Hence _.isNull(value) is used to fix this.
>>>
>>> *Feature Test cases:*
>>>
>>> - Introduced a new method create_table_with_query(server, db_name,
>>> query) in test_utils.py which executes the given query on connected
>>> server.
>>>
>>> - Added a new file test_data.json that has test data for test cases.
>>>
>>>
>>> On Fri, Apr 7, 2017 at 2:21 PM, Dave Page <[email protected]> wrote:
>>>
>>>> Hi
>>>>
>>>> On Sat, Apr 1, 2017 at 12:45 PM, Surinder Kumar
>>>> <[email protected]> wrote:
>>>> > Hi
>>>> >
>>>> > Issues fixed:
>>>> >
>>>> > 1. If a column is defined with a default modifier, there is now way to
>>>> > insert the row with those defaults.
>>>> > The column will be left blank and it will take default value
>>>> automatically.
>>>> >
>>>> > 2. If a column has a not-null constraint then an error is returned
>>>> and the
>>>> > row is not inserted.
>>>> > The column will be left blank
>>>> >
>>>> > The default values for new added rows will be displayed on
>>>> refresh/execute.
>>>> >
>>>> > Please find attached patch and review.
>>>>
>>>> This largely works as expected, but there is some weirdness. I have a
>>>> test table that looks like this:
>>>>
>>>> CREATE TABLE public.defaults
>>>> (
>>>> id bigint NOT NULL DEFAULT nextval('defaults_id_seq'::regclass),
>>>> data_default_nulls text COLLATE pg_catalog."default" DEFAULT
>>>> 'abc123'::text,
>>>> data_default_no_nulls text COLLATE pg_catalog."default" NOT NULL
>>>> DEFAULT 'def456'::text,
>>>> data_nulls text COLLATE pg_catalog."default",
>>>> data_no_nulls text COLLATE pg_catalog."default" NOT NULL,
>>>> CONSTRAINT defaults_pkey PRIMARY KEY (id)
>>>> )
>>>>
>>>> Remember that the expected behaviour is:
>>>>
>>>> - Set a value to empty to update the column to null.
>>>> - Set a value to '' to update the column to an empty string
>>>> - Set a value to anything else to update the column to that value
>>>>
>>>> 1) In a row with values in each column, if I try to set the value of
>>>> data_default_nulls to null, the query executed is:
>>>>
>>>> UPDATE public.defaults SET
>>>> data_default_nulls = '' WHERE
>>>> id = '2';
>>>>
>>>> 2) If I do the same in the data_nulls column, the value is immediately
>>>> shown as [null] and the query executed is:
>>>>
>>>> UPDATE public.defaults SET
>>>> data_nulls = NULL WHERE
>>>> id = '2';
>>>>
>>>> 3) If I then edit the value in data_default_nulls, it shows the
>>>> current value as ''. Removing the quotes (to set it to null) doesn't
>>>> get detected as a change.
>>>>
>>> Taken care.
>>>
>>>>
>>>> 4) When I manually executed "update defaults set data_default_nulls =
>>>> null where id = 2" in a query tool window, I got:
>>>>
>>>> 2017-04-07 09:43:02,987: INFO werkzeug: 127.0.0.1 - - [07/Apr/2017
>>>> 09:43:02] "GET /sqleditor/columns/8745675 HTTP/1.1" 500 -
>>>> Traceback (most recent call last):
>>>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa
>>>> ges/flask/app.py",
>>>> line 2000, in __call__
>>>> return self.wsgi_app(environ, start_response)
>>>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa
>>>> ges/flask/app.py",
>>>> line 1991, in wsgi_app
>>>> response = self.make_response(self.handle_exception(e))
>>>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa
>>>> ges/flask/app.py",
>>>> line 1567, in handle_exception
>>>> reraise(exc_type, exc_value, tb)
>>>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa
>>>> ges/flask/app.py",
>>>> line 1988, in wsgi_app
>>>> response = self.full_dispatch_request()
>>>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa
>>>> ges/flask/app.py",
>>>> line 1641, in full_dispatch_request
>>>> rv = self.handle_user_exception(e)
>>>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa
>>>> ges/flask/app.py",
>>>> line 1544, in handle_user_exception
>>>> reraise(exc_type, exc_value, tb)
>>>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa
>>>> ges/flask/app.py",
>>>> line 1639, in full_dispatch_request
>>>> rv = self.dispatch_request()
>>>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa
>>>> ges/flask/app.py",
>>>> line 1625, in dispatch_request
>>>> return self.view_functions[rule.endpoint](**req.view_args)
>>>> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa
>>>> ges/flask_login.py",
>>>> line 792, in decorated_view
>>>> return func(*args, **kwargs)
>>>> File "/Users/dpage/git/pgadmin4/web/pgadmin/tools/sqleditor/__ini
>>>> t__.py",
>>>> line 452, in get_columns
>>>> tid=command_obj.obj_id)
>>>> AttributeError: 'QueryToolCommand' object has no attribute 'obj_id'
>>>>
>>> Fixed.
>>>
>>>>
>>>> 5) When I run the query again in pgAdmin III, then refresh the data in
>>>> pgAdmin 4, the data_default_nulls column is displayed without the
>>>> [null] marker (despite having a null value, which I confirmed in
>>>> pgAdmin 3).
>>>>
>>> Fixed.
>>>
>>>>
>>>> I'm sure there are other combinations of issues here. Please fix and
>>>> thoroughly re-test to ensure behaviour is consistent - and to avoid
>>>> future issues, please add some appropriate feature tests to check
>>>> nulls, defaults and empty strings are properly handled in view, insert
>>>> and updates. Murtuza recently wrote some feature tests for the query
>>>> tool - you should be able to use those as a starting point.
>>>>
>>> Added feature tests
>>>
>>>>
>>>> Thanks.
>>>>
>>>> --
>>>> Dave Page
>>>> Blog: http://pgsnake.blogspot.com
>>>> Twitter: @pgsnake
>>>>
>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>> The Enterprise PostgreSQL Company
>>>>
>>>
>>>
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>
--
Sent via pgadmin-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers
Attachments:
[application/octet-stream] RM_2257_v2.patch (15.7K, 3-RM_2257_v2.patch)
download | inline diff:
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 759e657..f3353d6 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 7536a9c..4f1de2a 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/js/slickgrid/slick.pgadmin.editors.js b/web/pgadmin/static/js/slickgrid/slick.pgadmin.editors.js
index cdfba4d..0bdd1ab 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,10 @@
};
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.
+ return (!($input.val() == "" && _.isNull(defaultValue))) &&
+ ($input.val() != defaultValue);
};
this.validate = function () {
@@ -253,7 +261,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 +290,7 @@
};
this.isValueChanged = function () {
- return (!($input.val() == "" && defaultValue == null)) && ($input.val() != defaultValue);
+ return (!($input.val() == "" && _.isNull(defaultValue))) && ($input.val() != defaultValue);
};
this.validate = function () {
@@ -498,6 +506,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 +851,12 @@
};
this.isValueChanged = function () {
- return (!($input.val() == "" && defaultValue == null)) && ($input.val() != defaultValue);
+ 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 290bddd..61b1fc1 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 "<span class='pull-left'>[default]</span>";
+ }
+ else if (_.isUndefined(value) && columnDef.not_null) {
+ return null; // If null value not allowed, set cell to blank
+ }
+ else if (_.isUndefined(value) || value === null) {
+ return "<span class='pull-left'>[null]</span>";
} else {
// Stringify only if it's json object
if (typeof value === "object" && !Array.isArray(value)) {
@@ -42,7 +49,14 @@
}
function NumbersFormatter(row, cell, value, columnDef, dataContext) {
- if (_.isUndefined(value) || value === null) {
+ // If column has default value, set placeholder
+ if (_.isUndefined(value) && columnDef.has_default_val) {
+ return "<span class='pull-right'>[default]</span>";
+ }
+ else if (_.isUndefined(value) && columnDef.not_null) {
+ return ''; // If null value not allowed, set cell to blank
+ }
+ else if (_.isUndefined(value) || value === null) {
return "<span class='pull-right'>[null]</span>";
}
else if (value === "") {
@@ -57,16 +71,29 @@
/* Checkbox has 3 states
* 1) checked=true
* 2) unchecked=false
- * 3) indeterminate=null/''
+ * 3) indeterminate=null
*/
- if (value == null || value === "") {
+ if (_.isUndefined(value) && columnDef.has_default_val) {
+ return "<span class='pull-left'>[default]</span>";
+ }
+ else if (_.isUndefined(value) && columnDef.not_null) {
+ return ''; // If null value not allowed, set cell to blank
+ }
+ else if (value == null || value === "") {
return "<span class='pull-left'>[null]</span>";
}
return value ? "true" : "false";
}
function TextFormatter(row, cell, value, columnDef, dataContext) {
- if (_.isUndefined(value) || value === null) {
+ // If column has default value, set placeholder
+ if (_.isUndefined(value) && columnDef.has_default_val) {
+ return "<span class='pull-left'>[default]</span>";
+ }
+ else if (_.isUndefined(value) && columnDef.not_null) {
+ return ''; // If null value not allowed, set cell to blank
+ }
+ else if (_.isUndefined(value) || _.isNull(value)) {
return "<span class='pull-left'>[null]</span>";
}
else {
diff --git a/web/pgadmin/tools/sqleditor/__init__.py b/web/pgadmin/tools/sqleditor/__init__.py
index d114988..f7466d8 100644
--- a/web/pgadmin/tools/sqleditor/__init__.py
+++ b/web/pgadmin/tools/sqleditor/__init__.py
@@ -440,8 +440,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']
@@ -449,10 +464,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
@@ -602,6 +624,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 1795155..e396981 100644
--- a/web/pgadmin/tools/sqleditor/command.py
+++ b/web/pgadmin/tools/sqleditor/command.py
@@ -442,6 +442,23 @@ 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 = {}
+ 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] = ""
+ 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 +467,18 @@ 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)
list_of_sql.append(sql)
# For updated rows
diff --git a/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js b/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js
index 2062aa2..6915bb1 100644
--- a/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js
+++ b/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js
@@ -526,6 +526,19 @@ define(
render_grid: function(collection, columns, is_editable) {
var self = this;
+ var requiredFieldValidator = function (value) {
+ // If value if of boolean type,
+ // the comparision for value(false) == '' returns true
+ // so set it to some another value.
+ if (typeof(value) === 'boolean')
+ value = 2;
+
+ if (_.isUndefined(value) || _.isNull(value) || value == '') {
+ return {valid: false, msg: "This is a required field"};
+ } else {
+ return {valid: true, msg: null};
+ }
+ }
// This will work as data store and holds all the
// inserted/updated/deleted data from grid
self.handler.data_store = {
@@ -557,7 +570,10 @@ 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,
+ validator: c.not_null ? requiredFieldValidator : null
};
// Get the columns width based on data type
@@ -806,6 +822,32 @@ define(
$("#btn-save").prop('disabled', false);
}.bind(editor_data));
+ // Validate each cell of new added row.
+ var validateRow = function(grid, rowIdx) {
+ var is_valid = true;
+ $.each(grid.getColumns(), function(colIdx, column) {
+ // iterate through editable cells
+ // Skip first column - Select all
+ if (colIdx && colIdx !==0) {
+ var item = grid.getData()[rowIdx][colIdx-1] || '';
+
+ if (column.editor && column.validator && column.not_null &&
+ !column.has_default_val) {
+ var validationResults = column.validator(item);
+ if (!validationResults.valid) {
+ // show editor
+ grid.gotoCell(rowIdx, colIdx, true);
+ // validate (it will fail)
+ grid.getEditorLock().commitCurrentEdit();
+ // stop iteration
+ is_valid = false;
+ return is_valid;
+ }
+ }
+ }
+ });
+ return is_valid;
+ }
// Listener function which will be called when user adds new rows
grid.onAddNewRow.subscribe(function (e, args) {
// self.handler.data_store.added will holds all the newly added rows/data
@@ -826,8 +868,10 @@ define(
grid.invalidateRows([collection.length - 1]);
grid.updateRowCount();
grid.render();
+ // Once new row is rendered, validate row
+ var is_valid = validateRow(args.grid, args.grid.getActiveCell().row);
// Enable save button
- $("#btn-save").prop('disabled', false);
+ $("#btn-save").prop('disabled', !is_valid);
}.bind(editor_data));
// Resize SlickGrid when window resize
@@ -2077,7 +2121,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);
});
view thread (25+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: [pgAdmin4][Patch][RM2257]: Query tool - Insert row doesn't use default values
In-Reply-To: <CAM5-9D-kZEWrqgxr6-PyB_YVCdovJm8PpXGDpdfcpo65t_Sakg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox