Hi Apologies for the delay - I've finished travelling now.
I found a few issues, mostly minor: - The patch won't apply with "git apply" and only partially applies with patch -p0. Please "git add" all your changes and new files in your repo, and then run "git diff --cached --binary", which should create a usable patch. You can then un-stage your changes. - execute_query_utils.py is somewhat lacking in file header and any comments. - There is commented-out code in sqleditor.js - "committed" is miss-spelt in a number of places (2 m's, 2 t's) - On reflection, I don't think the "Data saved successfully, you still need to commit changes to the database." is prominent enough - in testing, I found it very easy to miss. That might also be compounded by the fact that "Alert on uncommitted transactions?" doesn't seem to be working for me. I get the "Save text" prompt to save the query text, but if I say no to that, the Query Tool instance closes with no further warning, despite having a transaction in progress. All in all, it's looking very good though. On Wed, Jul 3, 2019 at 5:22 PM Yosry Muhammad <yosry...@gmail.com> wrote: > I updated the patch to be in sync with recent commits made (since I sent > the patch) and fix merge conflicts. > > Please find the new patch attached. Waiting for review. > > Thanks. > > On Mon, Jul 1, 2019 at 9:19 PM Yosry Muhammad <yosry...@gmail.com> wrote: > >> Dear all, >> >> This is a final version of the patch I have been working on since the >> beginning of the GSoC project with tests and documentation. >> >> This patch includes: >> >> Features: >> - Implementeing the first version detecting updatable resultsets. Saving >> edited data works the same way as View Data mode (with small changes). A >> resultset is updatable if: >> - All columns belong to the same table. >> - All primary keys are selected. >> - No duplicate columns. >> >> - Adding the new Save Data icon and its shortcut in preferences. The old >> Save icon is used exclusively for Saving files now. >> - Integrating saving data changes into the ongoing transaction (if any). >> The user is notified that they need to commit the changes if auto-commit is >> off. >> - A failed save of data changes rolls back the data changes only, does >> not rollback previous queries in the same transaction. >> - Alerting the user when Execute/Explain is clicked with unsaved changes >> in the grid. >> - Modified/New cells are now highlighted. >> - Alerting the user when exiting with uncommited transactions. >> - Re-implementing the on tab close event of the query tool as multiple >> dialogs may be required for: unsaved data changes, unsaved file changes & >> uncommited transactions (they can all be enabled/disabled in preferences). >> - Hiding queries generated by pgAdmin from query history (until they are >> substituted by a mogrified version and have a checkbox added to >> enable/disable them). >> >> Bug fixes: >> - Fixed a bug where exit on save would exit even if the save was not >> successful. >> - Fixed a bug where alertify confirm dialogs had midword break wrapping. >> >> Tests: >> - Python tests: >> - test_is_query_resultset_updatable: Tests that updatable resultsets >> are detected correctly. >> - test_save_changed_data: Tests that additions, deletions & updates >> are performed correctly on updatable resultsets. >> - Feature tests: >> - query_tool_journey_test (existing - extended): Test that when the >> query resultset is updatable the user can modify cells and add new rows >> (and vice versa). >> - Updated other feature tests to match the new icon. >> - JS tests: >> - Updated call_render_after_poll_specs.js & >> keyboard_shortcuts_specs.js to test updates in related parts of the code. >> >> I could not add JS tests to test that the new Save Data button is enabled >> when the user edits a cell as I cannot mimic the actions of editing the >> grid. However, the new button is now used in View/Edit data feature tests >> and it works correctly. >> I also could not add JS tests to check that when the resultset is >> updatable the grid should be editable as the current code in sqleditor.js >> is not testable and it will required a lot of refactoring of this file, but >> again, this is covered by feature tests. >> >> Documentation: >> - Updated editgrid.rst, query_tool.rst, query_tool_shortcuts.rst, >> preferences.rst & keyboard_shortcuts.rst to match the new changes. >> - Updated the following screenshots: query_output_data.png, >> query_tool.png & query_toolbar.png >> >> The patch passes all tests performed by "make check" command. >> >> Please review, looking forward to any comments or feedback. >> >> 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/ >> > > > -- > *Yosry Muhammad Yosry* > > Computer Engineering student, > The Faculty of Engineering, > Cairo University (2021). > Class representative of CMP 2021. > https://www.linkedin.com/in/yosrym93/ > -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
