Re: [GSoC] Finalized First Patch

Patch committed (with some trivial editorial work on the docs).

Congratulations - that's impressive work!

On Tue, Jul 16, 2019 at 6:03 AM Yosry Muhammad <yosrym93(at)gmail(dot)com> wrote:

> Hi all,
>
> Please find attached an updated patch with the following modifications:
>
> - Fixed the bug noticed by Khushboo, it was caused because I was using the
> connection status checked periodically to know the transaction status and
> whether a transaction is active. Monitoring the connection status could be
> disabled using preferences (which I assume was the case at Khushboo's). I
> changed the code to store the last transaction status on any query
> execution, polling, or saving of data changes. This transaction status is
> used rather than the one checked periodically as it can be disabled. I also
> refactored other parts of the code to make use of the stored transaction
> status.
>
> - Created a new dialog that prompts the user to either commit or rollback
> when exiting with an active transaction. In addition, the commit button is
> disabled when the transaction is invalid (as the default behavior of
> clicking commit when the transaction is invalid is rolling back, this makes
> it clearer to the user that they can only rollback the transaction or
> cancel the exit). Preferences and documentation where updated accordingly.
>
> - When the user performs a failed save as a part of an ongoing transaction
> (with auto-commit off), a notification now clarifies that only the saving
> action was rolledback rather than the whole transaction, therefore, there
> previous queries are unaffected.
>
> Looking forward to your feedback.
> Thanks !
>
>
> On Fri, Jul 12, 2019 at 11:53 AM Yosry Muhammad <yosrym93(at)gmail(dot)com>
> wrote:
>
>> I will look into it and get back to you.
>> Thanks !
>>
>> On Fri, Jul 12, 2019, 11:20 AM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>>>
>>>
>>> On Fri, Jul 12, 2019 at 5:57 AM Khushboo Vashi <
>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi Yosry,
>>>>
>>>> On Thu, Jul 11, 2019 at 6:50 PM Yosry Muhammad <yosrym93(at)gmail(dot)com>
>>>> wrote:
>>>>
>>>>> Hi Khushboo,
>>>>> Please find an updated patch attached with the mentioned import line
>>>>> removed.
>>>>>
>>>>> Looks good to me.
>>>>
>>>>> On Thu, Jul 11, 2019 at 6:45 AM Khushboo Vashi <
>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> On Wed, Jul 10, 2019 at 3:11 PM Yosry Muhammad <yosrym93(at)gmail(dot)com>
>>>>>> wrote:
>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> On Wed, Jul 10, 2019, 9:14 AM Khushboo Vashi <
>>>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>>> Some points I missed:
>>>>>>>> 1. I assumed that in this patch modification in case of OIDs= True
>>>>>>>> (without primary key) has not considered as that is not working.
>>>>>>>>
>>>>>>>
>>>>>>> This is not implemented yet. I will work on that in a following
>>>>>>> patch soon enough.
>>>>>>>
>>>>>>> Okay.
>>>>>>
>>>>>>> 2. As we are already showing the changed Data prompt on closing the
>>>>>>>> Query Tool, do we really need the Uncommitted Transaction prompt?
>>>>>>>>
>>>>>>>
>>>>>>> This is needed when auto-commit is off. Saving changes in the data
>>>>>>> grid is performed as part of the ongoing transaction (or a new one if none
>>>>>>> is ongoing). After saving the data changes the user should still commit the
>>>>>>> current transaction for the changes to be commited to the database. This
>>>>>>> feature is also useful in general when auto-commit is off as users may
>>>>>>> forget to commit ongoing transactions.
>>>>>>>
>>>>>>> One thing I have noticed, when I add a new row and delete it
>>>>>> immediately without saving it and try to close the query tool, the
>>>>>> uncommitted prompt is coming.
>>>>>> In my opinion, it should not come, what do you think?
>>>>>>
>>>>>> We should disable the prompt if auto-commit and auto-rollback both
>>>>>> are enabled.
>>>>>>
>>>>>
>>>>> The uncommited prompt does not keep track of what the user has done so
>>>>> far, it only checks for the current transaction status. If a current
>>>>> transaction is ongoing, the prompt comes up. If you added a new row then
>>>>> deleted it without saving, the transaction status is not affected, you must
>>>>> have done a previous operation and had auto-commit turned off (probably the
>>>>> select statement).
>>>>> if auto-commit & auto-rollback are both enabled then there won't be
>>>>> any ongoing transaction at any point, thus, the prompt will never come up.
>>>>>
>>>>> Exactly, my point is. It should not prompt if auto-commit &
>>>> auto-rollback both are enabled, but it is coming. Please see the attached
>>>> video.
>>>>
>>>
>>> Agreed - this should be fixed.
>>>
>>> If auto-commit is turned off, it should also prompt to commit if the
>>> user hit Save in the prior step I think. Maybe reversing that prompt makes
>>> more sense in general - prompting to save rather than discard is quite
>>> normal; think about text editors etc.
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>
>
> --
> *Yosry Muhammad Yosry*
>
> Computer Engineering student,
> The Faculty of Engineering,
> Cairo University (2021).
> Class representative of CMP 2021.
> https://www.linkedin.com/in/yosrym93/
>

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

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