Re: [pgAdmin][RM6131] Port query tool to React

Hi,
Attached is the patch to fix the issues raised. Few of them are pending and
will send it later.
Fixed:

1. Add New Server Connection > Server options keep loading(For empty
Server group).
2. After clicking indent/Unindent(for all operations) for large query
option left as it is till operation completes
3. Also check sign beside options in Execute Option/Copy Header is
little bit big
4. In explain > Analysis tab does not show ROWS column
5. In explain > Explain > analysis previous explain output is NOT
cleared. New rows are appended. Same applies to the statistics tab.
6. Update new query tool connection tool tip.(7289)
7. Explain-Analyze > Loops column is empty.
8. Explain-Analyze with Verbose & Costs > in ROW X columns upward arrows
are missing.
9. Explain-Analyze with all option checked > background colors are
missing for timing.
10. Explain-Analyze > Additional bullet is added before Hash Cond.
11. Browser Tree > Filtered rows icon is not working.
12. Create table with timestamp and default value as function now() >
Add new row > Enter mandatory columns except column where default value is
function(now()) > Click Save > New row added but column with default value
has value [default]. not updated to actual value. / Default values are not
considered for any column while adding a new entry.
13. Disable execute options in View/Edit data.
14. The Boolean column always shows null.
15. In Query history Remove & Remove all buttons are stuck to each other.
16. On Remove all, the right panel is empty.
17. Create a column with boolean[]/ text[], Try to add a new entry from
data grid, enter “” quotes > Click Ok > Now try edit cell > You can not
change value.
18. In query history - Select queries are suffixed by ’Save Data’ icon
19. Edit any table with PK > Try to insert duplicate PK > Error thrown >
Correct pK value > Still old error shown > Not able to add new entry (This
works when focus is moved from edited cell)
20. Clicking arrows after opening dropdown options, does not collapse
dropdown.

I was not able to reproduce some of the bugs on webpack dev mode, but
reproducible on webpack prod mode bundles. After a lot of debugging it
turned out that webpack/babel transpile was changing the meaning of a piece
of code in prod mode. I tweaked the code then most issues were not
reproducible anymore.
That said, following issues were not reproducible and this fix could be the
reason:

1. Not able to load more than 1000 rows.
2. Find/Replace both opens the same dialogue box.
3. Try to edit cell with char varying data type(which opens text editor)
> Scroll result grid > Click on another cell > query edior shows white
screen & refresh is the only option left.(TypeError: Cannot read properties
of null (reading 'querySelector')
at getCellElement (sqleditor.js?ver=60800:1:995456))
4. Generate script is not working for schema diff for tables with target
only/ not working for any.(TypeError: Cannot read properties of undefined
(reading 'database'))
5. Query results are appended in the Notification tab.
6. Panel name is NOT updated on opening file. Panel-name should be
filename
7. Open a file in query tool > Open another file > Check panel name > It
is the first file name.
8. Incorrect CSV downloaded (film table) when CSV quotes select single
quote from preferences. *CSV generated at backend. No changes done.*
9. In Data grid > Add New data to cell > without clicking on other cell
click on Add New row > previous data is gone.

Please find the comment inline for other issues:

1. Small white line is added below Total rows status bar.
This is an existing issue with wcDocker. It is somehow not getting the
correct size for the query tool. You can verify this by opening query
tool in new tab.
2. In explain > Data output > Query Plan is editable.
To be precise, the JSON editor is editable but does not allow saving it.
This is inline with other editors like text editor which allows editing but
no save.
3. Color is NOT fainted in View/Edit data when query tool is NOT
editable.
This is inline with other places where the SQL is read only like the
properties dialog SQL tab or the RE-SQL tab. Plus, greying out of SQL
affects query readability.
4. If data in result grid is edited & changes are reverted, then also
Save button remain enabled/ Cell is shown in bold indicating data is edited.
This is based on existing behaviour. A separate RM can be raised to have
any improvement in this.
5. When the Save button is disabled then 'Save as' should be disabled as
well.
Save and Save As are different in behaviour. You can change a file and
save it. The save will be disabled but the user should be allowed. I also
checked the behaviour of VS-Code and PyCharm. They never disable the "save
as" button. After all, there is no harm in allowing a user to save as even
if it is empty.
6. Manage Macros - Help button is disabled. Remove SQL help button(Not
sure).
As I already mentioned in the review by Askhay, the existing help button
opens the query tool help. Query tool help is already added on the
toolbar and so this one is disabled.
7. Macros defined in one database are shown for other databases
also/even across servers.
As per the existing design.
8. Query tool notifier setting is missing in preferences.
Previously, the total time and number of rows were shown in the
notifier. And so, the notifier setting was added so that users can tweak it
to keep it open for a longer time. Now, we do not show those details on the
notifier since we have a fixed status bar for that. This setting is not
relevant anymore.

