Re: Regarding issue 1241

From: Harshal Dhumal <harshal(dot)dhumal(at)enterprisedb(dot)com>
To: Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>
Cc: Dave Page <dpage(at)pgadmin(dot)org>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: Regarding issue 1241
Date: 2016-07-18 09:32:12
Message-ID: CAFiP3vwBHTZKQ+74fB-fP1a3kBd_7cXR-3nv+DGX==H0X39hmw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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 <ashesh(dot)vashi(at)enterprisedb(dot)com
> wrote:

> On Wed, Jun 15, 2016 at 7:24 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>>
>>
>> On Wed, Jun 15, 2016 at 2:08 PM, Ashesh Vashi <
>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>
>>> On Wed, Jun 15, 2016 at 6:25 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>
>>>> On Wed, Jun 15, 2016 at 1:49 PM, Ashesh Vashi
>>>> <ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>> > On Thu, Jun 2, 2016 at 1:59 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>> >>
>>>> >>
>>>> >>
>>>> >> On Tue, May 31, 2016 at 8:53 PM, Harshal Dhumal
>>>> >> <harshal(dot)dhumal(at)enterprisedb(dot)com> 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
>>
>
>

Attachment Content-Type Size
RM1241.patch text/x-patch 29.0 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Neel Patel 2016-07-18 09:42:52 Re: PATCH: Fix the issue for saving query output as CSV
Previous Message Dave Page 2016-07-18 08:50:57 Re: [pgAdmin4][Patch]: RM#1462- AJAX request do not get fired second time