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)? 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_checks_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-packages/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-packages/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-packages/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-packages/selenium/webdriver/remote/webdriver.py", line 238, in execute self.error_handler.check_response(response) File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/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_checks_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-packages/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-packages/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-packages/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-packages/selenium/webdriver/remote/webdriver.py", line 238, in execute self.error_handler.check_response(response) File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/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) Thanks! On Fri, Apr 28, 2017 at 10:19 AM, Surinder Kumar < surinder.ku...@enterprisedb.com> 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 <dp...@pgadmin.org> wrote: > >> Hi >> >> On Sat, Apr 1, 2017 at 12:45 PM, Surinder Kumar >> <surinder.ku...@enterprisedb.com> 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