[pgAdmin][RM4642] port should not be mandatory when a service is provided
Akshay Joshi <[email protected]>
| Newsgroups | gmane.comp.db.postgresql.pgadmin.devel |
|---|---|
| Message-ID | <CANxoLDcBU3iygS=Dt4QshSXgrJ8BYkihmC2Zh+-BD=hB=2Ho8A@mail.gmail.com> |
Hi Hackers, Attached is the patch to fix the RM #4642 "port should not be mandatory when a service is provided". *Cyril Jouve* has sent the initial patch, but that only removes the validation on GUI, when we save the server properties without port and username backend throws an error because port and username is *NOT NULL* columns in the server table in SQLite. I have removed the *NOT NULL *constraint from the port and username. The maintenance database is required as our whole connection logic is based on that, so I have added the NOT NULL constraint for that column. Please review it. -- *Thanks & Regards* *Akshay Joshi* *Sr. Software Architect* *EnterpriseDB Software India Private Limited* *Mobile: +91 976-788-8246*
RM_4642.patch
(application/octet-stream, 7.4 KB)
diff --git a/web/migrations/versions/a77a0932a568_.py b/web/migrations/versions/a77a0932a568_.py
new file mode 100644
index 00000000..76ad244b
--- /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 eec37b20..dc39a139 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 c44a1f4c..4548bfff 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 2ba1274d..67cbe704 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.',
});
});
});