pgAdmin 4 commit: Add support for editing of resultsets in the Query To
Add support for editing of resultsets in the Query Tool, if the data can be identified as updatable. Fixes #1760 When a query is run in the Query Tool, check if the source of the columns can be identified as being from a single table, and that we have all columns that make up the primary key. If so, consider the resultset to be editable and allow the user to edit data and add/remove rows in the grid. Changes to data are saved using SAVEPOINTs as part of any transaction that's in progress, and rolled back if there are integrity violations, without otherwise affecting the ongoing transaction. Implemented by Yosry Muhammad as a Google Summer of Code project. Branch -- master Details --- https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=710d520631b1b4649eb4a8d924e3141ff74af449 Author: Yosry Muhammad Modified Files -- docs/en_US/editgrid.rst| 8 +- docs/en_US/images/query_output_data.png| Bin 83700 -> 50204 bytes docs/en_US/images/query_tool.png | Bin 85347 -> 50204 bytes docs/en_US/images/query_toolbar.png| Bin 20230 -> 7820 bytes docs/en_US/keyboard_shortcuts.rst | 2 + docs/en_US/preferences.rst | 4 + docs/en_US/query_tool.rst | 28 +- docs/en_US/query_tool_toolbar.rst | 9 +- docs/en_US/release_notes_4_11.rst | 1 + web/pgadmin/feature_tests/file_manager_test.py | 3 +- web/pgadmin/feature_tests/locators.py | 2 +- .../feature_tests/query_tool_journey_test.py | 75 ++ web/pgadmin/feature_tests/view_data_dml_queries.py | 2 +- web/pgadmin/static/js/keyboard_shortcuts.js| 4 + .../static/js/sqleditor/call_render_after_poll.js | 3 +- web/pgadmin/static/js/sqleditor/execute_query.js | 12 + .../static/js/sqleditor/query_tool_actions.js | 5 + .../static/js/sqleditor/query_tool_preferences.js | 6 +- web/pgadmin/static/scss/_alertify.overrides.scss | 2 +- .../tools/datagrid/templates/datagrid/index.html | 10 +- web/pgadmin/tools/sqleditor/__init__.py| 80 ++- web/pgadmin/tools/sqleditor/command.py | 364 +++--- .../tools/sqleditor/static/css/sqleditor.css | 6 +- .../sqleditor/static/img/save_data_changes.svg | 12 + web/pgadmin/tools/sqleditor/static/js/sqleditor.js | 759 + .../tools/sqleditor/static/scss/_sqleditor.scss| 7 + .../sqleditor/sql/11_plus/primary_keys.sql | 2 +- .../sqleditor/sql/default/primary_keys.sql | 4 +- .../tools/sqleditor/tests/execute_query_utils.py | 41 ++ .../tests/test_is_query_resultset_updatable.py | 125 .../sqleditor/tests/test_save_changed_data.py | 347 ++ .../utils/is_query_resultset_updatable.py | 120 .../sqleditor/utils/query_tool_preferences.py | 29 + .../tools/sqleditor/utils/save_changed_data.py | 310 + .../tools/sqleditor/utils/start_running_query.py | 3 + .../sqleditor/call_render_after_poll_spec.js | 25 +- .../sqleditor/keyboard_shortcuts_spec.js | 46 ++ web/regression/runtests.py | 1 + 38 files changed, 1860 insertions(+), 597 deletions(-)
Re: [GSoC] Finalized First Patch
Patch committed (with some trivial editorial work on the docs). Congratulations - that's impressive work! On Tue, Jul 16, 2019 at 6:03 AM Yosry Muhammad wrote: > Hi all, > > Please find attached an updated patch with the following modifications: > > - Fixed the bug noticed by Khushboo, it was caused because I was using the > connection status checked periodically to know the transaction status and > whether a transaction is active. Monitoring the connection status could be > disabled using preferences (which I assume was the case at Khushboo's). I > changed the code to store the last transaction status on any query > execution, polling, or saving of data changes. This transaction status is > used rather than the one checked periodically as it can be disabled. I also > refactored other parts of the code to make use of the stored transaction > status. > > - Created a new dialog that prompts the user to either commit or rollback > when exiting with an active transaction. In addition, the commit button is > disabled when the transaction is invalid (as the default behavior of > clicking commit when the transaction is invalid is rolling back, this makes > it clearer to the user that they can only rollback the transaction or > cancel the exit). Preferences and documentation where updated accordingly. > > - When the user performs a failed save as a part of an ongoing transaction > (with auto-commit off), a notification now clarifies that only the saving > action was rolledback rather than the whole transaction, therefore, there > previous queries are unaffected. > > Looking forward to your feedback. > Thanks ! > > > On Fri, Jul 12, 2019 at 11:53 AM Yosry Muhammad > wrote: > >> I will look into it and get back to you. >> Thanks ! >> >> On Fri, Jul 12, 2019, 11:20 AM Dave Page wrote: >> >>> >>> >>> On Fri, Jul 12, 2019 at 5:57 AM Khushboo Vashi < >>> khushboo.va...@enterprisedb.com> wrote: >>> Hi Yosry, On Thu, Jul 11, 2019 at 6:50 PM Yosry Muhammad wrote: > Hi Khushboo, > Please find an updated patch attached with the mentioned import line > removed. > > Looks good to me. > On Thu, Jul 11, 2019 at 6:45 AM Khushboo Vashi < > khushboo.va...@enterprisedb.com> wrote: > >> Hi, >> >> On Wed, Jul 10, 2019 at 3:11 PM Yosry Muhammad >> wrote: >> >>> Hi, >>> >>> On Wed, Jul 10, 2019, 9:14 AM Khushboo Vashi < >>> khushboo.va...@enterprisedb.com> wrote: >>> Some points I missed: 1. I assumed that in this patch modification in case of OIDs= True (without primary key) has not considered as that is not working. >>> >>> This is not implemented yet. I will work on that in a following >>> patch soon enough. >>> >>> Okay. >> >>> 2. As we are already showing the changed Data prompt on closing the Query Tool, do we really need the Uncommitted Transaction prompt? >>> >>> This is needed when auto-commit is off. Saving changes in the data >>> grid is performed as part of the ongoing transaction (or a new one if >>> none >>> is ongoing). After saving the data changes the user should still commit >>> the >>> current transaction for the changes to be commited to the database. This >>> feature is also useful in general when auto-commit is off as users may >>> forget to commit ongoing transactions. >>> >>> One thing I have noticed, when I add a new row and delete it >> immediately without saving it and try to close the query tool, the >> uncommitted prompt is coming. >> In my opinion, it should not come, what do you think? >> >> We should disable the prompt if auto-commit and auto-rollback both >> are enabled. >> > > The uncommited prompt does not keep track of what the user has done so > far, it only checks for the current transaction status. If a current > transaction is ongoing, the prompt comes up. If you added a new row then > deleted it without saving, the transaction status is not affected, you > must > have done a previous operation and had auto-commit turned off (probably > the > select statement). > if auto-commit & auto-rollback are both enabled then there won't be > any ongoing transaction at any point, thus, the prompt will never come up. > > Exactly, my point is. It should not prompt if auto-commit & auto-rollback both are enabled, but it is coming. Please see the attached video. >>> >>> Agreed - this should be fixed. >>> >>> If auto-commit is turned off, it should also prompt to commit if the >>> user hit Save in the prior step I think. Maybe reversing that prompt makes >>> more sense in general - prompting to save rather than discard is quite >>> normal; think about text editors etc. >>> >>> -- >>> Dave Page >>> Blog: http://pgsnake.blogspot.com >>> Twitter: @pgsnake >>> >>> EnterpriseDB UK: http:/
Re: [pgAdmin4][Patch] - RE-SQL and modified SQL tests for Check Constraint node
Thanks, patch applied. On Tue, Jul 16, 2019 at 12:01 PM Khushboo Vashi < khushboo.va...@enterprisedb.com> wrote: > Hi Dave, > > On Tue, Jul 16, 2019 at 2:05 PM Dave Page wrote: > >> Hi >> >> On Tue, Jul 16, 2019 at 7:22 AM Khushboo Vashi < >> khushboo.va...@enterprisedb.com> wrote: >> >>> Hi, >>> >>> Please find the attached patch for the RE-SQL and modified SQL tests for >>> check constraint node. >>> >>> For the table child nodes, *create table endpoint *with proper data is >>> required in the JSON file. >>> I have introduced one new parameter named *store_table_id* in the table >>> create endpoint, so the created table ID will be stored for the rest of the >>> scenarios >>> >> >> This breaks the type tests :-( >> >> Fixed. > Please find the attached updated patch. > > Thanks, > Khushboo > >> -- >> Dave Page >> Blog: http://pgsnake.blogspot.com >> Twitter: @pgsnake >> >> EnterpriseDB UK: http://www.enterprisedb.com >> The Enterprise PostgreSQL Company >> > -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
pgAdmin 4 commit: Add Reverse Engineered SQL tests for Constraints. Fix
Add Reverse Engineered SQL tests for Constraints. Fixes #4475 Branch -- master Details --- https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=8168f623c436ae586cb63f36859cca902690b032 Author: Khushboo Vashi Modified Files -- docs/en_US/release_notes_4_11.rst | 1 + .../tests/Default/alter_check_constraint.sql | 9 .../tests/Default/create_check_constraint.sql | 10 .../tests/Default/msql_check_constraint.sql| 6 +++ .../check_constraint/tests/Default/test.json | 56 ++ web/regression/re_sql/tests/test_resql.py | 8 6 files changed, 90 insertions(+)
Subtle test problem
Hi Yosry, Now that your patch is committed we're seeing a subtle failure on our internal (at EDB) CI/CD system. Essentially we have multiple tests running concurrently, and it looks like you're using a fixed table name. In other tests we make sure we have a random number on the end of the table name to avoid this issue. Can you fix that ASAP please? Thanks. -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: Subtle test problem
Working on it right now. On Wed, Jul 17, 2019, 2:35 PM Dave Page wrote: > Hi Yosry, > > Now that your patch is committed we're seeing a subtle failure on our > internal (at EDB) CI/CD system. Essentially we have multiple tests running > concurrently, and it looks like you're using a fixed table name. In other > tests we make sure we have a random number on the end of the table name to > avoid this issue. Can you fix that ASAP please? > > Thanks. > > -- > Dave Page > Blog: http://pgsnake.blogspot.com > Twitter: @pgsnake > > EnterpriseDB UK: http://www.enterprisedb.com > The Enterprise PostgreSQL Company >
pgAdmin 4 commit: Ensure Selenium is started only when it's needed.
Ensure Selenium is started only when it's needed. Branch -- master Details --- https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=d78dfbd30f0dbead00873f85ec3ae424a62637c7 Author: Akshay Joshi Modified Files -- web/regression/runtests.py | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-)
pgAdmin 4 commit: Randomise table names for tests.
Randomise table names for tests. Branch -- master Details --- https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=f5b927b9254beafd1e99027a1d22fe532615af57 Author: Yosry Muhammad Modified Files -- .../tests/test_is_query_resultset_updatable.py | 28 .../sqleditor/tests/test_save_changed_data.py | 30 ++ 2 files changed, 36 insertions(+), 22 deletions(-)
Bug in Database Nodes in the Tree
Hi Khushboo, There's a minor bug that I noticed a while back that when you right click a disconnected database (to drop it for example) it automatically connects to the database and expands the node. This behaviour is a little annoying to users (me too), I am trying to fix it. I looked around the code in the browser module (node.js, database.js, menu.js) and I couldn't find a way to modify this behaviour. Is this handled internally by jQuery? Is modifying this behaviour feasible? I think the problem is that the right click event triggers the left event click too. Am I correct? Thanks!
Re: Bug in Database Nodes in the Tree
Hi Yosry, On Wed, Jul 17, 2019 at 10:49 PM Yosry Muhammad wrote: > Hi Khushboo, > > There's a minor bug that I noticed a while back that when you right click > a disconnected database (to drop it for example) it automatically connects > to the database and expands the node. This behaviour is a little annoying > to users (me too), I am trying to fix it. > > This behavior is by design as we have considered some of the pgAdmin III behavior. One behavior we can change is that on the selection of the database node, we can just connect it and not expand it and when we expand the database node (by arrow icon or double click), we can connect and expand both. We need Dave's approval to change this. I looked around the code in the browser module (node.js, database.js, > menu.js) and I couldn't find a way to modify this behaviour. Is this > handled internally by jQuery? Is modifying this behaviour feasible? > > I think the problem is that the right click event triggers the left event > click too. Am I correct? > > Basically, the browser tree is generated through the aciTree library, so when required, the public APIs (provided by aciTree) of the events are being called. In this case, on the selection of the database node, the selected event is used in the database.js file. Thanks, Khushboo Thanks! >
Re: Bug in Database Nodes in the Tree
Hi, On Thu, Jul 18, 2019, 7:27 AM Khushboo Vashi < khushboo.va...@enterprisedb.com> wrote: > Hi Yosry, > > On Wed, Jul 17, 2019 at 10:49 PM Yosry Muhammad > wrote: > >> Hi Khushboo, >> >> There's a minor bug that I noticed a while back that when you right click >> a disconnected database (to drop it for example) it automatically connects >> to the database and expands the node. This behaviour is a little annoying >> to users (me too), I am trying to fix it. >> >> This behavior is by design as we have considered some of the pgAdmin > III behavior. One behavior we can change is that on the selection of the > database node, we can just connect it and not expand it and when we expand > the database node (by arrow icon or double click), we can connect and > expand both. > > We need Dave's approval to change this. > I think this makes more sense. > I looked around the code in the browser module (node.js, database.js, >> menu.js) and I couldn't find a way to modify this behaviour. Is this >> handled internally by jQuery? Is modifying this behaviour feasible? >> >> > I think the problem is that the right click event triggers the left event >> click too. Am I correct? >> >> Basically, the browser tree is generated through the aciTree library, so > when required, the public APIs (provided by aciTree) of the events are > being called. > In this case, on the selection of the database node, the selected event is > used in the database.js file. > > Thanks for the clarification.
