Re: [pgAdmin4] [Patch]: Grant Wizard

Hi Dave,

Please find updated patch with suggested changes.

On Thu, Mar 10, 2016 at 5:26 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

> On Thu, Mar 10, 2016 at 9:50 AM, Surinder Kumar
> <surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
> > Please apply Khusboo's patch for "Privileges macros under Schema" before
> > using grant wizard patch.
>
> Thanks, that works. Some (hopefully final) feedback:
>
> - Can we add a side-image? Not sure what yet - just a placeholder for
> now until I come up with something.
>
Already a placeholder for left side image.

>
> - Why is the closed button in an odd position (see File -> Test Alert
> for comparison)
>
Fixed

>
> - Why are we using a scrolling list AND pagination? I think a
> scrolling list alone should be fine.
>
Pagination is removed.

>
> - The grid sizing is wrong. See how the scrollbar on the right in the
> screenshot is off the edge of the dialogue, and there's a horizontal
> scrollbar?
>
Fixed.

>
> - We shouldn't truncate object names as that can be ambiguous. The
> column should extend as necessary, and there should be a horizontal
> scrollbar on the grid itself (not at the bottom of the dialogue
> content).
>
I have fixed it and scrollbar is now on the grid itself.

>
> - Function names should include the parameters, as they are part of
> the identifier. Without, it can be ambigous - e.g. do_stuff(int) vs.
> do_stuff(text).
>
Fixed.

>
> - If I select only functions (for example), the Privileges panel
> should only list privileges available for functions. If I select
> multiple object types, it should show the available options for only
> those object types.
>
Implemented this feature.

>
> - If I select functions and tables, and then choose (for example),
> usage and truncate, it will attempt to set usage on tables and
> truncate on functions. It should only attempt to set privileges on the
> objects for which they are appropriate.
>
Done

>
> - On the last page, the Next button is disabled. It is turned a
> marginally darker blue, but also highlights on mouse-over. The change
> in shade is so subtle it's hard to see, and the highlight implies the
> button is active when it isn't.
>
Fixed.

>
> - The buttons appear to have a smaller corner radius than those on the
> main browser, or in other dialogues.
>
Fixed.

>
> - Button labels should have an &nbsp; before them to properly space
> the label from the icon (or better yet, this should be done in CSS,
> though that would also need to be done elsewhere).
>
done with css.

>
> - Why do the URLs have a /wizard prefix? I think that should be removed.
>
Removed prefix.

>
> - The available privileges for each object type seem to be defined in
> both grant_wizard.js and allowed_acl.json. Can we just use
> allowed_acl.json?
>
Yes, allowed_acl.json is now used in grant_wizard.js.

>
> Thanks.
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>