Re: [GSoC] Query History Upgrade

[Yosry Muhammad]

Thanks all for the thorough review.

On Fri, Aug 16, 2019, 1:48 PM Akshay Joshi 
wrote:

> Thanks patch applied.
>
> On Wed, Aug 14, 2019 at 7:24 PM Yosry Muhammad  wrote:
>
>> Hi,
>>
>> On Wed, Aug 14, 2019 at 3:50 PM Yosry Muhammad 
>> wrote:
>>
>>> Please find an updated patch with the mentioned issue fixed. I am sorry
>>> you spent so much time reviewing this and finding bugs.
>>>
>>> On Wed, Aug 14, 2019 at 2:34 PM Akshay Joshi <
>>> akshay.jo...@enterprisedb.com> wrote:
>>>
 Hi Yosry

 I am still facing following issues:

- No icons for already saved(before applying your patch) query in
query history. Is this expected?

 I am sorry I missed this, yes this is expected as there is no way to
>> identify internally generated queries before the patch.
>> Thanks.
>>
>
>
> --
> *Thanks & Regards*
> *Akshay Joshi*
>
> *Sr. Software Architect*
> *EnterpriseDB Software India Private Limited*
> *Mobile: +91 976-788-8246*
>

Re: [GSoC] Query History Upgrade

[Akshay Joshi]

Thanks patch applied.

On Wed, Aug 14, 2019 at 7:24 PM Yosry Muhammad  wrote:

> Hi,
>
> On Wed, Aug 14, 2019 at 3:50 PM Yosry Muhammad  wrote:
>
>> Please find an updated patch with the mentioned issue fixed. I am sorry
>> you spent so much time reviewing this and finding bugs.
>>
>> On Wed, Aug 14, 2019 at 2:34 PM Akshay Joshi <
>> akshay.jo...@enterprisedb.com> wrote:
>>
>>> Hi Yosry
>>>
>>> I am still facing following issues:
>>>
>>>- No icons for already saved(before applying your patch) query in
>>>query history. Is this expected?
>>>
>>> I am sorry I missed this, yes this is expected as there is no way to
> identify internally generated queries before the patch.
> Thanks.
>


-- 
*Thanks & Regards*
*Akshay Joshi*

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

Re: [GSoC] Query History Upgrade

[Yosry Muhammad]

Hi,

On Wed, Aug 14, 2019 at 3:50 PM Yosry Muhammad  wrote:

> Please find an updated patch with the mentioned issue fixed. I am sorry
> you spent so much time reviewing this and finding bugs.
>
> On Wed, Aug 14, 2019 at 2:34 PM Akshay Joshi <
> akshay.jo...@enterprisedb.com> wrote:
>
>> Hi Yosry
>>
>> I am still facing following issues:
>>
>>- No icons for already saved(before applying your patch) query in
>>query history. Is this expected?
>>
>> I am sorry I missed this, yes this is expected as there is no way to
identify internally generated queries before the patch.
Thanks.

Re: [GSoC] Query History Upgrade

[Akshay Joshi]

Hi Yosry

I am still facing following issues:

   - No icons for already saved(before applying your patch) query in query
   history. Is this expected?
   - Internal queries are visible even if toggle switch is set to "No".
   Following are the steps to reproduce:
  - Create a table with primary key.
  - Open query tool and run "SELECT * FROM ".
  - Clear all old query history from query tool.
  - Make sure toggle switch for show internal query is set to "*Yes*".
  - Insert first record, query history will show "BEGIN" ,"INSERT" and
  COMMIT queries.
  - Set toggle switch to "*No*" and insert second record. Second record
  should not be visible as switch set to "*No*".
  - Now try switching toggle, when it is set to "Yes" it will show
  queries for first record and when it is set to "No" it will show queries
  for second record.


On Wed, Aug 14, 2019 at 4:47 PM Yosry Muhammad  wrote:

