Hi Yosry,
On Mon, Aug 12, 2019 at 2:00 PM Yosry Muhammad <yosry...@gmail.com> wrote:
> Hi Aditya,
>
> On Mon, Aug 12, 2019 at 9:04 AM Aditya Toshniwal <
> aditya.toshni...@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.
>> [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"
