Re: [GSoC] Query History Upgrade

From: Yosry Muhammad <yosrym93(at)gmail(dot)com>
To: Aditya Toshniwal <aditya(dot)toshniwal(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>, Dave Page <dpage(at)pgadmin(dot)org>
Subject: Re: [GSoC] Query History Upgrade
Date: 2019-08-12 23:33:18
Message-ID: CAFSMqn-nRwRM-HtPWBP4yxzz9Pg1=WNpkiX-waYM+m8U-Qyr8Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

The test failed after merging with master. A previously written test needed
to be updated after a previous commit.

Please find an updated patch attached with the fix.

On Mon, Aug 12, 2019 at 1:34 PM Aditya Toshniwal <
aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:

> Hi Yosry,
>
>
> On Mon, Aug 12, 2019 at 2:00 PM Yosry Muhammad <yosrym93(at)gmail(dot)com> wrote:
>
>> Hi Aditya,
>>
>> On Mon, Aug 12, 2019 at 9:04 AM Aditya Toshniwal <
>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Yosry,
>>>
>>> Nice work there. It seems to be working fine except a few suggestions:
>>> 1) Fix pep8 issues
>>> 2) DOM Statements like below can be avoided and html can be added
>>> directly to main template of $el instead of adding extra operations of
>>> find, prepend and append. Plus, it makes it difficult to understand what
>>> will the DOM look like.
>>> this.$el.find('.query').prepend('<i id="query_source_icon"
>>> class="query-history-icon sql-icon-lg"></i>');
>>> $container.append($toggleEl).append(self.$el);
>>> 3) Change the below to use class d-none with toggleClass('d-none') for
>>> consistency across.
>>> this.$el.find('.pgadmin-query-history-entry').each(function() {
>>> $(this).toggle();
>>> });
>>>
>>
>> Please find an updated patch attached with the above issues fixed. The
>> pep8 issue was in the test, I didn't re-check pep8 after writing the test -
>> my bad.
>>
>
>>
>>> 4) I may be wrong, but I'm seeing the flash icon for view/edit data
>>> queries and view table icon for query tool queries. Looks like they are
>>> swapped.
>>> [image: Screenshot 2019-08-12 at 12.15.18.png]
>>>
>>>
>>>
>> They seem to be in the right place for me, would you mind rechecking?
>>
> Now there are showing fine.
> However, the feature test case is failing for me. I tried 2 times:
> =============Running the test cases for 'PostgreSQL 11'=============
> runTest
> (pgadmin.feature_tests.query_tool_journey_test.QueryToolJourneyTest)
> Tests the path through the query tool ... Copy rows... OK.
> Copy columns... OK.
> History tab... OK.
> Updatable resultsets...FAIL
>
> ======================================================================
> FAIL: runTest
> (pgadmin.feature_tests.query_tool_journey_test.QueryToolJourneyTest)
> Tests the path through the query tool
> ----------------------------------------------------------------------
> Traceback (most recent call last):
> File
> "/Users/adityatoshniwal/projects/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
> line 79, in runTest
> self._test_updatable_resultset()
> File
> "/Users/adityatoshniwal/projects/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
> line 240, in _test_updatable_resultset
> self._check_query_results_editable(query, False)
> File
> "/Users/adityatoshniwal/projects/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
> line 378, in _check_query_results_editable
> is_editable = self._check_cell_editable(1)
> File
> "/Users/adityatoshniwal/projects/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
> line 395, in _check_cell_editable
> self.assertFalse('editable' in cell_classes)
> AssertionError: True is not false
>
> ----------------------------------------------------------------------
> Ran 1 test in 38.118s
>
> FAILED (failures=1)
>
>>
>> Thanks !
>>
>> --
>> *Yosry Muhammad Yosry*
>>
>> Computer Engineering student,
>> The Faculty of Engineering,
>> Cairo University (2021).
>> Class representative of CMP 2021.
>> https://www.linkedin.com/in/yosrym93/
>>
>
>
> --
> Thanks and Regards,
> Aditya Toshniwal
> Software Engineer | EnterpriseDB India | Pune
> "Don't Complain about Heat, Plant a TREE"
>

--
*Yosry Muhammad Yosry*

Computer Engineering student,
The Faculty of Engineering,
Cairo University (2021).
Class representative of CMP 2021.
https://www.linkedin.com/in/yosrym93/

Attachment Content-Type Size
query_history_v3.patch text/x-patch 230.3 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Aditya Toshniwal 2019-08-13 11:26:20 Re: [GSoC] Query History Upgrade
Previous Message Aditya Toshniwal 2019-08-12 11:33:56 Re: [GSoC] Query History Upgrade