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-25 05:52:02
Message-ID: CAM9w-_katju7x3+GnGUs7m8PfkXTNoka5+yoSF-jCUdqvRw_bA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Anthony/Joao,

That worked well !! Nice !! ( I am very new to Jasmine tests :P. Need to
dig in. )

Hackers,
Kindly review the patch attached in the trailing mail for RM#3271

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

On Fri, May 25, 2018 at 12:30 AM, Joao De Almeida Pereira <
jdealmeidapereira(at)pivotal(dot)io> wrote:

> Hello Aditya,
>
> We changed the way the tests were running and we could remove the function.
> Instead of piggybackiung on a deprecated field we created a html fixture
>
> $('body').append(
> '<div id="test-id">' +
> '<button id="btn-flash" disabled></button>' +
> '<button id="btn-cancel-query"></button>' +
> '</div>'
> );
>
>
> With it we could use the real JQuery instead of mocking it.
> Now the tests look like this
>
> it('disables the run query button', () => {
> let buttonFlash = $('#btn-flash');
>
> expect(buttonFlash.prop('disabled')).toEqual(true);
> });
>
>
>
> Thanks
> Anthony & Joao
>
> On Thu, May 24, 2018 at 12:26 AM Aditya Toshniwal <aditya.toshniwal@
> enterprisedb.com> wrote:
>
>> 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.toshniwal@
>> enterprisedb.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.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 Dave Page 2018-05-25 09:31:19 Re: [pgadmin4][patch] Use pytest test runner for unit tests
Previous Message Xuri Gong 2018-05-25 04:00:59 Re: Implement geospatial data viewer in pgAdmin4 for PostGIS