> Please find an updated patch attached with the aforementioned issues
> fixed.
>
> On Wed, Aug 14, 2019 at 12:33 PM Akshay Joshi <
> akshay.jo...@enterprisedb.com> wrote:
>
>>
>>
>> On Wed, Aug 14, 2019 at 3:53 PM Yosry Muhammad 
>> wrote:
>>
>>> Hi,
>>>
>>> On Wed, Aug 14, 2019 at 12:03 PM Akshay Joshi <
>>> akshay.jo...@enterprisedb.com> wrote:
>>>
 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]

 These problems did not occur when I checked the patch. I should have
>>> re-checked everything after merging with master, my bad. Sorry about that.
>>> Working on it.
>>>

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

 I am sorry I don't understand. Do you want me to make it toggle if the
>>> label is clicked as well?
>>>
>>
>> Currently it is happening. Queries should not be show/hide when click
>> on label.
>>
>>>
- For View/Edit Data it seems to take all the queries as internal,
is this expected ?

 Yes, as all queries in View/Edit Data are generated internally by
>>> pgAdmin.
>>>
>>> I am working on providing an updated patch ASAP.
>>>
>>
>>
>> --
>> *Thanks & Regards*
>> *Akshay Joshi*
>>
>> *Sr. Software Architect*
>> *EnterpriseDB Software India Private Limited*
>> *Mobile: +91 976-788-8246*
>>
>
>
> --
> *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*

Re: [GSoC] Query History Upgrade

[Akshay Joshi]

On Wed, Aug 14, 2019 at 3:53 PM Yosry Muhammad  wrote:

> Hi,
>
> On Wed, Aug 14, 2019 at 12:03 PM Akshay Joshi <
> akshay.jo...@enterprisedb.com> wrote:
>
>> 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]
>>
>> These problems did not occur when I checked the patch. I should have
> re-checked everything after merging with master, my bad. Sorry about that.
> Working on it.
>
>>
>>-
>>- 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.
>>
>> I am sorry I don't understand. Do you want me to make it toggle if the
> label is clicked as well?
>

Currently it is happening. Queries should not be show/hide when click
on label.

>
>>- For View/Edit Data it seems to take all the queries as internal, is
>>this expected ?
>>
>> Yes, as all queries in View/Edit Data are generated internally by pgAdmin.
>
> I am working on providing an updated patch ASAP.
>


-- 
*Thanks & Regards*
*Akshay Joshi*

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

Re: [GSoC] Query History Upgrade

[Yosry Muhammad]

Hi,

On Wed, Aug 14, 2019 at 12:03 PM Akshay Joshi 
wrote:

> 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]
>
> These problems did not occur when I checked the patch. I should have
re-checked everything after merging with master, my bad. Sorry about that.
Working on it.

>
>-
>- 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.
>
> I am sorry I don't understand. Do you want me to make it toggle if the
label is clicked as well?

>
>- For View/Edit Data it seems to take all the queries as internal, is
>this expected ?
>
> Yes, as all queries in View/Edit Data are generated internally by pgAdmin.

I am working on providing an updated patch ASAP.

Re: [GSoC] Query History Upgrade

[Akshay Joshi]

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  has been created
to track it.

On Wed, Aug 14, 2019 at 1:39 PM Yosry Muhammad  wrote:

> Great! thanks everyone.
>
> On Wed, Aug 14, 2019, 10:08 AM Akshay Joshi 
> wrote:
>
>> Hi Yosry
>>
>>
>> On Tue, Aug 13, 2019 at 6:33 PM Yosry Muhammad 
>> wrote:
>>
>>> Any more suggestions or comments on the patch?
>>>
>>
>> I am reviewing it.
>>
>>>
>>> On Tue, Aug 13, 2019 at 2:55 PM Aditya Toshniwal <
>>> aditya.toshni...@enterprisedb.com> wrote:
>>>


 On Tue, Aug 13, 2019 at 5:35 PM Yosry Muhammad 
 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 
> wrote:
>
>> Is it always the same error when it fails?
>>
>> On Tue, Aug 13, 2019 at 1:50 PM Aditya Toshniwal <
>> aditya.toshni...@enterprisedb.com> wrote:
>>
>>> The test cases fails intermittently. It passes sometimes.
>>>
>>> On Tue, Aug 13, 2019 at 5:15 PM Yosry Muhammad 
>>> 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 <
 aditya.toshni...@enterprisedb.com> 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 
