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

Thanks - applied!

On Fri, May 25, 2018 at 6:52 AM, Aditya Toshniwal <
aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:

> 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(dot)toshniwal(at)enterprisedb(dot)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(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"
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>
>>>
>

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

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