Re: [pgadmin-hackers] Fix for RM2421 [pgAdmin4][patch]

2017-06-08 Thread Dave Page
Thanks, applied.

On Thu, Jun 8, 2017 at 1:59 PM, Harshal Dhumal
 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  wrote:
>>
>> Can you rebase this please? I think Ashesh broke it :-p
>>
>> On Tue, Jun 6, 2017 at 7:42 AM, Harshal Dhumal
>>  wrote:
>> > Hi,
>> >
>> > On Mon, Jun 5, 2017 at 9:25 PM, Dave Page  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
>> >>  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
>> >> >  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
>> >> >>  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
>> >> >>>  wrote:
>> >> 
>> >>  Hi,
>> >> 
>> >>  On Thu, May 18, 2017 at 7:57 PM, Joao Pedro De Almeida Pereira
>> >>   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);
>> >> >

Re: [pgadmin-hackers] Fix for RM2421 [pgAdmin4][patch]

2017-06-08 Thread Harshal Dhumal
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  wrote:

> Can you rebase this please? I think Ashesh broke it :-p
>
> On Tue, Jun 6, 2017 at 7:42 AM, Harshal Dhumal
>  wrote:
> > Hi,
> >
> > On Mon, Jun 5, 2017 at 9:25 PM, Dave Page  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
> >>  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
> >> >  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
> >> >>  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
> >> >>>  wrote:
> >> 
> >>  Hi,
> >> 
> >>  On Thu, May 18, 2017 at 7:57 PM, Joao Pedro De Almeida Pereira
> >>   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 sa

Re: [pgadmin-hackers] Fix for RM2421 [pgAdmin4][patch]

2017-06-07 Thread Dave Page
Can you rebase this please? I think Ashesh broke it :-p

On Tue, Jun 6, 2017 at 7:42 AM, Harshal Dhumal
 wrote:
> Hi,
>
> On Mon, Jun 5, 2017 at 9:25 PM, Dave Page  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
>>  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
>> >  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
>> >>  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
>> >>>  wrote:
>> 
>>  Hi,
>> 
>>  On Thu, May 18, 2017 at 7:57 PM, Joao Pedro De Almeida Pereira
>>   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(

Re: [pgadmin-hackers] Fix for RM2421 [pgAdmin4][patch]

2017-06-05 Thread Harshal Dhumal
Hi,

On Mon, Jun 5, 2017 at 9:25 PM, Dave Page  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
>  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
> >  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
> >>  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
> >>>  wrote:
> 
>  Hi,
> 
>  On Thu, May 18, 2017 at 7:57 PM, Joao Pedro De Almeida Pereira
>   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
> 

Re: [pgadmin-hackers] Fix for RM2421 [pgAdmin4][patch]

2017-06-05 Thread Dave Page
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
 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
>  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
>>  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
>>>  wrote:

 Hi,

 On Thu, May 18, 2017 at 7:57 PM, Joao Pedro De Almeida Pereira
  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
>
>>>

Re: [pgadmin-hackers] Fix for RM2421 [pgAdmin4][patch]

2017-05-30 Thread Harshal Dhumal
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 <
harshal.dhu...@enterprisedb.com> 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 <
> harshal.dhu...@enterprisedb.com> 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 <
>> harshal.dhu...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> On Thu, May 18, 2017 at 7:57 PM, Joao Pedro De Almeida Pereira <
>>> jdealmeidapere...@pivotal.io> 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 <
 harshal.dhu...@enterprisedb.com> wrote:

> Hi,
>
> Please find attached patch for RM2421
>
> Issue fixed: 1. Integer/numeric Validation is not working properly.
>

Re: [pgadmin-hackers] Fix for RM2421 [pgAdmin4][patch]

2017-05-30 Thread Harshal Dhumal
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 <
harshal.dhu...@enterprisedb.com> 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 <
> harshal.dhu...@enterprisedb.com> wrote:
>
>> Hi,
>>
>> On Thu, May 18, 2017 at 7:57 PM, Joao Pedro De Almeida Pereira <
>> jdealmeidapere...@pivotal.io> 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 <
>>> harshal.dhu...@enterprisedb.com> 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


>>>
>>
>


Re: [pgadmin-hackers] Fix for RM2421 [pgAdmin4][patch]

2017-05-29 Thread Harshal Dhumal
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 <
harshal.dhu...@enterprisedb.com> wrote:

> Hi,
>
> On Thu, May 18, 2017 at 7:57 PM, Joao Pedro De Almeida Pereira <
> jdealmeidapere...@pivotal.io> 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 <
>> harshal.dhu...@enterprisedb.com> 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
>>>
>>>
>>
>
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/fo

Re: [pgadmin-hackers] Fix for RM2421 [pgAdmin4][patch]

2017-05-18 Thread Harshal Dhumal
Hi,

On Thu, May 18, 2017 at 7:57 PM, Joao Pedro De Almeida Pereira <
jdealmeidapere...@pivotal.io> 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 <
> harshal.dhu...@enterprisedb.com> 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
>>
>>
>


Re: [pgadmin-hackers] Fix for RM2421 [pgAdmin4][patch]

2017-05-18 Thread Joao Pedro De Almeida Pereira
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 <
harshal.dhu...@enterprisedb.com> 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
>
>


[pgadmin-hackers] Fix for RM2421 [pgAdmin4][patch]

2017-05-18 Thread Harshal Dhumal
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
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(
- 'pgadm