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

From: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
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-02 07:57:35
Message-ID: CANxoLDey0LQ=nqob6f7LeSm95+MdUXkKpW7Fpf=pO6T+uKVWTA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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*

Attachment Content-Type Size
RM_3659_v2.patch application/octet-stream 11.8 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Khushboo Vashi 2019-03-04 05:43:21 Re: Async queries pretending to be synchronous
Previous Message Khushboo Vashi 2019-03-02 06:14:26 Re: [pgAdmin4][Patch] - Encoding Fixes