MIME-Version: 1.0
In-Reply-To: 
 <CAFOhELeBmb02uLj36Y+UFXeQUN9Ri9SKYEsEDQz7nF4q09BaxQ@mail.gmail.com>
References: 
 <CAFOhELf-n8mM4h8RZuqxUs-Z+f97N6Ux5KT6aoAdWFiHVVURyw@mail.gmail.com>
	<CAFOhELf-QTMAgNkTT8AsRwJ38Jn_1BSFRV_G5ZmcHnrKmPsTog@mail.gmail.com>
	<CACCA4P1caumXOrpEXrrETMuNGj3G3ctY-PN9=V6GUPF=dTBnWg@mail.gmail.com>
	<CAFOhELdgb566u+4XXOOP4pDCF4GGzK8JZzLrrTuPt=OprsqG5Q@mail.gmail.com>
	<CACCA4P06PrJ2-VodT133sheH1i+7zc_M_y_me6KfRHkLqrQ7gg@mail.gmail.com>
	<CAFOhELc1OAz3BDWkKC0e0xA8sPBKiO-4vWJQ7ipMYmKC-C6YzA@mail.gmail.com>
	<CACCA4P1uFijfEV6vvETXX=qjafhGZsOacdi7Og+7edaBb4erxA@mail.gmail.com>
	<CAFOhELePQ_m8zmbORLmO3PUY5yTA5o_9suQKk0Tox8A7Fb8ovw@mail.gmail.com>
	<CAFOhELcq3oiXAC3LVCvxVA5i8qjCh=Sk77MsVUnZB2VhwzmLKw@mail.gmail.com>
	<CA+OCxozpzStRAm=rm3s2ZdtehMt6WfQTVEFT3fssorNc5M2UHQ@mail.gmail.com>
	<CA+OCxoxkows1UUgaddq3+R2VcTW9Vmc3QCKdrtd1sNP+fDsHTg@mail.gmail.com>
	<CAFOhELfzRV1WoA7A87=g5Nb9uvU3rLL2jPUCEid66pqzFdrfRQ@mail.gmail.com>
	<CA+OCxoxWUGqp5P_KSUbKBXH-i2AD4E8STWcfxsy01iVAymU-Og@mail.gmail.com>
	<CAFOhELfA8rTWKJr_P9=Z6--oeE0m7E40nZFr=WFSs2g8=3RmAg@mail.gmail.com>
	<CAFOhELewp6_jT35CkG8u8-+K+H9AhFS_QpiwXfMcCib6vJyJBg@mail.gmail.com>
	<CA+OCxoyRLYFv_SxNNGdsGN=Up7d3WT_EQz0-h9P+S+0oRtjaYg@mail.gmail.com>
	<CAFOhELc=PzdATON2Eo0sipm7AC9dtz33YZm-CqTfHUNesuUpwA@mail.gmail.com>
	<CAFOhELeBmb02uLj36Y+UFXeQUN9Ri9SKYEsEDQz7nF4q09BaxQ@mail.gmail.com>
Date: Wed, 23 Mar 2016 13:05:42 +0000
Message-ID: 
 <CA+OCxoxrV-_=UWOZ2WnJWA9z2E_hRbhYxpJYFNQyMAFDwsjVkw@mail.gmail.com>
Subject: Re: pgAdmin4 PATCH: Domain Module
From: Dave Page <dpage@pgadmin.org>
To: Khushboo Vashi <khushboo.vashi@enterprisedb.com>
Cc: pgadmin-hackers <pgadmin-hackers@postgresql.org>
Content-Type: text/plain; charset=UTF-8
Precedence: bulk
Sender: pgadmin-hackers-owner@postgresql.org

Hi

Almost there :-s

- The hint for default should be a placeholder in the textbox itself
(like combos have "Select from the list"

- I should be able to check the "Validate?" option on one or more
constraints from within the Domain dialogue

- Please ensure the capitalisation of all property labels is
consistent - it should be "Base type" not "Base Type", "System
domain?" not "System Domain?" etc.

- The check constraint reverse engineered SQL should include the path
to the constraint, e.g.

-- CHECK: schema.domain.check_at

Once that's done, it can be committed I think.

Thanks.

On Wed, Mar 23, 2016 at 7:27 AM, Khushboo Vashi
<khushboo.vashi@enterprisedb.com> wrote:
> Updated one comment.
>
> On Wed, Mar 23, 2016 at 12:48 PM, Khushboo Vashi
> <khushboo.vashi@enterprisedb.com> wrote:
>>
>> Hi,
>>
>> Please find attached updated patch for the Domains module.
>>
>> On Wed, Mar 16, 2016 at 9:40 PM, Dave Page <dpage@pgadmin.org> wrote:
>>>
>>> Hi
>>>
>>> On Wed, Mar 16, 2016 at 2:03 PM, Khushboo Vashi
>>> <khushboo.vashi@enterprisedb.com> wrote:
>>> > Hi,
>>> >
>>> > Please find the updated Domain Module Patch.
>>> >
>>> > To test this patch, please apply Backgrid Textarea Cell Patch before
>>> > this.
>>>
>>> Thanks. I believe with the following fixes, we'll be done :-)
>>>
>>> - Default values should be auto-quoted when necessary (ie. strings, on
>>> a text-based domain).
>>
>> As per our discussion, this should leave unquoted.
>
> And also added a hint to the field stated 'Enter an expression or a value.'
>>
>>
>>> - "System Domain?" should be in the General section, between owner and
>>> comment.
>>
>> Done
>>>
>>> - The switches should use the same colouring/styling as other objects,
>>> e.g.
>>>
>>>           options: {
>>>             'onText': 'Yes', 'offText': 'No',
>>>             'onColor': 'success', 'offColor': 'primary',
>>>             'size': 'small'
>>>           }
>>
>> Done
>>>
>>> - Please remove the Schema property from the main properties tab (not
>>> the properties dialogue).
>>
>> Done
>>>
>>> - No icon is show for Checks on the Dependents tab for a domain.
>>
>> Done
>>>
>>> - The add button on the Security Labels tab is spelt "Add". Why is
>>> that? Other instances of this grid use "ADD" which is the default in
>>> backform.pgadmin.js.
>>
>> Done
>>>
>>> - Dependencies on domain check constraints are listed as being on a
>>> "Type" not a "Domain".
>>
>> Done
>>>
>>> - If adding a domain constraint using the grid on the Domain dialogue,
>>> I cannot specify "NOT VALID". We need a checkbox for that in a narrow
>>> columns at the end. Unchecking it for an existing constraint should be
>>> the equivalent of doing "ALTER DOMAIN ... VALIDATE CONSTRAINT"
>>
>> Done
>>>
>>> - If I switch the "Don't Validate" switch on a constraint, there are
>>> leading blank lines in the generated SQL. The same occurs when adding
>>> a comment to a constraint.
>>
>> Done
>>>
>>> - I think we need to reverse the meaning of "Don't Validate" and
>>> rename to match the "Valid?" field that's on the properties list.
>>> Otherwise it's not clear they're the same thing.
>>
>> Done
>>>
>>> - s/Not Null/Not Null?/
>>
>> Done
>>>
>>>
>>>
>>> --
>>> 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 (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers