Re: [pgAdmin4][Patch]: RM #3659 Query tool does not give warning when Auto Commit is turned on in an incomplete transaction

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][Patch]: RM #3659 Query tool does not give warning when Auto Commit is turned on in an incomplete transaction
Date: 2019-03-04 10:44:34
Message-ID: CA+OCxoyOpymQM53GHDYtxsXWHaAmo-MaW3oXnEXj-0TYG0DE0Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Thanks - applied. Works nicely now :-)

On Sat, Mar 2, 2019 at 7:57 AM Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
wrote:

> Hi Dave
>
> On Fri, Mar 1, 2019 at 8:47 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>> Hi
>>
>> On Fri, Mar 1, 2019 at 9:07 AM Akshay Joshi <
>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Hackers,
>>>
>>> Attached is the patch to fix RM #3659 "Query tool does not give warning
>>> when Auto Commit is turned on in an incomplete transaction".
>>>
>>> Please review it.
>>>
>>
>> The behaviour doesn't seem quite right here. I see two issues:
>>
>> 1) It complains that a transaction is in progress if I try to turn
>> auto-commit on in a freshly opened Query Tool. As I haven't done anything
>> yet, I shouldn't get the warning. I think the logic to determine whether or
>> not we're in a transaction isn't correct.
>>
>> 2) In the case above, the setting is changed anyway. I think this has the
>> potential to be confusing (and potentially dangerous); we should only allow
>> the setting to actually be toggled when there's no transaction in progress.
>>
>
> The above behaviour I have followed from pgAdmin 3.
>
>>
>> Given 2), I wonder if we should remove the warning entirely, and just
>> disable the menu options whenever a transaction is in progress.
>>
>
> Done. I have disabled the drop down button when transaction is in
> progress. I have tried to disabled single menu option but that is an anchor
> tag. I have tried with CSS by setting "*pointer-events: none*" and from
> HTML adding "*disabled=’disabled*’" which is only supported in old
> browsers.
>
> Attached is the updated patch, please review it.
>
>>
>> I've attached an updated patch - I started tweaking the messages, so
>> please base additional work from that.
>>
>> Thanks.
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>
> --
> *Akshay Joshi*
>
> *Sr. Software Architect *
>
>
>
> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>

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

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

In response to

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2019-03-04 11:09:41 pgAdmin 4 commit: Fix the position of text editor of a cell in Query To
Previous Message Akshay Joshi 2019-03-04 10:42:08 Re: [pgAdmin4][Patch] - Encoding Fixes