public inbox for [email protected]
help / color / mirror / Atom feedFrom: Dave Page <[email protected]>
To: Surinder Kumar <[email protected]>
Cc: Ashesh Vashi <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: [pgAdmin4] [Patch]: Grant Wizard
Date: Thu, 7 Apr 2016 20:22:24 +0100
Message-ID: <CA+OCxoyC-U+u8eESm8NXchX+s9dwdCFKMJ3x5JcZNCAgN4CnPw@mail.gmail.com> (raw)
In-Reply-To: <CAM5-9D-ixhoFahG2dxmZCxhv_VVm0ncsqm075kemZ+M7kuoKPw@mail.gmail.com>
References: <CAM5-9D8oYyEaL89coD9FozPDziGhYutYAat9dMP9dAwhOp5OKQ@mail.gmail.com>
<CA+OCxozy4aH9=2T0Sp_WworU_3UQaaHBcQXTDhEUKHAkZJT8ow@mail.gmail.com>
<CA+OCxoyGKAorswp0fwUojT4gB0Ex+dcRxzKbLkfG48qeCmSCrQ@mail.gmail.com>
<CAM5-9D_D3TGq9-DbcR+07=p2cgT=yEtmEJnHU9rtmov_nby9+g@mail.gmail.com>
<CAM5-9D_6kD3yEcZ3A4A6WOT7aRk_EGru6LSswQv+PBmK=2QKfA@mail.gmail.com>
<CA+OCxozwOV+FixF3-9KgmGOaytZQ3Fsn3_BNAyCNqQPtLp+_VQ@mail.gmail.com>
<CAM5-9D9vGUgGaACFZc1YxFNp_gi-fAm=w2rf_JmNtD+HBi64=w@mail.gmail.com>
<CA+OCxozvNDNi0MBVQ3Gq7GhT27o2MX2=KuFUE+pUUqOCbufe+A@mail.gmail.com>
<CAM5-9D8ZHTvx53WhL2S0XUt8o+LE0aiCxMLQJGP2PUg_Q4S1dw@mail.gmail.com>
<CAM5-9D-bSJx-u_K+o2eQhd8r_39+CTxmabg9wkXP_NzVu9Eo2A@mail.gmail.com>
<CA+OCxozp8ZS5wHLAFuONT9Ap93De6so2Eg=ta028eusQwt=fEA@mail.gmail.com>
<CAM5-9D9Z9-x9Bbb0rFFS34xCy_GNsdot2bQ4rbYMPq=pWL3Vew@mail.gmail.com>
<CAM5-9D9hQZWsYETszCV7=dVsLf6KHojC9P+_icmHFGCh8jFHnQ@mail.gmail.com>
<CAG7mmowJONpMzMva7qxLQ=Wd5bFxXMsV7URosk8O5zVKLPXwnA@mail.gmail.com>
<CAM5-9D9R4vR6TfOLd+NiSVWZy+L0Z+8Jj=A-v-tP5f_5Of5ksA@mail.gmail.com>
<CA+OCxownatDkVQsZGKJUwO1=co0XOf0J=U5ydSYh8=xXi7ogTQ@mail.gmail.com>
<CAM5-9D-ixhoFahG2dxmZCxhv_VVm0ncsqm075kemZ+M7kuoKPw@mail.gmail.com>
List-Unsubscribe: <mailto:[email protected]?body=unsub%20pgadmin-hackers>
Hi
Nearly there :-). Assuming no regressions, I believe we'll be ready to
commit once the following issues are resolved:
- The select object grid should fill the available vertical space in the
dialogue - at present there's a gap below it.
- When scrolling to the top of bottom of the select object grid, the
dialogue jumps up or down the screen a little, if the dialogue has been
resized to a larger size.
- Please allow the wizard to be opened from a Database node, in which case
objects from all schemas should be listed. This will also require support
for schemas themselves.
- When selecting privileges, each time I click on a checkbox, the row
closes. Similar grids elsewhere in the app close the row when the cell
loses focus, *however*, that is also the incorrect behaviour - the row
should only close when the row itself loses focus (and should open when it
gets focus).
- The message:
Please select objects from the below list.
should read:
Please select objects from the list below.
- The message:
Following query will be executed on the database server for the selected
objects, and privileges. Please click on Finish to complete the process.
should read:
The SQL below will be executed on the database server to grant the selected
privileges. Please click on <b>Finish</b> to complete the process.
Thanks!
On Thu, Apr 7, 2016 at 6:10 PM, Surinder Kumar <
[email protected]> wrote:
> Hi Dave,
>
> Please find updated patch with above issue resolved.
>
> On Thu, Apr 7, 2016 at 7:47 PM, Dave Page <[email protected]> wrote:
>
>> Hi
>>
>> On Wed, Apr 6, 2016 at 12:37 PM, Surinder Kumar <
>> [email protected]> wrote:
>>
>>> Hi
>>>
>>> PFA updated patch with resolved review comments.
>>>
>>> On Tue, Apr 5, 2016 at 11:06 AM, Ashesh Vashi <
>>> [email protected]> wrote:
>>>
>>>> On Wed, Mar 30, 2016 at 5:14 PM, Surinder Kumar <
>>>> [email protected]> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> Please find updated patch.
>>>>>
>>>>> This patch has following changes:
>>>>> 1. Improved code commenting.
>>>>> 2. Properly handling memory leak issues in js code.
>>>>>
>>>> Hi Surinder,
>>>>
>>>> As discussed offline, here are the list of some of the review comments:
>>>>
>>>> * CSS should be relative to its parent element. Please make sure -
>>>> whenever you make
>>>> some changes in CSS, it should not affect the existing CSS unless
>>>> discussed.
>>>>
>>> Done
>>>
>>>>
>>>> * Change class name for 'error_msg_div' as it is common name. Please
>>>> name a class
>>>> with prefixed as the module name.
>>>>
>>> Done
>>>
>>>>
>>>> * Add comments for the blow line changed in node.ui.js file. Always
>>>> add logical
>>>> explanation for a change as a comment for any changes.
>>>> *while(p && p.length > 0) {*
>>>>
>>> Done
>>>
>>>>
>>>> * Please make sure, we wrap the code around 80 characters for better
>>>> readability.
>>>> Line length should not be greater than 80 characters.
>>>>
>>> Done
>>>
>>>>
>>>> * Put the allowed ACLs logic with server version support. We need to be
>>>> flexible
>>>> enough to accommodate possible future change in ACLs.
>>>>
>>> Done
>>>
>>>>
>>>> * Avoid using name as reference in each of the given. It will make the
>>>> search faster
>>>> in the database and less prone to character conversion issue.
>>>> i.e.
>>>> Use schema/namespace OID instead of nspname, object OID instead of
>>>> their name.
>>>>
>>> Done
>>>
>>>>
>>>> * Use separate templates for each type of objects.
>>>>
>>> Done
>>>
>>>>
>>>> * Use the existing functionalities as much as possible instead of
>>>> introducing new
>>>> one. That will make the code/results consistent across the
>>>> application.
>>>> i.e.
>>>> Use existing 'parse_priv_to_db' method, instead of creating new one.
>>>>
>>> Done
>>>
>>>>
>>>> * Please remove unnecessary suffixed white-spaces.
>>>>
>>> Done
>>>
>>
>> I get the attached error in the browser console when selecting "Grant
>> Wizard" from the menu when the current object is a schema. I've tried all
>> the normal refreshing/restarting.
>>
>> [image: Inline image 1]
>>
>>
>> --
>> 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
Attachments:
[image/png] Screen Shot 2016-04-07 at 15.15.21.png (117.8K, 3-Screen%20Shot%202016-04-07%20at%2015.15.21.png)
download | view image
view thread (24+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: [pgAdmin4] [Patch]: Grant Wizard
In-Reply-To: <CA+OCxoyC-U+u8eESm8NXchX+s9dwdCFKMJ3x5JcZNCAgN4CnPw@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox