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 04:26:10
Message-ID: CAM9w-_kfDX-pLT8dzhgf537un+DBTeRuDznj3KkT+LQ=zrE6QQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi,

No luck with data-test-selector. :(
I added data-test-selector to html tags. I get undefined for
.data('test-selector') as well as for .attr('data-test-selector').

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

On Thu, May 24, 2018 at 9:17 AM, Aditya Toshniwal <
aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:

> 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(dot)toshniwal(at)enterprisedb(dot)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(dot)toshniwal(at)enterprisedb(dot)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(dot)toshniwal(at)enterprisedb(dot)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 Ashesh Vashi 2018-05-24 06:59:16 Re: [pgadmin4][patch] Initial patch to decouple from ACI Tree
Previous Message Aditya Toshniwal 2018-05-24 03:47:14 Re: [pgAdmin4][RM#3271] Update to the latest 3.x version of jQuery