Issues that need to be checked and pending:

1. Explain-Analyze with all option checked > Statistics tab > % of query
is always 0 for node type. Need to check all the calculations.
2. In the Result grid multiple rows can not be selected with shift +
down arrow.
3. In Geometry Viewer , map disappears if taken to bottom.
4. Keyboard shortcut - Focus in query tool and try Previous/Next tab is
Not working add quotes in query tool
5. Keyboard shortcut Switch Panel is not working

On Thu, Apr 7, 2022 at 3:37 PM Aditya Toshniwal <
aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:

> Hi,
>
> Please find an updated patch with PEP8 issues fixed.
>
> On Thu, Apr 7, 2022 at 3:12 PM Aditya Toshniwal <
> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>
>> Hi Hackers,
>>
>> Attached is updated patch which now also includes:
>> Can't copy and paste row correctly if first column contains no data #7294
>>
>> On Tue, Apr 5, 2022 at 5:45 PM Aditya Toshniwal <
>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Akshay,
>>>
>>> Thank you for doing such a detailed review. Please find my comments
>>> inline below and attached patch.
>>>
>>> On Thu, Mar 17, 2022 at 4:05 PM Akshay Joshi <
>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi Aditya
>>>>
>>>> Following are the review comments:
>>>>
>>>> *GUI:*
>>>>
>>>> - The Maximize/Minimize button on the panel should be consistent
>>>> with other panels.
>>>>
>>>> Fixed.
>>>
>>>>
>>>> - Scratch Pad is missing or is there any new setting to add a
>>>> scratch pad?
>>>>
>>>> Added. The layout lib currently does not have a context menu on the
>>> header to add a panel. For now, you can use the reset layout button to add
>>> the scratch pad again if closed. Context menu can be added separately
>>> later. Reset layout will not refresh the page.
>>>
>>>>
>>>> - Press "Cmd + G" on Query Tool it opens the old search bar is it
>>>> still valid
>>>> - [image: Screenshot 2022-03-16 at 7.02.34 PM.png]
>>>>
>>>> Fixed.
>>>
>>>>
>>>> -
>>>> - In case of any error, we should move the cursor to the error
>>>> location. We are highlighting the error row but it should be scroll to that
>>>> location in the editor.
>>>>
>>>> Existing behaviour. Improvement done.
>>>
>>>>
>>>> - Error highlighting color should be aligned with the theme, check
>>>> the existing color.
>>>>
>>>> Fixed.
>>>
>>>>
>>>> - Error highlighting color is not cleared when running the
>>>> successful query. Run "SELECT * from pg_class123" and then run "SELECT *
>>>> from pg_class".
>>>>
>>>> Fixed.
>>>
>>>>
>>>> - *ToolBar buttons*:
>>>> - When the Save button is disabled then 'Save as' should be
>>>> disabled as well.
>>>>
>>>> This is not correct. A user should be allowed to "Save as" a file even
>>> if it is saved and save button is disabled.
>>>
>>>>
>>>> - Open any SQL file, change some text and click on the 'Save'
>>>> button, No notifier message has been flashed that 'File saved successfully'
>>>> and the button does not get disabled as well.
>>>>
>>>> Fixed.
>>>
>>>>
>>>> - Format SQL not working.
>>>>
>>>> Fixed.
>>>
>>>>
>>>> - Clear Query keyboard shortcut not working.
>>>>
>>>> Fixed.
>>>
>>>>
>>>> -
>>>> - Clicking on the 'Clear Query' menu should pop up a
>>>> confirmation dialog 'Are you sure you wish to discard the current changes?'
>>>>
>>>> Fixed.
>>>
>>>>
>>>> - Open any SQL file, change some text, and try to open another
>>>> file, it should pop up a confirmation dialog 'Are you sure you wish to
>>>> discard the current changes?'
>>>>
>>>> Fixed.
>>>
>>>>
>>>> - When clicking on the New Query Tool button it is not opening the
>>>> new query tool window.
>>>>
>>>> Fixed.
>>>
>>>>
>>>> - Shortcut Key for Replace is not correct on *Windows* (Tooltip
>>>> showing Alt + Ctrl + F) but actual is (Shift + Ctrl + F)
>>>>
>>>> Fixed.
>>>
>>>>
>>>> - Most of the Keyboard shortcuts are not working on Windows at all.
>>>>
>>>> Fixed.
>>>
>>>>
>>>> - *New Connection Dialog:*
>>>> - The close button should be right-aligned.
>>>>
>>>> Fixed.
>>>
>>>>
>>>> - Unable to test further because when selecting any disconnected
>>>> server it should pop up the password dialog to connect and then fetch the
>>>> details like databases, users, roles.
>>>>
>>>> Fixed.
>>>
>>>>
>>>> - Help buttons are disabled.
>>>>
>>>> The existing help button opens the query tool help. Query tool help is
>>> already added on the toolbar.
>>>
>>>>
>>>> - *Query History*:
>>>> - For some queries like 'ROLLBACK' and 'COMMIT,' Rows affected
>>>> shows in the negative (-1).
>>>>
>>>> Based on existing. I have made it blank.
>>>
>>>>
>>>> - *Remove All* should warn the user before removing everything "Are
>>>> you sure you wish to remove all the history? This will remove all of your
>>>> query histories from this and other sessions for this database."
>>>>
>>>> Fixed.
>>>
>>>>
>>>> - Duration is missing. It should be there with the Date and Rows
>>>> affected.
>>>>
>>>> Fixed.
>>>
>>>>
>>>> - *Status Bar*:
>>>> - We should display Milliseconds as well in Query Complete.
>>>>
>>>> It will now display in <hr>:<min>:<sec>.<msec> format. Fixed.
>>>
>>>>
>>>> - Total Rows: *N of N, *I always observe the same, can you please
>>>> change it to *N *only, Or am I missing some scenario where it is
>>>> changed?
>>>>
>>>> We're fetching rows on demand. "X of N" says total X rows fetched till
>>> now but total N are available.
>>>
>>>>
>>>> - *Data output*:
>>>> - Default message "No data output......" should be shown when
>>>> there are no rows/data, check the old behavior.
>>>>
>>>> Fixed.
>>>
>>>>
>>>> - Extra space in the JSON Editor, even if I resize it, its height
>>>> is not adjusted. Check the second screenshot
>>>> - [image: Screenshot 2022-03-17 at 11.29.21 AM.png] [image:
>>>> Screenshot 2022-03-17 at 11.29.59 AM.png]
>>>>
>>>> Fixed.
>>>
>>>>
>>>> -
>>>> - *Explain*:
>>>> - Old content should be cleared from the panel if we run the new
>>>> query by clicking the play button. It should show an informative message,
>>>> check the old behavior.
>>>>
>>>> Fixed.
>>>
>>>>
>>>> - *Macros*:
>>>> - The close button should be right-aligned.
>>>>
>>>> Fixed.
>>>
>>>>
>>>> - Help buttons are disabled.
>>>>
>>>> The existing help button opens the query tool help. Query tool help is
>>> already added on the toolbar.
>>>
>>>>
>>>> - *View/Edit Data*:
>>>> - Clipboard issues.
>>>>
>>>> Fixed.
>>>
>>>>
>>>> - Unable to edit table data even if the primary key is defined.
>>>>
>>>> Fixed.
>>>
>>>>
>>>> - Select All rows is missing at the left corner of the 'Data
>>>> Output' Panel.
>>>>
>>>> Fixed.
>>>
>>>>
>>>> - Change some data and try to save if an error comes then Spinner
>>>> is not cleared.
>>>>
>>>> Fixed.
>>>
>>>>
>>>> - Filtered Rows not working. Click on the 'Filtered Rows ...'
>>>> context menu.
>>>>
>>>> Fixed.
>>>
>>>>
>>>> - Sort/Filter Dialog is missing.
>>>>
>>>> The sort/filter dialog is present and opens when you click the filter
>>> button. I have removed the "Sort/Filter" menu item which does the same
>>> thing.
>>>
>>>>
>>>> - Select any cell of the table content, click on any filter menu
>>>> 'Filter by selection' or 'Exclude by selection' multiple times. It updates
>>>> the query every time and adds the condition which is not there previously.
>>>>
>>>> Fixed.
>>>
>>>>
>>>> - Limit (100 rows, 500 rows ...) not working. View data of any
>>>> table having 1000+ rows and then apply the limit.
>>>>
>>>> I am not able to reproduce this. Works fine.
>>>
>>>>
>>>> - *Themes*:
>>>> - Check and Fix all the issues related to the theme.
>>>> - [image: Screenshot 2022-03-17 at 1.43.01 PM.png].
>>>> [image: Screenshot 2022-03-17 at 1.44.10 PM.png].
>>>> -
>>>> - [image: Screenshot 2022-03-17 at 1.44.53 PM.png]
>>>> [image: Screenshot 2022-03-17 at 1.45.54 PM.png]
>>>>
>>>> Fixed.
>>>
>>>>
>>>> -
>>>>
>>>>
>>>> *Code:*
>>>>
>>>> - Fix pep8 issues.
>>>>
>>>> Fixed.
>>>
>>>>
>>>> - Can we fix the *Deprecation Warnings?*
>>>>
>>>> I think those are coming from bootstrap. Need to check that separately.
>>> Not related to the query tool.
>>>
>>>> *Note: *Code review still remains, meanwhile you can start fixing the
>>>> above issues.
>>>>
>>>
>>> Forgot to mention in initial mail - I have removed dependency on
>>> Snap.svg and have written Explain SVG codes from scratch.
>>>
>>>>
>>>> On Wed, Mar 16, 2022 at 5:54 PM Aditya Toshniwal <
>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> Attached is an updated one with few more improvements and fixes.
>>>>>
>>>>> On Wed, Mar 16, 2022 at 1:40 PM Aditya Toshniwal <
>>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Please find the attached patch :)
>>>>>>
>>>>>> On Wed, Mar 16, 2022 at 12:16 PM Akshay Joshi <
>>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>> Hi Aditya
>>>>>>>
>>>>>>> I think you forgot to attach the patch.
>>>>>>>
>>>>>>> On Tue, Mar 15, 2022 at 4:00 PM Aditya Toshniwal <
>>>>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>>> Hi Hackers,
>>>>>>>>
>>>>>>>> Attached is the initial patch that migrates the SQL Editor tool to
>>>>>>>> React based. Change highlights:
>>>>>>>> 1. Complete rewrite to React code.
>>>>>>>> 2. UI improvements based on suggestions and requests.
>>>>>>>> 3. Work towards stability and performance improvement.
>>>>>>>> 4. Keep row numbers in view when scrolling horizontally. Fixes #3989
>>>>>>>> 5. Fixed status bar at the bottom with useful details. Fixes #3253
>>>>>>>> 6. Relocate GIS Viewer Button to the Left Side of Results Table.
>>>>>>>> Fixed #6830
>>>>>>>> 7. Allow to remove single history records. Refs #4113
>>>>>>>> 8. Macros usability improvements. Ref #6969
>>>>>>>> 9. Connection bar visibility issue. Fixes #7188
>>>>>>>> 10. Query tool layout issues. Fixes #6725
>>>>>>>>
>>>>>>>> Please note, there are still few minor niggles at some places but
>>>>>>>> the patch qualified to be reviewed. We will need a good amount of time to
>>>>>>>> test this properly. So, I am sending the feature patch. JS test
>>>>>>>> cases and documentation patches will follow soon.
>>>>>>>>
>>>>>>>> Please review.
>>>>>>>>
>>>>>>>> [image: image.png]
>>>>>>>> --
>>>>>>>> Thanks,
>>>>>>>> Aditya Toshniwal
>>>>>>>> pgAdmin Hacker | Software Architect | *edbpostgres.com*
>>>>>>>> <http://edbpostgres.com>
>>>>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> *Thanks & Regards*
>>>>>>> *Akshay Joshi*
>>>>>>> *pgAdmin Hacker | Principal Software Architect*
>>>>>>> *EDB Postgres <http://edbpostgres.com>*
>>>>>>>
>>>>>>> *Mobile: +91 976-788-8246*
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Thanks,
>>>>>> Aditya Toshniwal
>>>>>> pgAdmin Hacker | Software Architect | *edbpostgres.com*
>>>>>> <http://edbpostgres.com>
>>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Thanks,
>>>>> Aditya Toshniwal
>>>>> pgAdmin Hacker | Software Architect | *edbpostgres.com*
>>>>> <http://edbpostgres.com>
>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>
>>>>
>>>>
>>>> --
>>>> *Thanks & Regards*
>>>> *Akshay Joshi*
>>>> *pgAdmin Hacker | Principal Software Architect*
>>>> *EDB Postgres <http://edbpostgres.com>*
>>>>
>>>> *Mobile: +91 976-788-8246*
>>>>
>>>
>>>
>>> --
>>> Thanks,
>>> Aditya Toshniwal
>>> pgAdmin Hacker | Software Architect | *edbpostgres.com*
>>> <http://edbpostgres.com>
>>> "Don't Complain about Heat, Plant a TREE"
>>>
>>
>>
>> --
>> Thanks,
>> Aditya Toshniwal
>> pgAdmin Hacker | Software Architect | *edbpostgres.com*
>> <http://edbpostgres.com>
>> "Don't Complain about Heat, Plant a TREE"
>>
>
>
> --
> Thanks,
> Aditya Toshniwal
> pgAdmin Hacker | Software Architect | *edbpostgres.com*
> <http://edbpostgres.com>
> "Don't Complain about Heat, Plant a TREE"
>

--
Thanks,
Aditya Toshniwal
pgAdmin Hacker | Software Architect | *edbpostgres.com*
<http://edbpostgres.com>
"Don't Complain about Heat, Plant a TREE"