public inbox for [email protected]
help / color / mirror / Atom feedFix for RM2421 [pgAdmin4][patch]
11+ messages / 3 participants
[nested] [flat]
* Fix for RM2421 [pgAdmin4][patch]
@ 2017-05-18 10:01 Harshal Dhumal <[email protected]>
2017-05-18 14:27 ` Re: Fix for RM2421 [pgAdmin4][patch] Joao Pedro De Almeida Pereira <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Harshal Dhumal @ 2017-05-18 10:01 UTC (permalink / raw)
To: pgadmin-hackers
Hi,
Please find attached patch for RM2421
Issue fixed: 1. Integer/numeric Validation is not working properly.
2. Wrong CPU rate unit
--
*Harshal Dhumal*
*Sr. Software Engineer*
EnterpriseDB India: 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:
[text/x-patch] RM2421.patch (6.0K, 3-RM2421.patch)
download | inline diff:
diff --git a/web/pgadmin/browser/server_groups/servers/resource_groups/templates/resource_groups/js/resource_groups.js b/web/pgadmin/browser/server_groups/servers/resource_groups/templates/resource_groups/js/resource_groups.js
index 3cc822e..596a4ee 100644
--- a/web/pgadmin/browser/server_groups/servers/resource_groups/templates/resource_groups/js/resource_groups.js
+++ b/web/pgadmin/browser/server_groups/servers/resource_groups/templates/resource_groups/js/resource_groups.js
@@ -72,7 +72,7 @@ define(
id: 'name', label: '{{ _('Name') }}', cell: 'string',
type: 'text',
},{
- id: 'cpu_rate_limit', label:'{{ _('CPU rate limit (%%)') }}', cell: 'string',
+ id: 'cpu_rate_limit', label:'{{ _('CPU rate limit (%)') }}', cell: 'string',
type: 'numeric', min:0, max:16777216
},{
id: 'dirty_rate_limit', label:'{{ _('Dirty rate limit (KB)') }}', cell: 'string',
diff --git a/web/pgadmin/static/js/backform.pgadmin.js b/web/pgadmin/static/js/backform.pgadmin.js
index 553676e..a822da8 100644
--- a/web/pgadmin/static/js/backform.pgadmin.js
+++ b/web/pgadmin/static/js/backform.pgadmin.js
@@ -1528,7 +1528,18 @@
max_value = field.max,
isValid = true,
intPattern = new RegExp("^-?[0-9]*$"),
- isMatched = intPattern.test(value);
+ isMatched = intPattern.test(value),
+ trigger_invalid_event = function(msg) {
+ if (this.model.collection || this.model.handler) {
+ (this.model.collection || this.model.handler).trigger(
+ 'pgadmin-session:model:invalid', msg, this.model
+ );
+ } else {
+ (this.model).trigger(
+ 'pgadmin-session:invalid', msg, this.model
+ );
+ }
+ }.bind(this);
// Below logic will validate input
if (!isMatched) {
@@ -1573,25 +1584,23 @@
this.model.errorModel.unset(name);
this.model.set(name, value);
this.listenTo(this.model, "change:" + name, this.render);
- if (this.model.collection || this.model.handler) {
- (this.model.collection || this.model.handler).trigger(
- 'pgadmin-session:model:valid', this.model, (this.model.collection || this.model.handler)
- );
+ // Check if other fields of same model are valid before
+ // triggering 'session:valid' event
+ if(_.size(this.model.errorModel.attributes) == 0) {
+ if (this.model.collection || this.model.handler) {
+ (this.model.collection || this.model.handler).trigger(
+ 'pgadmin-session:model:valid', this.model, (this.model.collection || this.model.handler)
+ );
+ } else {
+ (this.model).trigger(
+ 'pgadmin-session:valid', this.model.sessChanged(), this.model
+ );
+ }
} else {
- (this.model).trigger(
- 'pgadmin-session:valid', this.model.sessChanged(), this.model
- );
+ trigger_invalid_event(_.values(this.model.errorModel.attributes)[0]);
}
} else {
- if (this.model.collection || this.model.handler) {
- (this.model.collection || this.model.handler).trigger(
- 'pgadmin-session:model:invalid', this.model.errorModel.get(name), this.model
- );
- } else {
- (this.model).trigger(
- 'pgadmin-session:invalid', this.model.errorModel.get(name), this.model
- );
- }
+ trigger_invalid_event(this.model.errorModel.get(name));
}
}
});
@@ -1631,7 +1640,18 @@
max_value = field.max,
isValid = true,
intPattern = new RegExp("^-?[0-9]+(\.?[0-9]*)?$"),
- isMatched = intPattern.test(value);
+ isMatched = intPattern.test(value),
+ trigger_invalid_event = function(msg) {
+ if (this.model.collection || this.model.handler) {
+ (this.model.collection || this.model.handler).trigger(
+ 'pgadmin-session:model:invalid', msg, this.model
+ );
+ } else {
+ (this.model).trigger(
+ 'pgadmin-session:invalid', msg, this.model
+ );
+ }
+ }.bind(this);
// Below logic will validate input
if (!isMatched) {
@@ -1676,25 +1696,23 @@
this.model.errorModel.unset(name);
this.model.set(name, value);
this.listenTo(this.model, "change:" + name, this.render);
- if (this.model.collection || this.model.handler) {
- (this.model.collection || this.model.handler).trigger(
- 'pgadmin-session:model:valid', this.model, (this.model.collection || this.model.handler)
- );
+ // Check if other fields of same model are valid before
+ // triggering 'session:valid' event
+ if(_.size(this.model.errorModel.attributes) == 0) {
+ if (this.model.collection || this.model.handler) {
+ (this.model.collection || this.model.handler).trigger(
+ 'pgadmin-session:model:valid', this.model, (this.model.collection || this.model.handler)
+ );
+ } else {
+ (this.model).trigger(
+ 'pgadmin-session:valid', this.model.sessChanged(), this.model
+ );
+ }
} else {
- (this.model).trigger(
- 'pgadmin-session:valid', this.model.sessChanged(), this.model
- );
+ trigger_invalid_event(_.values(this.model.errorModel.attributes)[0]);
}
} else {
- if (this.model.collection || this.model.handler) {
- (this.model.collection || this.model.handler).trigger(
- 'pgadmin-session:model:invalid', this.model.errorModel.get(name), this.model
- );
- } else {
- (this.model).trigger(
- 'pgadmin-session:invalid', this.model.errorModel.get(name), this.model
- );
- }
+ trigger_invalid_event(this.model.errorModel.get(name));
}
}
});
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Fix for RM2421 [pgAdmin4][patch]
2017-05-18 10:01 Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
@ 2017-05-18 14:27 ` Joao Pedro De Almeida Pereira <[email protected]>
2017-05-19 06:52 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Joao Pedro De Almeida Pereira @ 2017-05-18 14:27 UTC (permalink / raw)
To: Harshal Dhumal <[email protected]>; +Cc: pgadmin-hackers; Shruti B Iyer <[email protected]>
Hello Harshal,
We review the patch and have some questions:
1) Is there any particular reason to initialize variables and functions in
the same place? We believe that it would be more readable there were no
chaining of variable creation, specially if those variables are functions.
Check line:
+++ b/web/pgadmin/static/js/backform.pgadmin.js
@@ -1528,7 +1528,18 @@
max_value = field.max,
isValid = true,
intPattern = new RegExp("^-?[0-9]*$"),
- isMatched = intPattern.test(value);
+ isMatched = intPattern.test(value),
+ trigger_invalid_event = function(msg) {
2) The functions added in both places look very similar, can they be merged
and extracted? We are talking about the trigger_invalid_event function.
3) The following change is very similar to the trigger_invalid_event, was
there a reason not to use it?
+++ b/web/pgadmin/static/js/backform.pgadmin.js
@@ -1573,25 +1584,23 @@
this.model.errorModel.unset(name);
this.model.set(name, value);
this.listenTo(this.model, "change:" + name, this.render);
- if (this.model.collection || this.model.handler) {
- (this.model.collection || this.model.handler).trigger(
- 'pgadmin-session:model:valid', this.model,
(this.model.collection || this.model.handler)
- );
+ // Check if other fields of same model are valid before
+ // triggering 'session:valid' event
+ if(_.size(this.model.errorModel.attributes) == 0) {
+ if (this.model.collection || this.model.handler) {
+ (this.model.collection || this.model.handler).trigger(
+ 'pgadmin-session:model:valid', this.model,
(this.model.collection || this.model.handler)
+ );
+ } else {
+ (this.model).trigger(
+ 'pgadmin-session:valid', this.model.sessChanged(), this.model
+ );
+ }
4) We also noticed that the following change sets look very similiar. Is
there any reason to have this code duplicated? If not this could be a good
time to refactor it.
+++ b/web/pgadmin/static/js/backform.pgadmin.js
@@ -1528,7 +1528,18 @@
@@ -1573,25 +1584,23 @@
@@ -1631,7 +1640,18 @@
@@ -1676,25 +1696,23 @@
Thanks
Joao & Shruti
On Thu, May 18, 2017 at 6:01 AM, Harshal Dhumal <
[email protected]> wrote:
> Hi,
>
> Please find attached patch for RM2421
>
> Issue fixed: 1. Integer/numeric Validation is not working properly.
> 2. Wrong CPU rate unit
> --
> *Harshal Dhumal*
> *Sr. Software Engineer*
>
> EnterpriseDB India: 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
>
>
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Fix for RM2421 [pgAdmin4][patch]
2017-05-18 10:01 Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-05-18 14:27 ` Re: Fix for RM2421 [pgAdmin4][patch] Joao Pedro De Almeida Pereira <[email protected]>
@ 2017-05-19 06:52 ` Harshal Dhumal <[email protected]>
2017-05-29 09:48 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Harshal Dhumal @ 2017-05-19 06:52 UTC (permalink / raw)
To: Joao Pedro De Almeida Pereira <[email protected]>; +Cc: pgadmin-hackers; Shruti B Iyer <[email protected]>
Hi,
On Thu, May 18, 2017 at 7:57 PM, Joao Pedro De Almeida Pereira <
[email protected]> wrote:
> Hello Harshal,
>
> We review the patch and have some questions:
> 1) Is there any particular reason to initialize variables and functions in
> the same place? We believe that it would be more readable there were no
> chaining of variable creation, specially if those variables are functions.
> Check line:
>
That function is only going to be used in checkNumeric function (in case of
Number control) and checkInt function (in case of Integer control) so
declared them locally.
Anyway I'm going to refactor both the controls as Number and Integer shares
some common properties.
+++ b/web/pgadmin/static/js/backform.pgadmin.js
> @@ -1528,7 +1528,18 @@
> max_value = field.max,
> isValid = true,
> intPattern = new RegExp("^-?[0-9]*$"),
> - isMatched = intPattern.test(value);
> + isMatched = intPattern.test(value),
> + trigger_invalid_event = function(msg) {
>
>
> 2) The functions added in both places look very similar, can they be
> merged and extracted? We are talking about the trigger_invalid_event
> function.
>
Yes they can be merged. As of now both NumericControl and IntegerControl
are derived from InputControl. Ideally
only NumericControl should be derived from InputControl and IntegerControl
should be derive from NumericControl.
> 3) The following change is very similar to the trigger_invalid_event, was
> there a reason not to use it?
>
Below code triggers "model valid" event; opposite to "model invalid" event (
trigger_invalid_event)
> +++ b/web/pgadmin/static/js/backform.pgadmin.js
> @@ -1573,25 +1584,23 @@
> this.model.errorModel.unset(name);
> this.model.set(name, value);
> this.listenTo(this.model, "change:" + name, this.render);
> - if (this.model.collection || this.model.handler) {
> - (this.model.collection || this.model.handler).trigger(
> - 'pgadmin-session:model:valid', this.model, (this.model.collection || this.model.handler)
> - );
> + // Check if other fields of same model are valid before
> + // triggering 'session:valid' event
> + if(_.size(this.model.errorModel.attributes) == 0) {
> + if (this.model.collection || this.model.handler) {
> + (this.model.collection || this.model.handler).trigger(
> + 'pgadmin-session:model:valid', this.model, (this.model.collection || this.model.handler)
> + );
> + } else {
> + (this.model).trigger(
> + 'pgadmin-session:valid', this.model.sessChanged(), this.model
> + );
> + }
>
>
> 4) We also noticed that the following change sets look very similiar. Is
> there any reason to have this code duplicated? If not this could be a good
> time to refactor it.
>
As said earlier in response of point 2 code duplication is because the way
controls are derived.
> +++ b/web/pgadmin/static/js/backform.pgadmin.js
> @@ -1528,7 +1528,18 @@
>
> @@ -1573,25 +1584,23 @@
>
> @@ -1631,7 +1640,18 @@
>
> @@ -1676,25 +1696,23 @@
>
>
>
> Thanks
> Joao & Shruti
>
> On Thu, May 18, 2017 at 6:01 AM, Harshal Dhumal <
> [email protected]> wrote:
>
>> Hi,
>>
>> Please find attached patch for RM2421
>>
>> Issue fixed: 1. Integer/numeric Validation is not working properly.
>> 2. Wrong CPU rate unit
>> --
>> *Harshal Dhumal*
>> *Sr. Software Engineer*
>>
>> EnterpriseDB India: 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
>>
>>
>
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Fix for RM2421 [pgAdmin4][patch]
2017-05-18 10:01 Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-05-18 14:27 ` Re: Fix for RM2421 [pgAdmin4][patch] Joao Pedro De Almeida Pereira <[email protected]>
2017-05-19 06:52 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
@ 2017-05-29 09:48 ` Harshal Dhumal <[email protected]>
2017-05-30 07:00 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Harshal Dhumal @ 2017-05-29 09:48 UTC (permalink / raw)
To: Joao Pedro De Almeida Pereira <[email protected]>; +Cc: pgadmin-hackers; Shruti B Iyer <[email protected]>
Hi,
Here is updated patch for RM2421.
Now I have moved all Numeric control level validations to datamodel. As
existing implementation was causing
issues with error messages in create/edit dialog when schema contains two
or more Numeric controls.
This is generic issue and not related to resource group. Also I have
updated all other nodes which uses Numeric controls
--
*Harshal Dhumal*
*Sr. Software Engineer*
EnterpriseDB India: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Fri, May 19, 2017 at 12:22 PM, Harshal Dhumal <
[email protected]> wrote:
> Hi,
>
> On Thu, May 18, 2017 at 7:57 PM, Joao Pedro De Almeida Pereira <
> [email protected]> wrote:
>
>> Hello Harshal,
>>
>> We review the patch and have some questions:
>> 1) Is there any particular reason to initialize variables and functions
>> in the same place? We believe that it would be more readable there were no
>> chaining of variable creation, specially if those variables are functions.
>> Check line:
>>
> That function is only going to be used in checkNumeric function (in case
> of Number control) and checkInt function (in case of Integer control) so
> declared them locally.
> Anyway I'm going to refactor both the controls as Number and Integer
> shares some common properties.
>
> +++ b/web/pgadmin/static/js/backform.pgadmin.js
>> @@ -1528,7 +1528,18 @@
>> max_value = field.max,
>> isValid = true,
>> intPattern = new RegExp("^-?[0-9]*$"),
>> - isMatched = intPattern.test(value);
>> + isMatched = intPattern.test(value),
>> + trigger_invalid_event = function(msg) {
>>
>>
>> 2) The functions added in both places look very similar, can they be
>> merged and extracted? We are talking about the trigger_invalid_event
>> function.
>>
> Yes they can be merged. As of now both NumericControl and IntegerControl
> are derived from InputControl. Ideally
> only NumericControl should be derived from InputControl and IntegerControl
> should be derive from NumericControl.
>
>
>
>> 3) The following change is very similar to the trigger_invalid_event,
>> was there a reason not to use it?
>>
> Below code triggers "model valid" event; opposite to "model invalid" event
> (trigger_invalid_event)
>
>> +++ b/web/pgadmin/static/js/backform.pgadmin.js
>> @@ -1573,25 +1584,23 @@
>> this.model.errorModel.unset(name);
>> this.model.set(name, value);
>> this.listenTo(this.model, "change:" + name, this.render);
>> - if (this.model.collection || this.model.handler) {
>> - (this.model.collection || this.model.handler).trigger(
>> - 'pgadmin-session:model:valid', this.model, (this.model.collection || this.model.handler)
>> - );
>> + // Check if other fields of same model are valid before
>> + // triggering 'session:valid' event
>> + if(_.size(this.model.errorModel.attributes) == 0) {
>> + if (this.model.collection || this.model.handler) {
>> + (this.model.collection || this.model.handler).trigger(
>> + 'pgadmin-session:model:valid', this.model, (this.model.collection || this.model.handler)
>> + );
>> + } else {
>> + (this.model).trigger(
>> + 'pgadmin-session:valid', this.model.sessChanged(), this.model
>> + );
>> + }
>>
>>
>> 4) We also noticed that the following change sets look very similiar. Is
>> there any reason to have this code duplicated? If not this could be a good
>> time to refactor it.
>>
> As said earlier in response of point 2 code duplication is because the way
> controls are derived.
>
>
>> +++ b/web/pgadmin/static/js/backform.pgadmin.js
>> @@ -1528,7 +1528,18 @@
>>
>> @@ -1573,25 +1584,23 @@
>>
>> @@ -1631,7 +1640,18 @@
>>
>> @@ -1676,25 +1696,23 @@
>>
>>
>>
>> Thanks
>> Joao & Shruti
>>
>> On Thu, May 18, 2017 at 6:01 AM, Harshal Dhumal <
>> [email protected]> wrote:
>>
>>> Hi,
>>>
>>> Please find attached patch for RM2421
>>>
>>> Issue fixed: 1. Integer/numeric Validation is not working properly.
>>> 2. Wrong CPU rate unit
>>> --
>>> *Harshal Dhumal*
>>> *Sr. Software Engineer*
>>>
>>> EnterpriseDB India: 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
>>>
>>>
>>
>
--
Sent via pgadmin-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers
Attachments:
[text/x-patch] RM2421_V2.patch (32.8K, 3-RM2421_V2.patch)
download | inline diff:
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/foreign_tables/templates/foreign_tables/js/foreign_tables.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/foreign_tables/templates/foreign_tables/js/foreign_tables.js
index 5f3dc69..b7df585 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/foreign_tables/templates/foreign_tables/js/foreign_tables.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/foreign_tables/templates/foreign_tables/js/foreign_tables.js
@@ -101,34 +101,56 @@ function($, _, S, pgAdmin, pgBrowser, alertify) {
cell: 'string', group: '{{ _('Definition') }}',
type: 'int', deps: ['datatype'],
disabled: function(m) {
- // We will store type from selected from combobox
+ var val = m.get('typlen');
+ // We will store type from selected from combobox
if(!(_.isUndefined(m.get('inheritedid'))
|| _.isNull(m.get('inheritedid'))
|| _.isUndefined(m.get('inheritedfrom'))
- || _.isNull(m.get('inheritedfrom')))) { return true; }
+ || _.isNull(m.get('inheritedfrom')))) {
- var of_type = m.get('datatype');
- if(m.type_options) {
- m.set('is_tlength', false, {silent: true});
+ if (!_.isUndefined(val)) {
+ setTimeout(function() {
+ m.set('typlen', undefined);
+ }, 10);
+ }
+ return true;
+ }
+
+ var of_type = m.get('datatype'),
+ has_length = false;
+ if(m.type_options) {
+ m.set('is_tlength', false, {silent: true});
- // iterating over all the types
- _.each(m.type_options, function(o) {
- // if type from selected from combobox matches in options
- if ( of_type == o.value ) {
- m.set('typlen', undefined);
+ // iterating over all the types
+ _.each(m.type_options, function(o) {
+ // if type from selected from combobox matches in options
+ if ( of_type == o.value ) {
// if length is allowed for selected type
if(o.length)
{
// set the values in model
+ has_length = true;
m.set('is_tlength', true, {silent: true});
m.set('min_val', o.min_val, {silent: true});
m.set('max_val', o.max_val, {silent: true});
}
+ }
+ });
+
+ if (!has_length && !_.isUndefined(val)) {
+ setTimeout(function() {
+ m.set('typlen', undefined);
+ }, 10);
}
- });
- return !(m.get('is_tlength'));
- }
- return true;
+
+ return !(m.get('is_tlength'));
+ }
+ if (!has_length && !_.isUndefined(val)) {
+ setTimeout(function() {
+ m.set('typlen', undefined);
+ }, 10);
+ }
+ return true;
},
cellHeaderClasses: 'width_percent_10'
},{
@@ -136,33 +158,54 @@ function($, _, S, pgAdmin, pgBrowser, alertify) {
type: 'int', deps: ['datatype'],
cell: 'string', group: '{{ _('Definition') }}',
disabled: function(m) {
+ var val = m.get('precision');
if(!(_.isUndefined(m.get('inheritedid'))
|| _.isNull(m.get('inheritedid'))
|| _.isUndefined(m.get('inheritedfrom'))
- || _.isNull(m.get('inheritedfrom')))) { return true; }
+ || _.isNull(m.get('inheritedfrom')))) {
+
+ if (!_.isUndefined(val)) {
+ setTimeout(function() {
+ m.set('precision', undefined);
+ }, 10);
+ }
+ return true;
+ }
+
+ var of_type = m.get('datatype'),
+ has_precision = false;
- var of_type = m.get('datatype');
if(m.type_options) {
m.set('is_precision', false, {silent: true});
// iterating over all the types
_.each(m.type_options, function(o) {
// if type from selected from combobox matches in options
if ( of_type == o.value ) {
- m.set('precision', undefined);
// if precession is allowed for selected type
if(o.precision)
{
+ has_precision = true;
// set the values in model
m.set('is_precision', true, {silent: true});
m.set('min_val', o.min_val, {silent: true});
m.set('max_val', o.max_val, {silent: true});
}
+ }
+ });
+ if (!has_precision && !_.isUndefined(val)) {
+ setTimeout(function() {
+ m.set('precision', undefined);
+ }, 10);
}
- });
- return !(m.get('is_precision'));
- }
- return true;
- }, cellHeaderClasses: 'width_percent_10'
+ return !(m.get('is_precision'));
+ }
+ if (!has_precision && !_.isUndefined(val)) {
+ setTimeout(function() {
+ m.set('precision', undefined);
+ }, 10);
+ }
+ return true;
+ }, cellHeaderClasses: 'width_percent_10'
},{
id: 'typdefault', label:'{{ _('Default') }}', type: 'text',
cell: 'string', min_version: 90300, group: '{{ _('Definition') }}',
@@ -216,22 +259,23 @@ function($, _, S, pgAdmin, pgBrowser, alertify) {
min_version: 90200
}],
validate: function() {
- var err = {},
- errmsg;
+ var errmsg = null;
if (_.isUndefined(this.get('attname')) || String(this.get('attname')).replace(/^\s+|\s+$/g, '') == '') {
- err['name'] = '{{ _('Column Name cannot be empty!') }}';
- errmsg = errmsg || err['attname'];
+ errmsg = '{{ _('Column Name cannot be empty!') }}';
+ this.errorModel.set('attname', errmsg);
+ } else {
+ this.errorModel.unset('attname');
}
if (_.isUndefined(this.get('datatype')) || String(this.get('datatype'))
.replace(/^\s+|\s+$/g, '') == '') {
- err['basensp'] = '{{ _('Column Datatype cannot be empty!') }}';
- errmsg = errmsg || err['datatype'];
+ errmsg = '{{ _('Column Datatype cannot be empty!') }}';
+ this.errorModel.set('datatype', errmsg);
+ } else {
+ this.errorModel.unset('datatype');
}
- this.errorModel.clear().set(err);
-
return errmsg;
},
is_editable_column: function(m) {
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/sequences/templates/sequence/js/sequence.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/sequences/templates/sequence/js/sequence.js
index 2fb7ae5..1b9db50 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/sequences/templates/sequence/js/sequence.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/sequences/templates/sequence/js/sequence.js
@@ -153,7 +153,10 @@ function($, _, S, pgAdmin, pgBrowser, alertify) {
min: 1
},{
id: 'start', label: '{{ _('Start') }}', type: 'int',
- mode: ['properties', 'create'], group: '{{ _('Definition') }}'
+ mode: ['properties', 'create', 'edit'], group: '{{ _('Definition') }}',
+ disabled: function(m) {
+ return !m.isNew();
+ }
},{
id: 'minimum', label: '{{ _('Minimum') }}', type: 'int',
mode: ['properties', 'create', 'edit'], group: '{{ _('Definition') }}'
@@ -199,14 +202,14 @@ function($, _, S, pgAdmin, pgBrowser, alertify) {
minimum = this.get('minimum'),
maximum = this.get('maximum');
start = this.get('start');
- // Clear any existing error msg.
- this.errorModel.clear();
if (_.isUndefined(this.get('name'))
|| String(this.get('name')).replace(/^\s+|\s+$/g, '') == '') {
msg = '{{ _('Name cannot be empty.') }}';
this.errorModel.set('name', msg);
return msg;
+ } else {
+ this.errorModel.unset('name');
}
if (_.isUndefined(this.get('seqowner'))
@@ -214,6 +217,8 @@ function($, _, S, pgAdmin, pgBrowser, alertify) {
msg = '{{ _('Owner cannot be empty.') }}';
this.errorModel.set('seqowner', msg);
return msg;
+ } else {
+ this.errorModel.unset('seqowner');
}
if (_.isUndefined(this.get('schema'))
@@ -221,26 +226,81 @@ function($, _, S, pgAdmin, pgBrowser, alertify) {
msg = '{{ _('Schema cannot be empty.') }}';
this.errorModel.set('schema', msg);
return msg;
+ } else {
+ this.errorModel.unset('schema');
+ }
+
+ if (!this.isNew()) {
+ if (_.isUndefined(this.get('current_value'))
+ || String(this.get('current_value')).replace(/^\s+|\s+$/g, '') == '') {
+ msg = '{{ _('Current value cannot be empty.') }}';
+ this.errorModel.set('current_value', msg);
+ return msg;
+ } else {
+ this.errorModel.unset('current_value');
+ }
+
+ if (_.isUndefined(this.get('increment'))
+ || String(this.get('increment')).replace(/^\s+|\s+$/g, '') == '') {
+ msg = '{{ _('Increment value cannot be empty.') }}';
+ this.errorModel.set('increment', msg);
+ return msg;
+ } else {
+ this.errorModel.unset('increment');
+ }
+
+ if (_.isUndefined(this.get('minimum'))
+ || String(this.get('minimum')).replace(/^\s+|\s+$/g, '') == '') {
+ msg = '{{ _('Minimum value cannot be empty.') }}';
+ this.errorModel.set('minimum', msg);
+ return msg;
+ } else {
+ this.errorModel.unset('minimum');
+ }
+
+ if (_.isUndefined(this.get('maximum'))
+ || String(this.get('maximum')).replace(/^\s+|\s+$/g, '') == '') {
+ msg = '{{ _('Maximum value cannot be empty.') }}';
+ this.errorModel.set('maximum', msg);
+ return msg;
+ } else {
+ this.errorModel.unset('maximum');
+ }
+
+ if (_.isUndefined(this.get('cache'))
+ || String(this.get('cache')).replace(/^\s+|\s+$/g, '') == '') {
+ msg = '{{ _('Cache value cannot be empty.') }}';
+ this.errorModel.set('cache', msg);
+ return msg;
+ } else {
+ this.errorModel.unset('cache');
+ }
}
var min_lt = '{{ _('Minimum value must be less than maximum value.') }}',
start_lt = '{{ _('Start value cannot be less than minimum value.') }}',
start_gt = '{{ _('Start value cannot be greater than maximum value.') }}';
+
if ((minimum == 0 && maximum == 0) ||
(parseInt(minimum, 10) >= parseInt(maximum, 10))) {
- msg = min_lt
- this.errorModel.set('minimum', msg);
- return msg;
+ this.errorModel.set('minimum', min_lt);
+ return min_lt;
+ } else {
+ this.errorModel.unset('minimum');
}
- else if (start < minimum) {
- msg = start_lt
- this.errorModel.set('start', msg);
- return msg;
+
+ if (start && minimum && parseInt(start) < parseInt(minimum)) {
+ this.errorModel.set('start', start_lt);
+ return start_lt;
+ } else {
+ this.errorModel.unset('start');
}
- else if (start > maximum) {
- msg = start_gt
- this.errorModel.set('start', msg);
- return msg;
+
+ if (start && maximum && parseInt(start) > parseInt(maximum)) {
+ this.errorModel.set('start', start_gt);
+ return start_gt;
+ } else {
+ this.errorModel.unset('start');
}
return null;
}
diff --git a/web/pgadmin/browser/server_groups/servers/resource_groups/templates/resource_groups/js/resource_groups.js b/web/pgadmin/browser/server_groups/servers/resource_groups/templates/resource_groups/js/resource_groups.js
index 3cc822e..26e63bf 100644
--- a/web/pgadmin/browser/server_groups/servers/resource_groups/templates/resource_groups/js/resource_groups.js
+++ b/web/pgadmin/browser/server_groups/servers/resource_groups/templates/resource_groups/js/resource_groups.js
@@ -85,54 +85,35 @@ define(
*/
validate: function(keys) {
- /* Check whether 'name' is present in 'keys', if it is present
- * it means there is a change in that field from the GUI, so we
- * need to validate it.
- */
- if (_.indexOf(keys, 'name') >= 0) {
- var name = this.get('name');
- if (_.isUndefined(name) || _.isNull(name) ||
- String(name).replace(/^\s+|\s+$/g, '') == '') {
- var msg = '{{ _('Name cannot be empty.') }}';
- this.errorModel.set('name', msg);
- return msg;
- } else {
- this.errorModel.unset('name');
- }
+ var name = this.get('name');
+ if (_.isUndefined(name) || _.isNull(name) ||
+ String(name).replace(/^\s+|\s+$/g, '') == '') {
+ var msg = '{{ _('Name cannot be empty.') }}';
+ this.errorModel.set('name', msg);
+ return msg;
+ } else {
+ this.errorModel.unset('name');
}
- /* Check whether 'cpu_rate_limit' is present in 'keys', if it is present
- * it means there is a change in that field from the GUI, so we
- * need to validate it.
- */
- if (_.indexOf(keys, 'cpu_rate_limit') >= 0) {
- var cpu_rate_limit = this.get('cpu_rate_limit');
- if (_.isUndefined(cpu_rate_limit) || _.isNull(cpu_rate_limit) ||
- String(cpu_rate_limit).replace(/^\s+|\s+$/g, '') == '') {
- var msg = '{{ _('CPU rate limit cannot be empty.') }}';
- this.errorModel.set('cpu_rate_limit', msg);
- return msg;
- } else {
- this.errorModel.unset('cpu_rate_limit');
- }
+ var cpu_rate_limit = this.get('cpu_rate_limit');
+ if (_.isUndefined(cpu_rate_limit) || _.isNull(cpu_rate_limit) ||
+ String(cpu_rate_limit).replace(/^\s+|\s+$/g, '') == '') {
+ var msg = '{{ _('CPU rate limit cannot be empty.') }}';
+ this.errorModel.set('cpu_rate_limit', msg);
+ return msg;
+ } else {
+ this.errorModel.unset('cpu_rate_limit');
}
- /* Check whether 'dirty_rate_limit' is present in 'keys', if it is present
- * it means there is a change in that field from the GUI, so we
- * need to validate it.
- */
- if (_.indexOf(keys, 'dirty_rate_limit') >= 0) {
- var dirty_rate_limit = this.get('dirty_rate_limit');
- if (_.isUndefined(dirty_rate_limit) || _.isNull(dirty_rate_limit) ||
- String(dirty_rate_limit).replace(/^\s+|\s+$/g, '') == '') {
- var msg = '{{ _('Dirty rate limit cannot be empty.') }}';
- this.errorModel.set('dirty_rate_limit', msg);
- return msg;
- } else {
- this.errorModel.unset('dirty_rate_limit');
- }
+ var dirty_rate_limit = this.get('dirty_rate_limit');
+ if (_.isUndefined(dirty_rate_limit) || _.isNull(dirty_rate_limit) ||
+ String(dirty_rate_limit).replace(/^\s+|\s+$/g, '') == '') {
+ var msg = '{{ _('Dirty rate limit cannot be empty.') }}';
+ this.errorModel.set('dirty_rate_limit', msg);
+ return msg;
+ } else {
+ this.errorModel.unset('dirty_rate_limit');
}
-
return null;
}
})
diff --git a/web/pgadmin/browser/server_groups/servers/roles/templates/role/js/role.js b/web/pgadmin/browser/server_groups/servers/roles/templates/role/js/role.js
index a85416c..c74cd59 100644
--- a/web/pgadmin/browser/server_groups/servers/roles/templates/role/js/role.js
+++ b/web/pgadmin/browser/server_groups/servers/roles/templates/role/js/role.js
@@ -427,7 +427,7 @@ function($, _, S, pgAdmin, pgBrowser, alertify, Backform) {
deps: ['rolcanlogin'], options: {format: 'YYYY-MM-DD HH:mm:ss Z'}
},{
id: 'rolconnlimit', type: 'int', group: '{{ _('Definition') }}',
- label: '{{ _('Connection limit') }}', cell: 'number',
+ label: '{{ _('Connection limit') }}', cell: 'number', min : -1,
mode: ['properties', 'edit', 'create'], disabled: 'readonly'
},{
id: 'rolcanlogin', label:'{{ _('Can login?') }}', type: 'switch',
diff --git a/web/pgadmin/browser/server_groups/servers/templates/servers/servers.js b/web/pgadmin/browser/server_groups/servers/templates/servers/servers.js
index 9eb82b8..bf28474 100644
--- a/web/pgadmin/browser/server_groups/servers/templates/servers/servers.js
+++ b/web/pgadmin/browser/server_groups/servers/templates/servers/servers.js
@@ -727,6 +727,9 @@ function($, _, S, pgAdmin, pgBrowser, alertify) {
check_for_empty(
'username', '{{ _('Username must be specified.') }}'
);
+ check_for_empty(
+ 'port', '{{ _('Port must be specified.') }}'
+ );
this.errorModel.set(err);
if (_.size(err)) {
diff --git a/web/pgadmin/browser/static/js/datamodel.js b/web/pgadmin/browser/static/js/datamodel.js
index 5b1c3a7..c3b5fac 100644
--- a/web/pgadmin/browser/static/js/datamodel.js
+++ b/web/pgadmin/browser/static/js/datamodel.js
@@ -1,6 +1,6 @@
define(
- ['underscore', 'pgadmin', 'jquery', 'backbone'],
-function(_, pgAdmin, $, Backbone) {
+ ['underscore', 'underscore.string', 'pgadmin', 'jquery', 'backbone'],
+function(_, S, pgAdmin, $, Backbone) {
var pgBrowser = pgAdmin.Browser = pgAdmin.Browser || {};
pgBrowser.DataModel = Backbone.Model.extend({
@@ -136,6 +136,7 @@ function(_, pgAdmin, $, Backbone) {
}
self.sessAttrs = {};
+ self.fieldData = {};
self.origSessAttrs = {};
self.objects = [];
self.arrays = [];
@@ -153,6 +154,23 @@ function(_, pgAdmin, $, Backbone) {
_.each(schema, function(s) {
switch(s.type) {
+ case 'int':
+ case 'numeric':
+ self.fieldData[s.id] = {
+ label: s.id,
+ type: s.type,
+ min: s.min || undefined,
+ max: s.max || undefined
+ }
+ break;
+ default:
+ self.fieldData[s.id] = {
+ label: s.id,
+ type: s.type
+ }
+ }
+
+ switch(s.type) {
case 'array':
self.arrays.push(s.id);
@@ -280,6 +298,12 @@ function(_, pgAdmin, $, Backbone) {
},
sessValid: function() {
var self = this;
+ // Perform default validations.
+ if ('default_validate' in self && typeof(self.default_validate) == 'function' &&
+ _.isString(self.default_validate())) {
+ return false;
+ }
+
if ('validate' in self && _.isFunction(self.validate) &&
_.isString(self.validate.apply(self))) {
return false;
@@ -301,8 +325,9 @@ function(_, pgAdmin, $, Backbone) {
}
if (key != null && res) {
- var attrs = {};
- var self = this;
+ var attrs = {},
+ self = this,
+ msg;
attrChanged = function(v, k) {
if (k in self.objects) {
@@ -327,9 +352,18 @@ function(_, pgAdmin, $, Backbone) {
if (!options || !options.silent) {
self.trigger('change', self, options);
}
+
+ // Perform default validations.
+
+ if ('default_validate' in self && typeof(self.default_validate) == 'function') {
+ msg = self.default_validate();
+ }
+
if ('validate' in self && typeof(self['validate']) === 'function') {
- var msg = self.validate(_.keys(attrs));
+ if (!msg) {
+ msg = self.validate(_.keys(attrs));
+ }
/*
* If any parent present, we will need to inform the parent - that
@@ -562,6 +596,13 @@ function(_, pgAdmin, $, Backbone) {
var msg = null,
validate = function(m, attrs) {
+ if ('default_validate' in m && typeof(m.default_validate) == 'function') {
+ msg = m.default_validate();
+ if (_.isString(msg)) {
+ return msg;
+ }
+ }
+
if ('validate' in m && typeof(m.validate) == 'function') {
msg = m.validate(attrs);
@@ -655,6 +696,79 @@ function(_, pgAdmin, $, Backbone) {
});
self.trigger('pgadmin-session:stop');
+ },
+ default_validate: function() {
+ var msg, field, value, type;
+
+ for (var i = 0, keys = _.keys(this.attributes), l = keys.length;
+ i<l;
+ i++) {
+
+ value = this.attributes[keys[i]];
+ field = this.fieldData[keys[i]]
+ msg = null;
+
+ if (!(_.isUndefined(value) || _.isNull(value) ||
+ String(value).replace(/^\s+|\s+$/g, '') == '')) {
+
+ if (!field) {
+ continue;
+ }
+
+ type = field.type || undefined;
+ if (!type) {
+ continue;
+ }
+
+ switch(type) {
+ case 'int':
+ msg = this.integer_validate(value, field);
+ break;
+ case 'numeric':
+ msg = this.number_validate(value, field);
+ break;
+ }
+
+ if (msg) {
+ this.errorModel.set(field.label, msg);
+ return msg;
+ } else {
+ this.errorModel.unset(field.label);
+ }
+ } else {
+ if (field) {
+ this.errorModel.unset(field.label);
+ }
+ }
+ }
+ return null;
+ },
+
+ check_min_max: function (value, field) {
+ var label = field.label,
+ min_value = field.min,
+ max_value = field.max;
+
+ if (min_value && value < min_value) {
+ return S(pgAdmin.Browser.messages.MUST_GR_EQ).sprintf(label, min_value).value();
+ } else if (max_value && value > max_value) {
+ return S(pgAdmin.Browser.messages.MUST_LESS_EQ).sprintf(label, max_value).value();
+ }
+ return null;
+ },
+ number_validate: function (value, field) {
+ var pattern = new RegExp("^-?[0-9]+(\.?[0-9]*)?$");
+ if (!pattern.test(value)) {
+ return S(pgAdmin.Browser.messages.MUST_BE_NUM).sprintf(field.label).value()
+ }
+ return this.check_min_max(value, field)
+ },
+ integer_validate: function(value, field) {
+ var pattern = new RegExp("^-?[0-9]*$");
+ if (!pattern.test(value)) {
+ return S(pgAdmin.Browser.messages.MUST_BE_INT).sprintf(field.label).value()
+ }
+ return this.check_min_max(value, field)
}
});
@@ -696,7 +810,8 @@ function(_, pgAdmin, $, Backbone) {
return self;
},
startNewSession: function() {
- var self = this;
+ var self = this,
+ msg;
if (self.trackChanges) {
// We're stopping the existing session.
@@ -718,8 +833,15 @@ function(_, pgAdmin, $, Backbone) {
if ('startNewSession' in m && _.isFunction(m.startNewSession)) {
m.startNewSession();
}
- if ('validate' in m && typeof(m.validate) === 'function') {
- var msg = m.validate();
+
+ if ('default_validate' in m && typeof(m.default_validate) == 'function') {
+ msg = m.default_validate();
+ }
+
+ if (_.isString(msg)) {
+ self.sessAttrs['invalid'][m.cid] = msg;
+ } else if ('validate' in m && typeof(m.validate) === 'function') {
+ msg = m.validate();
if (msg) {
self.sessAttrs['invalid'][m.cid] = msg;
@@ -900,7 +1022,14 @@ function(_, pgAdmin, $, Backbone) {
(self.handler || self).trigger('pgadmin-session:added', self, obj);
- if ('validate' in obj && typeof(obj.validate) === 'function') {
+
+ if ('default_validate' in obj && typeof(obj.default_validate) == 'function') {
+ msg = obj.default_validate();
+ }
+
+ if (_.isString(msg)) {
+ (self.sessAttrs['invalid'])[obj.cid] = msg;
+ } else if ('validate' in obj && typeof(obj.validate) === 'function') {
msg = obj.validate();
if (msg) {
@@ -908,7 +1037,14 @@ function(_, pgAdmin, $, Backbone) {
}
}
} else {
- if ('validate' in obj && typeof(obj.validate) === 'function') {
+
+ if ('default_validate' in obj && typeof(obj.default_validate) == 'function') {
+ msg = obj.default_validate();
+ }
+
+ if (_.isString(msg)) {
+ (self.sessAttrs['invalid'])[obj.cid] = msg;
+ } else if ('validate' in obj && typeof(obj.validate) === 'function') {
msg = obj.validate();
if (msg) {
diff --git a/web/pgadmin/static/js/backform.pgadmin.js b/web/pgadmin/static/js/backform.pgadmin.js
index 553676e..cf41297 100644
--- a/web/pgadmin/static/js/backform.pgadmin.js
+++ b/web/pgadmin/static/js/backform.pgadmin.js
@@ -1491,115 +1491,13 @@
Backform.Control.__super__.remove.apply(this, arguments);
}
-});
-
- /*
- * Integer input Control functionality just like backgrid
- */
- var IntegerControl = Backform.IntegerControl = Backform.InputControl.extend({
- defaults: {
- type: "number",
- label: "",
- min: undefined,
- max: undefined,
- maxlength: 255,
- extraClasses: [],
- helpMessage: null
- },
- template: _.template([
- '<label class="<%=Backform.controlLabelClassName%>"><%=label%></label>',
- '<div class="<%=Backform.controlsClassName%>">',
- ' <input type="<%=type%>" class="<%=Backform.controlClassName%> <%=extraClasses.join(\' \')%>" name="<%=name%>" min="<%=min%>" max="<%=max%>"maxlength="<%=maxlength%>" value="<%-value%>" placeholder="<%-placeholder%>" <%=disabled ? "disabled" : ""%> <%=required ? "required" : ""%> />',
- ' <% if (helpMessage && helpMessage.length) { %>',
- ' <span class="<%=Backform.helpMessageClassName%>"><%=helpMessage%></span>',
- ' <% } %>',
- '</div>'
- ].join("\n")),
- events: {
- "change input": "checkInt",
- "focus input": "clearInvalid"
- },
- checkInt: function(e) {
- var field = _.defaults(this.field.toJSON(), this.defaults),
- attrArr = this.field.get("name").split('.'),
- name = attrArr.shift(),
- value = this.getValueFromDOM(),
- min_value = field.min,
- max_value = field.max,
- isValid = true,
- intPattern = new RegExp("^-?[0-9]*$"),
- isMatched = intPattern.test(value);
-
- // Below logic will validate input
- if (!isMatched) {
- isValid = false;
- this.model.errorModel.unset(name);
- this.model.errorModel.set(
- name,
- S(pgAdmin.Browser.messages.MUST_BE_INT).sprintf(
- field.label
- ).value()
- );
- }
-
- // Below will check if entered value is in-between min & max range
- if (isValid && (!_.isUndefined(min_value) && value < min_value)) {
- isValid = false;
- this.model.errorModel.unset(name);
- this.model.errorModel.set(
- name,
- S(pgAdmin.Browser.messages.MUST_GR_EQ).sprintf(
- field.label,
- min_value
- ).value()
- );
- }
-
- if (isValid && (!_.isUndefined(max_value) && value > max_value)) {
- isValid = false;
- this.model.errorModel.unset(name);
- this.model.errorModel.set(
- name,
- S(pgAdmin.Browser.messages.MUST_LESS_EQ).sprintf(
- field.label,
- max_value
- ).value()
- );
- }
-
- // After validation we need to set that value into model (only if all flags are true)
- if (isValid) {
- this.stopListening(this.model, "change:" + name, this.render);
- this.model.errorModel.unset(name);
- this.model.set(name, value);
- this.listenTo(this.model, "change:" + name, this.render);
- if (this.model.collection || this.model.handler) {
- (this.model.collection || this.model.handler).trigger(
- 'pgadmin-session:model:valid', this.model, (this.model.collection || this.model.handler)
- );
- } else {
- (this.model).trigger(
- 'pgadmin-session:valid', this.model.sessChanged(), this.model
- );
- }
- } else {
- if (this.model.collection || this.model.handler) {
- (this.model.collection || this.model.handler).trigger(
- 'pgadmin-session:model:invalid', this.model.errorModel.get(name), this.model
- );
- } else {
- (this.model).trigger(
- 'pgadmin-session:invalid', this.model.errorModel.get(name), this.model
- );
- }
- }
- }
});
/*
* Numeric input Control functionality just like backgrid
*/
var NumericControl = Backform.NumericControl = Backform.InputControl.extend({
+
defaults: {
type: "number",
label: "",
@@ -1617,86 +1515,7 @@
' <span class="<%=Backform.helpMessageClassName%>"><%=helpMessage%></span>',
' <% } %>',
'</div>'
- ].join("\n")),
- events: {
- "change input": "checkNumeric",
- "focus input": "clearInvalid"
- },
- checkNumeric: function(e) {
- var field = _.defaults(this.field.toJSON(), this.defaults),
- attrArr = this.field.get("name").split('.'),
- name = attrArr.shift(),
- value = this.getValueFromDOM(),
- min_value = field.min,
- max_value = field.max,
- isValid = true,
- intPattern = new RegExp("^-?[0-9]+(\.?[0-9]*)?$"),
- isMatched = intPattern.test(value);
-
- // Below logic will validate input
- if (!isMatched) {
- isValid = false;
- this.model.errorModel.unset(name);
- this.model.errorModel.set(
- name,
- S(pgAdmin.Browser.messages.MUST_BE_NUM).sprintf(
- field.label
- ).value()
- );
- }
-
- // Below will check if entered value is in-between min & max range
- if (isValid && (!_.isUndefined(min_value) && value < min_value)) {
- isValid = false;
- this.model.errorModel.unset(name);
- this.model.errorModel.set(
- name,
- S(pgAdmin.Browser.messages.MUST_GR_EQ).sprintf(
- field.label,
- min_value
- ).value()
- );
- }
-
- if (isValid && (!_.isUndefined(max_value) && value > max_value)) {
- isValid = false;
- this.model.errorModel.unset(name);
- this.model.errorModel.set(
- name,
- S(pgAdmin.Browser.messages.MUST_LESS_EQ).sprintf(
- field.label,
- max_value
- ).value()
- );
- }
-
- // After validation we need to set that value into model (only if all flags are true)
- if (isValid) {
- this.stopListening(this.model, "change:" + name, this.render);
- this.model.errorModel.unset(name);
- this.model.set(name, value);
- this.listenTo(this.model, "change:" + name, this.render);
- if (this.model.collection || this.model.handler) {
- (this.model.collection || this.model.handler).trigger(
- 'pgadmin-session:model:valid', this.model, (this.model.collection || this.model.handler)
- );
- } else {
- (this.model).trigger(
- 'pgadmin-session:valid', this.model.sessChanged(), this.model
- );
- }
- } else {
- if (this.model.collection || this.model.handler) {
- (this.model.collection || this.model.handler).trigger(
- 'pgadmin-session:model:invalid', this.model.errorModel.get(name), this.model
- );
- } else {
- (this.model).trigger(
- 'pgadmin-session:invalid', this.model.errorModel.get(name), this.model
- );
- }
- }
- }
+ ].join("\n"))
});
///////
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Fix for RM2421 [pgAdmin4][patch]
2017-05-18 10:01 Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-05-18 14:27 ` Re: Fix for RM2421 [pgAdmin4][patch] Joao Pedro De Almeida Pereira <[email protected]>
2017-05-19 06:52 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-05-29 09:48 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
@ 2017-05-30 07:00 ` Harshal Dhumal <[email protected]>
2017-05-30 07:28 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Harshal Dhumal @ 2017-05-30 07:00 UTC (permalink / raw)
To: Joao Pedro De Almeida Pereira <[email protected]>; +Cc: pgadmin-hackers; Shruti B Iyer <[email protected]>
Hi,
Please ignore this patch as I forgot to include few changes. I'll send
updated one.
--
*Harshal Dhumal*
*Sr. Software Engineer*
EnterpriseDB India: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Mon, May 29, 2017 at 3:18 PM, Harshal Dhumal <
[email protected]> wrote:
> Hi,
>
> Here is updated patch for RM2421.
>
> Now I have moved all Numeric control level validations to datamodel. As
> existing implementation was causing
> issues with error messages in create/edit dialog when schema contains two
> or more Numeric controls.
>
> This is generic issue and not related to resource group. Also I have
> updated all other nodes which uses Numeric controls
>
>
>
> --
> *Harshal Dhumal*
> *Sr. Software Engineer*
>
> EnterpriseDB India: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Fri, May 19, 2017 at 12:22 PM, Harshal Dhumal <
> [email protected]> wrote:
>
>> Hi,
>>
>> On Thu, May 18, 2017 at 7:57 PM, Joao Pedro De Almeida Pereira <
>> [email protected]> wrote:
>>
>>> Hello Harshal,
>>>
>>> We review the patch and have some questions:
>>> 1) Is there any particular reason to initialize variables and functions
>>> in the same place? We believe that it would be more readable there were no
>>> chaining of variable creation, specially if those variables are functions.
>>> Check line:
>>>
>> That function is only going to be used in checkNumeric function (in case
>> of Number control) and checkInt function (in case of Integer control) so
>> declared them locally.
>> Anyway I'm going to refactor both the controls as Number and Integer
>> shares some common properties.
>>
>> +++ b/web/pgadmin/static/js/backform.pgadmin.js
>>> @@ -1528,7 +1528,18 @@
>>> max_value = field.max,
>>> isValid = true,
>>> intPattern = new RegExp("^-?[0-9]*$"),
>>> - isMatched = intPattern.test(value);
>>> + isMatched = intPattern.test(value),
>>> + trigger_invalid_event = function(msg) {
>>>
>>>
>>> 2) The functions added in both places look very similar, can they be
>>> merged and extracted? We are talking about the trigger_invalid_event
>>> function.
>>>
>> Yes they can be merged. As of now both NumericControl and IntegerControl
>> are derived from InputControl. Ideally
>> only NumericControl should be derived from InputControl and
>> IntegerControl should be derive from NumericControl.
>>
>>
>>
>>> 3) The following change is very similar to the trigger_invalid_event,
>>> was there a reason not to use it?
>>>
>> Below code triggers "model valid" event; opposite to "model invalid"
>> event (trigger_invalid_event)
>>
>>> +++ b/web/pgadmin/static/js/backform.pgadmin.js
>>> @@ -1573,25 +1584,23 @@
>>> this.model.errorModel.unset(name);
>>> this.model.set(name, value);
>>> this.listenTo(this.model, "change:" + name, this.render);
>>> - if (this.model.collection || this.model.handler) {
>>> - (this.model.collection || this.model.handler).trigger(
>>> - 'pgadmin-session:model:valid', this.model, (this.model.collection || this.model.handler)
>>> - );
>>> + // Check if other fields of same model are valid before
>>> + // triggering 'session:valid' event
>>> + if(_.size(this.model.errorModel.attributes) == 0) {
>>> + if (this.model.collection || this.model.handler) {
>>> + (this.model.collection || this.model.handler).trigger(
>>> + 'pgadmin-session:model:valid', this.model, (this.model.collection || this.model.handler)
>>> + );
>>> + } else {
>>> + (this.model).trigger(
>>> + 'pgadmin-session:valid', this.model.sessChanged(), this.model
>>> + );
>>> + }
>>>
>>>
>>> 4) We also noticed that the following change sets look very similiar. Is
>>> there any reason to have this code duplicated? If not this could be a good
>>> time to refactor it.
>>>
>> As said earlier in response of point 2 code duplication is because the
>> way controls are derived.
>>
>>
>>> +++ b/web/pgadmin/static/js/backform.pgadmin.js
>>> @@ -1528,7 +1528,18 @@
>>>
>>> @@ -1573,25 +1584,23 @@
>>>
>>> @@ -1631,7 +1640,18 @@
>>>
>>> @@ -1676,25 +1696,23 @@
>>>
>>>
>>>
>>> Thanks
>>> Joao & Shruti
>>>
>>> On Thu, May 18, 2017 at 6:01 AM, Harshal Dhumal <
>>> [email protected]> wrote:
>>>
>>>> Hi,
>>>>
>>>> Please find attached patch for RM2421
>>>>
>>>> Issue fixed: 1. Integer/numeric Validation is not working properly.
>>>> 2. Wrong CPU rate unit
>>>> --
>>>> *Harshal Dhumal*
>>>> *Sr. Software Engineer*
>>>>
>>>> EnterpriseDB India: 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
>>>>
>>>>
>>>
>>
>
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Fix for RM2421 [pgAdmin4][patch]
2017-05-18 10:01 Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-05-18 14:27 ` Re: Fix for RM2421 [pgAdmin4][patch] Joao Pedro De Almeida Pereira <[email protected]>
2017-05-19 06:52 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-05-29 09:48 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-05-30 07:00 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
@ 2017-05-30 07:28 ` Harshal Dhumal <[email protected]>
2017-06-05 15:55 ` Re: Fix for RM2421 [pgAdmin4][patch] Dave Page <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Harshal Dhumal @ 2017-05-30 07:28 UTC (permalink / raw)
To: Joao Pedro De Almeida Pereira <[email protected]>; +Cc: pgadmin-hackers; Shruti B Iyer <[email protected]>
Hi,
Please find updated patch.
--
*Harshal Dhumal*
*Sr. Software Engineer*
EnterpriseDB India: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Tue, May 30, 2017 at 12:30 PM, Harshal Dhumal <
[email protected]> wrote:
> Hi,
>
> Please ignore this patch as I forgot to include few changes. I'll send
> updated one.
>
> --
> *Harshal Dhumal*
> *Sr. Software Engineer*
>
> EnterpriseDB India: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Mon, May 29, 2017 at 3:18 PM, Harshal Dhumal <
> [email protected]> wrote:
>
>> Hi,
>>
>> Here is updated patch for RM2421.
>>
>> Now I have moved all Numeric control level validations to datamodel. As
>> existing implementation was causing
>> issues with error messages in create/edit dialog when schema contains two
>> or more Numeric controls.
>>
>> This is generic issue and not related to resource group. Also I have
>> updated all other nodes which uses Numeric controls
>>
>>
>>
>> --
>> *Harshal Dhumal*
>> *Sr. Software Engineer*
>>
>> EnterpriseDB India: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> On Fri, May 19, 2017 at 12:22 PM, Harshal Dhumal <
>> [email protected]> wrote:
>>
>>> Hi,
>>>
>>> On Thu, May 18, 2017 at 7:57 PM, Joao Pedro De Almeida Pereira <
>>> [email protected]> wrote:
>>>
>>>> Hello Harshal,
>>>>
>>>> We review the patch and have some questions:
>>>> 1) Is there any particular reason to initialize variables and functions
>>>> in the same place? We believe that it would be more readable there were no
>>>> chaining of variable creation, specially if those variables are functions.
>>>> Check line:
>>>>
>>> That function is only going to be used in checkNumeric function (in case
>>> of Number control) and checkInt function (in case of Integer control) so
>>> declared them locally.
>>> Anyway I'm going to refactor both the controls as Number and Integer
>>> shares some common properties.
>>>
>>> +++ b/web/pgadmin/static/js/backform.pgadmin.js
>>>> @@ -1528,7 +1528,18 @@
>>>> max_value = field.max,
>>>> isValid = true,
>>>> intPattern = new RegExp("^-?[0-9]*$"),
>>>> - isMatched = intPattern.test(value);
>>>> + isMatched = intPattern.test(value),
>>>> + trigger_invalid_event = function(msg) {
>>>>
>>>>
>>>> 2) The functions added in both places look very similar, can they be
>>>> merged and extracted? We are talking about the trigger_invalid_event
>>>> function.
>>>>
>>> Yes they can be merged. As of now both NumericControl and IntegerControl
>>> are derived from InputControl. Ideally
>>> only NumericControl should be derived from InputControl and
>>> IntegerControl should be derive from NumericControl.
>>>
>>>
>>>
>>>> 3) The following change is very similar to the trigger_invalid_event,
>>>> was there a reason not to use it?
>>>>
>>> Below code triggers "model valid" event; opposite to "model invalid"
>>> event (trigger_invalid_event)
>>>
>>>> +++ b/web/pgadmin/static/js/backform.pgadmin.js
>>>> @@ -1573,25 +1584,23 @@
>>>> this.model.errorModel.unset(name);
>>>> this.model.set(name, value);
>>>> this.listenTo(this.model, "change:" + name, this.render);
>>>> - if (this.model.collection || this.model.handler) {
>>>> - (this.model.collection || this.model.handler).trigger(
>>>> - 'pgadmin-session:model:valid', this.model, (this.model.collection || this.model.handler)
>>>> - );
>>>> + // Check if other fields of same model are valid before
>>>> + // triggering 'session:valid' event
>>>> + if(_.size(this.model.errorModel.attributes) == 0) {
>>>> + if (this.model.collection || this.model.handler) {
>>>> + (this.model.collection || this.model.handler).trigger(
>>>> + 'pgadmin-session:model:valid', this.model, (this.model.collection || this.model.handler)
>>>> + );
>>>> + } else {
>>>> + (this.model).trigger(
>>>> + 'pgadmin-session:valid', this.model.sessChanged(), this.model
>>>> + );
>>>> + }
>>>>
>>>>
>>>> 4) We also noticed that the following change sets look very similiar.
>>>> Is there any reason to have this code duplicated? If not this could be a
>>>> good time to refactor it.
>>>>
>>> As said earlier in response of point 2 code duplication is because the
>>> way controls are derived.
>>>
>>>
>>>> +++ b/web/pgadmin/static/js/backform.pgadmin.js
>>>> @@ -1528,7 +1528,18 @@
>>>>
>>>> @@ -1573,25 +1584,23 @@
>>>>
>>>> @@ -1631,7 +1640,18 @@
>>>>
>>>> @@ -1676,25 +1696,23 @@
>>>>
>>>>
>>>>
>>>> Thanks
>>>> Joao & Shruti
>>>>
>>>> On Thu, May 18, 2017 at 6:01 AM, Harshal Dhumal <
>>>> [email protected]> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> Please find attached patch for RM2421
>>>>>
>>>>> Issue fixed: 1. Integer/numeric Validation is not working properly.
>>>>> 2. Wrong CPU rate unit
>>>>> --
>>>>> *Harshal Dhumal*
>>>>> *Sr. Software Engineer*
>>>>>
>>>>> EnterpriseDB India: 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
>>>>>
>>>>>
>>>>
>>>
>>
>
--
Sent via pgadmin-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers
Attachments:
[text/x-patch] RM2421_V3.patch (32.9K, 3-RM2421_V3.patch)
download | inline diff:
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/foreign_tables/templates/foreign_tables/js/foreign_tables.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/foreign_tables/templates/foreign_tables/js/foreign_tables.js
index 5f3dc69..b7df585 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/foreign_tables/templates/foreign_tables/js/foreign_tables.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/foreign_tables/templates/foreign_tables/js/foreign_tables.js
@@ -101,34 +101,56 @@ function($, _, S, pgAdmin, pgBrowser, alertify) {
cell: 'string', group: '{{ _('Definition') }}',
type: 'int', deps: ['datatype'],
disabled: function(m) {
- // We will store type from selected from combobox
+ var val = m.get('typlen');
+ // We will store type from selected from combobox
if(!(_.isUndefined(m.get('inheritedid'))
|| _.isNull(m.get('inheritedid'))
|| _.isUndefined(m.get('inheritedfrom'))
- || _.isNull(m.get('inheritedfrom')))) { return true; }
+ || _.isNull(m.get('inheritedfrom')))) {
- var of_type = m.get('datatype');
- if(m.type_options) {
- m.set('is_tlength', false, {silent: true});
+ if (!_.isUndefined(val)) {
+ setTimeout(function() {
+ m.set('typlen', undefined);
+ }, 10);
+ }
+ return true;
+ }
+
+ var of_type = m.get('datatype'),
+ has_length = false;
+ if(m.type_options) {
+ m.set('is_tlength', false, {silent: true});
- // iterating over all the types
- _.each(m.type_options, function(o) {
- // if type from selected from combobox matches in options
- if ( of_type == o.value ) {
- m.set('typlen', undefined);
+ // iterating over all the types
+ _.each(m.type_options, function(o) {
+ // if type from selected from combobox matches in options
+ if ( of_type == o.value ) {
// if length is allowed for selected type
if(o.length)
{
// set the values in model
+ has_length = true;
m.set('is_tlength', true, {silent: true});
m.set('min_val', o.min_val, {silent: true});
m.set('max_val', o.max_val, {silent: true});
}
+ }
+ });
+
+ if (!has_length && !_.isUndefined(val)) {
+ setTimeout(function() {
+ m.set('typlen', undefined);
+ }, 10);
}
- });
- return !(m.get('is_tlength'));
- }
- return true;
+
+ return !(m.get('is_tlength'));
+ }
+ if (!has_length && !_.isUndefined(val)) {
+ setTimeout(function() {
+ m.set('typlen', undefined);
+ }, 10);
+ }
+ return true;
},
cellHeaderClasses: 'width_percent_10'
},{
@@ -136,33 +158,54 @@ function($, _, S, pgAdmin, pgBrowser, alertify) {
type: 'int', deps: ['datatype'],
cell: 'string', group: '{{ _('Definition') }}',
disabled: function(m) {
+ var val = m.get('precision');
if(!(_.isUndefined(m.get('inheritedid'))
|| _.isNull(m.get('inheritedid'))
|| _.isUndefined(m.get('inheritedfrom'))
- || _.isNull(m.get('inheritedfrom')))) { return true; }
+ || _.isNull(m.get('inheritedfrom')))) {
+
+ if (!_.isUndefined(val)) {
+ setTimeout(function() {
+ m.set('precision', undefined);
+ }, 10);
+ }
+ return true;
+ }
+
+ var of_type = m.get('datatype'),
+ has_precision = false;
- var of_type = m.get('datatype');
if(m.type_options) {
m.set('is_precision', false, {silent: true});
// iterating over all the types
_.each(m.type_options, function(o) {
// if type from selected from combobox matches in options
if ( of_type == o.value ) {
- m.set('precision', undefined);
// if precession is allowed for selected type
if(o.precision)
{
+ has_precision = true;
// set the values in model
m.set('is_precision', true, {silent: true});
m.set('min_val', o.min_val, {silent: true});
m.set('max_val', o.max_val, {silent: true});
}
+ }
+ });
+ if (!has_precision && !_.isUndefined(val)) {
+ setTimeout(function() {
+ m.set('precision', undefined);
+ }, 10);
}
- });
- return !(m.get('is_precision'));
- }
- return true;
- }, cellHeaderClasses: 'width_percent_10'
+ return !(m.get('is_precision'));
+ }
+ if (!has_precision && !_.isUndefined(val)) {
+ setTimeout(function() {
+ m.set('precision', undefined);
+ }, 10);
+ }
+ return true;
+ }, cellHeaderClasses: 'width_percent_10'
},{
id: 'typdefault', label:'{{ _('Default') }}', type: 'text',
cell: 'string', min_version: 90300, group: '{{ _('Definition') }}',
@@ -216,22 +259,23 @@ function($, _, S, pgAdmin, pgBrowser, alertify) {
min_version: 90200
}],
validate: function() {
- var err = {},
- errmsg;
+ var errmsg = null;
if (_.isUndefined(this.get('attname')) || String(this.get('attname')).replace(/^\s+|\s+$/g, '') == '') {
- err['name'] = '{{ _('Column Name cannot be empty!') }}';
- errmsg = errmsg || err['attname'];
+ errmsg = '{{ _('Column Name cannot be empty!') }}';
+ this.errorModel.set('attname', errmsg);
+ } else {
+ this.errorModel.unset('attname');
}
if (_.isUndefined(this.get('datatype')) || String(this.get('datatype'))
.replace(/^\s+|\s+$/g, '') == '') {
- err['basensp'] = '{{ _('Column Datatype cannot be empty!') }}';
- errmsg = errmsg || err['datatype'];
+ errmsg = '{{ _('Column Datatype cannot be empty!') }}';
+ this.errorModel.set('datatype', errmsg);
+ } else {
+ this.errorModel.unset('datatype');
}
- this.errorModel.clear().set(err);
-
return errmsg;
},
is_editable_column: function(m) {
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/sequences/templates/sequence/js/sequence.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/sequences/templates/sequence/js/sequence.js
index 2fb7ae5..1b9db50 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/sequences/templates/sequence/js/sequence.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/sequences/templates/sequence/js/sequence.js
@@ -153,7 +153,10 @@ function($, _, S, pgAdmin, pgBrowser, alertify) {
min: 1
},{
id: 'start', label: '{{ _('Start') }}', type: 'int',
- mode: ['properties', 'create'], group: '{{ _('Definition') }}'
+ mode: ['properties', 'create', 'edit'], group: '{{ _('Definition') }}',
+ disabled: function(m) {
+ return !m.isNew();
+ }
},{
id: 'minimum', label: '{{ _('Minimum') }}', type: 'int',
mode: ['properties', 'create', 'edit'], group: '{{ _('Definition') }}'
@@ -199,14 +202,14 @@ function($, _, S, pgAdmin, pgBrowser, alertify) {
minimum = this.get('minimum'),
maximum = this.get('maximum');
start = this.get('start');
- // Clear any existing error msg.
- this.errorModel.clear();
if (_.isUndefined(this.get('name'))
|| String(this.get('name')).replace(/^\s+|\s+$/g, '') == '') {
msg = '{{ _('Name cannot be empty.') }}';
this.errorModel.set('name', msg);
return msg;
+ } else {
+ this.errorModel.unset('name');
}
if (_.isUndefined(this.get('seqowner'))
@@ -214,6 +217,8 @@ function($, _, S, pgAdmin, pgBrowser, alertify) {
msg = '{{ _('Owner cannot be empty.') }}';
this.errorModel.set('seqowner', msg);
return msg;
+ } else {
+ this.errorModel.unset('seqowner');
}
if (_.isUndefined(this.get('schema'))
@@ -221,26 +226,81 @@ function($, _, S, pgAdmin, pgBrowser, alertify) {
msg = '{{ _('Schema cannot be empty.') }}';
this.errorModel.set('schema', msg);
return msg;
+ } else {
+ this.errorModel.unset('schema');
+ }
+
+ if (!this.isNew()) {
+ if (_.isUndefined(this.get('current_value'))
+ || String(this.get('current_value')).replace(/^\s+|\s+$/g, '') == '') {
+ msg = '{{ _('Current value cannot be empty.') }}';
+ this.errorModel.set('current_value', msg);
+ return msg;
+ } else {
+ this.errorModel.unset('current_value');
+ }
+
+ if (_.isUndefined(this.get('increment'))
+ || String(this.get('increment')).replace(/^\s+|\s+$/g, '') == '') {
+ msg = '{{ _('Increment value cannot be empty.') }}';
+ this.errorModel.set('increment', msg);
+ return msg;
+ } else {
+ this.errorModel.unset('increment');
+ }
+
+ if (_.isUndefined(this.get('minimum'))
+ || String(this.get('minimum')).replace(/^\s+|\s+$/g, '') == '') {
+ msg = '{{ _('Minimum value cannot be empty.') }}';
+ this.errorModel.set('minimum', msg);
+ return msg;
+ } else {
+ this.errorModel.unset('minimum');
+ }
+
+ if (_.isUndefined(this.get('maximum'))
+ || String(this.get('maximum')).replace(/^\s+|\s+$/g, '') == '') {
+ msg = '{{ _('Maximum value cannot be empty.') }}';
+ this.errorModel.set('maximum', msg);
+ return msg;
+ } else {
+ this.errorModel.unset('maximum');
+ }
+
+ if (_.isUndefined(this.get('cache'))
+ || String(this.get('cache')).replace(/^\s+|\s+$/g, '') == '') {
+ msg = '{{ _('Cache value cannot be empty.') }}';
+ this.errorModel.set('cache', msg);
+ return msg;
+ } else {
+ this.errorModel.unset('cache');
+ }
}
var min_lt = '{{ _('Minimum value must be less than maximum value.') }}',
start_lt = '{{ _('Start value cannot be less than minimum value.') }}',
start_gt = '{{ _('Start value cannot be greater than maximum value.') }}';
+
if ((minimum == 0 && maximum == 0) ||
(parseInt(minimum, 10) >= parseInt(maximum, 10))) {
- msg = min_lt
- this.errorModel.set('minimum', msg);
- return msg;
+ this.errorModel.set('minimum', min_lt);
+ return min_lt;
+ } else {
+ this.errorModel.unset('minimum');
}
- else if (start < minimum) {
- msg = start_lt
- this.errorModel.set('start', msg);
- return msg;
+
+ if (start && minimum && parseInt(start) < parseInt(minimum)) {
+ this.errorModel.set('start', start_lt);
+ return start_lt;
+ } else {
+ this.errorModel.unset('start');
}
- else if (start > maximum) {
- msg = start_gt
- this.errorModel.set('start', msg);
- return msg;
+
+ if (start && maximum && parseInt(start) > parseInt(maximum)) {
+ this.errorModel.set('start', start_gt);
+ return start_gt;
+ } else {
+ this.errorModel.unset('start');
}
return null;
}
diff --git a/web/pgadmin/browser/server_groups/servers/resource_groups/templates/resource_groups/js/resource_groups.js b/web/pgadmin/browser/server_groups/servers/resource_groups/templates/resource_groups/js/resource_groups.js
index 3cc822e..26e63bf 100644
--- a/web/pgadmin/browser/server_groups/servers/resource_groups/templates/resource_groups/js/resource_groups.js
+++ b/web/pgadmin/browser/server_groups/servers/resource_groups/templates/resource_groups/js/resource_groups.js
@@ -85,54 +85,35 @@ define(
*/
validate: function(keys) {
- /* Check whether 'name' is present in 'keys', if it is present
- * it means there is a change in that field from the GUI, so we
- * need to validate it.
- */
- if (_.indexOf(keys, 'name') >= 0) {
- var name = this.get('name');
- if (_.isUndefined(name) || _.isNull(name) ||
- String(name).replace(/^\s+|\s+$/g, '') == '') {
- var msg = '{{ _('Name cannot be empty.') }}';
- this.errorModel.set('name', msg);
- return msg;
- } else {
- this.errorModel.unset('name');
- }
+ var name = this.get('name');
+ if (_.isUndefined(name) || _.isNull(name) ||
+ String(name).replace(/^\s+|\s+$/g, '') == '') {
+ var msg = '{{ _('Name cannot be empty.') }}';
+ this.errorModel.set('name', msg);
+ return msg;
+ } else {
+ this.errorModel.unset('name');
}
- /* Check whether 'cpu_rate_limit' is present in 'keys', if it is present
- * it means there is a change in that field from the GUI, so we
- * need to validate it.
- */
- if (_.indexOf(keys, 'cpu_rate_limit') >= 0) {
- var cpu_rate_limit = this.get('cpu_rate_limit');
- if (_.isUndefined(cpu_rate_limit) || _.isNull(cpu_rate_limit) ||
- String(cpu_rate_limit).replace(/^\s+|\s+$/g, '') == '') {
- var msg = '{{ _('CPU rate limit cannot be empty.') }}';
- this.errorModel.set('cpu_rate_limit', msg);
- return msg;
- } else {
- this.errorModel.unset('cpu_rate_limit');
- }
+ var cpu_rate_limit = this.get('cpu_rate_limit');
+ if (_.isUndefined(cpu_rate_limit) || _.isNull(cpu_rate_limit) ||
+ String(cpu_rate_limit).replace(/^\s+|\s+$/g, '') == '') {
+ var msg = '{{ _('CPU rate limit cannot be empty.') }}';
+ this.errorModel.set('cpu_rate_limit', msg);
+ return msg;
+ } else {
+ this.errorModel.unset('cpu_rate_limit');
}
- /* Check whether 'dirty_rate_limit' is present in 'keys', if it is present
- * it means there is a change in that field from the GUI, so we
- * need to validate it.
- */
- if (_.indexOf(keys, 'dirty_rate_limit') >= 0) {
- var dirty_rate_limit = this.get('dirty_rate_limit');
- if (_.isUndefined(dirty_rate_limit) || _.isNull(dirty_rate_limit) ||
- String(dirty_rate_limit).replace(/^\s+|\s+$/g, '') == '') {
- var msg = '{{ _('Dirty rate limit cannot be empty.') }}';
- this.errorModel.set('dirty_rate_limit', msg);
- return msg;
- } else {
- this.errorModel.unset('dirty_rate_limit');
- }
+ var dirty_rate_limit = this.get('dirty_rate_limit');
+ if (_.isUndefined(dirty_rate_limit) || _.isNull(dirty_rate_limit) ||
+ String(dirty_rate_limit).replace(/^\s+|\s+$/g, '') == '') {
+ var msg = '{{ _('Dirty rate limit cannot be empty.') }}';
+ this.errorModel.set('dirty_rate_limit', msg);
+ return msg;
+ } else {
+ this.errorModel.unset('dirty_rate_limit');
}
-
return null;
}
})
diff --git a/web/pgadmin/browser/server_groups/servers/roles/templates/role/js/role.js b/web/pgadmin/browser/server_groups/servers/roles/templates/role/js/role.js
index a85416c..c74cd59 100644
--- a/web/pgadmin/browser/server_groups/servers/roles/templates/role/js/role.js
+++ b/web/pgadmin/browser/server_groups/servers/roles/templates/role/js/role.js
@@ -427,7 +427,7 @@ function($, _, S, pgAdmin, pgBrowser, alertify, Backform) {
deps: ['rolcanlogin'], options: {format: 'YYYY-MM-DD HH:mm:ss Z'}
},{
id: 'rolconnlimit', type: 'int', group: '{{ _('Definition') }}',
- label: '{{ _('Connection limit') }}', cell: 'number',
+ label: '{{ _('Connection limit') }}', cell: 'number', min : -1,
mode: ['properties', 'edit', 'create'], disabled: 'readonly'
},{
id: 'rolcanlogin', label:'{{ _('Can login?') }}', type: 'switch',
diff --git a/web/pgadmin/browser/server_groups/servers/templates/servers/servers.js b/web/pgadmin/browser/server_groups/servers/templates/servers/servers.js
index 9eb82b8..bf28474 100644
--- a/web/pgadmin/browser/server_groups/servers/templates/servers/servers.js
+++ b/web/pgadmin/browser/server_groups/servers/templates/servers/servers.js
@@ -727,6 +727,9 @@ function($, _, S, pgAdmin, pgBrowser, alertify) {
check_for_empty(
'username', '{{ _('Username must be specified.') }}'
);
+ check_for_empty(
+ 'port', '{{ _('Port must be specified.') }}'
+ );
this.errorModel.set(err);
if (_.size(err)) {
diff --git a/web/pgadmin/browser/static/js/datamodel.js b/web/pgadmin/browser/static/js/datamodel.js
index 5b1c3a7..c3b5fac 100644
--- a/web/pgadmin/browser/static/js/datamodel.js
+++ b/web/pgadmin/browser/static/js/datamodel.js
@@ -1,6 +1,6 @@
define(
- ['underscore', 'pgadmin', 'jquery', 'backbone'],
-function(_, pgAdmin, $, Backbone) {
+ ['underscore', 'underscore.string', 'pgadmin', 'jquery', 'backbone'],
+function(_, S, pgAdmin, $, Backbone) {
var pgBrowser = pgAdmin.Browser = pgAdmin.Browser || {};
pgBrowser.DataModel = Backbone.Model.extend({
@@ -136,6 +136,7 @@ function(_, pgAdmin, $, Backbone) {
}
self.sessAttrs = {};
+ self.fieldData = {};
self.origSessAttrs = {};
self.objects = [];
self.arrays = [];
@@ -153,6 +154,23 @@ function(_, pgAdmin, $, Backbone) {
_.each(schema, function(s) {
switch(s.type) {
+ case 'int':
+ case 'numeric':
+ self.fieldData[s.id] = {
+ label: s.id,
+ type: s.type,
+ min: s.min || undefined,
+ max: s.max || undefined
+ }
+ break;
+ default:
+ self.fieldData[s.id] = {
+ label: s.id,
+ type: s.type
+ }
+ }
+
+ switch(s.type) {
case 'array':
self.arrays.push(s.id);
@@ -280,6 +298,12 @@ function(_, pgAdmin, $, Backbone) {
},
sessValid: function() {
var self = this;
+ // Perform default validations.
+ if ('default_validate' in self && typeof(self.default_validate) == 'function' &&
+ _.isString(self.default_validate())) {
+ return false;
+ }
+
if ('validate' in self && _.isFunction(self.validate) &&
_.isString(self.validate.apply(self))) {
return false;
@@ -301,8 +325,9 @@ function(_, pgAdmin, $, Backbone) {
}
if (key != null && res) {
- var attrs = {};
- var self = this;
+ var attrs = {},
+ self = this,
+ msg;
attrChanged = function(v, k) {
if (k in self.objects) {
@@ -327,9 +352,18 @@ function(_, pgAdmin, $, Backbone) {
if (!options || !options.silent) {
self.trigger('change', self, options);
}
+
+ // Perform default validations.
+
+ if ('default_validate' in self && typeof(self.default_validate) == 'function') {
+ msg = self.default_validate();
+ }
+
if ('validate' in self && typeof(self['validate']) === 'function') {
- var msg = self.validate(_.keys(attrs));
+ if (!msg) {
+ msg = self.validate(_.keys(attrs));
+ }
/*
* If any parent present, we will need to inform the parent - that
@@ -562,6 +596,13 @@ function(_, pgAdmin, $, Backbone) {
var msg = null,
validate = function(m, attrs) {
+ if ('default_validate' in m && typeof(m.default_validate) == 'function') {
+ msg = m.default_validate();
+ if (_.isString(msg)) {
+ return msg;
+ }
+ }
+
if ('validate' in m && typeof(m.validate) == 'function') {
msg = m.validate(attrs);
@@ -655,6 +696,79 @@ function(_, pgAdmin, $, Backbone) {
});
self.trigger('pgadmin-session:stop');
+ },
+ default_validate: function() {
+ var msg, field, value, type;
+
+ for (var i = 0, keys = _.keys(this.attributes), l = keys.length;
+ i<l;
+ i++) {
+
+ value = this.attributes[keys[i]];
+ field = this.fieldData[keys[i]]
+ msg = null;
+
+ if (!(_.isUndefined(value) || _.isNull(value) ||
+ String(value).replace(/^\s+|\s+$/g, '') == '')) {
+
+ if (!field) {
+ continue;
+ }
+
+ type = field.type || undefined;
+ if (!type) {
+ continue;
+ }
+
+ switch(type) {
+ case 'int':
+ msg = this.integer_validate(value, field);
+ break;
+ case 'numeric':
+ msg = this.number_validate(value, field);
+ break;
+ }
+
+ if (msg) {
+ this.errorModel.set(field.label, msg);
+ return msg;
+ } else {
+ this.errorModel.unset(field.label);
+ }
+ } else {
+ if (field) {
+ this.errorModel.unset(field.label);
+ }
+ }
+ }
+ return null;
+ },
+
+ check_min_max: function (value, field) {
+ var label = field.label,
+ min_value = field.min,
+ max_value = field.max;
+
+ if (min_value && value < min_value) {
+ return S(pgAdmin.Browser.messages.MUST_GR_EQ).sprintf(label, min_value).value();
+ } else if (max_value && value > max_value) {
+ return S(pgAdmin.Browser.messages.MUST_LESS_EQ).sprintf(label, max_value).value();
+ }
+ return null;
+ },
+ number_validate: function (value, field) {
+ var pattern = new RegExp("^-?[0-9]+(\.?[0-9]*)?$");
+ if (!pattern.test(value)) {
+ return S(pgAdmin.Browser.messages.MUST_BE_NUM).sprintf(field.label).value()
+ }
+ return this.check_min_max(value, field)
+ },
+ integer_validate: function(value, field) {
+ var pattern = new RegExp("^-?[0-9]*$");
+ if (!pattern.test(value)) {
+ return S(pgAdmin.Browser.messages.MUST_BE_INT).sprintf(field.label).value()
+ }
+ return this.check_min_max(value, field)
}
});
@@ -696,7 +810,8 @@ function(_, pgAdmin, $, Backbone) {
return self;
},
startNewSession: function() {
- var self = this;
+ var self = this,
+ msg;
if (self.trackChanges) {
// We're stopping the existing session.
@@ -718,8 +833,15 @@ function(_, pgAdmin, $, Backbone) {
if ('startNewSession' in m && _.isFunction(m.startNewSession)) {
m.startNewSession();
}
- if ('validate' in m && typeof(m.validate) === 'function') {
- var msg = m.validate();
+
+ if ('default_validate' in m && typeof(m.default_validate) == 'function') {
+ msg = m.default_validate();
+ }
+
+ if (_.isString(msg)) {
+ self.sessAttrs['invalid'][m.cid] = msg;
+ } else if ('validate' in m && typeof(m.validate) === 'function') {
+ msg = m.validate();
if (msg) {
self.sessAttrs['invalid'][m.cid] = msg;
@@ -900,7 +1022,14 @@ function(_, pgAdmin, $, Backbone) {
(self.handler || self).trigger('pgadmin-session:added', self, obj);
- if ('validate' in obj && typeof(obj.validate) === 'function') {
+
+ if ('default_validate' in obj && typeof(obj.default_validate) == 'function') {
+ msg = obj.default_validate();
+ }
+
+ if (_.isString(msg)) {
+ (self.sessAttrs['invalid'])[obj.cid] = msg;
+ } else if ('validate' in obj && typeof(obj.validate) === 'function') {
msg = obj.validate();
if (msg) {
@@ -908,7 +1037,14 @@ function(_, pgAdmin, $, Backbone) {
}
}
} else {
- if ('validate' in obj && typeof(obj.validate) === 'function') {
+
+ if ('default_validate' in obj && typeof(obj.default_validate) == 'function') {
+ msg = obj.default_validate();
+ }
+
+ if (_.isString(msg)) {
+ (self.sessAttrs['invalid'])[obj.cid] = msg;
+ } else if ('validate' in obj && typeof(obj.validate) === 'function') {
msg = obj.validate();
if (msg) {
diff --git a/web/pgadmin/static/js/backform.pgadmin.js b/web/pgadmin/static/js/backform.pgadmin.js
index 553676e..13823b7 100644
--- a/web/pgadmin/static/js/backform.pgadmin.js
+++ b/web/pgadmin/static/js/backform.pgadmin.js
@@ -57,7 +57,7 @@
});
var controlMapper = Backform.controlMapper = {
- 'int': ['uneditable-input', 'integer', 'integer'],
+ 'int': ['uneditable-input', 'numeric', 'numeric'],
'text': ['uneditable-input', 'input', 'string'],
'numeric': ['uneditable-input', 'numeric', 'numeric'],
'date': 'datepicker',
@@ -1493,109 +1493,6 @@
}
});
- /*
- * Integer input Control functionality just like backgrid
- */
- var IntegerControl = Backform.IntegerControl = Backform.InputControl.extend({
- defaults: {
- type: "number",
- label: "",
- min: undefined,
- max: undefined,
- maxlength: 255,
- extraClasses: [],
- helpMessage: null
- },
- template: _.template([
- '<label class="<%=Backform.controlLabelClassName%>"><%=label%></label>',
- '<div class="<%=Backform.controlsClassName%>">',
- ' <input type="<%=type%>" class="<%=Backform.controlClassName%> <%=extraClasses.join(\' \')%>" name="<%=name%>" min="<%=min%>" max="<%=max%>"maxlength="<%=maxlength%>" value="<%-value%>" placeholder="<%-placeholder%>" <%=disabled ? "disabled" : ""%> <%=required ? "required" : ""%> />',
- ' <% if (helpMessage && helpMessage.length) { %>',
- ' <span class="<%=Backform.helpMessageClassName%>"><%=helpMessage%></span>',
- ' <% } %>',
- '</div>'
- ].join("\n")),
- events: {
- "change input": "checkInt",
- "focus input": "clearInvalid"
- },
- checkInt: function(e) {
- var field = _.defaults(this.field.toJSON(), this.defaults),
- attrArr = this.field.get("name").split('.'),
- name = attrArr.shift(),
- value = this.getValueFromDOM(),
- min_value = field.min,
- max_value = field.max,
- isValid = true,
- intPattern = new RegExp("^-?[0-9]*$"),
- isMatched = intPattern.test(value);
-
- // Below logic will validate input
- if (!isMatched) {
- isValid = false;
- this.model.errorModel.unset(name);
- this.model.errorModel.set(
- name,
- S(pgAdmin.Browser.messages.MUST_BE_INT).sprintf(
- field.label
- ).value()
- );
- }
-
- // Below will check if entered value is in-between min & max range
- if (isValid && (!_.isUndefined(min_value) && value < min_value)) {
- isValid = false;
- this.model.errorModel.unset(name);
- this.model.errorModel.set(
- name,
- S(pgAdmin.Browser.messages.MUST_GR_EQ).sprintf(
- field.label,
- min_value
- ).value()
- );
- }
-
- if (isValid && (!_.isUndefined(max_value) && value > max_value)) {
- isValid = false;
- this.model.errorModel.unset(name);
- this.model.errorModel.set(
- name,
- S(pgAdmin.Browser.messages.MUST_LESS_EQ).sprintf(
- field.label,
- max_value
- ).value()
- );
- }
-
- // After validation we need to set that value into model (only if all flags are true)
- if (isValid) {
- this.stopListening(this.model, "change:" + name, this.render);
- this.model.errorModel.unset(name);
- this.model.set(name, value);
- this.listenTo(this.model, "change:" + name, this.render);
- if (this.model.collection || this.model.handler) {
- (this.model.collection || this.model.handler).trigger(
- 'pgadmin-session:model:valid', this.model, (this.model.collection || this.model.handler)
- );
- } else {
- (this.model).trigger(
- 'pgadmin-session:valid', this.model.sessChanged(), this.model
- );
- }
- } else {
- if (this.model.collection || this.model.handler) {
- (this.model.collection || this.model.handler).trigger(
- 'pgadmin-session:model:invalid', this.model.errorModel.get(name), this.model
- );
- } else {
- (this.model).trigger(
- 'pgadmin-session:invalid', this.model.errorModel.get(name), this.model
- );
- }
- }
- }
- });
-
/*
* Numeric input Control functionality just like backgrid
*/
@@ -1617,86 +1514,7 @@
' <span class="<%=Backform.helpMessageClassName%>"><%=helpMessage%></span>',
' <% } %>',
'</div>'
- ].join("\n")),
- events: {
- "change input": "checkNumeric",
- "focus input": "clearInvalid"
- },
- checkNumeric: function(e) {
- var field = _.defaults(this.field.toJSON(), this.defaults),
- attrArr = this.field.get("name").split('.'),
- name = attrArr.shift(),
- value = this.getValueFromDOM(),
- min_value = field.min,
- max_value = field.max,
- isValid = true,
- intPattern = new RegExp("^-?[0-9]+(\.?[0-9]*)?$"),
- isMatched = intPattern.test(value);
-
- // Below logic will validate input
- if (!isMatched) {
- isValid = false;
- this.model.errorModel.unset(name);
- this.model.errorModel.set(
- name,
- S(pgAdmin.Browser.messages.MUST_BE_NUM).sprintf(
- field.label
- ).value()
- );
- }
-
- // Below will check if entered value is in-between min & max range
- if (isValid && (!_.isUndefined(min_value) && value < min_value)) {
- isValid = false;
- this.model.errorModel.unset(name);
- this.model.errorModel.set(
- name,
- S(pgAdmin.Browser.messages.MUST_GR_EQ).sprintf(
- field.label,
- min_value
- ).value()
- );
- }
-
- if (isValid && (!_.isUndefined(max_value) && value > max_value)) {
- isValid = false;
- this.model.errorModel.unset(name);
- this.model.errorModel.set(
- name,
- S(pgAdmin.Browser.messages.MUST_LESS_EQ).sprintf(
- field.label,
- max_value
- ).value()
- );
- }
-
- // After validation we need to set that value into model (only if all flags are true)
- if (isValid) {
- this.stopListening(this.model, "change:" + name, this.render);
- this.model.errorModel.unset(name);
- this.model.set(name, value);
- this.listenTo(this.model, "change:" + name, this.render);
- if (this.model.collection || this.model.handler) {
- (this.model.collection || this.model.handler).trigger(
- 'pgadmin-session:model:valid', this.model, (this.model.collection || this.model.handler)
- );
- } else {
- (this.model).trigger(
- 'pgadmin-session:valid', this.model.sessChanged(), this.model
- );
- }
- } else {
- if (this.model.collection || this.model.handler) {
- (this.model.collection || this.model.handler).trigger(
- 'pgadmin-session:model:invalid', this.model.errorModel.get(name), this.model
- );
- } else {
- (this.model).trigger(
- 'pgadmin-session:invalid', this.model.errorModel.get(name), this.model
- );
- }
- }
- }
+ ].join("\n"))
});
///////
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Fix for RM2421 [pgAdmin4][patch]
2017-05-18 10:01 Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-05-18 14:27 ` Re: Fix for RM2421 [pgAdmin4][patch] Joao Pedro De Almeida Pereira <[email protected]>
2017-05-19 06:52 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-05-29 09:48 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-05-30 07:00 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-05-30 07:28 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
@ 2017-06-05 15:55 ` Dave Page <[email protected]>
2017-06-06 06:42 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Dave Page @ 2017-06-05 15:55 UTC (permalink / raw)
To: Harshal Dhumal <[email protected]>; +Cc: Joao Pedro De Almeida Pereira <[email protected]>; pgadmin-hackers; Shruti B Iyer <[email protected]>
Hi
With this patch applied, it uses the field names instead of the labels
in error messages - e.g.
'dirty_rate_limit' must be numeric
instead of:
'Dirty Rate Limit (KB)' must be numeric.
Thanks.
On Tue, May 30, 2017 at 8:28 AM, Harshal Dhumal
<[email protected]> wrote:
> Hi,
>
> Please find updated patch.
>
> --
> Harshal Dhumal
> Sr. Software Engineer
>
> EnterpriseDB India: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Tue, May 30, 2017 at 12:30 PM, Harshal Dhumal
> <[email protected]> wrote:
>>
>> Hi,
>>
>> Please ignore this patch as I forgot to include few changes. I'll send
>> updated one.
>>
>> --
>> Harshal Dhumal
>> Sr. Software Engineer
>>
>> EnterpriseDB India: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> On Mon, May 29, 2017 at 3:18 PM, Harshal Dhumal
>> <[email protected]> wrote:
>>>
>>> Hi,
>>>
>>> Here is updated patch for RM2421.
>>>
>>> Now I have moved all Numeric control level validations to datamodel. As
>>> existing implementation was causing
>>> issues with error messages in create/edit dialog when schema contains two
>>> or more Numeric controls.
>>>
>>> This is generic issue and not related to resource group. Also I have
>>> updated all other nodes which uses Numeric controls
>>>
>>>
>>>
>>> --
>>> Harshal Dhumal
>>> Sr. Software Engineer
>>>
>>> EnterpriseDB India: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>> On Fri, May 19, 2017 at 12:22 PM, Harshal Dhumal
>>> <[email protected]> wrote:
>>>>
>>>> Hi,
>>>>
>>>> On Thu, May 18, 2017 at 7:57 PM, Joao Pedro De Almeida Pereira
>>>> <[email protected]> wrote:
>>>>>
>>>>> Hello Harshal,
>>>>>
>>>>> We review the patch and have some questions:
>>>>> 1) Is there any particular reason to initialize variables and functions
>>>>> in the same place? We believe that it would be more readable there were no
>>>>> chaining of variable creation, specially if those variables are functions.
>>>>> Check line:
>>>>
>>>> That function is only going to be used in checkNumeric function (in case
>>>> of Number control) and checkInt function (in case of Integer control) so
>>>> declared them locally.
>>>> Anyway I'm going to refactor both the controls as Number and Integer
>>>> shares some common properties.
>>>>
>>>>> +++ b/web/pgadmin/static/js/backform.pgadmin.js
>>>>> @@ -1528,7 +1528,18 @@
>>>>> max_value = field.max,
>>>>> isValid = true,
>>>>> intPattern = new RegExp("^-?[0-9]*$"),
>>>>> - isMatched = intPattern.test(value);
>>>>> + isMatched = intPattern.test(value),
>>>>> + trigger_invalid_event = function(msg) {
>>>>>
>>>>> 2) The functions added in both places look very similar, can they be
>>>>> merged and extracted? We are talking about the trigger_invalid_event
>>>>> function.
>>>>
>>>> Yes they can be merged. As of now both NumericControl and IntegerControl
>>>> are derived from InputControl. Ideally
>>>> only NumericControl should be derived from InputControl and
>>>> IntegerControl should be derive from NumericControl.
>>>>
>>>>
>>>>>
>>>>> 3) The following change is very similar to the trigger_invalid_event,
>>>>> was there a reason not to use it?
>>>>
>>>> Below code triggers "model valid" event; opposite to "model invalid"
>>>> event (trigger_invalid_event)
>>>>>
>>>>> +++ b/web/pgadmin/static/js/backform.pgadmin.js
>>>>> @@ -1573,25 +1584,23 @@
>>>>> this.model.errorModel.unset(name);
>>>>> this.model.set(name, value);
>>>>> this.listenTo(this.model, "change:" + name, this.render);
>>>>> - if (this.model.collection || this.model.handler) {
>>>>> - (this.model.collection || this.model.handler).trigger(
>>>>> - 'pgadmin-session:model:valid', this.model,
>>>>> (this.model.collection || this.model.handler)
>>>>> - );
>>>>> + // Check if other fields of same model are valid before
>>>>> + // triggering 'session:valid' event
>>>>> + if(_.size(this.model.errorModel.attributes) == 0) {
>>>>> + if (this.model.collection || this.model.handler) {
>>>>> + (this.model.collection || this.model.handler).trigger(
>>>>> + 'pgadmin-session:model:valid', this.model,
>>>>> (this.model.collection || this.model.handler)
>>>>> + );
>>>>> + } else {
>>>>> + (this.model).trigger(
>>>>> + 'pgadmin-session:valid', this.model.sessChanged(),
>>>>> this.model
>>>>> + );
>>>>> + }
>>>>>
>>>>> 4) We also noticed that the following change sets look very similiar.
>>>>> Is there any reason to have this code duplicated? If not this could be a
>>>>> good time to refactor it.
>>>>
>>>> As said earlier in response of point 2 code duplication is because the
>>>> way controls are derived.
>>>>
>>>>>
>>>>> +++ b/web/pgadmin/static/js/backform.pgadmin.js
>>>>> @@ -1528,7 +1528,18 @@
>>>>>
>>>>> @@ -1573,25 +1584,23 @@
>>>>>
>>>>> @@ -1631,7 +1640,18 @@
>>>>>
>>>>> @@ -1676,25 +1696,23 @@
>>>>>
>>>>>
>>>>> Thanks
>>>>> Joao & Shruti
>>>>>
>>>>> On Thu, May 18, 2017 at 6:01 AM, Harshal Dhumal
>>>>> <[email protected]> wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Please find attached patch for RM2421
>>>>>>
>>>>>> Issue fixed: 1. Integer/numeric Validation is not working properly.
>>>>>> 2. Wrong CPU rate unit
>>>>>> --
>>>>>> Harshal Dhumal
>>>>>> Sr. Software Engineer
>>>>>>
>>>>>> EnterpriseDB India: 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
>>>>>>
>>>>>
>>>>
>>>
>>
>
>
>
> --
> Sent via pgadmin-hackers mailing list ([email protected])
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgadmin-hackers
>
--
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
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Fix for RM2421 [pgAdmin4][patch]
2017-05-18 10:01 Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-05-18 14:27 ` Re: Fix for RM2421 [pgAdmin4][patch] Joao Pedro De Almeida Pereira <[email protected]>
2017-05-19 06:52 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-05-29 09:48 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-05-30 07:00 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-05-30 07:28 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-06-05 15:55 ` Re: Fix for RM2421 [pgAdmin4][patch] Dave Page <[email protected]>
@ 2017-06-06 06:42 ` Harshal Dhumal <[email protected]>
2017-06-07 13:29 ` Re: Fix for RM2421 [pgAdmin4][patch] Dave Page <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Harshal Dhumal @ 2017-06-06 06:42 UTC (permalink / raw)
To: Dave Page <[email protected]>; +Cc: pgadmin-hackers
Hi,
On Mon, Jun 5, 2017 at 9:25 PM, Dave Page <[email protected]> wrote:
> Hi
>
> With this patch applied, it uses the field names instead of the labels
> in error messages - e.g.
>
> 'dirty_rate_limit' must be numeric
>
> instead of:
>
> 'Dirty Rate Limit (KB)' must be numeric.
>
Fixed. Please find attached updated patch.
>
> Thanks.
>
> On Tue, May 30, 2017 at 8:28 AM, Harshal Dhumal
> <[email protected]> wrote:
> > Hi,
> >
> > Please find updated patch.
> >
> > --
> > Harshal Dhumal
> > Sr. Software Engineer
> >
> > EnterpriseDB India: http://www.enterprisedb.com
> > The Enterprise PostgreSQL Company
> >
> > On Tue, May 30, 2017 at 12:30 PM, Harshal Dhumal
> > <[email protected]> wrote:
> >>
> >> Hi,
> >>
> >> Please ignore this patch as I forgot to include few changes. I'll send
> >> updated one.
> >>
> >> --
> >> Harshal Dhumal
> >> Sr. Software Engineer
> >>
> >> EnterpriseDB India: http://www.enterprisedb.com
> >> The Enterprise PostgreSQL Company
> >>
> >> On Mon, May 29, 2017 at 3:18 PM, Harshal Dhumal
> >> <[email protected]> wrote:
> >>>
> >>> Hi,
> >>>
> >>> Here is updated patch for RM2421.
> >>>
> >>> Now I have moved all Numeric control level validations to datamodel. As
> >>> existing implementation was causing
> >>> issues with error messages in create/edit dialog when schema contains
> two
> >>> or more Numeric controls.
> >>>
> >>> This is generic issue and not related to resource group. Also I have
> >>> updated all other nodes which uses Numeric controls
> >>>
> >>>
> >>>
> >>> --
> >>> Harshal Dhumal
> >>> Sr. Software Engineer
> >>>
> >>> EnterpriseDB India: http://www.enterprisedb.com
> >>> The Enterprise PostgreSQL Company
> >>>
> >>> On Fri, May 19, 2017 at 12:22 PM, Harshal Dhumal
> >>> <[email protected]> wrote:
> >>>>
> >>>> Hi,
> >>>>
> >>>> On Thu, May 18, 2017 at 7:57 PM, Joao Pedro De Almeida Pereira
> >>>> <[email protected]> wrote:
> >>>>>
> >>>>> Hello Harshal,
> >>>>>
> >>>>> We review the patch and have some questions:
> >>>>> 1) Is there any particular reason to initialize variables and
> functions
> >>>>> in the same place? We believe that it would be more readable there
> were no
> >>>>> chaining of variable creation, specially if those variables are
> functions.
> >>>>> Check line:
> >>>>
> >>>> That function is only going to be used in checkNumeric function (in
> case
> >>>> of Number control) and checkInt function (in case of Integer control)
> so
> >>>> declared them locally.
> >>>> Anyway I'm going to refactor both the controls as Number and Integer
> >>>> shares some common properties.
> >>>>
> >>>>> +++ b/web/pgadmin/static/js/backform.pgadmin.js
> >>>>> @@ -1528,7 +1528,18 @@
> >>>>> max_value = field.max,
> >>>>> isValid = true,
> >>>>> intPattern = new RegExp("^-?[0-9]*$"),
> >>>>> - isMatched = intPattern.test(value);
> >>>>> + isMatched = intPattern.test(value),
> >>>>> + trigger_invalid_event = function(msg) {
> >>>>>
> >>>>> 2) The functions added in both places look very similar, can they be
> >>>>> merged and extracted? We are talking about the trigger_invalid_event
> >>>>> function.
> >>>>
> >>>> Yes they can be merged. As of now both NumericControl and
> IntegerControl
> >>>> are derived from InputControl. Ideally
> >>>> only NumericControl should be derived from InputControl and
> >>>> IntegerControl should be derive from NumericControl.
> >>>>
> >>>>
> >>>>>
> >>>>> 3) The following change is very similar to the trigger_invalid_event,
> >>>>> was there a reason not to use it?
> >>>>
> >>>> Below code triggers "model valid" event; opposite to "model invalid"
> >>>> event (trigger_invalid_event)
> >>>>>
> >>>>> +++ b/web/pgadmin/static/js/backform.pgadmin.js
> >>>>> @@ -1573,25 +1584,23 @@
> >>>>> this.model.errorModel.unset(name);
> >>>>> this.model.set(name, value);
> >>>>> this.listenTo(this.model, "change:" + name, this.render);
> >>>>> - if (this.model.collection || this.model.handler) {
> >>>>> - (this.model.collection || this.model.handler).trigger(
> >>>>> - 'pgadmin-session:model:valid', this.model,
> >>>>> (this.model.collection || this.model.handler)
> >>>>> - );
> >>>>> + // Check if other fields of same model are valid before
> >>>>> + // triggering 'session:valid' event
> >>>>> + if(_.size(this.model.errorModel.attributes) == 0) {
> >>>>> + if (this.model.collection || this.model.handler) {
> >>>>> + (this.model.collection || this.model.handler).trigger(
> >>>>> + 'pgadmin-session:model:valid', this.model,
> >>>>> (this.model.collection || this.model.handler)
> >>>>> + );
> >>>>> + } else {
> >>>>> + (this.model).trigger(
> >>>>> + 'pgadmin-session:valid', this.model.sessChanged(),
> >>>>> this.model
> >>>>> + );
> >>>>> + }
> >>>>>
> >>>>> 4) We also noticed that the following change sets look very similiar.
> >>>>> Is there any reason to have this code duplicated? If not this could
> be a
> >>>>> good time to refactor it.
> >>>>
> >>>> As said earlier in response of point 2 code duplication is because the
> >>>> way controls are derived.
> >>>>
> >>>>>
> >>>>> +++ b/web/pgadmin/static/js/backform.pgadmin.js
> >>>>> @@ -1528,7 +1528,18 @@
> >>>>>
> >>>>> @@ -1573,25 +1584,23 @@
> >>>>>
> >>>>> @@ -1631,7 +1640,18 @@
> >>>>>
> >>>>> @@ -1676,25 +1696,23 @@
> >>>>>
> >>>>>
> >>>>> Thanks
> >>>>> Joao & Shruti
> >>>>>
> >>>>> On Thu, May 18, 2017 at 6:01 AM, Harshal Dhumal
> >>>>> <[email protected]> wrote:
> >>>>>>
> >>>>>> Hi,
> >>>>>>
> >>>>>> Please find attached patch for RM2421
> >>>>>>
> >>>>>> Issue fixed: 1. Integer/numeric Validation is not working properly.
> >>>>>> 2. Wrong CPU rate unit
> >>>>>> --
> >>>>>> Harshal Dhumal
> >>>>>> Sr. Software Engineer
> >>>>>>
> >>>>>> EnterpriseDB India: http://www.enterprisedb.com
> >>>>>> The Enterprise PostgreSQL Company
> >>>>>>
> >>>>>>
> >>>>>> --
> >>>>>> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.
> org)
> >>>>>> To make changes to your subscription:
> >>>>>> http://www.postgresql.org/mailpref/pgadmin-hackers
> >>>>>>
> >>>>>
> >>>>
> >>>
> >>
> >
> >
> >
> > --
> > Sent via pgadmin-hackers mailing list ([email protected])
> > To make changes to your subscription:
> > http://www.postgresql.org/mailpref/pgadmin-hackers
> >
>
>
>
> --
> 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:
[text/x-patch] RM2421_V4.patch (33.0K, 3-RM2421_V4.patch)
download | inline diff:
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/foreign_tables/templates/foreign_tables/js/foreign_tables.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/foreign_tables/templates/foreign_tables/js/foreign_tables.js
index 5f3dc69..b7df585 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/foreign_tables/templates/foreign_tables/js/foreign_tables.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/foreign_tables/templates/foreign_tables/js/foreign_tables.js
@@ -101,34 +101,56 @@ function($, _, S, pgAdmin, pgBrowser, alertify) {
cell: 'string', group: '{{ _('Definition') }}',
type: 'int', deps: ['datatype'],
disabled: function(m) {
- // We will store type from selected from combobox
+ var val = m.get('typlen');
+ // We will store type from selected from combobox
if(!(_.isUndefined(m.get('inheritedid'))
|| _.isNull(m.get('inheritedid'))
|| _.isUndefined(m.get('inheritedfrom'))
- || _.isNull(m.get('inheritedfrom')))) { return true; }
+ || _.isNull(m.get('inheritedfrom')))) {
- var of_type = m.get('datatype');
- if(m.type_options) {
- m.set('is_tlength', false, {silent: true});
+ if (!_.isUndefined(val)) {
+ setTimeout(function() {
+ m.set('typlen', undefined);
+ }, 10);
+ }
+ return true;
+ }
+
+ var of_type = m.get('datatype'),
+ has_length = false;
+ if(m.type_options) {
+ m.set('is_tlength', false, {silent: true});
- // iterating over all the types
- _.each(m.type_options, function(o) {
- // if type from selected from combobox matches in options
- if ( of_type == o.value ) {
- m.set('typlen', undefined);
+ // iterating over all the types
+ _.each(m.type_options, function(o) {
+ // if type from selected from combobox matches in options
+ if ( of_type == o.value ) {
// if length is allowed for selected type
if(o.length)
{
// set the values in model
+ has_length = true;
m.set('is_tlength', true, {silent: true});
m.set('min_val', o.min_val, {silent: true});
m.set('max_val', o.max_val, {silent: true});
}
+ }
+ });
+
+ if (!has_length && !_.isUndefined(val)) {
+ setTimeout(function() {
+ m.set('typlen', undefined);
+ }, 10);
}
- });
- return !(m.get('is_tlength'));
- }
- return true;
+
+ return !(m.get('is_tlength'));
+ }
+ if (!has_length && !_.isUndefined(val)) {
+ setTimeout(function() {
+ m.set('typlen', undefined);
+ }, 10);
+ }
+ return true;
},
cellHeaderClasses: 'width_percent_10'
},{
@@ -136,33 +158,54 @@ function($, _, S, pgAdmin, pgBrowser, alertify) {
type: 'int', deps: ['datatype'],
cell: 'string', group: '{{ _('Definition') }}',
disabled: function(m) {
+ var val = m.get('precision');
if(!(_.isUndefined(m.get('inheritedid'))
|| _.isNull(m.get('inheritedid'))
|| _.isUndefined(m.get('inheritedfrom'))
- || _.isNull(m.get('inheritedfrom')))) { return true; }
+ || _.isNull(m.get('inheritedfrom')))) {
+
+ if (!_.isUndefined(val)) {
+ setTimeout(function() {
+ m.set('precision', undefined);
+ }, 10);
+ }
+ return true;
+ }
+
+ var of_type = m.get('datatype'),
+ has_precision = false;
- var of_type = m.get('datatype');
if(m.type_options) {
m.set('is_precision', false, {silent: true});
// iterating over all the types
_.each(m.type_options, function(o) {
// if type from selected from combobox matches in options
if ( of_type == o.value ) {
- m.set('precision', undefined);
// if precession is allowed for selected type
if(o.precision)
{
+ has_precision = true;
// set the values in model
m.set('is_precision', true, {silent: true});
m.set('min_val', o.min_val, {silent: true});
m.set('max_val', o.max_val, {silent: true});
}
+ }
+ });
+ if (!has_precision && !_.isUndefined(val)) {
+ setTimeout(function() {
+ m.set('precision', undefined);
+ }, 10);
}
- });
- return !(m.get('is_precision'));
- }
- return true;
- }, cellHeaderClasses: 'width_percent_10'
+ return !(m.get('is_precision'));
+ }
+ if (!has_precision && !_.isUndefined(val)) {
+ setTimeout(function() {
+ m.set('precision', undefined);
+ }, 10);
+ }
+ return true;
+ }, cellHeaderClasses: 'width_percent_10'
},{
id: 'typdefault', label:'{{ _('Default') }}', type: 'text',
cell: 'string', min_version: 90300, group: '{{ _('Definition') }}',
@@ -216,22 +259,23 @@ function($, _, S, pgAdmin, pgBrowser, alertify) {
min_version: 90200
}],
validate: function() {
- var err = {},
- errmsg;
+ var errmsg = null;
if (_.isUndefined(this.get('attname')) || String(this.get('attname')).replace(/^\s+|\s+$/g, '') == '') {
- err['name'] = '{{ _('Column Name cannot be empty!') }}';
- errmsg = errmsg || err['attname'];
+ errmsg = '{{ _('Column Name cannot be empty!') }}';
+ this.errorModel.set('attname', errmsg);
+ } else {
+ this.errorModel.unset('attname');
}
if (_.isUndefined(this.get('datatype')) || String(this.get('datatype'))
.replace(/^\s+|\s+$/g, '') == '') {
- err['basensp'] = '{{ _('Column Datatype cannot be empty!') }}';
- errmsg = errmsg || err['datatype'];
+ errmsg = '{{ _('Column Datatype cannot be empty!') }}';
+ this.errorModel.set('datatype', errmsg);
+ } else {
+ this.errorModel.unset('datatype');
}
- this.errorModel.clear().set(err);
-
return errmsg;
},
is_editable_column: function(m) {
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/sequences/templates/sequence/js/sequence.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/sequences/templates/sequence/js/sequence.js
index 2fb7ae5..1b9db50 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/sequences/templates/sequence/js/sequence.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/sequences/templates/sequence/js/sequence.js
@@ -153,7 +153,10 @@ function($, _, S, pgAdmin, pgBrowser, alertify) {
min: 1
},{
id: 'start', label: '{{ _('Start') }}', type: 'int',
- mode: ['properties', 'create'], group: '{{ _('Definition') }}'
+ mode: ['properties', 'create', 'edit'], group: '{{ _('Definition') }}',
+ disabled: function(m) {
+ return !m.isNew();
+ }
},{
id: 'minimum', label: '{{ _('Minimum') }}', type: 'int',
mode: ['properties', 'create', 'edit'], group: '{{ _('Definition') }}'
@@ -199,14 +202,14 @@ function($, _, S, pgAdmin, pgBrowser, alertify) {
minimum = this.get('minimum'),
maximum = this.get('maximum');
start = this.get('start');
- // Clear any existing error msg.
- this.errorModel.clear();
if (_.isUndefined(this.get('name'))
|| String(this.get('name')).replace(/^\s+|\s+$/g, '') == '') {
msg = '{{ _('Name cannot be empty.') }}';
this.errorModel.set('name', msg);
return msg;
+ } else {
+ this.errorModel.unset('name');
}
if (_.isUndefined(this.get('seqowner'))
@@ -214,6 +217,8 @@ function($, _, S, pgAdmin, pgBrowser, alertify) {
msg = '{{ _('Owner cannot be empty.') }}';
this.errorModel.set('seqowner', msg);
return msg;
+ } else {
+ this.errorModel.unset('seqowner');
}
if (_.isUndefined(this.get('schema'))
@@ -221,26 +226,81 @@ function($, _, S, pgAdmin, pgBrowser, alertify) {
msg = '{{ _('Schema cannot be empty.') }}';
this.errorModel.set('schema', msg);
return msg;
+ } else {
+ this.errorModel.unset('schema');
+ }
+
+ if (!this.isNew()) {
+ if (_.isUndefined(this.get('current_value'))
+ || String(this.get('current_value')).replace(/^\s+|\s+$/g, '') == '') {
+ msg = '{{ _('Current value cannot be empty.') }}';
+ this.errorModel.set('current_value', msg);
+ return msg;
+ } else {
+ this.errorModel.unset('current_value');
+ }
+
+ if (_.isUndefined(this.get('increment'))
+ || String(this.get('increment')).replace(/^\s+|\s+$/g, '') == '') {
+ msg = '{{ _('Increment value cannot be empty.') }}';
+ this.errorModel.set('increment', msg);
+ return msg;
+ } else {
+ this.errorModel.unset('increment');
+ }
+
+ if (_.isUndefined(this.get('minimum'))
+ || String(this.get('minimum')).replace(/^\s+|\s+$/g, '') == '') {
+ msg = '{{ _('Minimum value cannot be empty.') }}';
+ this.errorModel.set('minimum', msg);
+ return msg;
+ } else {
+ this.errorModel.unset('minimum');
+ }
+
+ if (_.isUndefined(this.get('maximum'))
+ || String(this.get('maximum')).replace(/^\s+|\s+$/g, '') == '') {
+ msg = '{{ _('Maximum value cannot be empty.') }}';
+ this.errorModel.set('maximum', msg);
+ return msg;
+ } else {
+ this.errorModel.unset('maximum');
+ }
+
+ if (_.isUndefined(this.get('cache'))
+ || String(this.get('cache')).replace(/^\s+|\s+$/g, '') == '') {
+ msg = '{{ _('Cache value cannot be empty.') }}';
+ this.errorModel.set('cache', msg);
+ return msg;
+ } else {
+ this.errorModel.unset('cache');
+ }
}
var min_lt = '{{ _('Minimum value must be less than maximum value.') }}',
start_lt = '{{ _('Start value cannot be less than minimum value.') }}',
start_gt = '{{ _('Start value cannot be greater than maximum value.') }}';
+
if ((minimum == 0 && maximum == 0) ||
(parseInt(minimum, 10) >= parseInt(maximum, 10))) {
- msg = min_lt
- this.errorModel.set('minimum', msg);
- return msg;
+ this.errorModel.set('minimum', min_lt);
+ return min_lt;
+ } else {
+ this.errorModel.unset('minimum');
}
- else if (start < minimum) {
- msg = start_lt
- this.errorModel.set('start', msg);
- return msg;
+
+ if (start && minimum && parseInt(start) < parseInt(minimum)) {
+ this.errorModel.set('start', start_lt);
+ return start_lt;
+ } else {
+ this.errorModel.unset('start');
}
- else if (start > maximum) {
- msg = start_gt
- this.errorModel.set('start', msg);
- return msg;
+
+ if (start && maximum && parseInt(start) > parseInt(maximum)) {
+ this.errorModel.set('start', start_gt);
+ return start_gt;
+ } else {
+ this.errorModel.unset('start');
}
return null;
}
diff --git a/web/pgadmin/browser/server_groups/servers/resource_groups/templates/resource_groups/js/resource_groups.js b/web/pgadmin/browser/server_groups/servers/resource_groups/templates/resource_groups/js/resource_groups.js
index 3cc822e..26e63bf 100644
--- a/web/pgadmin/browser/server_groups/servers/resource_groups/templates/resource_groups/js/resource_groups.js
+++ b/web/pgadmin/browser/server_groups/servers/resource_groups/templates/resource_groups/js/resource_groups.js
@@ -85,54 +85,35 @@ define(
*/
validate: function(keys) {
- /* Check whether 'name' is present in 'keys', if it is present
- * it means there is a change in that field from the GUI, so we
- * need to validate it.
- */
- if (_.indexOf(keys, 'name') >= 0) {
- var name = this.get('name');
- if (_.isUndefined(name) || _.isNull(name) ||
- String(name).replace(/^\s+|\s+$/g, '') == '') {
- var msg = '{{ _('Name cannot be empty.') }}';
- this.errorModel.set('name', msg);
- return msg;
- } else {
- this.errorModel.unset('name');
- }
+ var name = this.get('name');
+ if (_.isUndefined(name) || _.isNull(name) ||
+ String(name).replace(/^\s+|\s+$/g, '') == '') {
+ var msg = '{{ _('Name cannot be empty.') }}';
+ this.errorModel.set('name', msg);
+ return msg;
+ } else {
+ this.errorModel.unset('name');
}
- /* Check whether 'cpu_rate_limit' is present in 'keys', if it is present
- * it means there is a change in that field from the GUI, so we
- * need to validate it.
- */
- if (_.indexOf(keys, 'cpu_rate_limit') >= 0) {
- var cpu_rate_limit = this.get('cpu_rate_limit');
- if (_.isUndefined(cpu_rate_limit) || _.isNull(cpu_rate_limit) ||
- String(cpu_rate_limit).replace(/^\s+|\s+$/g, '') == '') {
- var msg = '{{ _('CPU rate limit cannot be empty.') }}';
- this.errorModel.set('cpu_rate_limit', msg);
- return msg;
- } else {
- this.errorModel.unset('cpu_rate_limit');
- }
+ var cpu_rate_limit = this.get('cpu_rate_limit');
+ if (_.isUndefined(cpu_rate_limit) || _.isNull(cpu_rate_limit) ||
+ String(cpu_rate_limit).replace(/^\s+|\s+$/g, '') == '') {
+ var msg = '{{ _('CPU rate limit cannot be empty.') }}';
+ this.errorModel.set('cpu_rate_limit', msg);
+ return msg;
+ } else {
+ this.errorModel.unset('cpu_rate_limit');
}
- /* Check whether 'dirty_rate_limit' is present in 'keys', if it is present
- * it means there is a change in that field from the GUI, so we
- * need to validate it.
- */
- if (_.indexOf(keys, 'dirty_rate_limit') >= 0) {
- var dirty_rate_limit = this.get('dirty_rate_limit');
- if (_.isUndefined(dirty_rate_limit) || _.isNull(dirty_rate_limit) ||
- String(dirty_rate_limit).replace(/^\s+|\s+$/g, '') == '') {
- var msg = '{{ _('Dirty rate limit cannot be empty.') }}';
- this.errorModel.set('dirty_rate_limit', msg);
- return msg;
- } else {
- this.errorModel.unset('dirty_rate_limit');
- }
+ var dirty_rate_limit = this.get('dirty_rate_limit');
+ if (_.isUndefined(dirty_rate_limit) || _.isNull(dirty_rate_limit) ||
+ String(dirty_rate_limit).replace(/^\s+|\s+$/g, '') == '') {
+ var msg = '{{ _('Dirty rate limit cannot be empty.') }}';
+ this.errorModel.set('dirty_rate_limit', msg);
+ return msg;
+ } else {
+ this.errorModel.unset('dirty_rate_limit');
}
-
return null;
}
})
diff --git a/web/pgadmin/browser/server_groups/servers/roles/templates/role/js/role.js b/web/pgadmin/browser/server_groups/servers/roles/templates/role/js/role.js
index a85416c..c74cd59 100644
--- a/web/pgadmin/browser/server_groups/servers/roles/templates/role/js/role.js
+++ b/web/pgadmin/browser/server_groups/servers/roles/templates/role/js/role.js
@@ -427,7 +427,7 @@ function($, _, S, pgAdmin, pgBrowser, alertify, Backform) {
deps: ['rolcanlogin'], options: {format: 'YYYY-MM-DD HH:mm:ss Z'}
},{
id: 'rolconnlimit', type: 'int', group: '{{ _('Definition') }}',
- label: '{{ _('Connection limit') }}', cell: 'number',
+ label: '{{ _('Connection limit') }}', cell: 'number', min : -1,
mode: ['properties', 'edit', 'create'], disabled: 'readonly'
},{
id: 'rolcanlogin', label:'{{ _('Can login?') }}', type: 'switch',
diff --git a/web/pgadmin/browser/server_groups/servers/templates/servers/servers.js b/web/pgadmin/browser/server_groups/servers/templates/servers/servers.js
index 9eb82b8..bf28474 100644
--- a/web/pgadmin/browser/server_groups/servers/templates/servers/servers.js
+++ b/web/pgadmin/browser/server_groups/servers/templates/servers/servers.js
@@ -727,6 +727,9 @@ function($, _, S, pgAdmin, pgBrowser, alertify) {
check_for_empty(
'username', '{{ _('Username must be specified.') }}'
);
+ check_for_empty(
+ 'port', '{{ _('Port must be specified.') }}'
+ );
this.errorModel.set(err);
if (_.size(err)) {
diff --git a/web/pgadmin/browser/static/js/datamodel.js b/web/pgadmin/browser/static/js/datamodel.js
index 5b1c3a7..ad8f95f 100644
--- a/web/pgadmin/browser/static/js/datamodel.js
+++ b/web/pgadmin/browser/static/js/datamodel.js
@@ -1,6 +1,6 @@
define(
- ['underscore', 'pgadmin', 'jquery', 'backbone'],
-function(_, pgAdmin, $, Backbone) {
+ ['underscore', 'underscore.string', 'pgadmin', 'jquery', 'backbone'],
+function(_, S, pgAdmin, $, Backbone) {
var pgBrowser = pgAdmin.Browser = pgAdmin.Browser || {};
pgBrowser.DataModel = Backbone.Model.extend({
@@ -136,6 +136,7 @@ function(_, pgAdmin, $, Backbone) {
}
self.sessAttrs = {};
+ self.fieldData = {};
self.origSessAttrs = {};
self.objects = [];
self.arrays = [];
@@ -153,6 +154,25 @@ function(_, pgAdmin, $, Backbone) {
_.each(schema, function(s) {
switch(s.type) {
+ case 'int':
+ case 'numeric':
+ self.fieldData[s.id] = {
+ id: s.id,
+ label: s.label,
+ type: s.type,
+ min: s.min || undefined,
+ max: s.max || undefined
+ }
+ break;
+ default:
+ self.fieldData[s.id] = {
+ id: s.id,
+ label: s.label,
+ type: s.type
+ }
+ }
+
+ switch(s.type) {
case 'array':
self.arrays.push(s.id);
@@ -280,6 +300,12 @@ function(_, pgAdmin, $, Backbone) {
},
sessValid: function() {
var self = this;
+ // Perform default validations.
+ if ('default_validate' in self && typeof(self.default_validate) == 'function' &&
+ _.isString(self.default_validate())) {
+ return false;
+ }
+
if ('validate' in self && _.isFunction(self.validate) &&
_.isString(self.validate.apply(self))) {
return false;
@@ -301,8 +327,9 @@ function(_, pgAdmin, $, Backbone) {
}
if (key != null && res) {
- var attrs = {};
- var self = this;
+ var attrs = {},
+ self = this,
+ msg;
attrChanged = function(v, k) {
if (k in self.objects) {
@@ -327,9 +354,18 @@ function(_, pgAdmin, $, Backbone) {
if (!options || !options.silent) {
self.trigger('change', self, options);
}
+
+ // Perform default validations.
+
+ if ('default_validate' in self && typeof(self.default_validate) == 'function') {
+ msg = self.default_validate();
+ }
+
if ('validate' in self && typeof(self['validate']) === 'function') {
- var msg = self.validate(_.keys(attrs));
+ if (!msg) {
+ msg = self.validate(_.keys(attrs));
+ }
/*
* If any parent present, we will need to inform the parent - that
@@ -562,6 +598,13 @@ function(_, pgAdmin, $, Backbone) {
var msg = null,
validate = function(m, attrs) {
+ if ('default_validate' in m && typeof(m.default_validate) == 'function') {
+ msg = m.default_validate();
+ if (_.isString(msg)) {
+ return msg;
+ }
+ }
+
if ('validate' in m && typeof(m.validate) == 'function') {
msg = m.validate(attrs);
@@ -655,6 +698,79 @@ function(_, pgAdmin, $, Backbone) {
});
self.trigger('pgadmin-session:stop');
+ },
+ default_validate: function() {
+ var msg, field, value, type;
+
+ for (var i = 0, keys = _.keys(this.attributes), l = keys.length;
+ i<l;
+ i++) {
+
+ value = this.attributes[keys[i]];
+ field = this.fieldData[keys[i]]
+ msg = null;
+
+ if (!(_.isUndefined(value) || _.isNull(value) ||
+ String(value).replace(/^\s+|\s+$/g, '') == '')) {
+
+ if (!field) {
+ continue;
+ }
+
+ type = field.type || undefined;
+ if (!type) {
+ continue;
+ }
+
+ switch(type) {
+ case 'int':
+ msg = this.integer_validate(value, field);
+ break;
+ case 'numeric':
+ msg = this.number_validate(value, field);
+ break;
+ }
+
+ if (msg) {
+ this.errorModel.set(field.id, msg);
+ return msg;
+ } else {
+ this.errorModel.unset(field.id);
+ }
+ } else {
+ if (field) {
+ this.errorModel.unset(field.id);
+ }
+ }
+ }
+ return null;
+ },
+
+ check_min_max: function (value, field) {
+ var label = field.label,
+ min_value = field.min,
+ max_value = field.max;
+
+ if (min_value && value < min_value) {
+ return S(pgAdmin.Browser.messages.MUST_GR_EQ).sprintf(label, min_value).value();
+ } else if (max_value && value > max_value) {
+ return S(pgAdmin.Browser.messages.MUST_LESS_EQ).sprintf(label, max_value).value();
+ }
+ return null;
+ },
+ number_validate: function (value, field) {
+ var pattern = new RegExp("^-?[0-9]+(\.?[0-9]*)?$");
+ if (!pattern.test(value)) {
+ return S(pgAdmin.Browser.messages.MUST_BE_NUM).sprintf(field.label).value()
+ }
+ return this.check_min_max(value, field)
+ },
+ integer_validate: function(value, field) {
+ var pattern = new RegExp("^-?[0-9]*$");
+ if (!pattern.test(value)) {
+ return S(pgAdmin.Browser.messages.MUST_BE_INT).sprintf(field.label).value()
+ }
+ return this.check_min_max(value, field)
}
});
@@ -696,7 +812,8 @@ function(_, pgAdmin, $, Backbone) {
return self;
},
startNewSession: function() {
- var self = this;
+ var self = this,
+ msg;
if (self.trackChanges) {
// We're stopping the existing session.
@@ -718,8 +835,15 @@ function(_, pgAdmin, $, Backbone) {
if ('startNewSession' in m && _.isFunction(m.startNewSession)) {
m.startNewSession();
}
- if ('validate' in m && typeof(m.validate) === 'function') {
- var msg = m.validate();
+
+ if ('default_validate' in m && typeof(m.default_validate) == 'function') {
+ msg = m.default_validate();
+ }
+
+ if (_.isString(msg)) {
+ self.sessAttrs['invalid'][m.cid] = msg;
+ } else if ('validate' in m && typeof(m.validate) === 'function') {
+ msg = m.validate();
if (msg) {
self.sessAttrs['invalid'][m.cid] = msg;
@@ -900,7 +1024,14 @@ function(_, pgAdmin, $, Backbone) {
(self.handler || self).trigger('pgadmin-session:added', self, obj);
- if ('validate' in obj && typeof(obj.validate) === 'function') {
+
+ if ('default_validate' in obj && typeof(obj.default_validate) == 'function') {
+ msg = obj.default_validate();
+ }
+
+ if (_.isString(msg)) {
+ (self.sessAttrs['invalid'])[obj.cid] = msg;
+ } else if ('validate' in obj && typeof(obj.validate) === 'function') {
msg = obj.validate();
if (msg) {
@@ -908,7 +1039,14 @@ function(_, pgAdmin, $, Backbone) {
}
}
} else {
- if ('validate' in obj && typeof(obj.validate) === 'function') {
+
+ if ('default_validate' in obj && typeof(obj.default_validate) == 'function') {
+ msg = obj.default_validate();
+ }
+
+ if (_.isString(msg)) {
+ (self.sessAttrs['invalid'])[obj.cid] = msg;
+ } else if ('validate' in obj && typeof(obj.validate) === 'function') {
msg = obj.validate();
if (msg) {
diff --git a/web/pgadmin/static/js/backform.pgadmin.js b/web/pgadmin/static/js/backform.pgadmin.js
index 553676e..13823b7 100644
--- a/web/pgadmin/static/js/backform.pgadmin.js
+++ b/web/pgadmin/static/js/backform.pgadmin.js
@@ -57,7 +57,7 @@
});
var controlMapper = Backform.controlMapper = {
- 'int': ['uneditable-input', 'integer', 'integer'],
+ 'int': ['uneditable-input', 'numeric', 'numeric'],
'text': ['uneditable-input', 'input', 'string'],
'numeric': ['uneditable-input', 'numeric', 'numeric'],
'date': 'datepicker',
@@ -1493,109 +1493,6 @@
}
});
- /*
- * Integer input Control functionality just like backgrid
- */
- var IntegerControl = Backform.IntegerControl = Backform.InputControl.extend({
- defaults: {
- type: "number",
- label: "",
- min: undefined,
- max: undefined,
- maxlength: 255,
- extraClasses: [],
- helpMessage: null
- },
- template: _.template([
- '<label class="<%=Backform.controlLabelClassName%>"><%=label%></label>',
- '<div class="<%=Backform.controlsClassName%>">',
- ' <input type="<%=type%>" class="<%=Backform.controlClassName%> <%=extraClasses.join(\' \')%>" name="<%=name%>" min="<%=min%>" max="<%=max%>"maxlength="<%=maxlength%>" value="<%-value%>" placeholder="<%-placeholder%>" <%=disabled ? "disabled" : ""%> <%=required ? "required" : ""%> />',
- ' <% if (helpMessage && helpMessage.length) { %>',
- ' <span class="<%=Backform.helpMessageClassName%>"><%=helpMessage%></span>',
- ' <% } %>',
- '</div>'
- ].join("\n")),
- events: {
- "change input": "checkInt",
- "focus input": "clearInvalid"
- },
- checkInt: function(e) {
- var field = _.defaults(this.field.toJSON(), this.defaults),
- attrArr = this.field.get("name").split('.'),
- name = attrArr.shift(),
- value = this.getValueFromDOM(),
- min_value = field.min,
- max_value = field.max,
- isValid = true,
- intPattern = new RegExp("^-?[0-9]*$"),
- isMatched = intPattern.test(value);
-
- // Below logic will validate input
- if (!isMatched) {
- isValid = false;
- this.model.errorModel.unset(name);
- this.model.errorModel.set(
- name,
- S(pgAdmin.Browser.messages.MUST_BE_INT).sprintf(
- field.label
- ).value()
- );
- }
-
- // Below will check if entered value is in-between min & max range
- if (isValid && (!_.isUndefined(min_value) && value < min_value)) {
- isValid = false;
- this.model.errorModel.unset(name);
- this.model.errorModel.set(
- name,
- S(pgAdmin.Browser.messages.MUST_GR_EQ).sprintf(
- field.label,
- min_value
- ).value()
- );
- }
-
- if (isValid && (!_.isUndefined(max_value) && value > max_value)) {
- isValid = false;
- this.model.errorModel.unset(name);
- this.model.errorModel.set(
- name,
- S(pgAdmin.Browser.messages.MUST_LESS_EQ).sprintf(
- field.label,
- max_value
- ).value()
- );
- }
-
- // After validation we need to set that value into model (only if all flags are true)
- if (isValid) {
- this.stopListening(this.model, "change:" + name, this.render);
- this.model.errorModel.unset(name);
- this.model.set(name, value);
- this.listenTo(this.model, "change:" + name, this.render);
- if (this.model.collection || this.model.handler) {
- (this.model.collection || this.model.handler).trigger(
- 'pgadmin-session:model:valid', this.model, (this.model.collection || this.model.handler)
- );
- } else {
- (this.model).trigger(
- 'pgadmin-session:valid', this.model.sessChanged(), this.model
- );
- }
- } else {
- if (this.model.collection || this.model.handler) {
- (this.model.collection || this.model.handler).trigger(
- 'pgadmin-session:model:invalid', this.model.errorModel.get(name), this.model
- );
- } else {
- (this.model).trigger(
- 'pgadmin-session:invalid', this.model.errorModel.get(name), this.model
- );
- }
- }
- }
- });
-
/*
* Numeric input Control functionality just like backgrid
*/
@@ -1617,86 +1514,7 @@
' <span class="<%=Backform.helpMessageClassName%>"><%=helpMessage%></span>',
' <% } %>',
'</div>'
- ].join("\n")),
- events: {
- "change input": "checkNumeric",
- "focus input": "clearInvalid"
- },
- checkNumeric: function(e) {
- var field = _.defaults(this.field.toJSON(), this.defaults),
- attrArr = this.field.get("name").split('.'),
- name = attrArr.shift(),
- value = this.getValueFromDOM(),
- min_value = field.min,
- max_value = field.max,
- isValid = true,
- intPattern = new RegExp("^-?[0-9]+(\.?[0-9]*)?$"),
- isMatched = intPattern.test(value);
-
- // Below logic will validate input
- if (!isMatched) {
- isValid = false;
- this.model.errorModel.unset(name);
- this.model.errorModel.set(
- name,
- S(pgAdmin.Browser.messages.MUST_BE_NUM).sprintf(
- field.label
- ).value()
- );
- }
-
- // Below will check if entered value is in-between min & max range
- if (isValid && (!_.isUndefined(min_value) && value < min_value)) {
- isValid = false;
- this.model.errorModel.unset(name);
- this.model.errorModel.set(
- name,
- S(pgAdmin.Browser.messages.MUST_GR_EQ).sprintf(
- field.label,
- min_value
- ).value()
- );
- }
-
- if (isValid && (!_.isUndefined(max_value) && value > max_value)) {
- isValid = false;
- this.model.errorModel.unset(name);
- this.model.errorModel.set(
- name,
- S(pgAdmin.Browser.messages.MUST_LESS_EQ).sprintf(
- field.label,
- max_value
- ).value()
- );
- }
-
- // After validation we need to set that value into model (only if all flags are true)
- if (isValid) {
- this.stopListening(this.model, "change:" + name, this.render);
- this.model.errorModel.unset(name);
- this.model.set(name, value);
- this.listenTo(this.model, "change:" + name, this.render);
- if (this.model.collection || this.model.handler) {
- (this.model.collection || this.model.handler).trigger(
- 'pgadmin-session:model:valid', this.model, (this.model.collection || this.model.handler)
- );
- } else {
- (this.model).trigger(
- 'pgadmin-session:valid', this.model.sessChanged(), this.model
- );
- }
- } else {
- if (this.model.collection || this.model.handler) {
- (this.model.collection || this.model.handler).trigger(
- 'pgadmin-session:model:invalid', this.model.errorModel.get(name), this.model
- );
- } else {
- (this.model).trigger(
- 'pgadmin-session:invalid', this.model.errorModel.get(name), this.model
- );
- }
- }
- }
+ ].join("\n"))
});
///////
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Fix for RM2421 [pgAdmin4][patch]
2017-05-18 10:01 Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-05-18 14:27 ` Re: Fix for RM2421 [pgAdmin4][patch] Joao Pedro De Almeida Pereira <[email protected]>
2017-05-19 06:52 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-05-29 09:48 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-05-30 07:00 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-05-30 07:28 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-06-05 15:55 ` Re: Fix for RM2421 [pgAdmin4][patch] Dave Page <[email protected]>
2017-06-06 06:42 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
@ 2017-06-07 13:29 ` Dave Page <[email protected]>
2017-06-08 12:59 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Dave Page @ 2017-06-07 13:29 UTC (permalink / raw)
To: Harshal Dhumal <[email protected]>; +Cc: pgadmin-hackers
Can you rebase this please? I think Ashesh broke it :-p
On Tue, Jun 6, 2017 at 7:42 AM, Harshal Dhumal
<[email protected]> wrote:
> Hi,
>
> On Mon, Jun 5, 2017 at 9:25 PM, Dave Page <[email protected]> wrote:
>>
>> Hi
>>
>> With this patch applied, it uses the field names instead of the labels
>> in error messages - e.g.
>>
>> 'dirty_rate_limit' must be numeric
>>
>> instead of:
>>
>> 'Dirty Rate Limit (KB)' must be numeric.
>
> Fixed. Please find attached updated patch.
>
>>
>>
>> Thanks.
>>
>> On Tue, May 30, 2017 at 8:28 AM, Harshal Dhumal
>> <[email protected]> wrote:
>> > Hi,
>> >
>> > Please find updated patch.
>> >
>> > --
>> > Harshal Dhumal
>> > Sr. Software Engineer
>> >
>> > EnterpriseDB India: http://www.enterprisedb.com
>> > The Enterprise PostgreSQL Company
>> >
>> > On Tue, May 30, 2017 at 12:30 PM, Harshal Dhumal
>> > <[email protected]> wrote:
>> >>
>> >> Hi,
>> >>
>> >> Please ignore this patch as I forgot to include few changes. I'll send
>> >> updated one.
>> >>
>> >> --
>> >> Harshal Dhumal
>> >> Sr. Software Engineer
>> >>
>> >> EnterpriseDB India: http://www.enterprisedb.com
>> >> The Enterprise PostgreSQL Company
>> >>
>> >> On Mon, May 29, 2017 at 3:18 PM, Harshal Dhumal
>> >> <[email protected]> wrote:
>> >>>
>> >>> Hi,
>> >>>
>> >>> Here is updated patch for RM2421.
>> >>>
>> >>> Now I have moved all Numeric control level validations to datamodel.
>> >>> As
>> >>> existing implementation was causing
>> >>> issues with error messages in create/edit dialog when schema contains
>> >>> two
>> >>> or more Numeric controls.
>> >>>
>> >>> This is generic issue and not related to resource group. Also I have
>> >>> updated all other nodes which uses Numeric controls
>> >>>
>> >>>
>> >>>
>> >>> --
>> >>> Harshal Dhumal
>> >>> Sr. Software Engineer
>> >>>
>> >>> EnterpriseDB India: http://www.enterprisedb.com
>> >>> The Enterprise PostgreSQL Company
>> >>>
>> >>> On Fri, May 19, 2017 at 12:22 PM, Harshal Dhumal
>> >>> <[email protected]> wrote:
>> >>>>
>> >>>> Hi,
>> >>>>
>> >>>> On Thu, May 18, 2017 at 7:57 PM, Joao Pedro De Almeida Pereira
>> >>>> <[email protected]> wrote:
>> >>>>>
>> >>>>> Hello Harshal,
>> >>>>>
>> >>>>> We review the patch and have some questions:
>> >>>>> 1) Is there any particular reason to initialize variables and
>> >>>>> functions
>> >>>>> in the same place? We believe that it would be more readable there
>> >>>>> were no
>> >>>>> chaining of variable creation, specially if those variables are
>> >>>>> functions.
>> >>>>> Check line:
>> >>>>
>> >>>> That function is only going to be used in checkNumeric function (in
>> >>>> case
>> >>>> of Number control) and checkInt function (in case of Integer control)
>> >>>> so
>> >>>> declared them locally.
>> >>>> Anyway I'm going to refactor both the controls as Number and Integer
>> >>>> shares some common properties.
>> >>>>
>> >>>>> +++ b/web/pgadmin/static/js/backform.pgadmin.js
>> >>>>> @@ -1528,7 +1528,18 @@
>> >>>>> max_value = field.max,
>> >>>>> isValid = true,
>> >>>>> intPattern = new RegExp("^-?[0-9]*$"),
>> >>>>> - isMatched = intPattern.test(value);
>> >>>>> + isMatched = intPattern.test(value),
>> >>>>> + trigger_invalid_event = function(msg) {
>> >>>>>
>> >>>>> 2) The functions added in both places look very similar, can they be
>> >>>>> merged and extracted? We are talking about the trigger_invalid_event
>> >>>>> function.
>> >>>>
>> >>>> Yes they can be merged. As of now both NumericControl and
>> >>>> IntegerControl
>> >>>> are derived from InputControl. Ideally
>> >>>> only NumericControl should be derived from InputControl and
>> >>>> IntegerControl should be derive from NumericControl.
>> >>>>
>> >>>>
>> >>>>>
>> >>>>> 3) The following change is very similar to the
>> >>>>> trigger_invalid_event,
>> >>>>> was there a reason not to use it?
>> >>>>
>> >>>> Below code triggers "model valid" event; opposite to "model invalid"
>> >>>> event (trigger_invalid_event)
>> >>>>>
>> >>>>> +++ b/web/pgadmin/static/js/backform.pgadmin.js
>> >>>>> @@ -1573,25 +1584,23 @@
>> >>>>> this.model.errorModel.unset(name);
>> >>>>> this.model.set(name, value);
>> >>>>> this.listenTo(this.model, "change:" + name, this.render);
>> >>>>> - if (this.model.collection || this.model.handler) {
>> >>>>> - (this.model.collection || this.model.handler).trigger(
>> >>>>> - 'pgadmin-session:model:valid', this.model,
>> >>>>> (this.model.collection || this.model.handler)
>> >>>>> - );
>> >>>>> + // Check if other fields of same model are valid before
>> >>>>> + // triggering 'session:valid' event
>> >>>>> + if(_.size(this.model.errorModel.attributes) == 0) {
>> >>>>> + if (this.model.collection || this.model.handler) {
>> >>>>> + (this.model.collection || this.model.handler).trigger(
>> >>>>> + 'pgadmin-session:model:valid', this.model,
>> >>>>> (this.model.collection || this.model.handler)
>> >>>>> + );
>> >>>>> + } else {
>> >>>>> + (this.model).trigger(
>> >>>>> + 'pgadmin-session:valid', this.model.sessChanged(),
>> >>>>> this.model
>> >>>>> + );
>> >>>>> + }
>> >>>>>
>> >>>>> 4) We also noticed that the following change sets look very
>> >>>>> similiar.
>> >>>>> Is there any reason to have this code duplicated? If not this could
>> >>>>> be a
>> >>>>> good time to refactor it.
>> >>>>
>> >>>> As said earlier in response of point 2 code duplication is because
>> >>>> the
>> >>>> way controls are derived.
>> >>>>
>> >>>>>
>> >>>>> +++ b/web/pgadmin/static/js/backform.pgadmin.js
>> >>>>> @@ -1528,7 +1528,18 @@
>> >>>>>
>> >>>>> @@ -1573,25 +1584,23 @@
>> >>>>>
>> >>>>> @@ -1631,7 +1640,18 @@
>> >>>>>
>> >>>>> @@ -1676,25 +1696,23 @@
>> >>>>>
>> >>>>>
>> >>>>> Thanks
>> >>>>> Joao & Shruti
>> >>>>>
>> >>>>> On Thu, May 18, 2017 at 6:01 AM, Harshal Dhumal
>> >>>>> <[email protected]> wrote:
>> >>>>>>
>> >>>>>> Hi,
>> >>>>>>
>> >>>>>> Please find attached patch for RM2421
>> >>>>>>
>> >>>>>> Issue fixed: 1. Integer/numeric Validation is not working properly.
>> >>>>>> 2. Wrong CPU rate unit
>> >>>>>> --
>> >>>>>> Harshal Dhumal
>> >>>>>> Sr. Software Engineer
>> >>>>>>
>> >>>>>> EnterpriseDB India: 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
>> >>>>>>
>> >>>>>
>> >>>>
>> >>>
>> >>
>> >
>> >
>> >
>> > --
>> > Sent via pgadmin-hackers mailing list ([email protected])
>> > To make changes to your subscription:
>> > http://www.postgresql.org/mailpref/pgadmin-hackers
>> >
>>
>>
>>
>> --
>> 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
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Fix for RM2421 [pgAdmin4][patch]
2017-05-18 10:01 Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-05-18 14:27 ` Re: Fix for RM2421 [pgAdmin4][patch] Joao Pedro De Almeida Pereira <[email protected]>
2017-05-19 06:52 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-05-29 09:48 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-05-30 07:00 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-05-30 07:28 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-06-05 15:55 ` Re: Fix for RM2421 [pgAdmin4][patch] Dave Page <[email protected]>
2017-06-06 06:42 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-06-07 13:29 ` Re: Fix for RM2421 [pgAdmin4][patch] Dave Page <[email protected]>
@ 2017-06-08 12:59 ` Harshal Dhumal <[email protected]>
2017-06-08 13:59 ` Re: Fix for RM2421 [pgAdmin4][patch] Dave Page <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Harshal Dhumal @ 2017-06-08 12:59 UTC (permalink / raw)
To: Dave Page <[email protected]>; +Cc: pgadmin-hackers
Please find attached rebased patch.
--
*Harshal Dhumal*
*Sr. Software Engineer*
EnterpriseDB India: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Wed, Jun 7, 2017 at 6:59 PM, Dave Page <[email protected]> wrote:
> Can you rebase this please? I think Ashesh broke it :-p
>
> On Tue, Jun 6, 2017 at 7:42 AM, Harshal Dhumal
> <[email protected]> wrote:
> > Hi,
> >
> > On Mon, Jun 5, 2017 at 9:25 PM, Dave Page <[email protected]> wrote:
> >>
> >> Hi
> >>
> >> With this patch applied, it uses the field names instead of the labels
> >> in error messages - e.g.
> >>
> >> 'dirty_rate_limit' must be numeric
> >>
> >> instead of:
> >>
> >> 'Dirty Rate Limit (KB)' must be numeric.
> >
> > Fixed. Please find attached updated patch.
> >
> >>
> >>
> >> Thanks.
> >>
> >> On Tue, May 30, 2017 at 8:28 AM, Harshal Dhumal
> >> <[email protected]> wrote:
> >> > Hi,
> >> >
> >> > Please find updated patch.
> >> >
> >> > --
> >> > Harshal Dhumal
> >> > Sr. Software Engineer
> >> >
> >> > EnterpriseDB India: http://www.enterprisedb.com
> >> > The Enterprise PostgreSQL Company
> >> >
> >> > On Tue, May 30, 2017 at 12:30 PM, Harshal Dhumal
> >> > <[email protected]> wrote:
> >> >>
> >> >> Hi,
> >> >>
> >> >> Please ignore this patch as I forgot to include few changes. I'll
> send
> >> >> updated one.
> >> >>
> >> >> --
> >> >> Harshal Dhumal
> >> >> Sr. Software Engineer
> >> >>
> >> >> EnterpriseDB India: http://www.enterprisedb.com
> >> >> The Enterprise PostgreSQL Company
> >> >>
> >> >> On Mon, May 29, 2017 at 3:18 PM, Harshal Dhumal
> >> >> <[email protected]> wrote:
> >> >>>
> >> >>> Hi,
> >> >>>
> >> >>> Here is updated patch for RM2421.
> >> >>>
> >> >>> Now I have moved all Numeric control level validations to datamodel.
> >> >>> As
> >> >>> existing implementation was causing
> >> >>> issues with error messages in create/edit dialog when schema
> contains
> >> >>> two
> >> >>> or more Numeric controls.
> >> >>>
> >> >>> This is generic issue and not related to resource group. Also I have
> >> >>> updated all other nodes which uses Numeric controls
> >> >>>
> >> >>>
> >> >>>
> >> >>> --
> >> >>> Harshal Dhumal
> >> >>> Sr. Software Engineer
> >> >>>
> >> >>> EnterpriseDB India: http://www.enterprisedb.com
> >> >>> The Enterprise PostgreSQL Company
> >> >>>
> >> >>> On Fri, May 19, 2017 at 12:22 PM, Harshal Dhumal
> >> >>> <[email protected]> wrote:
> >> >>>>
> >> >>>> Hi,
> >> >>>>
> >> >>>> On Thu, May 18, 2017 at 7:57 PM, Joao Pedro De Almeida Pereira
> >> >>>> <[email protected]> wrote:
> >> >>>>>
> >> >>>>> Hello Harshal,
> >> >>>>>
> >> >>>>> We review the patch and have some questions:
> >> >>>>> 1) Is there any particular reason to initialize variables and
> >> >>>>> functions
> >> >>>>> in the same place? We believe that it would be more readable there
> >> >>>>> were no
> >> >>>>> chaining of variable creation, specially if those variables are
> >> >>>>> functions.
> >> >>>>> Check line:
> >> >>>>
> >> >>>> That function is only going to be used in checkNumeric function (in
> >> >>>> case
> >> >>>> of Number control) and checkInt function (in case of Integer
> control)
> >> >>>> so
> >> >>>> declared them locally.
> >> >>>> Anyway I'm going to refactor both the controls as Number and
> Integer
> >> >>>> shares some common properties.
> >> >>>>
> >> >>>>> +++ b/web/pgadmin/static/js/backform.pgadmin.js
> >> >>>>> @@ -1528,7 +1528,18 @@
> >> >>>>> max_value = field.max,
> >> >>>>> isValid = true,
> >> >>>>> intPattern = new RegExp("^-?[0-9]*$"),
> >> >>>>> - isMatched = intPattern.test(value);
> >> >>>>> + isMatched = intPattern.test(value),
> >> >>>>> + trigger_invalid_event = function(msg) {
> >> >>>>>
> >> >>>>> 2) The functions added in both places look very similar, can they
> be
> >> >>>>> merged and extracted? We are talking about the
> trigger_invalid_event
> >> >>>>> function.
> >> >>>>
> >> >>>> Yes they can be merged. As of now both NumericControl and
> >> >>>> IntegerControl
> >> >>>> are derived from InputControl. Ideally
> >> >>>> only NumericControl should be derived from InputControl and
> >> >>>> IntegerControl should be derive from NumericControl.
> >> >>>>
> >> >>>>
> >> >>>>>
> >> >>>>> 3) The following change is very similar to the
> >> >>>>> trigger_invalid_event,
> >> >>>>> was there a reason not to use it?
> >> >>>>
> >> >>>> Below code triggers "model valid" event; opposite to "model
> invalid"
> >> >>>> event (trigger_invalid_event)
> >> >>>>>
> >> >>>>> +++ b/web/pgadmin/static/js/backform.pgadmin.js
> >> >>>>> @@ -1573,25 +1584,23 @@
> >> >>>>> this.model.errorModel.unset(name);
> >> >>>>> this.model.set(name, value);
> >> >>>>> this.listenTo(this.model, "change:" + name, this.render);
> >> >>>>> - if (this.model.collection || this.model.handler) {
> >> >>>>> - (this.model.collection || this.model.handler).trigger(
> >> >>>>> - 'pgadmin-session:model:valid', this.model,
> >> >>>>> (this.model.collection || this.model.handler)
> >> >>>>> - );
> >> >>>>> + // Check if other fields of same model are valid before
> >> >>>>> + // triggering 'session:valid' event
> >> >>>>> + if(_.size(this.model.errorModel.attributes) == 0) {
> >> >>>>> + if (this.model.collection || this.model.handler) {
> >> >>>>> + (this.model.collection ||
> this.model.handler).trigger(
> >> >>>>> + 'pgadmin-session:model:valid', this.model,
> >> >>>>> (this.model.collection || this.model.handler)
> >> >>>>> + );
> >> >>>>> + } else {
> >> >>>>> + (this.model).trigger(
> >> >>>>> + 'pgadmin-session:valid', this.model.sessChanged(),
> >> >>>>> this.model
> >> >>>>> + );
> >> >>>>> + }
> >> >>>>>
> >> >>>>> 4) We also noticed that the following change sets look very
> >> >>>>> similiar.
> >> >>>>> Is there any reason to have this code duplicated? If not this
> could
> >> >>>>> be a
> >> >>>>> good time to refactor it.
> >> >>>>
> >> >>>> As said earlier in response of point 2 code duplication is because
> >> >>>> the
> >> >>>> way controls are derived.
> >> >>>>
> >> >>>>>
> >> >>>>> +++ b/web/pgadmin/static/js/backform.pgadmin.js
> >> >>>>> @@ -1528,7 +1528,18 @@
> >> >>>>>
> >> >>>>> @@ -1573,25 +1584,23 @@
> >> >>>>>
> >> >>>>> @@ -1631,7 +1640,18 @@
> >> >>>>>
> >> >>>>> @@ -1676,25 +1696,23 @@
> >> >>>>>
> >> >>>>>
> >> >>>>> Thanks
> >> >>>>> Joao & Shruti
> >> >>>>>
> >> >>>>> On Thu, May 18, 2017 at 6:01 AM, Harshal Dhumal
> >> >>>>> <[email protected]> wrote:
> >> >>>>>>
> >> >>>>>> Hi,
> >> >>>>>>
> >> >>>>>> Please find attached patch for RM2421
> >> >>>>>>
> >> >>>>>> Issue fixed: 1. Integer/numeric Validation is not working
> properly.
> >> >>>>>> 2. Wrong CPU rate unit
> >> >>>>>> --
> >> >>>>>> Harshal Dhumal
> >> >>>>>> Sr. Software Engineer
> >> >>>>>>
> >> >>>>>> EnterpriseDB India: 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
> >> >>>>>>
> >> >>>>>
> >> >>>>
> >> >>>
> >> >>
> >> >
> >> >
> >> >
> >> > --
> >> > Sent via pgadmin-hackers mailing list ([email protected]
> )
> >> > To make changes to your subscription:
> >> > http://www.postgresql.org/mailpref/pgadmin-hackers
> >> >
> >>
> >>
> >>
> >> --
> >> 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:
[text/x-patch] RM2421_V5.patch (32.6K, 3-RM2421_V5.patch)
download | inline diff:
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/foreign_tables/templates/foreign_tables/js/foreign_tables.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/foreign_tables/templates/foreign_tables/js/foreign_tables.js
index b239970..0ea196a 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/foreign_tables/templates/foreign_tables/js/foreign_tables.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/foreign_tables/templates/foreign_tables/js/foreign_tables.js
@@ -102,34 +102,56 @@ define([
cell: 'string', group: gettext('Definition'),
type: 'int', deps: ['datatype'],
disabled: function(m) {
- // We will store type from selected from combobox
+ var val = m.get('typlen');
+ // We will store type from selected from combobox
if(!(_.isUndefined(m.get('inheritedid'))
|| _.isNull(m.get('inheritedid'))
|| _.isUndefined(m.get('inheritedfrom'))
- || _.isNull(m.get('inheritedfrom')))) { return true; }
+ || _.isNull(m.get('inheritedfrom')))) {
- var of_type = m.get('datatype');
- if(m.type_options) {
- m.set('is_tlength', false, {silent: true});
+ if (!_.isUndefined(val)) {
+ setTimeout(function() {
+ m.set('typlen', undefined);
+ }, 10);
+ }
+ return true;
+ }
+
+ var of_type = m.get('datatype'),
+ has_length = false;
+ if(m.type_options) {
+ m.set('is_tlength', false, {silent: true});
- // iterating over all the types
- _.each(m.type_options, function(o) {
- // if type from selected from combobox matches in options
- if ( of_type == o.value ) {
- m.set('typlen', undefined);
+ // iterating over all the types
+ _.each(m.type_options, function(o) {
+ // if type from selected from combobox matches in options
+ if ( of_type == o.value ) {
// if length is allowed for selected type
if(o.length)
{
// set the values in model
+ has_length = true;
m.set('is_tlength', true, {silent: true});
m.set('min_val', o.min_val, {silent: true});
m.set('max_val', o.max_val, {silent: true});
}
+ }
+ });
+
+ if (!has_length && !_.isUndefined(val)) {
+ setTimeout(function() {
+ m.set('typlen', undefined);
+ }, 10);
}
- });
- return !(m.get('is_tlength'));
- }
- return true;
+
+ return !(m.get('is_tlength'));
+ }
+ if (!has_length && !_.isUndefined(val)) {
+ setTimeout(function() {
+ m.set('typlen', undefined);
+ }, 10);
+ }
+ return true;
},
cellHeaderClasses: 'width_percent_10'
},{
@@ -137,33 +159,54 @@ define([
type: 'int', deps: ['datatype'],
cell: 'string', group: gettext('Definition'),
disabled: function(m) {
+ var val = m.get('precision');
if(!(_.isUndefined(m.get('inheritedid'))
|| _.isNull(m.get('inheritedid'))
|| _.isUndefined(m.get('inheritedfrom'))
- || _.isNull(m.get('inheritedfrom')))) { return true; }
+ || _.isNull(m.get('inheritedfrom')))) {
+
+ if (!_.isUndefined(val)) {
+ setTimeout(function() {
+ m.set('precision', undefined);
+ }, 10);
+ }
+ return true;
+ }
+
+ var of_type = m.get('datatype'),
+ has_precision = false;
- var of_type = m.get('datatype');
if(m.type_options) {
m.set('is_precision', false, {silent: true});
// iterating over all the types
_.each(m.type_options, function(o) {
// if type from selected from combobox matches in options
if ( of_type == o.value ) {
- m.set('precision', undefined);
// if precession is allowed for selected type
if(o.precision)
{
+ has_precision = true;
// set the values in model
m.set('is_precision', true, {silent: true});
m.set('min_val', o.min_val, {silent: true});
m.set('max_val', o.max_val, {silent: true});
}
+ }
+ });
+ if (!has_precision && !_.isUndefined(val)) {
+ setTimeout(function() {
+ m.set('precision', undefined);
+ }, 10);
}
- });
- return !(m.get('is_precision'));
- }
- return true;
- }, cellHeaderClasses: 'width_percent_10'
+ return !(m.get('is_precision'));
+ }
+ if (!has_precision && !_.isUndefined(val)) {
+ setTimeout(function() {
+ m.set('precision', undefined);
+ }, 10);
+ }
+ return true;
+ }, cellHeaderClasses: 'width_percent_10'
},{
id: 'typdefault', label: gettext('Default'), type: 'text',
cell: 'string', min_version: 90300, group: gettext('Definition'),
@@ -217,22 +260,23 @@ define([
min_version: 90200
}],
validate: function() {
- var err = {},
- errmsg;
+ var errmsg = null;
if (_.isUndefined(this.get('attname')) || String(this.get('attname')).replace(/^\s+|\s+$/g, '') == '') {
- err['name'] = gettext('Column Name cannot be empty!');
- errmsg = errmsg || err['attname'];
+ errmsg = gettext('Column Name cannot be empty!');
+ this.errorModel.set('attname', errmsg);
+ } else {
+ this.errorModel.unset('attname');
}
if (_.isUndefined(this.get('datatype')) || String(this.get('datatype'))
.replace(/^\s+|\s+$/g, '') == '') {
- err['basensp'] = gettext('Column Datatype cannot be empty!');
- errmsg = errmsg || err['datatype'];
+ errmsg = gettext('Column Datatype cannot be empty!');
+ this.errorModel.set('datatype', errmsg);
+ } else {
+ this.errorModel.unset('datatype');
}
- this.errorModel.clear().set(err);
-
return errmsg;
},
is_editable_column: function(m) {
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/sequences/templates/sequence/js/sequence.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/sequences/templates/sequence/js/sequence.js
index 9e6711e..e8d9ac9 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/sequences/templates/sequence/js/sequence.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/sequences/templates/sequence/js/sequence.js
@@ -154,7 +154,10 @@ define([
min: 1
},{
id: 'start', label: gettext('Start'), type: 'int',
- mode: ['properties', 'create'], group: gettext('Definition')
+ mode: ['properties', 'create'], group: gettext('Definition'),
+ disabled: function(m) {
+ return !m.isNew();
+ }
},{
id: 'minimum', label: gettext('Minimum'), type: 'int',
mode: ['properties', 'create', 'edit'], group: gettext('Definition')
@@ -200,14 +203,14 @@ define([
minimum = this.get('minimum'),
maximum = this.get('maximum');
start = this.get('start');
- // Clear any existing error msg.
- this.errorModel.clear();
if (_.isUndefined(this.get('name'))
|| String(this.get('name')).replace(/^\s+|\s+$/g, '') == '') {
msg = gettext('Name cannot be empty.');
this.errorModel.set('name', msg);
return msg;
+ } else {
+ this.errorModel.unset('name');
}
if (_.isUndefined(this.get('seqowner'))
@@ -215,6 +218,8 @@ define([
msg = gettext('Owner cannot be empty.');
this.errorModel.set('seqowner', msg);
return msg;
+ } else {
+ this.errorModel.unset('seqowner');
}
if (_.isUndefined(this.get('schema'))
@@ -222,26 +227,80 @@ define([
msg = gettext('Schema cannot be empty.');
this.errorModel.set('schema', msg);
return msg;
+ } else {
+ this.errorModel.unset('schema');
}
+ if (!this.isNew()) {
+ if (_.isUndefined(this.get('current_value'))
+ || String(this.get('current_value')).replace(/^\s+|\s+$/g, '') == '') {
+ msg = '{{ _('Current value cannot be empty.') }}';
+ this.errorModel.set('current_value', msg);
+ return msg;
+ } else {
+ this.errorModel.unset('current_value');
+ }
+
+ if (_.isUndefined(this.get('increment'))
+ || String(this.get('increment')).replace(/^\s+|\s+$/g, '') == '') {
+ msg = '{{ _('Increment value cannot be empty.') }}';
+ this.errorModel.set('increment', msg);
+ return msg;
+ } else {
+ this.errorModel.unset('increment');
+ }
+
+ if (_.isUndefined(this.get('minimum'))
+ || String(this.get('minimum')).replace(/^\s+|\s+$/g, '') == '') {
+ msg = '{{ _('Minimum value cannot be empty.') }}';
+ this.errorModel.set('minimum', msg);
+ return msg;
+ } else {
+ this.errorModel.unset('minimum');
+ }
+
+ if (_.isUndefined(this.get('maximum'))
+ || String(this.get('maximum')).replace(/^\s+|\s+$/g, '') == '') {
+ msg = '{{ _('Maximum value cannot be empty.') }}';
+ this.errorModel.set('maximum', msg);
+ return msg;
+ } else {
+ this.errorModel.unset('maximum');
+ }
+
+ if (_.isUndefined(this.get('cache'))
+ || String(this.get('cache')).replace(/^\s+|\s+$/g, '') == '') {
+ msg = '{{ _('Cache value cannot be empty.') }}';
+ this.errorModel.set('cache', msg);
+ return msg;
+ } else {
+ this.errorModel.unset('cache');
+ }
+ }
var min_lt = gettext('Minimum value must be less than maximum value.'),
start_lt = gettext('Start value cannot be less than minimum value.'),
start_gt = gettext('Start value cannot be greater than maximum value.');
+
if ((minimum == 0 && maximum == 0) ||
(parseInt(minimum, 10) >= parseInt(maximum, 10))) {
- msg = min_lt
- this.errorModel.set('minimum', msg);
- return msg;
+ this.errorModel.set('minimum', min_lt);
+ return min_lt;
+ } else {
+ this.errorModel.unset('minimum');
}
- else if (start < minimum) {
- msg = start_lt
- this.errorModel.set('start', msg);
- return msg;
+
+ if (start && minimum && parseInt(start) < parseInt(minimum)) {
+ this.errorModel.set('start', start_lt);
+ return start_lt;
+ } else {
+ this.errorModel.unset('start');
}
- else if (start > maximum) {
- msg = start_gt
- this.errorModel.set('start', msg);
- return msg;
+
+ if (start && maximum && parseInt(start) > parseInt(maximum)) {
+ this.errorModel.set('start', start_gt);
+ return start_gt;
+ } else {
+ this.errorModel.unset('start');
}
return null;
}
diff --git a/web/pgadmin/browser/server_groups/servers/resource_groups/templates/resource_groups/js/resource_groups.js b/web/pgadmin/browser/server_groups/servers/resource_groups/templates/resource_groups/js/resource_groups.js
index cec3917..389a2e6 100644
--- a/web/pgadmin/browser/server_groups/servers/resource_groups/templates/resource_groups/js/resource_groups.js
+++ b/web/pgadmin/browser/server_groups/servers/resource_groups/templates/resource_groups/js/resource_groups.js
@@ -86,53 +86,36 @@ define([
* the GUI for the respective control.
*/
validate: function(keys) {
- var msg, cpu_rate_limit, dirty_rate_limit, name;
-
- /* Check whether 'name' is present in 'keys', if it is present
- * it means there is a change in that field from the GUI, so we
- * need to validate it.
- */
- if (_.indexOf(keys, 'name') >= 0) {
- name = this.get('name');
- if (_.isUndefined(name) || _.isNull(name) ||
- String(name).replace(/^\s+|\s+$/g, '') === '') {
- msg = gettext('Name cannot be empty.');
- this.errorModel.set('name', msg);
- return msg;
- }
+ var msg, cpu_rate_limit, dirty_rate_limit, name,
+ name = this.get('name');
+ if (_.isUndefined(name) || _.isNull(name) ||
+ String(name).replace(/^\s+|\s+$/g, '') == '') {
+ var msg = gettext('Name cannot be empty.');
+ this.errorModel.set('name', msg);
+ return msg;
+ } else {
this.errorModel.unset('name');
}
- /* Check whether 'cpu_rate_limit' is present in 'keys', if it is present
- * it means there is a change in that field from the GUI, so we
- * need to validate it.
- */
- if (_.indexOf(keys, 'cpu_rate_limit') >= 0) {
- cpu_rate_limit = this.get('cpu_rate_limit');
- if (_.isUndefined(cpu_rate_limit) || _.isNull(cpu_rate_limit) ||
- String(cpu_rate_limit).replace(/^\s+|\s+$/g, '') === '') {
- msg = gettext('CPU rate limit cannot be empty.');
- this.errorModel.set('cpu_rate_limit', msg);
- return msg;
- }
+ var cpu_rate_limit = this.get('cpu_rate_limit');
+ if (_.isUndefined(cpu_rate_limit) || _.isNull(cpu_rate_limit) ||
+ String(cpu_rate_limit).replace(/^\s+|\s+$/g, '') == '') {
+ var msg = gettext('CPU rate limit cannot be empty.');
+ this.errorModel.set('cpu_rate_limit', msg);
+ return msg;
+ } else {
this.errorModel.unset('cpu_rate_limit');
}
- /* Check whether 'dirty_rate_limit' is present in 'keys', if it is present
- * it means there is a change in that field from the GUI, so we
- * need to validate it.
- */
- if (_.indexOf(keys, 'dirty_rate_limit') >= 0) {
- dirty_rate_limit = this.get('dirty_rate_limit');
- if (_.isUndefined(dirty_rate_limit) || _.isNull(dirty_rate_limit) ||
- String(dirty_rate_limit).replace(/^\s+|\s+$/g, '') === '') {
- msg = gettext('Dirty rate limit cannot be empty.');
- this.errorModel.set('dirty_rate_limit', msg);
- return msg;
- }
+ var dirty_rate_limit = this.get('dirty_rate_limit');
+ if (_.isUndefined(dirty_rate_limit) || _.isNull(dirty_rate_limit) ||
+ String(dirty_rate_limit).replace(/^\s+|\s+$/g, '') == '') {
+ var msg = gettext('Dirty rate limit cannot be empty.');
+ this.errorModel.set('dirty_rate_limit', msg);
+ return msg;
+ } else {
this.errorModel.unset('dirty_rate_limit');
}
-
return null;
}
})
diff --git a/web/pgadmin/browser/server_groups/servers/roles/templates/role/js/role.js b/web/pgadmin/browser/server_groups/servers/roles/templates/role/js/role.js
index 97f0738..e55761d 100644
--- a/web/pgadmin/browser/server_groups/servers/roles/templates/role/js/role.js
+++ b/web/pgadmin/browser/server_groups/servers/roles/templates/role/js/role.js
@@ -426,7 +426,7 @@ define([
deps: ['rolcanlogin'], options: {format: 'YYYY-MM-DD HH:mm:ss Z'}
},{
id: 'rolconnlimit', type: 'int', group: gettext('Definition'),
- label: gettext('Connection limit'), cell: 'number',
+ label: gettext('Connection limit'), cell: 'number', min : -1,
mode: ['properties', 'edit', 'create'], disabled: 'readonly'
},{
id: 'rolcanlogin', label: gettext('Can login?'), type: 'switch',
diff --git a/web/pgadmin/browser/server_groups/servers/templates/servers/servers.js b/web/pgadmin/browser/server_groups/servers/templates/servers/servers.js
index 5ff911b..6bb827e 100644
--- a/web/pgadmin/browser/server_groups/servers/templates/servers/servers.js
+++ b/web/pgadmin/browser/server_groups/servers/templates/servers/servers.js
@@ -721,6 +721,9 @@ define([
check_for_empty(
'username', gettext('Username must be specified.')
);
+ check_for_empty(
+ 'port', '{{ _('Port must be specified.') }}'
+ );
this.errorModel.set(err);
if (_.size(err)) {
diff --git a/web/pgadmin/browser/static/js/datamodel.js b/web/pgadmin/browser/static/js/datamodel.js
index 5b1c3a7..ad8f95f 100644
--- a/web/pgadmin/browser/static/js/datamodel.js
+++ b/web/pgadmin/browser/static/js/datamodel.js
@@ -1,6 +1,6 @@
define(
- ['underscore', 'pgadmin', 'jquery', 'backbone'],
-function(_, pgAdmin, $, Backbone) {
+ ['underscore', 'underscore.string', 'pgadmin', 'jquery', 'backbone'],
+function(_, S, pgAdmin, $, Backbone) {
var pgBrowser = pgAdmin.Browser = pgAdmin.Browser || {};
pgBrowser.DataModel = Backbone.Model.extend({
@@ -136,6 +136,7 @@ function(_, pgAdmin, $, Backbone) {
}
self.sessAttrs = {};
+ self.fieldData = {};
self.origSessAttrs = {};
self.objects = [];
self.arrays = [];
@@ -153,6 +154,25 @@ function(_, pgAdmin, $, Backbone) {
_.each(schema, function(s) {
switch(s.type) {
+ case 'int':
+ case 'numeric':
+ self.fieldData[s.id] = {
+ id: s.id,
+ label: s.label,
+ type: s.type,
+ min: s.min || undefined,
+ max: s.max || undefined
+ }
+ break;
+ default:
+ self.fieldData[s.id] = {
+ id: s.id,
+ label: s.label,
+ type: s.type
+ }
+ }
+
+ switch(s.type) {
case 'array':
self.arrays.push(s.id);
@@ -280,6 +300,12 @@ function(_, pgAdmin, $, Backbone) {
},
sessValid: function() {
var self = this;
+ // Perform default validations.
+ if ('default_validate' in self && typeof(self.default_validate) == 'function' &&
+ _.isString(self.default_validate())) {
+ return false;
+ }
+
if ('validate' in self && _.isFunction(self.validate) &&
_.isString(self.validate.apply(self))) {
return false;
@@ -301,8 +327,9 @@ function(_, pgAdmin, $, Backbone) {
}
if (key != null && res) {
- var attrs = {};
- var self = this;
+ var attrs = {},
+ self = this,
+ msg;
attrChanged = function(v, k) {
if (k in self.objects) {
@@ -327,9 +354,18 @@ function(_, pgAdmin, $, Backbone) {
if (!options || !options.silent) {
self.trigger('change', self, options);
}
+
+ // Perform default validations.
+
+ if ('default_validate' in self && typeof(self.default_validate) == 'function') {
+ msg = self.default_validate();
+ }
+
if ('validate' in self && typeof(self['validate']) === 'function') {
- var msg = self.validate(_.keys(attrs));
+ if (!msg) {
+ msg = self.validate(_.keys(attrs));
+ }
/*
* If any parent present, we will need to inform the parent - that
@@ -562,6 +598,13 @@ function(_, pgAdmin, $, Backbone) {
var msg = null,
validate = function(m, attrs) {
+ if ('default_validate' in m && typeof(m.default_validate) == 'function') {
+ msg = m.default_validate();
+ if (_.isString(msg)) {
+ return msg;
+ }
+ }
+
if ('validate' in m && typeof(m.validate) == 'function') {
msg = m.validate(attrs);
@@ -655,6 +698,79 @@ function(_, pgAdmin, $, Backbone) {
});
self.trigger('pgadmin-session:stop');
+ },
+ default_validate: function() {
+ var msg, field, value, type;
+
+ for (var i = 0, keys = _.keys(this.attributes), l = keys.length;
+ i<l;
+ i++) {
+
+ value = this.attributes[keys[i]];
+ field = this.fieldData[keys[i]]
+ msg = null;
+
+ if (!(_.isUndefined(value) || _.isNull(value) ||
+ String(value).replace(/^\s+|\s+$/g, '') == '')) {
+
+ if (!field) {
+ continue;
+ }
+
+ type = field.type || undefined;
+ if (!type) {
+ continue;
+ }
+
+ switch(type) {
+ case 'int':
+ msg = this.integer_validate(value, field);
+ break;
+ case 'numeric':
+ msg = this.number_validate(value, field);
+ break;
+ }
+
+ if (msg) {
+ this.errorModel.set(field.id, msg);
+ return msg;
+ } else {
+ this.errorModel.unset(field.id);
+ }
+ } else {
+ if (field) {
+ this.errorModel.unset(field.id);
+ }
+ }
+ }
+ return null;
+ },
+
+ check_min_max: function (value, field) {
+ var label = field.label,
+ min_value = field.min,
+ max_value = field.max;
+
+ if (min_value && value < min_value) {
+ return S(pgAdmin.Browser.messages.MUST_GR_EQ).sprintf(label, min_value).value();
+ } else if (max_value && value > max_value) {
+ return S(pgAdmin.Browser.messages.MUST_LESS_EQ).sprintf(label, max_value).value();
+ }
+ return null;
+ },
+ number_validate: function (value, field) {
+ var pattern = new RegExp("^-?[0-9]+(\.?[0-9]*)?$");
+ if (!pattern.test(value)) {
+ return S(pgAdmin.Browser.messages.MUST_BE_NUM).sprintf(field.label).value()
+ }
+ return this.check_min_max(value, field)
+ },
+ integer_validate: function(value, field) {
+ var pattern = new RegExp("^-?[0-9]*$");
+ if (!pattern.test(value)) {
+ return S(pgAdmin.Browser.messages.MUST_BE_INT).sprintf(field.label).value()
+ }
+ return this.check_min_max(value, field)
}
});
@@ -696,7 +812,8 @@ function(_, pgAdmin, $, Backbone) {
return self;
},
startNewSession: function() {
- var self = this;
+ var self = this,
+ msg;
if (self.trackChanges) {
// We're stopping the existing session.
@@ -718,8 +835,15 @@ function(_, pgAdmin, $, Backbone) {
if ('startNewSession' in m && _.isFunction(m.startNewSession)) {
m.startNewSession();
}
- if ('validate' in m && typeof(m.validate) === 'function') {
- var msg = m.validate();
+
+ if ('default_validate' in m && typeof(m.default_validate) == 'function') {
+ msg = m.default_validate();
+ }
+
+ if (_.isString(msg)) {
+ self.sessAttrs['invalid'][m.cid] = msg;
+ } else if ('validate' in m && typeof(m.validate) === 'function') {
+ msg = m.validate();
if (msg) {
self.sessAttrs['invalid'][m.cid] = msg;
@@ -900,7 +1024,14 @@ function(_, pgAdmin, $, Backbone) {
(self.handler || self).trigger('pgadmin-session:added', self, obj);
- if ('validate' in obj && typeof(obj.validate) === 'function') {
+
+ if ('default_validate' in obj && typeof(obj.default_validate) == 'function') {
+ msg = obj.default_validate();
+ }
+
+ if (_.isString(msg)) {
+ (self.sessAttrs['invalid'])[obj.cid] = msg;
+ } else if ('validate' in obj && typeof(obj.validate) === 'function') {
msg = obj.validate();
if (msg) {
@@ -908,7 +1039,14 @@ function(_, pgAdmin, $, Backbone) {
}
}
} else {
- if ('validate' in obj && typeof(obj.validate) === 'function') {
+
+ if ('default_validate' in obj && typeof(obj.default_validate) == 'function') {
+ msg = obj.default_validate();
+ }
+
+ if (_.isString(msg)) {
+ (self.sessAttrs['invalid'])[obj.cid] = msg;
+ } else if ('validate' in obj && typeof(obj.validate) === 'function') {
msg = obj.validate();
if (msg) {
diff --git a/web/pgadmin/static/js/backform.pgadmin.js b/web/pgadmin/static/js/backform.pgadmin.js
index 0cc1e2f..a82205e 100644
--- a/web/pgadmin/static/js/backform.pgadmin.js
+++ b/web/pgadmin/static/js/backform.pgadmin.js
@@ -58,7 +58,7 @@
});
var controlMapper = Backform.controlMapper = {
- 'int': ['uneditable-input', 'integer', 'integer'],
+ 'int': ['uneditable-input', 'numeric', 'numeric'],
'text': ['uneditable-input', 'input', 'string'],
'numeric': ['uneditable-input', 'numeric', 'numeric'],
'date': 'datepicker',
@@ -1493,110 +1493,6 @@
Backform.Control.__super__.remove.apply(this, arguments);
}
});
-
- /*
- * Integer input Control functionality just like backgrid
- */
- var IntegerControl = Backform.IntegerControl = Backform.InputControl.extend({
- defaults: {
- type: "number",
- label: "",
- min: undefined,
- max: undefined,
- maxlength: 255,
- extraClasses: [],
- helpMessage: null
- },
- template: _.template([
- '<label class="<%=Backform.controlLabelClassName%>"><%=label%></label>',
- '<div class="<%=Backform.controlsClassName%>">',
- ' <input type="<%=type%>" class="<%=Backform.controlClassName%> <%=extraClasses.join(\' \')%>" name="<%=name%>" min="<%=min%>" max="<%=max%>"maxlength="<%=maxlength%>" value="<%-value%>" placeholder="<%-placeholder%>" <%=disabled ? "disabled" : ""%> <%=required ? "required" : ""%> />',
- ' <% if (helpMessage && helpMessage.length) { %>',
- ' <span class="<%=Backform.helpMessageClassName%>"><%=helpMessage%></span>',
- ' <% } %>',
- '</div>'
- ].join("\n")),
- events: {
- "change input": "checkInt",
- "focus input": "clearInvalid"
- },
- checkInt: function(e) {
- var field = _.defaults(this.field.toJSON(), this.defaults),
- attrArr = this.field.get("name").split('.'),
- name = attrArr.shift(),
- value = this.getValueFromDOM(),
- min_value = field.min,
- max_value = field.max,
- isValid = true,
- intPattern = new RegExp("^-?[0-9]*$"),
- isMatched = intPattern.test(value);
-
- // Below logic will validate input
- if (!isMatched) {
- isValid = false;
- this.model.errorModel.unset(name);
- this.model.errorModel.set(
- name,
- S(gettext("'%s' must be an integer.")).sprintf(
- field.label
- ).value()
- );
- }
-
- // Below will check if entered value is in-between min & max range
- if (isValid && (!_.isUndefined(min_value) && value < min_value)) {
- isValid = false;
- this.model.errorModel.unset(name);
- this.model.errorModel.set(
- name,
- S(gettext("%s' must be greater than or equal to %d.")).sprintf(
- field.label,
- min_value
- ).value()
- );
- }
-
- if (isValid && (!_.isUndefined(max_value) && value > max_value)) {
- isValid = false;
- this.model.errorModel.unset(name);
- this.model.errorModel.set(
- name,
- S(gettext("'%s' must be less than or equal to %d.")).sprintf(
- field.label,
- max_value
- ).value()
- );
- }
-
- // After validation we need to set that value into model (only if all flags are true)
- if (isValid) {
- this.stopListening(this.model, "change:" + name, this.render);
- this.model.errorModel.unset(name);
- this.model.set(name, value);
- this.listenTo(this.model, "change:" + name, this.render);
- if (this.model.collection || this.model.handler) {
- (this.model.collection || this.model.handler).trigger(
- 'pgadmin-session:model:valid', this.model, (this.model.collection || this.model.handler)
- );
- } else {
- (this.model).trigger(
- 'pgadmin-session:valid', this.model.sessChanged(), this.model
- );
- }
- } else {
- if (this.model.collection || this.model.handler) {
- (this.model.collection || this.model.handler).trigger(
- 'pgadmin-session:model:invalid', this.model.errorModel.get(name), this.model
- );
- } else {
- (this.model).trigger(
- 'pgadmin-session:invalid', this.model.errorModel.get(name), this.model
- );
- }
- }
- }
- });
-
/*
* Numeric input Control functionality just like backgrid
*/
@@ -1618,86 +1514,7 @@
' <span class="<%=Backform.helpMessageClassName%>"><%=helpMessage%></span>',
' <% } %>',
'</div>'
- ].join("\n")),
- events: {
- "change input": "checkNumeric",
- "focus input": "clearInvalid"
- },
- checkNumeric: function(e) {
- var field = _.defaults(this.field.toJSON(), this.defaults),
- attrArr = this.field.get("name").split('.'),
- name = attrArr.shift(),
- value = this.getValueFromDOM(),
- min_value = field.min,
- max_value = field.max,
- isValid = true,
- intPattern = new RegExp("^-?[0-9]+(\.?[0-9]*)?$"),
- isMatched = intPattern.test(value);
-
- // Below logic will validate input
- if (!isMatched) {
- isValid = false;
- this.model.errorModel.unset(name);
- this.model.errorModel.set(
- name,
- S(gettext("'%s' must be a numeric.")).sprintf(
- field.label
- ).value()
- );
- }
-
- // Below will check if entered value is in-between min & max range
- if (isValid && (!_.isUndefined(min_value) && value < min_value)) {
- isValid = false;
- this.model.errorModel.unset(name);
- this.model.errorModel.set(
- name,
- S(gettext("%s' must be greater than or equal to %d.")).sprintf(
- field.label,
- min_value
- ).value()
- );
- }
-
- if (isValid && (!_.isUndefined(max_value) && value > max_value)) {
- isValid = false;
- this.model.errorModel.unset(name);
- this.model.errorModel.set(
- name,
- S(gettext("'%s' must be less than or equal to %d.")).sprintf(
- field.label,
- max_value
- ).value()
- );
- }
-
- // After validation we need to set that value into model (only if all flags are true)
- if (isValid) {
- this.stopListening(this.model, "change:" + name, this.render);
- this.model.errorModel.unset(name);
- this.model.set(name, value);
- this.listenTo(this.model, "change:" + name, this.render);
- if (this.model.collection || this.model.handler) {
- (this.model.collection || this.model.handler).trigger(
- 'pgadmin-session:model:valid', this.model, (this.model.collection || this.model.handler)
- );
- } else {
- (this.model).trigger(
- 'pgadmin-session:valid', this.model.sessChanged(), this.model
- );
- }
- } else {
- if (this.model.collection || this.model.handler) {
- (this.model.collection || this.model.handler).trigger(
- 'pgadmin-session:model:invalid', this.model.errorModel.get(name), this.model
- );
- } else {
- (this.model).trigger(
- 'pgadmin-session:invalid', this.model.errorModel.get(name), this.model
- );
- }
- }
- }
+ ].join("\n"))
});
///////
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Fix for RM2421 [pgAdmin4][patch]
2017-05-18 10:01 Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-05-18 14:27 ` Re: Fix for RM2421 [pgAdmin4][patch] Joao Pedro De Almeida Pereira <[email protected]>
2017-05-19 06:52 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-05-29 09:48 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-05-30 07:00 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-05-30 07:28 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-06-05 15:55 ` Re: Fix for RM2421 [pgAdmin4][patch] Dave Page <[email protected]>
2017-06-06 06:42 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-06-07 13:29 ` Re: Fix for RM2421 [pgAdmin4][patch] Dave Page <[email protected]>
2017-06-08 12:59 ` Re: Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
@ 2017-06-08 13:59 ` Dave Page <[email protected]>
0 siblings, 0 replies; 11+ messages in thread
From: Dave Page @ 2017-06-08 13:59 UTC (permalink / raw)
To: Harshal Dhumal <[email protected]>; +Cc: pgadmin-hackers
Thanks, applied.
On Thu, Jun 8, 2017 at 1:59 PM, Harshal Dhumal
<[email protected]> wrote:
> Please find attached rebased patch.
>
> --
> Harshal Dhumal
> Sr. Software Engineer
>
> EnterpriseDB India: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Wed, Jun 7, 2017 at 6:59 PM, Dave Page <[email protected]> wrote:
>>
>> Can you rebase this please? I think Ashesh broke it :-p
>>
>> On Tue, Jun 6, 2017 at 7:42 AM, Harshal Dhumal
>> <[email protected]> wrote:
>> > Hi,
>> >
>> > On Mon, Jun 5, 2017 at 9:25 PM, Dave Page <[email protected]> wrote:
>> >>
>> >> Hi
>> >>
>> >> With this patch applied, it uses the field names instead of the labels
>> >> in error messages - e.g.
>> >>
>> >> 'dirty_rate_limit' must be numeric
>> >>
>> >> instead of:
>> >>
>> >> 'Dirty Rate Limit (KB)' must be numeric.
>> >
>> > Fixed. Please find attached updated patch.
>> >
>> >>
>> >>
>> >> Thanks.
>> >>
>> >> On Tue, May 30, 2017 at 8:28 AM, Harshal Dhumal
>> >> <[email protected]> wrote:
>> >> > Hi,
>> >> >
>> >> > Please find updated patch.
>> >> >
>> >> > --
>> >> > Harshal Dhumal
>> >> > Sr. Software Engineer
>> >> >
>> >> > EnterpriseDB India: http://www.enterprisedb.com
>> >> > The Enterprise PostgreSQL Company
>> >> >
>> >> > On Tue, May 30, 2017 at 12:30 PM, Harshal Dhumal
>> >> > <[email protected]> wrote:
>> >> >>
>> >> >> Hi,
>> >> >>
>> >> >> Please ignore this patch as I forgot to include few changes. I'll
>> >> >> send
>> >> >> updated one.
>> >> >>
>> >> >> --
>> >> >> Harshal Dhumal
>> >> >> Sr. Software Engineer
>> >> >>
>> >> >> EnterpriseDB India: http://www.enterprisedb.com
>> >> >> The Enterprise PostgreSQL Company
>> >> >>
>> >> >> On Mon, May 29, 2017 at 3:18 PM, Harshal Dhumal
>> >> >> <[email protected]> wrote:
>> >> >>>
>> >> >>> Hi,
>> >> >>>
>> >> >>> Here is updated patch for RM2421.
>> >> >>>
>> >> >>> Now I have moved all Numeric control level validations to
>> >> >>> datamodel.
>> >> >>> As
>> >> >>> existing implementation was causing
>> >> >>> issues with error messages in create/edit dialog when schema
>> >> >>> contains
>> >> >>> two
>> >> >>> or more Numeric controls.
>> >> >>>
>> >> >>> This is generic issue and not related to resource group. Also I
>> >> >>> have
>> >> >>> updated all other nodes which uses Numeric controls
>> >> >>>
>> >> >>>
>> >> >>>
>> >> >>> --
>> >> >>> Harshal Dhumal
>> >> >>> Sr. Software Engineer
>> >> >>>
>> >> >>> EnterpriseDB India: http://www.enterprisedb.com
>> >> >>> The Enterprise PostgreSQL Company
>> >> >>>
>> >> >>> On Fri, May 19, 2017 at 12:22 PM, Harshal Dhumal
>> >> >>> <[email protected]> wrote:
>> >> >>>>
>> >> >>>> Hi,
>> >> >>>>
>> >> >>>> On Thu, May 18, 2017 at 7:57 PM, Joao Pedro De Almeida Pereira
>> >> >>>> <[email protected]> wrote:
>> >> >>>>>
>> >> >>>>> Hello Harshal,
>> >> >>>>>
>> >> >>>>> We review the patch and have some questions:
>> >> >>>>> 1) Is there any particular reason to initialize variables and
>> >> >>>>> functions
>> >> >>>>> in the same place? We believe that it would be more readable
>> >> >>>>> there
>> >> >>>>> were no
>> >> >>>>> chaining of variable creation, specially if those variables are
>> >> >>>>> functions.
>> >> >>>>> Check line:
>> >> >>>>
>> >> >>>> That function is only going to be used in checkNumeric function
>> >> >>>> (in
>> >> >>>> case
>> >> >>>> of Number control) and checkInt function (in case of Integer
>> >> >>>> control)
>> >> >>>> so
>> >> >>>> declared them locally.
>> >> >>>> Anyway I'm going to refactor both the controls as Number and
>> >> >>>> Integer
>> >> >>>> shares some common properties.
>> >> >>>>
>> >> >>>>> +++ b/web/pgadmin/static/js/backform.pgadmin.js
>> >> >>>>> @@ -1528,7 +1528,18 @@
>> >> >>>>> max_value = field.max,
>> >> >>>>> isValid = true,
>> >> >>>>> intPattern = new RegExp("^-?[0-9]*$"),
>> >> >>>>> - isMatched = intPattern.test(value);
>> >> >>>>> + isMatched = intPattern.test(value),
>> >> >>>>> + trigger_invalid_event = function(msg) {
>> >> >>>>>
>> >> >>>>> 2) The functions added in both places look very similar, can they
>> >> >>>>> be
>> >> >>>>> merged and extracted? We are talking about the
>> >> >>>>> trigger_invalid_event
>> >> >>>>> function.
>> >> >>>>
>> >> >>>> Yes they can be merged. As of now both NumericControl and
>> >> >>>> IntegerControl
>> >> >>>> are derived from InputControl. Ideally
>> >> >>>> only NumericControl should be derived from InputControl and
>> >> >>>> IntegerControl should be derive from NumericControl.
>> >> >>>>
>> >> >>>>
>> >> >>>>>
>> >> >>>>> 3) The following change is very similar to the
>> >> >>>>> trigger_invalid_event,
>> >> >>>>> was there a reason not to use it?
>> >> >>>>
>> >> >>>> Below code triggers "model valid" event; opposite to "model
>> >> >>>> invalid"
>> >> >>>> event (trigger_invalid_event)
>> >> >>>>>
>> >> >>>>> +++ b/web/pgadmin/static/js/backform.pgadmin.js
>> >> >>>>> @@ -1573,25 +1584,23 @@
>> >> >>>>> this.model.errorModel.unset(name);
>> >> >>>>> this.model.set(name, value);
>> >> >>>>> this.listenTo(this.model, "change:" + name,
>> >> >>>>> this.render);
>> >> >>>>> - if (this.model.collection || this.model.handler) {
>> >> >>>>> - (this.model.collection || this.model.handler).trigger(
>> >> >>>>> - 'pgadmin-session:model:valid', this.model,
>> >> >>>>> (this.model.collection || this.model.handler)
>> >> >>>>> - );
>> >> >>>>> + // Check if other fields of same model are valid before
>> >> >>>>> + // triggering 'session:valid' event
>> >> >>>>> + if(_.size(this.model.errorModel.attributes) == 0) {
>> >> >>>>> + if (this.model.collection || this.model.handler) {
>> >> >>>>> + (this.model.collection ||
>> >> >>>>> this.model.handler).trigger(
>> >> >>>>> + 'pgadmin-session:model:valid', this.model,
>> >> >>>>> (this.model.collection || this.model.handler)
>> >> >>>>> + );
>> >> >>>>> + } else {
>> >> >>>>> + (this.model).trigger(
>> >> >>>>> + 'pgadmin-session:valid',
>> >> >>>>> this.model.sessChanged(),
>> >> >>>>> this.model
>> >> >>>>> + );
>> >> >>>>> + }
>> >> >>>>>
>> >> >>>>> 4) We also noticed that the following change sets look very
>> >> >>>>> similiar.
>> >> >>>>> Is there any reason to have this code duplicated? If not this
>> >> >>>>> could
>> >> >>>>> be a
>> >> >>>>> good time to refactor it.
>> >> >>>>
>> >> >>>> As said earlier in response of point 2 code duplication is because
>> >> >>>> the
>> >> >>>> way controls are derived.
>> >> >>>>
>> >> >>>>>
>> >> >>>>> +++ b/web/pgadmin/static/js/backform.pgadmin.js
>> >> >>>>> @@ -1528,7 +1528,18 @@
>> >> >>>>>
>> >> >>>>> @@ -1573,25 +1584,23 @@
>> >> >>>>>
>> >> >>>>> @@ -1631,7 +1640,18 @@
>> >> >>>>>
>> >> >>>>> @@ -1676,25 +1696,23 @@
>> >> >>>>>
>> >> >>>>>
>> >> >>>>> Thanks
>> >> >>>>> Joao & Shruti
>> >> >>>>>
>> >> >>>>> On Thu, May 18, 2017 at 6:01 AM, Harshal Dhumal
>> >> >>>>> <[email protected]> wrote:
>> >> >>>>>>
>> >> >>>>>> Hi,
>> >> >>>>>>
>> >> >>>>>> Please find attached patch for RM2421
>> >> >>>>>>
>> >> >>>>>> Issue fixed: 1. Integer/numeric Validation is not working
>> >> >>>>>> properly.
>> >> >>>>>> 2. Wrong CPU rate unit
>> >> >>>>>> --
>> >> >>>>>> Harshal Dhumal
>> >> >>>>>> Sr. Software Engineer
>> >> >>>>>>
>> >> >>>>>> EnterpriseDB India: 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
>> >> >>>>>>
>> >> >>>>>
>> >> >>>>
>> >> >>>
>> >> >>
>> >> >
>> >> >
>> >> >
>> >> > --
>> >> > Sent via pgadmin-hackers mailing list
>> >> > ([email protected])
>> >> > To make changes to your subscription:
>> >> > http://www.postgresql.org/mailpref/pgadmin-hackers
>> >> >
>> >>
>> >>
>> >>
>> >> --
>> >> 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
>
>
--
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
^ permalink raw reply [nested|flat] 11+ messages in thread
end of thread, other threads:[~2017-06-08 13:59 UTC | newest]
Thread overview: 11+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2017-05-18 10:01 Fix for RM2421 [pgAdmin4][patch] Harshal Dhumal <[email protected]>
2017-05-18 14:27 ` Joao Pedro De Almeida Pereira <[email protected]>
2017-05-19 06:52 ` Harshal Dhumal <[email protected]>
2017-05-29 09:48 ` Harshal Dhumal <[email protected]>
2017-05-30 07:00 ` Harshal Dhumal <[email protected]>
2017-05-30 07:28 ` Harshal Dhumal <[email protected]>
2017-06-05 15:55 ` Dave Page <[email protected]>
2017-06-06 06:42 ` Harshal Dhumal <[email protected]>
2017-06-07 13:29 ` Dave Page <[email protected]>
2017-06-08 12:59 ` Harshal Dhumal <[email protected]>
2017-06-08 13:59 ` Dave Page <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox