Re: [GSoC] Query History Upgrade

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 <
[email protected]> wrote:

> Hi Yosry,
>
>
> On Mon, Aug 12, 2019 at 2:00 PM Yosry Muhammad <[email protected]> wrote:
>
>> Hi Aditya,
>>
>> On Mon, Aug 12, 2019 at 9:04 AM Aditya Toshniwal <
>> [email protected]> 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/