public inbox for [email protected]
help / color / mirror / Atom feedRegarding issue 1241
9+ messages / 3 participants
[nested] [flat]
* Regarding issue 1241
@ 2016-05-31 19:53 Harshal Dhumal <[email protected]>
2016-06-02 08:29 ` Re: Regarding issue 1241 Dave Page <[email protected]>
0 siblings, 1 reply; 9+ messages in thread
From: Harshal Dhumal @ 2016-05-31 19:53 UTC (permalink / raw)
To: Dave Page <[email protected]>; pgadmin-hackers
Hi Dave,
Regarding Issue 1241 <https://redmine.postgresql.org/issues/1241;:
We have added header section for parameter tab deliberately so that we can
force user to select parameter name (and therefore parameter's data type)
before adding new row. This is required because behavior of second cell
(Value cell) is dependent on what parameter name user has selected in first
cell (Name cell). See attached screen-shot.
For example:
1. If user selects parameter 'array_nulls' then value for this should be
either true or false (and hence switch cell).
2. If user selects parameter 'cpu_index_tuple_cost' then value for this
should be Integer (and hence Integer cell).
Without the custom header (and forcing user to select parameter) we cannot
decide what type of cell we need in second column.
Let me know your opinion on this.
Apart from this I have fixed column resize issue for Security label tab.
For Privileges tab I have reduced column resizing margin at some extent but
not 100%.
Regards,
--
*Harshal Dhumal*
*Software Engineer *
EenterpriseDB <http://www.enterprisedb.com;
--
Sent via pgadmin-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers
Attachments:
[image/png] parameter_tab_header_issue.png (68.0K, 3-parameter_tab_header_issue.png)
download | view image
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Regarding issue 1241
2016-05-31 19:53 Regarding issue 1241 Harshal Dhumal <[email protected]>
@ 2016-06-02 08:29 ` Dave Page <[email protected]>
2016-06-15 12:49 ` Re: Regarding issue 1241 Ashesh Vashi <[email protected]>
0 siblings, 1 reply; 9+ messages in thread
From: Dave Page @ 2016-06-02 08:29 UTC (permalink / raw)
To: Harshal Dhumal <[email protected]>; +Cc: pgadmin-hackers
On Tue, May 31, 2016 at 8:53 PM, Harshal Dhumal <
[email protected]> wrote:
> Hi Dave,
>
> Regarding Issue 1241 <https://redmine.postgresql.org/issues/1241;:
>
> We have added header section for parameter tab deliberately so that we can
> force user to select parameter name (and therefore parameter's data type)
> before adding new row. This is required because behavior of second cell
> (Value cell) is dependent on what parameter name user has selected in first
> cell (Name cell). See attached screen-shot.
>
> For example:
> 1. If user selects parameter 'array_nulls' then value for this should be
> either true or false (and hence switch cell).
> 2. If user selects parameter 'cpu_index_tuple_cost' then value for this
> should be Integer (and hence Integer cell).
>
> Without the custom header (and forcing user to select parameter) we cannot
> decide what type of cell we need in second column.
>
> Let me know your opinion on this.
>
We need to figure out a way to fix it. Our difficulties encountered writing
code should not dictate usability compromises. In this case, something that
needs some thought and maybe some tricky code has caused us to create an
inconsistent UI workflow to side-step the problem, which is not appropriate
as it leads to a poor look and feel and potentially confusion for the user.
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Regarding issue 1241
2016-05-31 19:53 Regarding issue 1241 Harshal Dhumal <[email protected]>
2016-06-02 08:29 ` Re: Regarding issue 1241 Dave Page <[email protected]>
@ 2016-06-15 12:49 ` Ashesh Vashi <[email protected]>
2016-06-15 12:55 ` Re: Regarding issue 1241 Dave Page <[email protected]>
0 siblings, 1 reply; 9+ messages in thread
From: Ashesh Vashi @ 2016-06-15 12:49 UTC (permalink / raw)
To: Dave Page <[email protected]>; +Cc: Harshal Dhumal <[email protected]>; pgadmin-hackers
On Thu, Jun 2, 2016 at 1:59 PM, Dave Page <[email protected]> wrote:
>
>
> On Tue, May 31, 2016 at 8:53 PM, Harshal Dhumal <
> [email protected]> wrote:
>
>> Hi Dave,
>>
>> Regarding Issue 1241 <https://redmine.postgresql.org/issues/1241;:
>>
>> We have added header section for parameter tab deliberately so that we
>> can force user to select parameter name (and therefore parameter's data
>> type) before adding new row. This is required because behavior of second
>> cell (Value cell) is dependent on what parameter name user has selected in
>> first cell (Name cell). See attached screen-shot.
>>
>> For example:
>> 1. If user selects parameter 'array_nulls' then value for this should be
>> either true or false (and hence switch cell).
>> 2. If user selects parameter 'cpu_index_tuple_cost' then value for this
>> should be Integer (and hence Integer cell).
>>
>> Without the custom header (and forcing user to select parameter) we
>> cannot decide what type of cell we need in second column.
>>
>> Let me know your opinion on this.
>>
>
> We need to figure out a way to fix it. Our difficulties encountered
> writing code should not dictate usability compromises.
>
In this case, something that needs some thought and maybe some tricky code
> has caused us to create an inconsistent UI workflow to side-step the
> problem, which is not appropriate as it leads to a poor look and feel and
> potentially confusion for the user.
>
Agree - we should handle these cases gracefully.
We need to over come the limitation by brain storming, which we already
started offline. :-)
To be honest - it is a time consuming work, and there is no quick fix for
this.
We can handle it as one case for each change instead of targeting all UI
changes as one whole problem.
And, we can utilize the same time to fix a lot more cases in beta 2.
I can ask Harshal to find out all possible places, where the similar
changes are required, and create a separate case for each (though - not
without your agreement).
--
Thanks & Regards,
Ashesh Vashi
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Regarding issue 1241
2016-05-31 19:53 Regarding issue 1241 Harshal Dhumal <[email protected]>
2016-06-02 08:29 ` Re: Regarding issue 1241 Dave Page <[email protected]>
2016-06-15 12:49 ` Re: Regarding issue 1241 Ashesh Vashi <[email protected]>
@ 2016-06-15 12:55 ` Dave Page <[email protected]>
2016-06-15 13:08 ` Re: Regarding issue 1241 Ashesh Vashi <[email protected]>
0 siblings, 1 reply; 9+ messages in thread
From: Dave Page @ 2016-06-15 12:55 UTC (permalink / raw)
To: Ashesh Vashi <[email protected]>; +Cc: Harshal Dhumal <[email protected]>; pgadmin-hackers
On Wed, Jun 15, 2016 at 1:49 PM, Ashesh Vashi
<[email protected]> wrote:
> On Thu, Jun 2, 2016 at 1:59 PM, Dave Page <[email protected]> wrote:
>>
>>
>>
>> On Tue, May 31, 2016 at 8:53 PM, Harshal Dhumal
>> <[email protected]> wrote:
>>>
>>> Hi Dave,
>>>
>>> Regarding Issue 1241:
>>>
>>> We have added header section for parameter tab deliberately so that we
>>> can force user to select parameter name (and therefore parameter's data
>>> type) before adding new row. This is required because behavior of second
>>> cell (Value cell) is dependent on what parameter name user has selected in
>>> first cell (Name cell). See attached screen-shot.
>>>
>>> For example:
>>> 1. If user selects parameter 'array_nulls' then value for this should be
>>> either true or false (and hence switch cell).
>>> 2. If user selects parameter 'cpu_index_tuple_cost' then value for this
>>> should be Integer (and hence Integer cell).
>>>
>>> Without the custom header (and forcing user to select parameter) we
>>> cannot decide what type of cell we need in second column.
>>>
>>> Let me know your opinion on this.
>>
>>
>> We need to figure out a way to fix it. Our difficulties encountered
>> writing code should not dictate usability compromises.
>>
>> In this case, something that needs some thought and maybe some tricky code
>> has caused us to create an inconsistent UI workflow to side-step the
>> problem, which is not appropriate as it leads to a poor look and feel and
>> potentially confusion for the user.
>
> Agree - we should handle these cases gracefully.
> We need to over come the limitation by brain storming, which we already
> started offline. :-)
>
> To be honest - it is a time consuming work, and there is no quick fix for
> this.
> We can handle it as one case for each change instead of targeting all UI
> changes as one whole problem.
> And, we can utilize the same time to fix a lot more cases in beta 2.
As far as I'm aware, this is the only case where there's a real problem.
> I can ask Harshal to find out all possible places, where the similar changes
> are required, and create a separate case for each (though - not without your
> agreement).
I don't think we need to. This one sub-node grid (parameters) is the
only one that I've seen where we deviate from the intended design -
and I think I've seen them all now!
--
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] 9+ messages in thread
* Re: Regarding issue 1241
2016-05-31 19:53 Regarding issue 1241 Harshal Dhumal <[email protected]>
2016-06-02 08:29 ` Re: Regarding issue 1241 Dave Page <[email protected]>
2016-06-15 12:49 ` Re: Regarding issue 1241 Ashesh Vashi <[email protected]>
2016-06-15 12:55 ` Re: Regarding issue 1241 Dave Page <[email protected]>
@ 2016-06-15 13:08 ` Ashesh Vashi <[email protected]>
2016-06-15 13:54 ` Re: Regarding issue 1241 Dave Page <[email protected]>
0 siblings, 1 reply; 9+ messages in thread
From: Ashesh Vashi @ 2016-06-15 13:08 UTC (permalink / raw)
To: Dave Page <[email protected]>; +Cc: Harshal Dhumal <[email protected]>; pgadmin-hackers
On Wed, Jun 15, 2016 at 6:25 PM, Dave Page <[email protected]> wrote:
> On Wed, Jun 15, 2016 at 1:49 PM, Ashesh Vashi
> <[email protected]> wrote:
> > On Thu, Jun 2, 2016 at 1:59 PM, Dave Page <[email protected]> wrote:
> >>
> >>
> >>
> >> On Tue, May 31, 2016 at 8:53 PM, Harshal Dhumal
> >> <[email protected]> wrote:
> >>>
> >>> Hi Dave,
> >>>
> >>> Regarding Issue 1241:
> >>>
> >>> We have added header section for parameter tab deliberately so that we
> >>> can force user to select parameter name (and therefore parameter's data
> >>> type) before adding new row. This is required because behavior of
> second
> >>> cell (Value cell) is dependent on what parameter name user has
> selected in
> >>> first cell (Name cell). See attached screen-shot.
> >>>
> >>> For example:
> >>> 1. If user selects parameter 'array_nulls' then value for this should
> be
> >>> either true or false (and hence switch cell).
> >>> 2. If user selects parameter 'cpu_index_tuple_cost' then value for this
> >>> should be Integer (and hence Integer cell).
> >>>
> >>> Without the custom header (and forcing user to select parameter) we
> >>> cannot decide what type of cell we need in second column.
> >>>
> >>> Let me know your opinion on this.
> >>
> >>
> >> We need to figure out a way to fix it. Our difficulties encountered
> >> writing code should not dictate usability compromises.
> >>
> >> In this case, something that needs some thought and maybe some tricky
> code
> >> has caused us to create an inconsistent UI workflow to side-step the
> >> problem, which is not appropriate as it leads to a poor look and feel
> and
> >> potentially confusion for the user.
> >
> > Agree - we should handle these cases gracefully.
> > We need to over come the limitation by brain storming, which we already
> > started offline. :-)
> >
> > To be honest - it is a time consuming work, and there is no quick fix for
> > this.
> > We can handle it as one case for each change instead of targeting all UI
> > changes as one whole problem.
> > And, we can utilize the same time to fix a lot more cases in beta 2.
>
> As far as I'm aware, this is the only case where there's a real problem.
>
> > I can ask Harshal to find out all possible places, where the similar
> changes
> > are required, and create a separate case for each (though - not without
> your
> > agreement).
>
> I don't think we need to. This one sub-node grid (parameters) is the
> only one that I've seen where we deviate from the intended design -
> and I think I've seen them all now!
>
Hmm..
Unfortunately - some set of columns needs to be unique in most of the cases
(where these controls are used), and the checks for the unique dataset is
done at the control side, which was wrong at our end.
And, we will need to change the model validation code to check the
uniqueness of data set at data level (through Backbone.Model) now, which
will require a lot more changes than it looks.
For example - in table node, we have too many UniqueCollControl, which
requires these changes.
--
Thanks & Regards,
Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company
<http://www.enterprisedb.com/;
*http://www.linkedin.com/in/asheshvashi*
<http://www.linkedin.com/in/asheshvashi;
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Regarding issue 1241
2016-05-31 19:53 Regarding issue 1241 Harshal Dhumal <[email protected]>
2016-06-02 08:29 ` Re: Regarding issue 1241 Dave Page <[email protected]>
2016-06-15 12:49 ` Re: Regarding issue 1241 Ashesh Vashi <[email protected]>
2016-06-15 12:55 ` Re: Regarding issue 1241 Dave Page <[email protected]>
2016-06-15 13:08 ` Re: Regarding issue 1241 Ashesh Vashi <[email protected]>
@ 2016-06-15 13:54 ` Dave Page <[email protected]>
2016-06-15 14:27 ` Re: Regarding issue 1241 Ashesh Vashi <[email protected]>
0 siblings, 1 reply; 9+ messages in thread
From: Dave Page @ 2016-06-15 13:54 UTC (permalink / raw)
To: Ashesh Vashi <[email protected]>; +Cc: Harshal Dhumal <[email protected]>; pgadmin-hackers
On Wed, Jun 15, 2016 at 2:08 PM, Ashesh Vashi <[email protected]
> wrote:
> On Wed, Jun 15, 2016 at 6:25 PM, Dave Page <[email protected]> wrote:
>
>> On Wed, Jun 15, 2016 at 1:49 PM, Ashesh Vashi
>> <[email protected]> wrote:
>> > On Thu, Jun 2, 2016 at 1:59 PM, Dave Page <[email protected]> wrote:
>> >>
>> >>
>> >>
>> >> On Tue, May 31, 2016 at 8:53 PM, Harshal Dhumal
>> >> <[email protected]> wrote:
>> >>>
>> >>> Hi Dave,
>> >>>
>> >>> Regarding Issue 1241:
>> >>>
>> >>> We have added header section for parameter tab deliberately so that we
>> >>> can force user to select parameter name (and therefore parameter's
>> data
>> >>> type) before adding new row. This is required because behavior of
>> second
>> >>> cell (Value cell) is dependent on what parameter name user has
>> selected in
>> >>> first cell (Name cell). See attached screen-shot.
>> >>>
>> >>> For example:
>> >>> 1. If user selects parameter 'array_nulls' then value for this should
>> be
>> >>> either true or false (and hence switch cell).
>> >>> 2. If user selects parameter 'cpu_index_tuple_cost' then value for
>> this
>> >>> should be Integer (and hence Integer cell).
>> >>>
>> >>> Without the custom header (and forcing user to select parameter) we
>> >>> cannot decide what type of cell we need in second column.
>> >>>
>> >>> Let me know your opinion on this.
>> >>
>> >>
>> >> We need to figure out a way to fix it. Our difficulties encountered
>> >> writing code should not dictate usability compromises.
>> >>
>> >> In this case, something that needs some thought and maybe some tricky
>> code
>> >> has caused us to create an inconsistent UI workflow to side-step the
>> >> problem, which is not appropriate as it leads to a poor look and feel
>> and
>> >> potentially confusion for the user.
>> >
>> > Agree - we should handle these cases gracefully.
>> > We need to over come the limitation by brain storming, which we already
>> > started offline. :-)
>> >
>> > To be honest - it is a time consuming work, and there is no quick fix
>> for
>> > this.
>> > We can handle it as one case for each change instead of targeting all UI
>> > changes as one whole problem.
>> > And, we can utilize the same time to fix a lot more cases in beta 2.
>>
>> As far as I'm aware, this is the only case where there's a real problem.
>>
>> > I can ask Harshal to find out all possible places, where the similar
>> changes
>> > are required, and create a separate case for each (though - not without
>> your
>> > agreement).
>>
>> I don't think we need to. This one sub-node grid (parameters) is the
>> only one that I've seen where we deviate from the intended design -
>> and I think I've seen them all now!
>>
> Hmm..
>
> Unfortunately - some set of columns needs to be unique in most of the
> cases (where these controls are used), and the checks for the unique
> dataset is done at the control side, which was wrong at our end.
> And, we will need to change the model validation code to check the
> uniqueness of data set at data level (through Backbone.Model) now, which
> will require a lot more changes than it looks.
>
> For example - in table node, we have too many UniqueCollControl, which
> requires these changes.
>
Perhaps - but I fail to see how this justifies the different UI design for
this one use. Are we talking about the same thing?
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Regarding issue 1241
2016-05-31 19:53 Regarding issue 1241 Harshal Dhumal <[email protected]>
2016-06-02 08:29 ` Re: Regarding issue 1241 Dave Page <[email protected]>
2016-06-15 12:49 ` Re: Regarding issue 1241 Ashesh Vashi <[email protected]>
2016-06-15 12:55 ` Re: Regarding issue 1241 Dave Page <[email protected]>
2016-06-15 13:08 ` Re: Regarding issue 1241 Ashesh Vashi <[email protected]>
2016-06-15 13:54 ` Re: Regarding issue 1241 Dave Page <[email protected]>
@ 2016-06-15 14:27 ` Ashesh Vashi <[email protected]>
2016-07-18 09:32 ` Re: Regarding issue 1241 Harshal Dhumal <[email protected]>
0 siblings, 1 reply; 9+ messages in thread
From: Ashesh Vashi @ 2016-06-15 14:27 UTC (permalink / raw)
To: Dave Page <[email protected]>; +Cc: Harshal Dhumal <[email protected]>; pgadmin-hackers
On Wed, Jun 15, 2016 at 7:24 PM, Dave Page <[email protected]> wrote:
>
>
> On Wed, Jun 15, 2016 at 2:08 PM, Ashesh Vashi <
> [email protected]> wrote:
>
>> On Wed, Jun 15, 2016 at 6:25 PM, Dave Page <[email protected]> wrote:
>>
>>> On Wed, Jun 15, 2016 at 1:49 PM, Ashesh Vashi
>>> <[email protected]> wrote:
>>> > On Thu, Jun 2, 2016 at 1:59 PM, Dave Page <[email protected]> wrote:
>>> >>
>>> >>
>>> >>
>>> >> On Tue, May 31, 2016 at 8:53 PM, Harshal Dhumal
>>> >> <[email protected]> wrote:
>>> >>>
>>> >>> Hi Dave,
>>> >>>
>>> >>> Regarding Issue 1241:
>>> >>>
>>> >>> We have added header section for parameter tab deliberately so that
>>> we
>>> >>> can force user to select parameter name (and therefore parameter's
>>> data
>>> >>> type) before adding new row. This is required because behavior of
>>> second
>>> >>> cell (Value cell) is dependent on what parameter name user has
>>> selected in
>>> >>> first cell (Name cell). See attached screen-shot.
>>> >>>
>>> >>> For example:
>>> >>> 1. If user selects parameter 'array_nulls' then value for this
>>> should be
>>> >>> either true or false (and hence switch cell).
>>> >>> 2. If user selects parameter 'cpu_index_tuple_cost' then value for
>>> this
>>> >>> should be Integer (and hence Integer cell).
>>> >>>
>>> >>> Without the custom header (and forcing user to select parameter) we
>>> >>> cannot decide what type of cell we need in second column.
>>> >>>
>>> >>> Let me know your opinion on this.
>>> >>
>>> >>
>>> >> We need to figure out a way to fix it. Our difficulties encountered
>>> >> writing code should not dictate usability compromises.
>>> >>
>>> >> In this case, something that needs some thought and maybe some tricky
>>> code
>>> >> has caused us to create an inconsistent UI workflow to side-step the
>>> >> problem, which is not appropriate as it leads to a poor look and feel
>>> and
>>> >> potentially confusion for the user.
>>> >
>>> > Agree - we should handle these cases gracefully.
>>> > We need to over come the limitation by brain storming, which we already
>>> > started offline. :-)
>>> >
>>> > To be honest - it is a time consuming work, and there is no quick fix
>>> for
>>> > this.
>>> > We can handle it as one case for each change instead of targeting all
>>> UI
>>> > changes as one whole problem.
>>> > And, we can utilize the same time to fix a lot more cases in beta 2.
>>>
>>> As far as I'm aware, this is the only case where there's a real problem.
>>>
>>> > I can ask Harshal to find out all possible places, where the similar
>>> changes
>>> > are required, and create a separate case for each (though - not
>>> without your
>>> > agreement).
>>>
>>> I don't think we need to. This one sub-node grid (parameters) is the
>>> only one that I've seen where we deviate from the intended design -
>>> and I think I've seen them all now!
>>>
>> Hmm..
>>
>> Unfortunately - some set of columns needs to be unique in most of the
>> cases (where these controls are used), and the checks for the unique
>> dataset is done at the control side, which was wrong at our end.
>> And, we will need to change the model validation code to check the
>> uniqueness of data set at data level (through Backbone.Model) now, which
>> will require a lot more changes than it looks.
>>
>> For example - in table node, we have too many UniqueCollControl, which
>> requires these changes.
>>
>
> Perhaps - but I fail to see how this justifies the different UI design for
> this one use. Are we talking about the same thing?
>
Yes - we do.
It is not change in the design of the UI control, but - we will need to
replace simplified subnode control (which is already present in the
system), and make the validation check in each of the data model one at a
time.
We need to keep the UI at other place, until we fix the data validation
part at each of those places.
We will remove the UniqueColControl once we complete all these changes.
That's why - I said it was mistake to do the validation in Control rather
than the data (Backbone.Model).
--
Thanks & Regards,
Ashesh Vashi
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Regarding issue 1241
2016-05-31 19:53 Regarding issue 1241 Harshal Dhumal <[email protected]>
2016-06-02 08:29 ` Re: Regarding issue 1241 Dave Page <[email protected]>
2016-06-15 12:49 ` Re: Regarding issue 1241 Ashesh Vashi <[email protected]>
2016-06-15 12:55 ` Re: Regarding issue 1241 Dave Page <[email protected]>
2016-06-15 13:08 ` Re: Regarding issue 1241 Ashesh Vashi <[email protected]>
2016-06-15 13:54 ` Re: Regarding issue 1241 Dave Page <[email protected]>
2016-06-15 14:27 ` Re: Regarding issue 1241 Ashesh Vashi <[email protected]>
@ 2016-07-18 09:32 ` Harshal Dhumal <[email protected]>
2016-07-18 10:51 ` Re: Regarding issue 1241 Dave Page <[email protected]>
0 siblings, 1 reply; 9+ messages in thread
From: Harshal Dhumal @ 2016-07-18 09:32 UTC (permalink / raw)
To: Ashesh Vashi <[email protected]>; +Cc: Dave Page <[email protected]>; pgadmin-hackers
Hi,
PFA patch for issue RM1241
Changes: 1. Altered variable control to make its UI consistence with
privileges and Security labels.
2. Changed datamodel.js to handle duplicate rows at datamodel level and not
UI/Control level. (See variable control for example)
--
*Harshal Dhumal*
*Software Engineer*
EnterpriseDB India: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Wed, Jun 15, 2016 at 7:57 PM, Ashesh Vashi <[email protected]
> wrote:
> On Wed, Jun 15, 2016 at 7:24 PM, Dave Page <[email protected]> wrote:
>
>>
>>
>> On Wed, Jun 15, 2016 at 2:08 PM, Ashesh Vashi <
>> [email protected]> wrote:
>>
>>> On Wed, Jun 15, 2016 at 6:25 PM, Dave Page <[email protected]> wrote:
>>>
>>>> On Wed, Jun 15, 2016 at 1:49 PM, Ashesh Vashi
>>>> <[email protected]> wrote:
>>>> > On Thu, Jun 2, 2016 at 1:59 PM, Dave Page <[email protected]> wrote:
>>>> >>
>>>> >>
>>>> >>
>>>> >> On Tue, May 31, 2016 at 8:53 PM, Harshal Dhumal
>>>> >> <[email protected]> wrote:
>>>> >>>
>>>> >>> Hi Dave,
>>>> >>>
>>>> >>> Regarding Issue 1241:
>>>> >>>
>>>> >>> We have added header section for parameter tab deliberately so that
>>>> we
>>>> >>> can force user to select parameter name (and therefore parameter's
>>>> data
>>>> >>> type) before adding new row. This is required because behavior of
>>>> second
>>>> >>> cell (Value cell) is dependent on what parameter name user has
>>>> selected in
>>>> >>> first cell (Name cell). See attached screen-shot.
>>>> >>>
>>>> >>> For example:
>>>> >>> 1. If user selects parameter 'array_nulls' then value for this
>>>> should be
>>>> >>> either true or false (and hence switch cell).
>>>> >>> 2. If user selects parameter 'cpu_index_tuple_cost' then value for
>>>> this
>>>> >>> should be Integer (and hence Integer cell).
>>>> >>>
>>>> >>> Without the custom header (and forcing user to select parameter) we
>>>> >>> cannot decide what type of cell we need in second column.
>>>> >>>
>>>> >>> Let me know your opinion on this.
>>>> >>
>>>> >>
>>>> >> We need to figure out a way to fix it. Our difficulties encountered
>>>> >> writing code should not dictate usability compromises.
>>>> >>
>>>> >> In this case, something that needs some thought and maybe some
>>>> tricky code
>>>> >> has caused us to create an inconsistent UI workflow to side-step the
>>>> >> problem, which is not appropriate as it leads to a poor look and
>>>> feel and
>>>> >> potentially confusion for the user.
>>>> >
>>>> > Agree - we should handle these cases gracefully.
>>>> > We need to over come the limitation by brain storming, which we
>>>> already
>>>> > started offline. :-)
>>>> >
>>>> > To be honest - it is a time consuming work, and there is no quick fix
>>>> for
>>>> > this.
>>>> > We can handle it as one case for each change instead of targeting all
>>>> UI
>>>> > changes as one whole problem.
>>>> > And, we can utilize the same time to fix a lot more cases in beta 2.
>>>>
>>>> As far as I'm aware, this is the only case where there's a real problem.
>>>>
>>>> > I can ask Harshal to find out all possible places, where the similar
>>>> changes
>>>> > are required, and create a separate case for each (though - not
>>>> without your
>>>> > agreement).
>>>>
>>>> I don't think we need to. This one sub-node grid (parameters) is the
>>>> only one that I've seen where we deviate from the intended design -
>>>> and I think I've seen them all now!
>>>>
>>> Hmm..
>>>
>>> Unfortunately - some set of columns needs to be unique in most of the
>>> cases (where these controls are used), and the checks for the unique
>>> dataset is done at the control side, which was wrong at our end.
>>> And, we will need to change the model validation code to check the
>>> uniqueness of data set at data level (through Backbone.Model) now, which
>>> will require a lot more changes than it looks.
>>>
>>> For example - in table node, we have too many UniqueCollControl, which
>>> requires these changes.
>>>
>>
>> Perhaps - but I fail to see how this justifies the different UI design
>> for this one use. Are we talking about the same thing?
>>
> Yes - we do.
> It is not change in the design of the UI control, but - we will need to
> replace simplified subnode control (which is already present in the
> system), and make the validation check in each of the data model one at a
> time.
>
> We need to keep the UI at other place, until we fix the data validation
> part at each of those places.
> We will remove the UniqueColControl once we complete all these changes.
>
> That's why - I said it was mistake to do the validation in Control rather
> than the data (Backbone.Model).
>
>
> --
> Thanks & Regards,
> Ashesh Vashi
>
>
>>
>> --
>> 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] RM1241.patch (29.0K, 3-RM1241.patch)
download | inline diff:
diff --git a/web/pgadmin/browser/server_groups/servers/databases/templates/databases/js/databases.js b/web/pgadmin/browser/server_groups/servers/databases/templates/databases/js/databases.js
index 61671f4..f2a49c9 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/templates/databases/js/databases.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/templates/databases/js/databases.js
@@ -284,7 +284,7 @@ function($, _, S, pgAdmin, pgBrowser, Alertify) {
canAdd: true, canDelete: true, control: 'unique-col-collection',
},{
id: 'variables', label: '{{ _('Parameters') }}', type: 'collection',
- model: pgBrowser.Node.VariableModel, editable: false,
+ model: pgBrowser.Node.VariableModel.extend({keys:['name', 'role']}), editable: false,
group: '{{ _('Parameters') }}', mode: ['edit', 'create'],
canAdd: true, canEdit: false, canDelete: true, hasRole: true,
control: Backform.VariableCollectionControl, node: 'role'
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 64fd32b..444e8ca 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
@@ -476,7 +476,8 @@ function($, _, S, pgAdmin, pgBrowser, alertify, Backform) {
},{
id: 'variables', label: '{{ _('Parameters') }}', type: 'collection',
group: '{{ _('Parameters') }}', hasDatabase: true, url: 'variables',
- model: pgBrowser.Node.VariableModel, control: 'variable-collection',
+ model: pgBrowser.Node.VariableModel.extend({keys:['name', 'database']}),
+ control: 'variable-collection',
mode: [ 'edit', 'create'], canAdd: true, canDelete: true,
url: "variables", disabled: 'readonly'
},{
diff --git a/web/pgadmin/browser/server_groups/servers/static/js/variable.js b/web/pgadmin/browser/server_groups/servers/static/js/variable.js
index 2d13728..82b0ac5 100644
--- a/web/pgadmin/browser/server_groups/servers/static/js/variable.js
+++ b/web/pgadmin/browser/server_groups/servers/static/js/variable.js
@@ -20,6 +20,7 @@
Alertify = require('alertify') || root.Alertify;
pgAdmin = require('pgadmin') || root.pgAdmin,
pgNode = require('pgadmin.browser.node') || root.pgAdmin.Browser.Node;
+
factory(root, _, $, Backbone, Backform, Alertify, pgAdmin, pgNode);
// Finally, as a browser global.
@@ -38,8 +39,17 @@
var cellFunction = function(model) {
var self = this,
name = model.get("name"),
- availVariables = self.get('availVariables'),
- variable = availVariables[name];
+ availVariables = {};
+
+ self.collection.each(function(col) {
+ if (col.get("name") == "name") {
+ availVariables = col.get('availVariables');
+ }
+ });
+
+
+ var variable = name ? availVariables[name]: undefined,
+ value = model.get("value");
switch(variable && variable.vartype) {
case "bool":
@@ -47,13 +57,13 @@
* bool cell and variable can not be stateless (i.e undefined).
* It should be either true or false.
*/
- if (_.isUndefined(model.get("value"))) {
- model.set("value", false);
- }
+
+ model.set("value", !!model.get("value"), {silent: true});
return Backgrid.Extension.SwitchCell;
break;
case "enum":
+ model.set({'value': undefined}, {silent:true});
var options = [],
enumVals = variable.enumvals;
@@ -64,40 +74,138 @@
return Backgrid.Extension.Select2Cell.extend({optionValues: options});
break;
case "integer":
+ if (!_.isNaN(parseInt(value))) {
+ model.set({'value': parseInt(value)}, {silent:true});
+ } else {
+ model.set({'value': undefined}, {silent:true});
+ }
return Backgrid.IntegerCell;
break;
case "real":
+ if (!_.isNaN(parseFloat(value))) {
+ model.set({'value': parseFloat(value)}, {silent:true});
+ } else {
+ model.set({'value': undefined}, {silent:true});
+ }
return Backgrid.NumberCell.extend({decimals: 0});
break;
case "string":
return Backgrid.StringCell;
break;
default:
+ model.set({'value': undefined}, {silent:true});
return Backgrid.Cell;
break;
}
+ model.set({'value': undefined}, {silent:true});
+ return Backgrid.Cell;
}
+ /*
+ * This row will define behaviour or value column cell depending upon
+ * variable name.
+ */
+ var VariableRow = Backgrid.Row.extend({
+ modelDuplicateColor: "lightYellow",
+
+ modelUniqueColor: "#fff",
+
+ initialize: function () {
+ Backgrid.Row.prototype.initialize.apply(this, arguments);
+ var self = this;
+ self.model.on("change:name", function() {
+ setTimeout(function() {
+ self.columns.each(function(col) {
+ if (col.get('name') == 'value') {
+
+ var idx = self.columns.indexOf(col),
+ cf = col.get("cellFunction"),
+ cell = new (cf.apply(col, [self.model]))({
+ column: col,
+ model: self.model
+ }),
+ oldCell = self.cells[idx];
+ oldCell.remove();
+ self.cells[idx] = cell;
+ self.render();
+ }
+
+ });
+ }, 10);
+ });
+ self.listenTo(self.model, 'pgadmin-session:model:duplicate', self.modelDuplicate);
+ self.listenTo(self.model, 'pgadmin-session:model:unique', self.modelUnique);
+ },
+ modelDuplicate: function() {
+ $(this.el).removeClass("new");
+ this.el.style.backgroundColor = this.modelDuplicateColor;
+ },
+ modelUnique: function() {
+ this.el.style.backgroundColor = this.modelUniqueColor;
+ }
+
+ })
/**
* VariableModel used to represent configuration parameters (variables tab)
* for database objects.
**/
var VariableModel = pgNode.VariableModel = pgNode.Model.extend({
+ keys: ['name'],
defaults: {
name: undefined,
value: undefined,
- role: undefined,
- database: undefined,
+ role: null,
+ database: null,
},
- keys: ['name', 'role', 'database'],
schema: [
- {id: 'name', label:'Name', type:'text', editable: false, cellHeaderClasses: 'width_percent_30'},
+ {
+ id: 'name', label:'Name', type:'text', editable: true, cellHeaderClasses: 'width_percent_30',
+ editable: function(m) {
+ return (m instanceof Backbone.Collection) ? true : m.isNew();
+ },
+ cell: Backgrid.Extension.NodeAjaxOptionsCell.extend({
+ initialize: function() {
+ Backgrid.Extension.NodeAjaxOptionsCell.prototype.initialize.apply(this, arguments);
+
+ // Immediately process options as we need them before render.
+
+ var opVals = _.clone(this.optionValues ||
+ (_.isFunction(this.column.get('options')) ?
+ (this.column.get('options'))(this) :
+ this.column.get('options')));
+
+ this.column.set('options', opVals);
+ }
+ }),
+ url: 'vopts',
+ select2: { allowClear: false },
+ transform: function(vars, cell) {
+ var self = this,
+ res = [],
+ availVariables = {};
+
+ _.each(vars, function(v) {
+ res.push({
+ 'value': v.name,
+ 'image': undefined,
+ 'label': v.name
+ });
+ availVariables[v.name] = v;
+ });
+
+ cell.column.set("availVariables", availVariables);
+ return res;
+ }
+ },
{
id: 'value', label:'Value', type: 'text', editable: true,
- cellFunction: cellFunction, cellHeaderClasses: 'width_percent_50'
+ cellFunction: cellFunction, cellHeaderClasses: 'width_percent_40'
+ },
+ {id: 'database', label:'Database', type: 'text', editable: true,
+ node: 'database', cell: Backgrid.Extension.NodeListByNameCell
},
- {id: 'database', label:'Database', type: 'text', editable: false},
- {id: 'role', label:'Role', type: 'text', editable: false}
+ {id: 'role', label:'Role', type: 'text', editable: true,
+ node: 'role', cell: Backgrid.Extension.NodeListByNameCell}
],
toJSON: function() {
var d = Backbone.Model.prototype.toJSON.apply(this);
@@ -144,7 +252,7 @@
initialize: function(opts) {
var self = this,
- uniqueCols = ['name'];
+ keys = ['name'];
/*
* Read from field schema whether user wants to use database and role
@@ -155,24 +263,22 @@
// Update unique coll field based on above flag status.
if (self.hasDatabase) {
- uniqueCols.push('database')
+ keys.push('database');
} else if (self.hasRole) {
- uniqueCols.push('role')
+ keys.push('role');
}
// Overriding the uniqueCol in the field
if (opts && opts.field) {
if (opts.field instanceof Backform.Field) {
opts.field.set({
- uniqueCol: uniqueCols,
- model: pgNode.VariableModel
+ model: pgNode.VariableModel.extend({keys:keys})
},
{
silent: true
});
} else {
opts.field.extend({
- uniqueCol: uniqueCols,
- model: pgNode.VariableModel
+ model: pgNode.VariableModel.extend({keys:keys})
});
}
}
@@ -181,94 +287,25 @@
self, arguments
);
-
self.availVariables = {};
var node = self.field.get('node').type,
- headerSchema = [{
- id: 'name', label:'', type:'text',
- url: self.field.get('variable_opts') || 'vopts',
- control: Backform.NodeAjaxOptionsControl,
- cache_level: 'server',
- select2: {
- allowClear: false, width: 'style'
- },
- availVariables: self.availVariables,
- node: node, first_empty: false,
- version_compatible: self.field.get('version_compatible'),
- transform: function(vars) {
- var self = this,
- opts = self.field.get('availVariables');
-
- res = [];
-
- for (var prop in opts) {
- if (opts.hasOwnProperty(prop)) {
- delete opts[prop];
- }
- }
-
- _.each(vars, function(v) {
- opts[v.name] = _.extend({}, v);
- res.push({
- 'label': v.name,
- 'value': v.name
- });
- });
-
- return res;
- }
- }],
- headerDefaults = {name: null},
gridCols = ['name', 'value'];
if (self.hasDatabase) {
- headerSchema.push({
- id: 'database', label:'', type: 'text', cache_level: 'server',
- control: Backform.NodeListByNameControl, node: 'database',
- version_compatible: self.field.get('version_compatible')
- });
- headerDefaults['database'] = null;
gridCols.push('database');
}
if (self.hasRole) {
- headerSchema.push({
- id: 'role', label:'', type: 'text', cache_level: 'server',
- control: Backform.NodeListByNameControl, node: 'role',
- version_compatible: self.field.get('version_compatible')
- });
- headerDefaults['role'] = null;
gridCols.push('role');
}
- self.headerData = new (Backbone.Model.extend({
- defaults: headerDefaults,
- schema: headerSchema
- }))({});
-
- var headerGroups = Backform.generateViewSchema(
- self.field.get('node_info'), self.headerData, 'create',
- node, self.field.get('node_data')
- ),
- fields = [];
-
- _.each(headerGroups, function(o) {
- fields = fields.concat(o.fields);
- });
-
- self.headerFields = new Backform.Fields(fields);
self.gridSchema = Backform.generateGridColumnsFromModel(
- null, VariableModel, 'edit', gridCols
+ self.field.get('node_info'), VariableModel.extend({keys:keys}), 'edit', gridCols, self.field.get('schema_node')
);
// Make sure - we do have the data for variables
self.getVariables();
-
- self.controls = [];
- self.listenTo(self.headerData, "change", self.headerDataChanged);
- self.listenTo(self.headerData, "select2", self.headerDataChanged);
- self.listenTo(self.collection, "remove", self.onRemoveVariable);
},
/*
* Get the variable data for this node.
@@ -321,101 +358,19 @@
}
},
- generateHeader: function(data) {
- var header = [
- '<div class="subnode-header-form">',
- ' <div class="container-fluid">',
- ' <div class="row">',
- ' <div class="col-md-4">',
- ' <label class="control-label"><%-variable_label%></label>',
- ' </div>',
- ' <div class="col-md-4" header="name"></div>',
- ' <div class="col-md-4">',
- ' <button class="btn-sm btn-default add" <%=canAdd ? "" : "disabled=\'disabled\'"%> ><%-add_label%></buttton>',
- ' </div>',
- ' </div>'];
-
- if(this.hasDatabase) {
- header.push([
- ' <div class="row">',
- ' <div class="col-md-4">',
- ' <label class="control-label"><%-database_label%></label>',
- ' </div>',
- ' <div class="col-md-4" header="database"></div>',
- ' </div>'].join("\n")
- );
- }
-
- if (this.hasRole) {
- header.push([
- ' <div class="row">',
- ' <div class="col-md-4">',
- ' <label class="control-label"><%-role_label%></label>',
- ' </div>',
- ' <div class="col-md-4" header="role"></div>',
- ' </div>'].join("\n")
- );
- }
-
- header.push([
- ' </div>',
- '</div>'].join("\n"));
-
- // TODO:: Do the i18n
- _.extend(data, {
- variable_label: "Parameter name",
- add_label: "ADD",
- database_label: "Database",
- role_label: "Role"
- });
-
- var self = this,
- headerTmpl = _.template(header.join("\n")),
- $header = $(headerTmpl(data)),
- controls = this.controls;
-
- this.headerFields.each(function(field) {
- var control = new (field.get("control"))({
- field: field,
- model: self.headerData
- });
-
- $header.find('div[header="' + field.get('name') + '"]').append(
- control.render().$el
- );
-
- controls.push(control);
- });
-
- // We should not show add but in properties mode
- if (data.mode == 'properties') {
- $header.find("button.add").remove();
- }
-
- self.$header = $header;
-
- return $header;
- },
-
- events: _.extend(
- {}, Backform.UniqueColCollectionControl.prototype.events,
- {'click button.add': 'addVariable'}
- ),
-
showGridControl: function(data) {
var self = this,
titleTmpl = _.template([
"<div class='subnode-header'>",
"<label class='control-label'><%-label%></label>",
+ "<button class='btn-sm btn-default add' <%=canAdd ? '' : 'disabled=\"disabled\"'%>>Add</buttton>",
"</div>"].join("\n")),
$gridBody =
$("<div class='pgadmin-control-group backgrid form-group col-xs-12 object subnode'></div>").append(
- titleTmpl({label: data.label})
+ titleTmpl(data)
);
- $gridBody.append(self.generateHeader(data));
-
var gridSchema = _.clone(this.gridSchema);
_.each(gridSchema.columns, function(col) {
@@ -460,18 +415,61 @@
var grid = self.grid = new Backgrid.Grid({
columns: gridSchema.columns,
collection: self.collection,
+ row: VariableRow,
className: "backgrid table-bordered"
});
self.$grid = grid.render().$el;
$gridBody.append(self.$grid);
- self.headerData.set(
- 'name',
- self.$header.find(
- 'div[header="name"] select option:first'
- ).val()
- );
+ // Add button callback
+ if (!(data.disabled || data.canAdd == false)) {
+ $gridBody.find('button.add').first().click(function(e) {
+ e.preventDefault();
+ var canAddRow = _.isFunction(data.canAddRow) ?
+ data.canAddRow.apply(self, [self.model]) : true;
+ if (canAddRow) {
+
+ var allowMultipleEmptyRows = !!self.field.get('allowMultipleEmptyRows');
+
+ // If allowMultipleEmptyRows is not set or is false then don't allow second new empty row.
+ // There should be only one empty row.
+ if (!allowMultipleEmptyRows && self.collection) {
+ var isEmpty = false;
+ self.collection.each(function(model) {
+ var modelValues = [];
+ _.each(model.attributes, function(val, key) {
+ modelValues.push(val);
+ })
+ if(!_.some(modelValues, _.identity)) {
+ isEmpty = true;
+ }
+ });
+ if(isEmpty) {
+ return false;
+ }
+ }
+
+ $(grid.body.$el.find($("tr.new"))).removeClass("new")
+ var m = new (data.model) (null, {
+ silent: true,
+ handler: self.collection,
+ top: self.model.top || self.model,
+ collection: self.collection,
+ node_info: self.model.node_info
+ });
+ self.collection.add(m);
+
+ var idx = self.collection.indexOf(m),
+ newRow = grid.body.rows[idx].$el;
+
+ newRow.addClass("new");
+ $(newRow).pgMakeVisible('backform-tab');
+
+ return false;
+ }
+ });
+ }
// Render node grid
return $gridBody;
@@ -505,83 +503,7 @@
delete m;
}
- this.headerDataChanged();
-
return false;
- },
-
- headerDataChanged: function() {
- var self = this, val,
- data = this.headerData.toJSON(),
- inSelected = false,
- checkVars = ['name'];
-
- if (!self.$header) {
- return;
- }
-
- if (self.hasDatabase) {
- checkVars.push('database');
- }
-
- if (self.hasRole) {
- checkVars.push('role');
- }
-
- if (self.control_data.canAdd) {
- self.collection.each(function(m) {
- if (!inSelected) {
- var has = true;
- _.each(checkVars, function(v) {
- val = m.get(v);
- has = has && ((
- (_.isUndefined(val) || _.isNull(val)) &&
- (_.isUndefined(data[v]) || _.isNull(data[v]))
- ) ||
- (val == data[v]));
- });
-
- inSelected = has;
- }
- });
- }
- else {
- inSelected = true;
- }
-
- self.$header.find('button.add').prop('disabled', inSelected);
- },
-
- onRemoveVariable: function() {
- var self = this;
-
- // Wait for collection to be updated before checking for the button to be
- // enabled, or not.
- setTimeout(function() {
- self.headerDataChanged();
- }, 10);
- },
-
- remove: function() {
- /*
- * Stop listening the events registered by this control.
- */
- this.stopListening(this.headerData, "change", this.headerDataChanged);
- this.listenTo(this.headerData, "select2", this.headerDataChanged);
- this.listenTo(this.collection, "remove", this.onRemoveVariable);
-
- // Remove header controls.
- _.each(this.controls, function(control) {
- control.remove();
- });
-
- VariableCollectionControl.__super__.remove.apply(this, arguments);
-
- // Remove the header model
- delete (this.headerData);
-
- // Clear the available Variables object
- self.availVariables = {};
}
});
diff --git a/web/pgadmin/browser/static/js/datamodel.js b/web/pgadmin/browser/static/js/datamodel.js
index 87b5a64..8cb7034 100644
--- a/web/pgadmin/browser/static/js/datamodel.js
+++ b/web/pgadmin/browser/static/js/datamodel.js
@@ -7,6 +7,7 @@ function(_, pgAdmin, $, Backbone) {
/*
* Parsing the existing data
*/
+ on_server: false,
parse: function(res) {
var self = this;
if (res && _.isObject(res) && 'node' in res && res['node']) {
@@ -33,9 +34,21 @@ function(_, pgAdmin, $, Backbone) {
silent: true,
attrName: s.id
});
- self.set(s.id, obj, {silent: true, parse: true});
+
+ /*
+ * Nested collection models may or may not have idAttribute.
+ * So to decide whether model is new or not set 'on_server'
+ * flag on such models.
+ */
+
+ self.set(s.id, obj, {silent: true, parse: true, on_server : true});
} else {
- obj.reset(val, {silent: true, parse: true});
+ /*
+ * Nested collection models may or may not have idAttribute.
+ * So to decide whether model is new or not set 'on_server'
+ * flag on such models.
+ */
+ obj.reset(val, {silent: true, parse: true, on_server : true});
}
}
else {
@@ -89,6 +102,12 @@ function(_, pgAdmin, $, Backbone) {
return res;
},
+ isNew: function() {
+ if (this.has(this.idAttribute)) {
+ return !this.has(this.idAttribute);
+ }
+ return !this.on_server;
+ },
primary_key: function() {
if (this.keys && _.isArray(this.keys)) {
var res = {}, self = this;
@@ -103,6 +122,11 @@ function(_, pgAdmin, $, Backbone) {
},
initialize: function(attributes, options) {
var self = this;
+ self._previous_key_values = {};
+
+ if ('on_server' in options && options.on_server) {
+ self.on_server = true;
+ }
Backbone.Model.prototype.initialize.apply(self, arguments);
@@ -198,6 +222,13 @@ function(_, pgAdmin, $, Backbone) {
self.startNewSession();
}
+ if ('keys' in self && _.isArray(self.keys)) {
+ _.each(self.keys, function(key) {
+ self.on("change:" + key, function(m) {
+ self._previous_key_values[key] = m.previous(key);
+ })
+ })
+ }
return self;
},
// Create a reset function, which allow us to remove the nested object.
@@ -725,13 +756,19 @@ function(_, pgAdmin, $, Backbone) {
invalidModels = self.sessAttrs['invalid'];
if (self.trackChanges) {
- // Find the object the invalid list, if found remove it from the list
+ // Now check uniqueness of current model with other models.
+ var isUnique = self.checkDuplicateWithModel(m);
+
+ // If unique then find the object the invalid list, if found remove it from the list
// and inform the parent that - I am a valid object now.
- if (m.cid in invalidModels) {
+
+ if (isUnique && m.cid in invalidModels) {
delete invalidModels[m.cid];
}
- this.triggerValidationEvent.apply(this);
+ if (isUnique) {
+ this.triggerValidationEvent.apply(this);
+ }
}
return true;
@@ -837,7 +874,6 @@ function(_, pgAdmin, $, Backbone) {
return (_.findIndex(this.sessAttrs[type], comparator));
},
onModelAdd: function(obj) {
-
if (!this.trackChanges)
return true;
@@ -871,25 +907,21 @@ function(_, pgAdmin, $, Backbone) {
(self.sessAttrs['invalid'])[obj.cid] = msg;
}
}
+ } else {
+ if ('validate' in obj && typeof(obj.validate) === 'function') {
+ msg = obj.validate();
- // Let the parent/listener know about my status (valid/invalid).
- this.triggerValidationEvent.apply(this);
-
- return true;
- }
- if ('validate' in obj && typeof(obj.validate) === 'function') {
- msg = obj.validate();
-
- if (msg) {
- (self.sessAttrs['invalid'])[obj.cid] = msg;
+ if (msg) {
+ (self.sessAttrs['invalid'])[obj.cid] = msg;
+ }
}
- }
- self.sessAttrs['added'].push(obj);
+ self.sessAttrs['added'].push(obj);
- /*
- * Session has been changed
- */
- (self.handler || self).trigger('pgadmin-session:added', self, obj);
+ /*
+ * Session has been changed
+ */
+ (self.handler || self).trigger('pgadmin-session:added', self, obj);
+ }
// Let the parent/listener know about my status (valid/invalid).
this.triggerValidationEvent.apply(this);
@@ -897,7 +929,6 @@ function(_, pgAdmin, $, Backbone) {
return true;
},
onModelRemove: function(obj) {
-
if (!this.trackChanges)
return true;
@@ -917,6 +948,8 @@ function(_, pgAdmin, $, Backbone) {
(self.handler || self).trigger('pgadmin-session:removed', self, copy);
+ self.checkDuplicateWithModel(copy);
+
// Let the parent/listener know about my status (valid/invalid).
this.triggerValidationEvent.apply(this);
@@ -936,6 +969,8 @@ function(_, pgAdmin, $, Backbone) {
self.sessAttrs['deleted'].push(obj);
+ self.checkDuplicateWithModel(obj);
+
// Let the parent/listener know about my status (valid/invalid).
this.triggerValidationEvent.apply(this);
@@ -985,12 +1020,12 @@ function(_, pgAdmin, $, Backbone) {
}
},
onModelChange: function(obj) {
+ var self = this;
if (!this.trackChanges || !(obj instanceof pgBrowser.Node.Model))
return true;
- var self = this,
- idx = self.objFindInSession(obj, 'added');
+ var idx = self.objFindInSession(obj, 'added');
// It was newly added model, we don't need to add into the changed
// list.
@@ -1034,6 +1069,74 @@ function(_, pgAdmin, $, Backbone) {
(self.handler || self).trigger('pgadmin-session:changed', self, obj);
return true;
+ },
+
+ /*
+ * This function will check if given model is unique or duplicate in
+ * collection and set/clear duplicate errors on models.
+ */
+ checkDuplicateWithModel: function(model) {
+ if (!('keys' in model) || _.isEmpty(model.keys)) {
+ return true;
+ }
+
+ var self = this,
+ condition = {},
+ previous_condition = {};
+
+ _.each(model.keys, function(key) {
+ condition[key] = model.get(key);
+ if(key in model._previous_key_values) {
+ previous_condition[key] = model._previous_key_values[key];
+ } else {
+ previous_condition[key] = model.previous(key);
+ }
+ });
+
+ // Reset previously changed values.
+ model._previous_key_values = {};
+
+ var old_conflicting_models = self.where(previous_condition);
+
+ if (old_conflicting_models.length == 1) {
+ var m = old_conflicting_models[0];
+ self.clearInvalidSessionIfModelValid(m);
+ }
+
+ new_conflicting_models = self.where(condition);
+ if (new_conflicting_models.length == 0) {
+ self.clearInvalidSessionIfModelValid(model);
+ } else if (new_conflicting_models.length == 1) {
+ self.clearInvalidSessionIfModelValid(model);
+ self.clearInvalidSessionIfModelValid(new_conflicting_models[0]);
+ } else {
+ var msg = "Duplicate rows.";
+ setTimeout(function() {
+ _.each(new_conflicting_models, function(m) {
+ self.trigger(
+ 'pgadmin-session:model:invalid', msg, m, self.handler
+ );
+ m.trigger(
+ 'pgadmin-session:model:duplicate', m, msg
+ );
+ });
+ }, 10);
+
+ return false;
+ }
+ return true;
+ },
+ clearInvalidSessionIfModelValid: function(m) {
+ var errors = m.errorModel.attributes,
+ invalidModels = this.sessAttrs['invalid'];
+
+ m.trigger('pgadmin-session:model:unique', m
+ );
+ if (_.size(errors) == 0) {
+ delete invalidModels[m.cid];
+ } else {
+ invalidModels[m.cid] = errors[Object.keys(errors)[0]];
+ }
}
});
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Regarding issue 1241
2016-05-31 19:53 Regarding issue 1241 Harshal Dhumal <[email protected]>
2016-06-02 08:29 ` Re: Regarding issue 1241 Dave Page <[email protected]>
2016-06-15 12:49 ` Re: Regarding issue 1241 Ashesh Vashi <[email protected]>
2016-06-15 12:55 ` Re: Regarding issue 1241 Dave Page <[email protected]>
2016-06-15 13:08 ` Re: Regarding issue 1241 Ashesh Vashi <[email protected]>
2016-06-15 13:54 ` Re: Regarding issue 1241 Dave Page <[email protected]>
2016-06-15 14:27 ` Re: Regarding issue 1241 Ashesh Vashi <[email protected]>
2016-07-18 09:32 ` Re: Regarding issue 1241 Harshal Dhumal <[email protected]>
@ 2016-07-18 10:51 ` Dave Page <[email protected]>
0 siblings, 0 replies; 9+ messages in thread
From: Dave Page @ 2016-07-18 10:51 UTC (permalink / raw)
To: Harshal Dhumal <[email protected]>; +Cc: Ashesh Vashi <[email protected]>; pgadmin-hackers
Thanks - committed with minor tweaks!
On Mon, Jul 18, 2016 at 10:32 AM, Harshal Dhumal
<[email protected]> wrote:
> Hi,
>
> PFA patch for issue RM1241
>
> Changes: 1. Altered variable control to make its UI consistence with
> privileges and Security labels.
> 2. Changed datamodel.js to handle duplicate rows at datamodel level and not
> UI/Control level. (See variable control for example)
>
>
>
> --
> Harshal Dhumal
> Software Engineer
>
> EnterpriseDB India: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Wed, Jun 15, 2016 at 7:57 PM, Ashesh Vashi
> <[email protected]> wrote:
>>
>> On Wed, Jun 15, 2016 at 7:24 PM, Dave Page <[email protected]> wrote:
>>>
>>>
>>>
>>> On Wed, Jun 15, 2016 at 2:08 PM, Ashesh Vashi
>>> <[email protected]> wrote:
>>>>
>>>> On Wed, Jun 15, 2016 at 6:25 PM, Dave Page <[email protected]> wrote:
>>>>>
>>>>> On Wed, Jun 15, 2016 at 1:49 PM, Ashesh Vashi
>>>>> <[email protected]> wrote:
>>>>> > On Thu, Jun 2, 2016 at 1:59 PM, Dave Page <[email protected]> wrote:
>>>>> >>
>>>>> >>
>>>>> >>
>>>>> >> On Tue, May 31, 2016 at 8:53 PM, Harshal Dhumal
>>>>> >> <[email protected]> wrote:
>>>>> >>>
>>>>> >>> Hi Dave,
>>>>> >>>
>>>>> >>> Regarding Issue 1241:
>>>>> >>>
>>>>> >>> We have added header section for parameter tab deliberately so that
>>>>> >>> we
>>>>> >>> can force user to select parameter name (and therefore parameter's
>>>>> >>> data
>>>>> >>> type) before adding new row. This is required because behavior of
>>>>> >>> second
>>>>> >>> cell (Value cell) is dependent on what parameter name user has
>>>>> >>> selected in
>>>>> >>> first cell (Name cell). See attached screen-shot.
>>>>> >>>
>>>>> >>> For example:
>>>>> >>> 1. If user selects parameter 'array_nulls' then value for this
>>>>> >>> should be
>>>>> >>> either true or false (and hence switch cell).
>>>>> >>> 2. If user selects parameter 'cpu_index_tuple_cost' then value for
>>>>> >>> this
>>>>> >>> should be Integer (and hence Integer cell).
>>>>> >>>
>>>>> >>> Without the custom header (and forcing user to select parameter) we
>>>>> >>> cannot decide what type of cell we need in second column.
>>>>> >>>
>>>>> >>> Let me know your opinion on this.
>>>>> >>
>>>>> >>
>>>>> >> We need to figure out a way to fix it. Our difficulties encountered
>>>>> >> writing code should not dictate usability compromises.
>>>>> >>
>>>>> >> In this case, something that needs some thought and maybe some
>>>>> >> tricky code
>>>>> >> has caused us to create an inconsistent UI workflow to side-step the
>>>>> >> problem, which is not appropriate as it leads to a poor look and
>>>>> >> feel and
>>>>> >> potentially confusion for the user.
>>>>> >
>>>>> > Agree - we should handle these cases gracefully.
>>>>> > We need to over come the limitation by brain storming, which we
>>>>> > already
>>>>> > started offline. :-)
>>>>> >
>>>>> > To be honest - it is a time consuming work, and there is no quick fix
>>>>> > for
>>>>> > this.
>>>>> > We can handle it as one case for each change instead of targeting all
>>>>> > UI
>>>>> > changes as one whole problem.
>>>>> > And, we can utilize the same time to fix a lot more cases in beta 2.
>>>>>
>>>>> As far as I'm aware, this is the only case where there's a real
>>>>> problem.
>>>>>
>>>>> > I can ask Harshal to find out all possible places, where the similar
>>>>> > changes
>>>>> > are required, and create a separate case for each (though - not
>>>>> > without your
>>>>> > agreement).
>>>>>
>>>>> I don't think we need to. This one sub-node grid (parameters) is the
>>>>> only one that I've seen where we deviate from the intended design -
>>>>> and I think I've seen them all now!
>>>>
>>>> Hmm..
>>>>
>>>> Unfortunately - some set of columns needs to be unique in most of the
>>>> cases (where these controls are used), and the checks for the unique dataset
>>>> is done at the control side, which was wrong at our end.
>>>> And, we will need to change the model validation code to check the
>>>> uniqueness of data set at data level (through Backbone.Model) now, which
>>>> will require a lot more changes than it looks.
>>>>
>>>> For example - in table node, we have too many UniqueCollControl, which
>>>> requires these changes.
>>>
>>>
>>> Perhaps - but I fail to see how this justifies the different UI design
>>> for this one use. Are we talking about the same thing?
>>
>> Yes - we do.
>> It is not change in the design of the UI control, but - we will need to
>> replace simplified subnode control (which is already present in the system),
>> and make the validation check in each of the data model one at a time.
>>
>> We need to keep the UI at other place, until we fix the data validation
>> part at each of those places.
>> We will remove the UniqueColControl once we complete all these changes.
>>
>> That's why - I said it was mistake to do the validation in Control rather
>> than the data (Backbone.Model).
>>
>>
>> --
>> Thanks & Regards,
>> Ashesh Vashi
>>
>>>
>>>
>>> --
>>> 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] 9+ messages in thread
end of thread, other threads:[~2016-07-18 10:51 UTC | newest]
Thread overview: 9+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2016-05-31 19:53 Regarding issue 1241 Harshal Dhumal <[email protected]>
2016-06-02 08:29 ` Dave Page <[email protected]>
2016-06-15 12:49 ` Ashesh Vashi <[email protected]>
2016-06-15 12:55 ` Dave Page <[email protected]>
2016-06-15 13:08 ` Ashesh Vashi <[email protected]>
2016-06-15 13:54 ` Dave Page <[email protected]>
2016-06-15 14:27 ` Ashesh Vashi <[email protected]>
2016-07-18 09:32 ` Harshal Dhumal <[email protected]>
2016-07-18 10:51 ` 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