Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1agBM3-0000Dt-Iy for pgadmin-hackers@arkaria.postgresql.org; Wed, 16 Mar 2016 13:19:15 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1agBM2-0006F9-N6 for pgadmin-hackers@arkaria.postgresql.org; Wed, 16 Mar 2016 13:19:14 +0000 Received: from makus.postgresql.org ([2001:4800:1501:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1agBLo-00060d-JJ for pgadmin-hackers@postgresql.org; Wed, 16 Mar 2016 13:19:00 +0000 Received: from mail-io0-x22b.google.com ([2607:f8b0:4001:c06::22b]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1agBLc-0007BI-KX for pgadmin-hackers@postgresql.org; Wed, 16 Mar 2016 13:18:59 +0000 Received: by mail-io0-x22b.google.com with SMTP id g184so1284555ioa.3 for ; Wed, 16 Mar 2016 06:18:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=v2o62BXmfcYGrQ05DOWk0LGF2x/mIIkR1Lf6wKu77ds=; b=jvlkuSjPuT9tgotvITEm5xho/Z7LPfNFVtfEzrr/fp8mv3Mi+MbvCgPMjbkoTpbMqs tOtM5gtQyTRW4fo0dGybhGy7PcJiT+BcRVscGjrWyvdBK1DpSTb9zekH8HASqsq306a9 SVmKG2DPlLMjA4BKhQhoWk8QimhzEhIqu5UFpYU1LmeVQ5T0AH/5kpRdM8w7L3Zgg1ud RS/QBW+t91BeoUiEtKvZytKXtao6RWifKrrr4dbItJwgSQIgJQgZ5iKBz3q3z6a6ZdWG edlx85d9Gl4xex107mN6PQP9tFTIdmVKmfMif2+AE6wcpEzlcH+HGt+2PBPX+aoyaAZR LygQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=v2o62BXmfcYGrQ05DOWk0LGF2x/mIIkR1Lf6wKu77ds=; b=ZZj43KlMoZro1iCBszFxcjVqdYBgTcaM9YKemUCh8xBv7A/35ABZlUXuMQXXCbLOVb uldu4WacZr53+ZKP54RE6ohSnL4FXPhXxNjJ1HNfZrF208B175IDAmckXoUz2Ek5fDL5 bjKAACzSy3qYRlkaWqVZAW7YWJ4GbCT/R9azcvfq5UyihSSaJcyeFB1lDPw9POt7oYYH ZWthxSpgWIWVUTuL4D3TKnGSTmvsq8hBGf3O8nYaxXatpLRPH6pJa2uOKbjNOdnUFTMp tt/LWpDq4qfGCh9boyF3rRAFvdtzOw6hsQmRXNKCpPv+7MRTc7PfaMXPQpBdyHnQyVvy XDRg== X-Gm-Message-State: AD7BkJLRdpLu8Ik9y5qu8j02UbEhuqCKFQluBpgD9AxpzSobvZX2q6MuIWALPR3TsUg+Yjw5He12EY/4LMCf6iE5 X-Received: by 10.107.156.193 with SMTP id f184mr3389934ioe.117.1458134327525; Wed, 16 Mar 2016 06:18:47 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.39.5 with HTTP; Wed, 16 Mar 2016 06:18:27 -0700 (PDT) In-Reply-To: References: From: Ashesh Vashi Date: Wed, 16 Mar 2016 18:48:27 +0530 Message-ID: Subject: Re: PATCH: Added Node Type & Catalog objects [pgAdmin4] To: Dave Page Cc: Murtuza Zabuawala , pgadmin-hackers Content-Type: multipart/alternative; boundary=001a1140cd38a08fe6052e2a5a6a X-Pg-Spam-Score: -2.6 (--) List-Archive: List-Help: List-ID: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: X-Mailing-List: pgadmin-hackers Precedence: bulk Sender: pgadmin-hackers-owner@postgresql.org --001a1140cd38a08fe6052e2a5a6a Content-Type: text/plain; charset=UTF-8 On Wed, Mar 16, 2016 at 6:32 PM, Dave Page wrote: > Hi > > On Mon, Mar 14, 2016 at 12:21 PM, Murtuza Zabuawala > wrote: > > Hi Dave, > > > > Thank you for reviewing my patch. > > > > I have made changes according to your feedback. > > > > Please review updated patch. > > OK, I think it's in pretty good shape now. Here's what I hope are the > last of the review comments, mostly minor things :-) > > - I'm allowed to edit ENUM label names, but PostgreSQL doesn't support > that. > > - Security label fields resize in Edit mode. See recent changes by > Arun to fix that elsewhere. > > - When editing a type, I cannot select a Grantee on the Privileges tab. > Is the grantor same as the current user? If yes - then it is a bug, otherwise it is by design. Because - you can not change privileges of a grantee, when it was given by some other grantor. But - if you delete the privileges for its grantor, it will remove privileges for the privileges given by it too. (which is though, we have not yet take care, and we will do it in later.) -- Thanks & Regards, Ashesh Vashi EnterpriseDB INDIA: Enterprise PostgreSQL Company *http://www.linkedin.com/in/asheshvashi* > > - When adding a composite type, I cannot type/select directly in the > grid, I have to expand the row. > > - If I click ADD to add a new member to a composite type whilst the > previous row is expanded, the new row is added, but the expanded panel > remains linked to the first row. > > - If I then click on the Edit button on the new row, I see two > expanded panels, neither of which is attached to their row (and > they're in the wrong order). I would only expect to see one panel at a > time. > > - When creating an External type, the Input, Output, Send, Receive and > Analyze function lists are all blank. Is this expected? I would have > thought there would be system functions listed as there are for Typmod > in/out. > > - Labels for Yes/No fields should typically end in a '?', e.g. > "Variable?", "Preferred?" > > - s/Prefered/Preferred > > - Why does "Range" type have a frame around it? > > - If a field is there to select/display a function, it should have > "function" at the end of its label, e.g. "Canonical function" > > - As above, but with type. E.g. "Element type" > > - Some of the combo boxes have hint text of "Select from the list", > whilst others don't. This should be consistent. > > - The colouring of "No" on the switches should be blue (see "System > Schema?") > > - s/Subtype/Sub-type/ > > - s/SubType diff/Sub-type diff function/ > > - Please remove the pre-9.1 support. > > - In def create(self, gid, sid, did, scid), ensure error messages are > properly pluralised, e.g. 'Composite types require at least two > members' or 'External types require both Input & Output conversion > functions.' > > - additional_features.sql is poorly formatted (lack of proper indentation) > > Thanks! > > -- > 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 > --001a1140cd38a08fe6052e2a5a6a Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
On Wed, Mar 16, 2016 at 6:32 PM, Dave Page <dpage@pgadmin= .org> wrote:
Hi

On Mon, Mar 14, 2016 at 12:21 PM, Murtuza Zabuawala
<murtuza.zabuawala= @enterprisedb.com> wrote:
> Hi Dave,
>
> Thank you for reviewing my patch.
>
> I have made changes according to your feedback.
>
> Please review updated patch.

OK, I think it's in pretty good shape now. Here's what I hop= e are the
last of the review comments, mostly minor things :-)

- I'm allowed to edit ENUM label names, but PostgreSQL doesn't supp= ort that.

- Security label fields resize in Edit mode. See recent changes by
Arun to fix that elsewhere.

- When editing a type, I cannot select a Grantee on the Privileges tab.
=
Is the grantor same as the current user?

If yes - then it is a bug, otherwise it is by design.
Be= cause - you can not change privileges of a grantee, when it was given by so= me other grantor.

But - if you delete the privileg= es for its grantor, it will remove privileges for the privileges given by i= t too. (which is though, we have not yet take care, and we will do it in la= ter.)

--

Thanks & Regar= ds,

<= span style=3D"font-style:italic">Ashesh Vashi
EnterpriseDB INDIA:=C2=A0= Enterprise Postg= reSQL Company



- When adding a composite type, I cannot type/select directly in the
grid, I have to expand the row.

- If I click ADD to add a new member to a composite type whilst the
previous row is expanded, the new row is added, but the expanded panel
remains linked to the first row.

- If I then click on the Edit button on the new row, I see two
expanded panels, neither of which is attached to their row (and
they're in the wrong order). I would only expect to see one panel at a<= br> time.

- When creating an External type, the Input, Output, Send, Receive and
Analyze function lists are all blank. Is this expected? I would have
thought there would be system functions listed as there are for Typmod
in/out.

- Labels for Yes/No fields should typically end in a '?', e.g.
"Variable?", "Preferred?"

- s/Prefered/Preferred

- Why does "Range" type have a frame around it?

- If a field is there to select/display a function, it should have
"function" at the end of its label, e.g. "Canonical function= "

- As above, but with type. E.g. "Element type"

- Some of the combo boxes have hint text of "Select from the list"= ;,
whilst others don't. This should be consistent.

- The colouring of "No" on the switches should be blue (see "= ;System Schema?")

- s/Subtype/Sub-type/

- s/SubType diff/Sub-type diff function/

- Please remove the pre-9.1 support.

- In def create(self, gid, sid, did, scid), ensure error messages are
properly pluralised, e.g. 'Composite types require at least two
members' or 'External types require both Input & Output convers= ion
functions.'

- additional_features.sql is poorly formatted (lack of proper indentation)<= br>
Thanks!

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


--
Sent via pgadmin-hackers mailing l= ist (pgadmin-hackers@post= gresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers=

--001a1140cd38a08fe6052e2a5a6a--