[RM#3605] View Data: Deleting N number of rows makes first N number of rows disable

[Murtuza Zabuawala]

Hi,

PFA minor patch to fix the issue where using view data, when we try to
delete N row, it will make the first N row from the grid disabled, you try
to double click on the cell and try to update the value of that row it will
not open the editor.

*Steps:*
1) Create a simple table as shown in screenshot.
2) Open view data for the table
3) Select row number 6
4) Click on Trash (delete button)
5) Click on Save grid icon
6) Try to update the first row's column b value


--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


RM_3605.diff
Description: Binary data

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]

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

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]

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]

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

[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

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