Skip to content

Ensure worspaces/pgadmin state is saved on abrupt shutdown of pgadmin.#GH_3319 #8671

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yogeshmahajan-1903
Copy link
Contributor

No description provided.

@yogeshmahajan-1903 yogeshmahajan-1903 force-pushed the GH_3319 branch 5 times, most recently from ae9bd5f to 7441c9e Compare April 22, 2025 09:58
@yogeshmahajan-1903 yogeshmahajan-1903 force-pushed the GH_3319 branch 6 times, most recently from 72d6539 to c2980a1 Compare May 2, 2025 06:31
Copy link
Contributor

@akshay-joshi akshay-joshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GUI Review Comments:

  1. When opening pgAdmin again in the browser Default Workspace should be in focus. Open Schema Diff tool and refresh the browser.

  2. PSQL tools database name not showing in the single line on reopening pgAdmin.

  3. Delete all the states from the table if the user changes the preferences and sets them to False.

  4. Getting error AttributeError: 'NoneType' object has no attribute 'connection' in line 604, in _get_database_role psql/init.py

  5. params['db'] = underscore_escape(data['db_name'])  TypeError: 'NoneType' object is not subscriptable in line 104 in psql/init.py

  6. Open Query Tool/PSQL in the respective workspace and then refresh the browser. It opens in the Default workspace, which seems to be correct, but we need to document this behaviour.

  7. Query Tool/PSQL for adHoc servers are still opening with error. We should not store state of the adHoc server.

  8. The show_query_tool.js, show_erd_tool.js, showSchemaDiffTool.js and show_psql_tool.js files have duplicate code to open the respective tools. We can easily make some part of the logic generic.

  9. Why file name 'showSchemaDiffTool.js' different in respect to others like show_query_tool.js

  10. Documentation/Screenshot updates are missing for Preferences.

  11. Create a new .rst file to explain this new feature.

  12. Add comments in the new function added in this PR and wherever needed in the code, so that others can understand the logic.

@@ -0,0 +1,33 @@

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a copyright header in the file.

@@ -392,6 +392,17 @@ class QueryHistoryModel(db.Model):
last_updated_flag = db.Column(db.String(), nullable=False)


class PgadminStateData(db.Model):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename this class based on the table name you decide (as per the above review comments).

@@ -392,6 +392,17 @@ class QueryHistoryModel(db.Model):
last_updated_flag = db.Column(db.String(), nullable=False)


class PgadminStateData(db.Model):
"""Define the history SQL table."""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct the comment.


def upgrade():
op.create_table(
'pgadmin_state_data',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We haven't started any of the tables with pgadmin. Rename this table to any name of your choice:

  1. 'application_state'
  2. 'app_state'
  3. 'workspace_state'

Or any other name that you can think of.


data = _get_database_role(params['sid'], params['did'])
print(data)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this print statement.

'public')
self.assertTrue(self.page.expand_tables_node(
"Server", self.server['name'], self.server['db_password'],
self.test_db,'public'), 'Table node nmot exoanded')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change the message to 'Tree not expanded to the table node.'

self.database_name, 'public')
self.assertTrue(self.page.expand_tables_node(
"Server", self.server['name'], self.server['db_password'],
self.database_name, 'public'),'Table dialog not opend')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change the message to 'Tree not expanded to the table node.'

'public')
self.assertTrue(self.page.expand_tables_node(
"Server", self.server['name'], self.server['db_password'],
self.test_db, 'public'),'Tree not expanded to table')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change the message to 'Tree not expanded to the table node.'

'public')
self.assertTrue(self.page.expand_tables_node(
"Server", self.server['name'], self.server['db_password'],
self.test_db, 'public'),'Table node not expanded')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change the message to 'Tree not expanded to the table node.'

'public')
self.assertTrue(self.page.expand_tables_node(
"Server", self.server['name'],self.server['db_password'],
self.test_db, 'public'), 'Table node not expanded')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change the message to 'Tree not expanded to the table node.'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants