From da553eec9b2324dde5259457bd654683e2133190 Mon Sep 17 00:00:00 2001 From: Akshay Joshi Date: Fri, 13 Sep 2019 11:54:16 +0530 Subject: [PATCH] Ensure port and username should not be mandatory when a service is provided. Fixes #4642 --- docs/en_US/release_notes_4_13.rst | 1 + web/migrations/versions/a77a0932a568_.py | 91 +++++++++++++++++++ .../browser/server_groups/servers/__init__.py | 6 +- .../server_groups/servers/model_validation.js | 8 +- .../servers/model_validation_spec.js | 13 ++- 5 files changed, 105 insertions(+), 14 deletions(-) create mode 100644 web/migrations/versions/a77a0932a568_.py diff --git a/docs/en_US/release_notes_4_13.rst b/docs/en_US/release_notes_4_13.rst index c2ad6ffe1..e197467c6 100644 --- a/docs/en_US/release_notes_4_13.rst +++ b/docs/en_US/release_notes_4_13.rst @@ -45,6 +45,7 @@ Bug fixes | `Issue #4577 `_ - Fix an error that could be seen when click on any system column of a table. | `Issue #4584 `_ - Unescape HTML entities in database names in the Query Tool title bar. | `Issue #4631 `_ - Add editor options for plain text mode and to disable block folding to workaround rendering speed issues in CodeMirror with very large scripts. +| `Issue #4642 `_ - Ensure port and username should not be mandatory when a service is provided. | `Issue #4643 `_ - Fix Truncate option deselect issue for compound triggers. | `Issue #4644 `_ - Fix length and precision enable/disable issue when changing the data type for Domain node. | `Issue #4650 `_ - Fix SQL tab issue for Views. It's a regression of compound triggers. diff --git a/web/migrations/versions/a77a0932a568_.py b/web/migrations/versions/a77a0932a568_.py new file mode 100644 index 000000000..76ad244bf --- /dev/null +++ b/web/migrations/versions/a77a0932a568_.py @@ -0,0 +1,91 @@ + +"""Change the not null constraints for port, username as it should not + compulsory when service is provided. RM #4642 + +Revision ID: a77a0932a568 +Revises: 35f29b1701bd +Create Date: 2019-09-09 15:41:30.084753 + +""" +from alembic import op +import sqlalchemy as sa +from pgadmin.model import db + +# revision identifiers, used by Alembic. +revision = 'a77a0932a568' +down_revision = '35f29b1701bd' +branch_labels = None +depends_on = None + + +def upgrade(): + # To Save previous data + db.engine.execute("ALTER TABLE server RENAME TO server_old") + + # Create table with drop constraint for port and username definition + db.engine.execute(""" + CREATE TABLE server ( + id INTEGER NOT NULL, + user_id INTEGER NOT NULL, + servergroup_id INTEGER NOT NULL, + name VARCHAR(128) NOT NULL, + host VARCHAR(128), + port INTEGER, + maintenance_db VARCHAR(64) NOT NULL, + username VARCHAR(64), + password VARCHAR(64), + role VARCHAR(64), + ssl_mode VARCHAR(16) NOT NULL CHECK(ssl_mode IN + ( 'allow' , 'prefer' , 'require' , 'disable' , + 'verify-ca' , 'verify-full' ) + ), + comment VARCHAR(1024), + discovery_id VARCHAR(128), + hostaddr TEXT(1024), + db_res TEXT, + passfile TEXT, + sslcert TEXT, + sslkey TEXT, + sslrootcert TEXT, + sslcrl TEXT, + sslcompression INTEGER DEFAULT 0, + bgcolor TEXT(10), + fgcolor TEXT(10), + service TEXT, + use_ssh_tunnel INTEGER DEFAULT 0, + tunnel_host TEXT, + tunnel_port TEXT, + tunnel_username TEXT, + tunnel_authentication INTEGER DEFAULT 0, + tunnel_identity_file TEXT, connect_timeout INTEGER DEFAULT 0, tunnel_password TEXT(64), + PRIMARY KEY(id), + FOREIGN KEY(user_id) REFERENCES user(id), + FOREIGN KEY(servergroup_id) REFERENCES servergroup(id) + ) + """) + + # Copy old data again into table + db.engine.execute(""" + INSERT INTO server ( + id, user_id, servergroup_id, name, host, port, maintenance_db, + username, password, role, ssl_mode, comment, discovery_id, hostaddr, + db_res, passfile, sslcert, sslkey, sslrootcert, sslcrl, + sslcompression, bgcolor, fgcolor, service, use_ssh_tunnel, + tunnel_host, tunnel_port, tunnel_username, tunnel_authentication, + tunnel_identity_file + + ) SELECT + id, user_id, servergroup_id, name, host, port, maintenance_db, + username, password, role, ssl_mode, comment, discovery_id, hostaddr, + db_res, passfile, sslcert, sslkey, sslrootcert, sslcrl, + sslcompression, bgcolor, fgcolor, service, use_ssh_tunnel, + tunnel_host, tunnel_port, tunnel_username, tunnel_authentication, + tunnel_identity_file + FROM server_old""") + + # Remove old data + db.engine.execute("DROP TABLE server_old") + + +def downgrade(): + pass diff --git a/web/pgadmin/browser/server_groups/servers/__init__.py b/web/pgadmin/browser/server_groups/servers/__init__.py index eec37b205..dc39a1393 100644 --- a/web/pgadmin/browser/server_groups/servers/__init__.py +++ b/web/pgadmin/browser/server_groups/servers/__init__.py @@ -723,9 +723,8 @@ class ServerNode(PGChildNodeView): """Add a server node to the settings database""" required_args = [ u'name', - u'port', + u'db', u'sslmode', - u'username' ] data = request.form if request.form else json.loads( @@ -741,7 +740,8 @@ class ServerNode(PGChildNodeView): if 'service' in data and not data['service']: required_args.extend([ u'host', - u'db', + u'port', + u'username', u'role' ]) for arg in required_args: diff --git a/web/pgadmin/static/js/browser/server_groups/servers/model_validation.js b/web/pgadmin/static/js/browser/server_groups/servers/model_validation.js index c44a1f4cd..4548bfffa 100644 --- a/web/pgadmin/static/js/browser/server_groups/servers/model_validation.js +++ b/web/pgadmin/static/js/browser/server_groups/servers/model_validation.js @@ -36,13 +36,13 @@ export class ModelValidation { this.checkHostAndHostAddress(); this.checkForEmpty('db', gettext('Maintenance database must be specified.')); + this.checkForEmpty('username', gettext('Username must be specified.')); + this.checkForEmpty('port', gettext('Port must be specified.')); } else { + this.checkForEmpty('db', gettext('Maintenance database must be specified.')); this.clearHostAddressAndDbErrors(); } - this.checkForEmpty('username', gettext('Username must be specified.')); - this.checkForEmpty('port', gettext('Port must be specified.')); - if (this.model.get('use_ssh_tunnel')) { this.checkForEmpty('tunnel_host', gettext('SSH Tunnel host must be specified.')); this.checkForEmpty('tunnel_port', gettext('SSH Tunnel port must be specified.')); @@ -73,7 +73,7 @@ export class ModelValidation { } clearHostAddressAndDbErrors() { - _.each(['host', 'hostaddr', 'db'], (item) => { + _.each(['host', 'hostaddr', 'db', 'username', 'port'], (item) => { this.setNullValueForEmptyString(item); this.model.errorModel.unset(item); }); diff --git a/web/regression/javascript/browser/server_groups/servers/model_validation_spec.js b/web/regression/javascript/browser/server_groups/servers/model_validation_spec.js index 2ba1274d7..67cbe704e 100644 --- a/web/regression/javascript/browser/server_groups/servers/model_validation_spec.js +++ b/web/regression/javascript/browser/server_groups/servers/model_validation_spec.js @@ -47,9 +47,10 @@ describe('Server#ModelValidation', () => { }); }); - describe('Service id present', () => { + describe('Service id and Maintenance database present', () => { it('does not set any error in the model', () => { model.allValues['service'] = 'asdfg'; + model.allValues['db'] = 'postgres'; expect(modelValidation.validate()).toBeNull(); expect(model.errorModel.set).toHaveBeenCalledWith({}); }); @@ -147,14 +148,12 @@ describe('Server#ModelValidation', () => { }); describe('Service id present', () => { - it('does not set any error in the model', () => { + it('set the "Maintenance database must be specified." error', () => { + model.allValues['name'] = 'some name'; model.allValues['service'] = 'asdfg'; - expect(modelValidation.validate()).toEqual('Name must be specified.'); - expect(model.errorModel.set).toHaveBeenCalledTimes(1); + expect(modelValidation.validate()).toEqual('Maintenance database must be specified.'); expect(model.errorModel.set).toHaveBeenCalledWith({ - name: 'Name must be specified.', - username: 'Username must be specified.', - port: 'Port must be specified.', + db: 'Maintenance database must be specified.', }); }); });