> 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 <
>> aditya.toshni...@enterprisedb.com> wrote:
>>
>>> 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:
>>>

Re: [GSoC] Query History Upgrade

[Yosry Muhammad]

Great! thanks everyone.

On Wed, Aug 14, 2019, 10:08 AM Akshay Joshi 
wrote:

> Hi Yosry
>
>
> On Tue, Aug 13, 2019 at 6:33 PM Yosry Muhammad  wrote:
>
>> Any more suggestions or comments on the patch?
>>
>
> I am reviewing it.
>
>>
>> On Tue, Aug 13, 2019 at 2:55 PM Aditya Toshniwal <
>> aditya.toshni...@enterprisedb.com> wrote:
>>
>>>
>>>
>>> On Tue, Aug 13, 2019 at 5:35 PM Yosry Muhammad 
>>> 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 
 wrote:

> Is it always the same error when it fails?
>
> On Tue, Aug 13, 2019 at 1:50 PM Aditya Toshniwal <
> aditya.toshni...@enterprisedb.com> wrote:
>
>> The test cases fails intermittently. It passes sometimes.
>>
>> On Tue, Aug 13, 2019 at 5:15 PM Yosry Muhammad 
>> 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 <
>>> aditya.toshni...@enterprisedb.com> 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 
 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 <
> aditya.toshni...@enterprisedb.com> wrote:
>
>> 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('>>> class="query-history-icon sql-icon-lg">');
 $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();
 });

Re: [GSoC] Query History Upgrade

[Akshay Joshi]

Hi Yosry


On Tue, Aug 13, 2019 at 6:33 PM Yosry Muhammad  wrote:

> Any more suggestions or comments on the patch?
>

I am reviewing it.

>
> On Tue, Aug 13, 2019 at 2:55 PM Aditya Toshniwal <
> aditya.toshni...@enterprisedb.com> wrote:
>
>>
>>
>> On Tue, Aug 13, 2019 at 5:35 PM Yosry Muhammad 
>> 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 
>>> wrote:
>>>
 Is it always the same error when it fails?

 On Tue, Aug 13, 2019 at 1:50 PM Aditya Toshniwal <
 aditya.toshni...@enterprisedb.com> wrote:

> The test cases fails intermittently. It passes sometimes.
>
> On Tue, Aug 13, 2019 at 5:15 PM Yosry Muhammad 
> 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 <
>> aditya.toshni...@enterprisedb.com> 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 
>>> 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 <
 aditya.toshni...@enterprisedb.com> wrote:

> Hi Yosry,
>
>
> On Mon, Aug 12, 2019 at 2:00 PM Yosry Muhammad 
> 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('>> class="query-history-icon sql-icon-lg">');
>>> $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.
>>
>

Re: [GSoC] Query History Upgrade

[Yosry Muhammad]

Any more suggestions or comments on the patch?

On Tue, Aug 13, 2019 at 2:55 PM Aditya Toshniwal <
aditya.toshni...@enterprisedb.com> wrote:

>
>
> On Tue, Aug 13, 2019 at 5:35 PM Yosry Muhammad  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 
>> wrote:
>>
>>> Is it always the same error when it fails?
>>>
>>> On Tue, Aug 13, 2019 at 1:50 PM Aditya Toshniwal <
>>> aditya.toshni...@enterprisedb.com> wrote:
>>>
 The test cases fails intermittently. It passes sometimes.

 On Tue, Aug 13, 2019 at 5:15 PM Yosry Muhammad 
 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 <
> aditya.toshni...@enterprisedb.com> 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 
>> 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 <
>>> aditya.toshni...@enterprisedb.com> wrote:
>>>
 Hi Yosry,


 On Mon, Aug 12, 2019 at 2:00 PM Yosry Muhammad 
 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('> class="query-history-icon sql-icon-lg">');
>> $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

Re: [GSoC] Query History Upgrade

[Aditya Toshniwal]

