Re: [pgAdmin4][RM#3271] Update to the latest 3.x version of jQuery

From: Aditya Toshniwal <aditya(dot)toshniwal(at)enterprisedb(dot)com>
To: Joao De Almeida Pereira <jdealmeidapereira(at)pivotal(dot)io>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][RM#3271] Update to the latest 3.x version of jQuery
Date: 2018-05-24 03:47:14
Message-ID: CAM9w-_ktQn7sk8GF+FevcmxmknevXRz7NreYU8qtGwJ9ud6dfg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Victoria/Joao,

Thank you for reviewing. The above function is probably the last solution
which I added.
The test cases in web/regression/javascript/sqleditor/execute_query_spec.js
calls the execute query function of pgAdmin4 sqleditor. The execution
disables/enables the buttons internally and is not done by the test cases.
So there is spy on prop function (jqueryPropSpy = spyOn($.fn, 'prop')) of
jQuery. The problem is, the spy jQuery object (in function
findJQueryCallWithSelector) returned seems to have very less information,
not sure why. I had tried using css selector for id, tried extracting id
directly, class, but none of them were present in the object.

I can try adding data-test-selector to the HTML5 tag, if it works then will
send the updated patch.

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

On Wed, May 23, 2018 at 7:41 PM, Joao De Almeida Pereira <
jdealmeidapereira(at)pivotal(dot)io> wrote:

> HI Aditya,
>
> Good job porting the app from the ancient version of JQuery to the latest
> build :D
>
> The patch in general looks good, and the patches-bot says all tests pass.
>
> The only thing that we would like to point out is this piece of code:
>
> /* jQuery has removed selector property in version 3.x
> * To allow JS test cases to run, the below function is
> * used to set the selector property
> */
> function setPropAndSelector(selector, propName, propVal) {
> let tmpObj = $(selector);
> tmpObj['selector'] = selector;
> tmpObj.prop(propName,propVal);
> return tmpObj;
> }
>
> As tests are also part of the code we believe that when there is a change
> that affect testing we should change the tests to work accordingly. So our
> suggestion is, instead of creating this function, why don’t we use a CSS
> selector in the tests to do the clicking or even use the HTML5 tag
> data-test-selector?
> This way the tests will also be coherent with the upgrade from JQuery 1 to
> 3
> ​
>
> Thanks
> Victoria & Joao
>
> On Wed, May 23, 2018 at 6:44 AM Aditya Toshniwal <aditya.toshniwal@
> enterprisedb.com> wrote:
>
>> Hi Hackers,
>>
>> Please find the updated patch for upgrading the jQuery version to 3.x.
>> The patch is only for pgAdmin4 code changes replacing old deprecated
>> functions with new replacement.
>> We need to look into external libraries used for any vulnerabilities.
>> Request you to kindly review.
>>
>> Thanks and Regards,
>> Aditya Toshniwal
>> Software Engineer | EnterpriseDB Software Solutions | Pune
>> "Don't Complain about Heat, Plant a tree"
>>
>> On Mon, May 14, 2018 at 1:23 PM, Aditya Toshniwal <aditya.toshniwal@
>> enterprisedb.com> wrote:
>>
>>> Hi Hackers,
>>>
>>> Please hold on with the patch. Will send the updated patch soon.
>>> Apologies.
>>>
>>>
>>>
>>> Thanks and Regards,
>>> Aditya Toshniwal
>>> Software Engineer | EnterpriseDB Software Solutions | Pune
>>> "Don't Complain about Heat, Plant a tree"
>>>
>>> On Mon, May 14, 2018 at 12:52 PM, Aditya Toshniwal <aditya.toshniwal@
>>> enterprisedb.com> wrote:
>>>
>>>> Hi Hackers,
>>>>
>>>> PFA patch for upgrading jQuery version to 3.3.1 from current version
>>>> 1.12.4. Patch includes replacing deprecated functions of jquery to latest
>>>> one.
>>>> Kindly review.
>>>>
>>>> Thanks and Regards,
>>>> Aditya Toshniwal
>>>> Software Engineer | EnterpriseDB Software Solutions | Pune
>>>> "Don't Complain about Heat, Plant a tree"
>>>>
>>>
>>>
>>

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Aditya Toshniwal 2018-05-24 04:26:10 Re: [pgAdmin4][RM#3271] Update to the latest 3.x version of jQuery
Previous Message Anthony Emengo 2018-05-23 21:48:45 [pgadmin4][patch] Use pytest test runner for unit tests