Re: User management functionality patch [pgadmin4]

From: Harshal Dhumal <harshal(dot)dhumal(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: User management functionality patch [pgadmin4]
Date: 2016-06-07 07:13:38
Message-ID: CAFiP3vwdDB4pV4JbmPx=qyWVCAu6fhZsTAvjJZHU37oiuFZh_A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi,

PFA patch for user management issues.

--
*Harshal Dhumal*
*Software Engineer*

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

On Mon, Jun 6, 2016 at 6:06 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

> Hi
>
> Thanks - I've commit as-is (with some minor tweaks), however the
> following issues are present:
>
> 1) I get an error: "Invalid Email id: dpage+pg(at)pgadmin(dot)org". That is a
> perfectly valid email address, but I guess we're not recognising the
> +.
>
Fixed.

>
> 2) When I mouse-over the Users menu option, the cursor isn't changing
> to a pointer.
>
Fixed.

>
> 3) The font in the Close button is still not quite the same as for the
> other dialogues.
>
Fixed font size issue.

>
> Please fix and submit patch(es).
>
> Thanks.
>
> On Mon, Jun 6, 2016 at 11:58 AM, Harshal Dhumal
> <harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:
> > Hi,
> >
> > PFA updated patch (V6) for user management functionality.
> >
> > Changes: As per Ashesh's suggestion I have disabled email update of
> existing
> > user.
> >
> >
> > --
> > Harshal Dhumal
> > Software Engineer
> >
> > EnterpriseDB India: http://www.enterprisedb.com
> > The Enterprise PostgreSQL Company
> >
> > On Mon, Jun 6, 2016 at 2:16 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
> >>
> >> Hi
> >>
> >> On Fri, Jun 3, 2016 at 10:52 PM, Harshal Dhumal
> >> <harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:
> >> > Hi,
> >> >
> >> >
> >> > PFA attached patch (V5) for user management functionality.
> >> >
> >> > Note: If you've applied any of the previous patch of this
> functionality
> >> > then
> >> > set ConfigDB value to 10 in version table of and also delete role
> >> > 'Standard'
> >> > from role table before applying this patch.
> >>
> >> Done - also restarted my app server, and done a hard refresh of the
> >> browser...
> >>
> >> And I get "(index):310 Uncaught TypeError: Cannot read property
> >> 'show_users' of undefined" when I try to open the Users menu option.
> >
> >
> > This was an issue. Ideally Users menu shouldn't be visible to non admin
> > users. I have fixed in this patch.
> >
> >>
> >>
> >> >> - The Close button should be disabled if errors are present.
> >> >
> >> >
> >> > I'm not convinced that to deny superuser from closing dialog for his
> >> > mistakes (accidental mistakes).
> >> >
> >> > Consider a case when superuser clears email for any old user
> >> > inadvertently
> >> > (obviously this won't reflect on server). At this point there is no
> >> > proper
> >> > way that he can roll back or close the dialog without saving it if we
> >> > disable close button. He has to either enter correct email for that
> user
> >> > or
> >> > refresh the browser.
> >> >
> >> > Another case while adding new user if he plans not to add user then he
> >> > has
> >> > to clear that partially filled user from grid before he can close the
> >> > dialog.
> >>
> >> Well we either need that, or a message box asking the user if he wants
> >> to discard his changes and offering OK/Cancel options.
> >
> >
> > I have added confirmation before closing dialog if any unsaved changes
> are
> > present.
> >
> >
> >>
> >>
> >> >> - If I enter all the details for a new user and then hit Close, the
> >> >> dialog
> >> >> is closed and the new user is NOT added. I have to click something
> else
> >> >> first so the row loses focus, and then click close.
> >> >
> >> >
> >> > I was not able to reproduce this issue. I tried with both close
> buttons
> >> > (top-right and bottom-right). Users were created in both the cases by
> >> > adding
> >> > all details and directly closing dialog without clicking anywhere on
> the
> >> > dialog.
> >>
> >> Hmm, I'll re-test when I get an updated patch.
> >
> > Ok
> >
> >>
> >>
> >> --
> >> 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
>

Attachment Content-Type Size
user_management_issues.patch text/x-patch 4.2 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Ashesh Vashi 2016-06-07 07:57:22 pgAdmin 4 commit: During encryption password is converted to buffer (in
Previous Message Sandeep Thakkar 2016-06-07 07:02:36 Re: Patch for pgAdmin4 RPM package