-
Notifications
You must be signed in to change notification settings - Fork 879
Skip virtualisation for small DataGridView grids to avoid a re-measure on tab show #10331
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
base: master
Are you sure you want to change the base?
Changes from all commits
bd252ca
1f8a075
4173ddf
0713356
be8985e
3b04d60
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,14 @@ | ||
| SELECT | ||
| rolname, rolcanlogin, rolsuper | ||
| rolname, rolcanlogin, rolsuper, | ||
| EXISTS ( | ||
| SELECT 1 FROM pg_catalog.pg_auth_members am | ||
| WHERE am.roleid = {{ rid }}::OID | ||
| AND am.member = ( | ||
| SELECT oid FROM pg_catalog.pg_roles | ||
| WHERE rolname = current_user | ||
| ) | ||
| AND am.admin_option | ||
| ) AS has_admin_option | ||
| FROM | ||
| pg_catalog.pg_roles | ||
| WHERE oid = {{ rid }}::OID |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| ########################################################################## | ||
| # | ||
| # pgAdmin 4 - PostgreSQL Tools | ||
| # | ||
| # Copyright (C) 2013 - 2026, The pgAdmin Development Team | ||
| # This software is released under the PostgreSQL Licence | ||
| # | ||
| ########################################################################## | ||
|
|
||
| from unittest.mock import MagicMock | ||
|
|
||
| from pgadmin.utils.route import BaseTestGenerator | ||
| from pgadmin.browser.server_groups.servers.roles import RoleView | ||
|
|
||
|
|
||
| class RoleCheckPermissionTest(BaseTestGenerator): | ||
| """Unit tests for RoleView._check_permission's ADMIN OPTION carve-out. | ||
|
|
||
| A role holder who is neither a superuser nor a CREATEROLE holder, but | ||
| who has been granted ADMIN OPTION on the specific role being updated, | ||
| should be allowed through the permission gate so they can manage that | ||
| role's membership - but only for 'update', never for 'drop', and the | ||
| view should record that the request must be restricted to membership | ||
| changes only. | ||
| """ | ||
| scenarios = [ | ||
| ('Check Role Node', dict(url='/browser/role/obj/')) | ||
| ] | ||
|
|
||
| def setUp(self): | ||
| pass | ||
|
|
||
| def runTest(self): | ||
| view = RoleView(cmd=None) | ||
| view.manager = MagicMock() | ||
|
|
||
| # Plain user, no admin option: update is forbidden. | ||
| view.manager.user_info = { | ||
| 'is_superuser': False, 'can_create_role': False, 'id': 5 | ||
| } | ||
| view.has_admin_option = False | ||
| self.assertTrue(view._check_permission(True, 'update', {'rid': 10})) | ||
| self.assertFalse(view.membership_only_update) | ||
|
|
||
| # Same user, but with ADMIN OPTION on the target role: allowed | ||
| # through, flagged as membership-only. | ||
| view.has_admin_option = True | ||
| self.assertFalse(view._check_permission(True, 'update', {'rid': 10})) | ||
| self.assertTrue(view.membership_only_update) | ||
|
|
||
| # ADMIN OPTION does not extend to dropping the role. | ||
| self.assertTrue(view._check_permission(True, 'drop', {'rid': 10})) | ||
|
|
||
| # Superusers are unaffected by the ADMIN OPTION check. | ||
| view.manager.user_info = { | ||
| 'is_superuser': True, 'can_create_role': False, 'id': 5 | ||
| } | ||
| view.has_admin_option = False | ||
| self.assertFalse(view._check_permission(True, 'update', {'rid': 10})) | ||
|
|
||
| def tearDown(self): | ||
| pass |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1173,6 +1173,19 @@ def execute_void(self, query, params=None, formatted_exception_msg=False): | |
|
|
||
| if not status: | ||
| return False, str(cur) | ||
|
|
||
| if isinstance(cur, AsyncDictServerCursor): | ||
| # A named/server-side cursor's execute() always runs the query | ||
| # as `DECLARE ... CURSOR FOR <query>`, which cannot express a | ||
| # transaction-control statement such as BEGIN/COMMIT/ROLLBACK. | ||
| # Run this one statement through a throwaway plain cursor | ||
| # instead, leaving the cached server-side cursor untouched, and | ||
| # treat it as leaving no result set for whatever poll() call | ||
| # comes next. | ||
| cur = self.conn.cursor() | ||
| self.column_info = None | ||
| self.row_count = 0 | ||
|
Comment on lines
+1177
to
+1187
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Update the cursor that
📍 Affects 2 files
🤖 Prompt for AI Agents |
||
|
|
||
| query_id = str(secrets.choice(range(1, 9999999))) | ||
|
|
||
| current_app.logger.log( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Check the submitted fields before validation mutates the request.
_validate_rolemembersaddsrol_members_listand revocation fields toself.request. A valid payload that contains onlyrolmembersthen failsset(self.request) <= {'rolmembers'}. ADMIN OPTION holders cannot update membership.Capture the request keys before validation. Check that saved set here.
Proposed fix
🤖 Prompt for AI Agents