On Tue, Aug 13, 2019 at 5:35 PM Yosry Muhammad  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  wrote:
>
>> Is it always the same error when it fails?
>>
>> On Tue, Aug 13, 2019 at 1:50 PM Aditya Toshniwal <
>> aditya.toshni...@enterprisedb.com> wrote:
>>
>>> The test cases fails intermittently. It passes sometimes.
>>>
>>> On Tue, Aug 13, 2019 at 5:15 PM Yosry Muhammad 
>>> 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 <
 aditya.toshni...@enterprisedb.com> 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 
> 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 <
>> aditya.toshni...@enterprisedb.com> wrote:
>>
>>> Hi Yosry,
>>>
>>>
>>> On Mon, Aug 12, 2019 at 2:00 PM Yosry Muhammad 
>>> 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(' class="query-history-icon sql-icon-lg">');
> $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 c

Re: [GSoC] Query History Upgrade

[Yosry Muhammad]

Is it always the same error when it fails?

On Tue, Aug 13, 2019 at 1:50 PM Aditya Toshniwal <
aditya.toshni...@enterprisedb.com> wrote:

> The test cases fails intermittently. It passes sometimes.
>
> On Tue, Aug 13, 2019 at 5:15 PM Yosry Muhammad  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 <
>> aditya.toshni...@enterprisedb.com> 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 
>>> 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 <
 aditya.toshni...@enterprisedb.com> wrote:

> Hi Yosry,
>
>
> On Mon, Aug 12, 2019 at 2:00 PM Yosry Muhammad 
> 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('>> class="query-history-icon sql-icon-lg">');
>>> $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
>  

Re: [GSoC] Query History Upgrade

[Aditya Toshniwal]

The test cases fails intermittently. It passes sometimes.

On Tue, Aug 13, 2019 at 5:15 PM Yosry Muhammad  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 <
> aditya.toshni...@enterprisedb.com> 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 
>> 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 <
>>> aditya.toshni...@enterprisedb.com> wrote:
>>>
 Hi Yosry,


 On Mon, Aug 12, 2019 at 2:00 PM Yosry Muhammad 
 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('> class="query-history-icon sql-icon-lg">');
>> $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)
   Fil

Re: [GSoC] Query History Upgrade

[Yosry Muhammad]

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 <
aditya.toshni...@enterprisedb.com> 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  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 <
>> aditya.toshni...@enterprisedb.com> wrote:
>>
>>> Hi Yosry,
>>>
>>>
>>> On Mon, Aug 12, 2019 at 2:00 PM Yosry Muhammad 
>>> 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(' class="query-history-icon sql-icon-lg">');
> $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/pgadmi

Re: [GSoC] Query History Upgrade

[Aditya Toshniwal]

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  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 <
> aditya.toshni...@enterprisedb.com> wrote:
>
>> Hi Yosry,
>>
>>
>> On Mon, Aug 12, 2019 at 2:00 PM Yosry Muhammad 
>> 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('>>> class="query-history-icon sql-icon-lg">');
 $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/
>>>
>>
>

Re: [GSoC] Query History Upgrade

[Aditya Toshniwal]

Hi Yosry,


On Mon, Aug 12, 2019 at 2:00 PM Yosry Muhammad  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('> class="query-history-icon sql-icon-lg">');
>> $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"

Re: [GSoC] Query History Upgrade

[Aditya Toshniwal]

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('');
$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();
});
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]


On Sun, Aug 11, 2019 at 11:58 AM Yosry Muhammad  wrote:

> Hi Hackers,
>
> Please find attached a patch including new features for the Query History
> as follows:
>
> - Integration with updatable resultsets: Queries generated by pgAdmin
> during Save Data operations are now shown in the history panel, including
> transaction control queries.
> - Source indication: The source of each query in the history panel is
> indicated using an icon corresponding to the toolbar (Save Data, Execute,
> Explain, Commit, Rollback & View/Edit).
> - Allow showing/hiding queries generated internally by pgAdmin using a
> toggle button.
> - Some refactoring in save_data_changes.py
>
> The patch includes the code for the feature, added feature tests and
> documentation updates.
> Please review !
>
> Thanks. Regards.
>
> --
> *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"