Re: [GSoC] Query History Upgrade

Hi Yosry

I have found following issues:

   - Jasmine test cases are failing.
   - Browser error when applying the patch and open the query tool.
   Previously saved queries are there:
   - [image: Screenshot 2019-08-14 at 3.21.21 PM.png]
   - Toggle Switch should have text "Yes/No" as we already have label "Show
   queries generated internally by pgAdmin?"
   - Show/Hide query works on click of label as well.
   - For View/Edit Data it seems to take all the queries as internal, is
   this expected ?

Feature #4612 <https://redmine.postgresql.org/issues/4612> has been created
to track it.

On Wed, Aug 14, 2019 at 1:39 PM Yosry Muhammad <[email protected]> wrote:

> Great! thanks everyone.
>
> On Wed, Aug 14, 2019, 10:08 AM Akshay Joshi <[email protected]>
> wrote:
>
>> Hi Yosry
>>
>>
>> On Tue, Aug 13, 2019 at 6:33 PM Yosry Muhammad <[email protected]>
>> wrote:
>>
>>> Any more suggestions or comments on the patch?
>>>
>>
>>     I am reviewing it.
>>
>>>
>>> On Tue, Aug 13, 2019 at 2:55 PM Aditya Toshniwal <
>>> [email protected]> wrote:
>>>
>>>>
>>>>
>>>> On Tue, Aug 13, 2019 at 5:35 PM Yosry Muhammad <[email protected]>
>>>> wrote:
>>>>
>>>>> Please find attached an updated patch with a small modification to the
>>>>> feature test. This makes sure the first history element is selected and
>>>>> focused before trying to move through entries using the down arrow key.
>>>>> When the test fails, is it always the same error you sent before ?
>>>>>
>>>> Seems to be working now.  And yes, I got the same error every time it
>>>> had failed.
>>>>
>>>>>
>>>>> On Tue, Aug 13, 2019 at 1:51 PM Yosry Muhammad <[email protected]>
>>>>> wrote:
>>>>>
>>>>>> Is it always the same error when it fails?
>>>>>>
>>>>>> On Tue, Aug 13, 2019 at 1:50 PM Aditya Toshniwal <
>>>>>> [email protected]> wrote:
>>>>>>
>>>>>>> The test cases fails intermittently. It passes sometimes.
>>>>>>>
>>>>>>> On Tue, Aug 13, 2019 at 5:15 PM Yosry Muhammad <[email protected]>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Hi Aditya,
>>>>>>>>
>>>>>>>> The test passes on my computer, is there anything I could try to
>>>>>>>> reproduced the failure?
>>>>>>>>
>>>>>>>> By looking at the error, I suspect clicking the down arrow key on
>>>>>>>> your machine did not move to the next history entry during the test. Does
>>>>>>>> clicking the down arrow normally go to the next history entry on your
>>>>>>>> machine?
>>>>>>>>
>>>>>>>> On Tue, Aug 13, 2019 at 1:26 PM Aditya Toshniwal <
>>>>>>>> [email protected]> wrote:
>>>>>>>>
>>>>>>>>> Hi Yosry,
>>>>>>>>>
>>>>>>>>> Everything looks fine to me. Except intermediate feature test
>>>>>>>>> failure. May be @committers can try on their machine.
>>>>>>>>>
>>>>>>>>> ======================================================================
>>>>>>>>> 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 78, in runTest
>>>>>>>>>     self._test_query_sources_and_generated_queries()
>>>>>>>>>   File
>>>>>>>>> "/Users/adityatoshniwal/projects/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
>>>>>>>>> line 186, in _test_query_sources_and_generated_queries
>>>>>>>>>     self._test_history_query_sources()
>>>>>>>>>   File
>>>>>>>>> "/Users/adityatoshniwal/projects/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
>>>>>>>>> line 215, in _test_history_query_sources
>>>>>>>>>     history_entries_icons)
>>>>>>>>>   File
>>>>>>>>> "/Users/adityatoshniwal/projects/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
>>>>>>>>> line 287, in _check_history_queries_and_icons
>>>>>>>>>     self.assertIn(query, query_history_selected_item.text)
>>>>>>>>> AssertionError: "UPDATE public.test_editable_table2293 SET
>>>>>>>>> normal_column = '10'::numeric WHERE pk_column = '1';" not found in
>>>>>>>>> 'COMMIT;\n15:00:40'
>>>>>>>>>
>>>>>>>>> On Tue, Aug 13, 2019 at 5:03 AM Yosry Muhammad <[email protected]>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>> 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/
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> 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/
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> 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/
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> *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/
>>>
>>
>>
>> --
>> *Thanks & Regards*
>> *Akshay Joshi*
>>
>> *Sr. Software Architect*
>> *EnterpriseDB Software India Private Limited*
>> *Mobile: +91 976-788-8246*
>>
>

-- 
*Thanks & Regards*
*Akshay Joshi*

*Sr. Software Architect*
*EnterpriseDB Software India Private Limited*
*Mobile: +91 976-788-8246*