Subject: [pgAdmin4][RM3849] Ask to save unsaved query changes stopped working

From: Aditya Toshniwal <[email-masked]>
Date: Mon, 21 Jan 2019 13:24:18 +0530

Hi Hackers,

Attached is the patch to throw alert warning when user reloads or closes
the browser.

Kindly review.

-- 
Thanks and Regards,
Aditya Toshniwal
Software Engineer | EnterpriseDB Software Solutions | Pune
"Don't Complain about Heat, Plant a tree"

---

From: Akshay Joshi <[email-masked]>
Date: Tue, 22 Jan 2019 11:57:16 +0530

Hi Aditya

Below are my review comments:

   - You have added new config parameter "*DISABLE_BEFOREUNLOAD_MESSAGE*"
   to make this request configurable, should it be the part of preferences
   setting instead of configuration? @Dave can you please comment on this.
   - Name "*DISABLE_BEFOREUNLOAD_MESSAGE"* is not clear as users
   perspective, can you please change it to some meaningful name.

Apart from that code looks good to me.

-- 
*Akshay Joshi*

*Sr. Software Architect *

*Phone: +91 20-3058-9517Mobile: +91 976-788-8246*

---

From: Dave Page <[email-masked]>
Date: Tue, 22 Jan 2019 09:31:12 +0000

Hi

On Tue, Jan 22, 2019 at 6:27 AM Akshay Joshi
<[email-masked]> wrote:
> You have added new config parameter "DISABLE_BEFOREUNLOAD_MESSAGE" to make this request configurable, should it be the part of preferences setting instead of configuration? @Dave can you please comment on this.

That does seem like something that should be a per-user preference,
not a global config option.

> Name "DISABLE_BEFOREUNLOAD_MESSAGE" is not clear as users perspective, can you please change it to some meaningful name.
>
> On Mon, Jan 21, 2019 at 1:24 PM Aditya Toshniwal <[email-masked]> wrote:
>>
>> Hi Hackers,
>>
>> Attached is the patch to throw alert warning when user reloads or closes the browser.
>>
>> Kindly review.
>>
>> --
>> Thanks and Regards,
>> Aditya Toshniwal
> Akshay Joshi
> Sr. Software Architect
> Phone: +91 20-3058-9517
> Mobile: +91 976-788-8246

-- 
Dave Page
VP, Chief Architect, Tools & Installers
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

---

From: Aditya Toshniwal <[email-masked]>
Date: Tue, 22 Jan 2019 17:24:18 +0530

Any suggestions on preferences name/desc ?

On Tue, Jan 22, 2019 at 3:01 PM Dave Page <[email-masked]>
wrote:

> > You have added new config parameter "DISABLE_BEFOREUNLOAD_MESSAGE" to
> make this request configurable, should it be the part of preferences
> > Name "DISABLE_BEFOREUNLOAD_MESSAGE" is not clear as users perspective,
> can you please change it to some meaningful name.
> >

---

From: Dave Page <[email-masked]>
Date: Tue, 22 Jan 2019 11:57:18 +0000

> Any suggestions on preferences name/desc ?

Name: "Confirm on close"
Desc: "Confirm closure of the browser or browser tab is intended
before proceeding."

> On Tue, Jan 22, 2019 at 3:01 PM Dave Page <[email-masked]> wrote:
>>
>> Hi
>>

---

From: Aditya Toshniwal <[email-masked]>
Date: Tue, 22 Jan 2019 17:31:32 +0530

> Desc: "Confirm closure of the browser or browser tab is intended
It also warns on refresh.

> >> > Name "DISABLE_BEFOREUNLOAD_MESSAGE" is not clear as users

---

From: Dave Page <[email-masked]>
Date: Tue, 22 Jan 2019 12:08:26 +0000

On Tue, Jan 22, 2019 at 12:01 PM Aditya Toshniwal
<[email-masked]> wrote:
>
>
>
> On Tue, Jan 22, 2019 at 5:27 PM Dave Page <[email-masked]> wrote:
>>

Name: "Confirm on close or refresh"
Desc: "Confirm closure or refresh of the browser or browser tab is
intended before proceeding."

---

From: Aditya Toshniwal <[email-masked]>
Date: Tue, 22 Jan 2019 18:28:11 +0530

Hi Hackers,

Attached is the revised patch. The warning occurrence is configurable from
Preferences > Browser > Display.

Kindly review.

On Tue, Jan 22, 2019 at 5:38 PM Dave Page <[email-masked]>
wrote:

> >> >> > You have added new config parameter "DISABLE_BEFOREUNLOAD_MESSAGE"

---

From: Akshay Joshi <[email-masked]>
Date: Wed, 23 Jan 2019 11:19:55 +0530

Hi Aditya

Found one issue you have remove the function "*disable_tree_state_save*" from
"*web/regression/python_test_utils/test_utils.py*" and move that code into "
*set_preference*" function and it is called from "
*web/regression/runtests.py*" which is conditional, so please correct that
behaviour.

On Tue, Jan 22, 2019 at 6:28 PM Aditya Toshniwal <
[email-masked]> wrote:

>> >> >> > You have added new config parameter
>> "DISABLE_BEFOREUNLOAD_MESSAGE" to make this request configurable, should it
>> be the part of preferences setting instead of configuration? @Dave can you
>> please comment on this.
>> >> >>

-- 
*Akshay Joshi*

---

From: Aditya Toshniwal <[email-masked]>
Date: Wed, 23 Jan 2019 11:32:02 +0530

Hi Hackers,

PFA revised patch as per review.

On Wed, Jan 23, 2019 at 11:20 AM Akshay Joshi <[email-masked]>
wrote:

> "*web/regression/python_test_utils/test_utils.py*" and move that code
> into "*set_preference*" function and it is called from "
> *web/regression/runtests.py*" which is conditional, so please correct
> that behaviour.
>
>> Attached is the revised patch. The warning occurrence is configurable
>> from Preferences > Browser > Display.
>>
>> Kindly review.
>>

---

From: Akshay Joshi <[email-masked]>
Date: Wed, 23 Jan 2019 11:58:45 +0530

Thanks patch applied.

On Wed, Jan 23, 2019 at 11:32 AM Aditya Toshniwal <
[email-masked]> wrote:

> On Wed, Jan 23, 2019 at 11:20 AM Akshay Joshi <
> [email-masked]> wrote:
>
>> Hi Aditya
>>

-- 
*Akshay Joshi*