Re: [GSoC] Query History Upgrade - Mailing list pgadmin-hackers

Hi Aditya,

On Mon, Aug 12, 2019 at 9:04 AM Aditya Toshniwal <aditya.toshniwal@enterprisedb.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.

 

They seem to be in the right place for me, would you mind rechecking?

=============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

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/