Re: User management functionality patch [pgadmin4]

Thanks - applied with a minor tweak for the button styling so it
looked consistent in webkit browsers.

On Tue, Jun 7, 2016 at 8:13 AM, Harshal Dhumal
<harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:
> 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
>
>

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

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