webapp/pgadmin4
1
0
Fork 0
mirror of synced 2026-10-23 07:08:55 +03:00
Code Activity

Keep Query Tool execution disabled until a restored tab's execute options reach the server. #10618 (#10619)

Browse Source
This commit is contained in:
Dave Page
2026-10-15 20:49:10 +01:00
parent f73b51f912
commit 1cac42d986
2 changed files with 171 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

51
web/pgadmin/tools/sqleditor/static/js/components/sections/MainToolBar.jsx
View File

@@ -57,6 +57,10 @@ function changeQueryExecutionSettings(type, api, transId, value) {
// they must be sent to the server when changed.
const EXECUTION_OPTIONS = ['auto_commit', 'auto_rollback', 'server_cursor'];
// Buttons kept disabled whilst restored execute options are being sent to the
// server, so that nothing runs with the preference values instead.
const BUTTONS_GATED_BY_EXEC_OPTIONS = ['execute', 'execute-options', 'explain', 'explain_analyse'];
// Check menu options that are saved and restored with the tab's state.
export const RESTORABLE_TOOLBAR_OPTIONS = [
...EXECUTION_OPTIONS,
@@ -101,6 +105,8 @@ export function MainToolBar({containerRef, onFilterClick, onManageMacros, onAddT
const checkedMenuItemsRef = React.useRef(checkedMenuItems);
// Restored execute options waiting for the connection to be established.
const [pendingExecOptions, setPendingExecOptions] = useState(null);
// The number of restored execute options still being sent to the server.
const [applyingExecOptions, setApplyingExecOptions] = useState(0);
const {isSaveToolDataEnabled} = useApplicationState() ?? {};
/* Menu button refs */
const saveAsMenuRef = React.useRef(null);
@@ -116,6 +122,15 @@ export function MainToolBar({containerRef, onFilterClick, onManageMacros, onAddT
const setDisableButton = useCallback((name, disable=true)=>{
setButtonsDisabled((prev)=>({...prev, [name]: disable}));
}, []);
/* Kept separate from buttonsDisabled, which the execution events set
directly, so that nothing else can enable these buttons early and the
gate always lifts once the requests have settled. Pending options count
only once connected, as they are sent in the effect after that render;
before then Execute is what re-makes a failed connection. */
const execOptionsGated = applyingExecOptions > 0
|| (!_.isEmpty(pendingExecOptions) && Boolean(queryToolConnCtx.connected));
const isButtonDisabled = (name)=>(buttonsDisabled[name]
|| (execOptionsGated && BUTTONS_GATED_BY_EXEC_OPTIONS.includes(name)));
const executeCursor = useCallback(()=>{
if(!queryToolCtx.preferences.sqleditor.underline_query_cursor && queryToolCtx.preferences.sqleditor.underlined_query_execute_warning){
@@ -224,8 +239,12 @@ export function MainToolBar({containerRef, onFilterClick, onManageMacros, onAddT
return;
}
setPendingExecOptions(null);
Object.entries(pendingExecOptions).forEach(([option, value])=>{
changeQueryExecutionSettings(option, queryToolCtx.api, queryToolCtx.params.trans_id, value)
const options = Object.entries(pendingExecOptions);
setApplyingExecOptions((prev)=>prev + options.length);
options.forEach(([option, value])=>{
// Start from a promise so that a synchronous failure is handled too.
Promise.resolve()
.then(()=>changeQueryExecutionSettings(option, queryToolCtx.api, queryToolCtx.params.trans_id, value))
.catch((error)=>{
setCheckedMenuItems((prev)=>({...prev, [option]: !value}));
if(option === 'server_cursor') {
@@ -238,6 +257,9 @@ export function MainToolBar({containerRef, onFilterClick, onManageMacros, onAddT
eventBus.fireEvent(QUERY_TOOL_EVENTS.HANDLE_API_ERROR, error, {
checkTransaction: true,
});
})
.finally(()=>{
setApplyingExecOptions((prev)=>prev - 1);
});
});
}, [pendingExecOptions, queryToolConnCtx.connected]);
@@ -394,6 +416,9 @@ export function MainToolBar({containerRef, onFilterClick, onManageMacros, onAddT
eventBus.fireEvent(QUERY_TOOL_EVENTS.EXECUTION_START, 'ROLLBACK;', {external: true});
};
const executeMacro = (m)=>{
/* Macros run through the same transaction, so they must wait for the
restored execute options too. */
if(execOptionsGated) return;
eventBus.fireEvent(QUERY_TOOL_EVENTS.TRIGGER_EXECUTION, null, m.sql);
};
const onLimitChange=(e)=>{
@@ -500,7 +525,7 @@ export function MainToolBar({containerRef, onFilterClick, onManageMacros, onAddT
{
shortcut: queryToolPref.btn_execute_options,
options: {
callback: ()=>{!buttonsDisabled['execute-options']&&toggleMenu({
callback: ()=>{!isButtonDisabled('execute-options')&&toggleMenu({
currentTarget: {name: 'menu-autocommit'}
});}
}
@@ -508,25 +533,25 @@ export function MainToolBar({containerRef, onFilterClick, onManageMacros, onAddT
{
shortcut: queryToolPref.execute_script,
options: {
callback: ()=>{!buttonsDisabled['execute']&&executeScript();}
callback: ()=>{!isButtonDisabled('execute')&&executeScript();}
}
},
{
shortcut: queryToolPref.execute_cursor,
options: {
callback: ()=>{!buttonsDisabled['execute']&&executeCursor();}
callback: ()=>{!isButtonDisabled('execute')&&executeCursor();}
}
},
{
shortcut: queryToolPref.explain_query,
options: {
callback: (e)=>{e.preventDefault(); !buttonsDisabled['explain']&&explain();}
callback: (e)=>{e.preventDefault(); !isButtonDisabled('explain')&&explain();}
}
},
{
shortcut: queryToolPref.explain_analyze_query,
options: {
callback: ()=>{!buttonsDisabled['explain_analyse']&&explainAnalyse();}
callback: ()=>{!isButtonDisabled('explain_analyse')&&explainAnalyse();}
}
},
{
@@ -643,18 +668,18 @@ export function MainToolBar({containerRef, onFilterClick, onManageMacros, onAddT
<PgIconButton title={gettext('Cancel query')} icon={<StopRoundedIcon style={{height: 'unset'}} />}
onClick={cancelQuery} disabled={buttonsDisabled['cancel']} shortcut={queryToolPref.btn_cancel_query} />
<PgIconButton title={gettext('Execute script')} icon={<PlayArrowRoundedIcon style={{height: 'unset'}} />}
onClick={executeScript} disabled={buttonsDisabled['execute']} shortcut={queryToolPref.execute_script}/>
onClick={executeScript} disabled={isButtonDisabled('execute')} shortcut={queryToolPref.execute_script}/>
<PgIconButton title={gettext('Execute query')} icon={<ExecuteQueryIcon style={{padding: '2px 5px'}} />}
onClick={executeCursor} disabled={buttonsDisabled['execute'] || !queryToolCtx.params.is_query_tool} shortcut={queryToolPref.execute_cursor}/>
onClick={executeCursor} disabled={isButtonDisabled('execute') || !queryToolCtx.params.is_query_tool} shortcut={queryToolPref.execute_cursor}/>
<PgIconButton title={gettext('Execute options')} icon={<KeyboardArrowDownIcon />} splitButton
name="menu-autocommit" ref={autoCommitMenuRef} shortcut={queryToolPref.btn_execute_options}
onClick={toggleMenu} disabled={buttonsDisabled['execute-options']}/>
onClick={toggleMenu} disabled={isButtonDisabled('execute-options')}/>
</PgButtonGroup>
<PgButtonGroup size="small">
<PgIconButton title={gettext('Explain')} icon={<ExplicitRoundedIcon />}
onClick={()=>{explain();}} disabled={buttonsDisabled['explain'] || !queryToolCtx.params.is_query_tool} shortcut={queryToolPref.explain_query}/>
onClick={()=>{explain();}} disabled={isButtonDisabled('explain') || !queryToolCtx.params.is_query_tool} shortcut={queryToolPref.explain_query}/>
<PgIconButton title={gettext('Explain Analyze')} icon={<AssessmentRoundedIcon />}
onClick={()=>{explainAnalyse();}} disabled={buttonsDisabled['explain_analyse'] || !queryToolCtx.params.is_query_tool} shortcut={queryToolPref.explain_analyze_query}/>
onClick={()=>{explainAnalyse();}} disabled={isButtonDisabled('explain_analyse') || !queryToolCtx.params.is_query_tool} shortcut={queryToolPref.explain_analyze_query}/>
<PgIconButton title={gettext('Explain Settings')} icon={<KeyboardArrowDownIcon />} splitButton
disabled={!queryToolCtx.params.is_query_tool}
name="menu-explain" ref={explainMenuRef} onClick={toggleMenu} />
@@ -668,7 +693,7 @@ export function MainToolBar({containerRef, onFilterClick, onManageMacros, onAddT
<PgButtonGroup size="small">
<PgIconButton title={gettext('Macros')} icon={
<><FormatListNumberedRoundedIcon /><KeyboardArrowDownIcon style={{marginLeft: '-10px'}} /></>}
disabled={!queryToolCtx.params.is_query_tool} name="menu-macros" ref={macrosMenuRef} onClick={toggleMenu} />
disabled={!queryToolCtx.params.is_query_tool || execOptionsGated} name="menu-macros" ref={macrosMenuRef} onClick={toggleMenu} />
</PgButtonGroup>
<PgButtonGroup size="small">
<PgIconButton title={gettext('Help')} icon={<HelpIcon />} onClick={onHelpClick} />

134
web/regression/javascript/sqleditor/MainToolBarRestore.spec.js
View File

@@ -28,7 +28,7 @@ jest.mock('../../../pgadmin/settings/static/ApplicationStateProvider', () => ({
}));
import React from 'react';
import { render, act, waitFor } from '@testing-library/react';
import { render, act, waitFor, fireEvent as fireDomEvent } from '@testing-library/react';
import '@testing-library/jest-dom';
import { withTheme } from '../fake_theme';
import EventBus from '../../../pgadmin/static/js/helpers/EventBus';
@@ -151,6 +151,8 @@ describe('MainToolBar toolbar options restore', () => {
expect(queryToolCtx.updateServerCursor).toHaveBeenCalledWith({server_cursor: true});
// Nothing can be sent to the server until the transaction exists.
expect(queryToolCtx.api.post).not.toHaveBeenCalled();
// Execution must stay possible, as it is what re-makes a failed connection.
expectGated(false);
setConnected(true);
await waitFor(() => expect(queryToolCtx.api.post).toHaveBeenCalledTimes(2));
@@ -177,6 +179,136 @@ describe('MainToolBar toolbar options restore', () => {
fireEvent.mockRestore();
});
// A disabled button loses its accessible name to a wrapping span.
const gatedButtons = () => ['Execute script', 'Execute query', 'Execute options', 'Explain', 'Explain Analyze']
.map((label) => document.querySelector(`button[data-label="${label}"]`));
const expectGated = (disabled) => {
gatedButtons().forEach((btn) => {
if(disabled) {
expect(btn).toBeDisabled();
} else {
expect(btn).toBeEnabled();
}
});
};
it('keeps execution disabled until restored execute options are applied', async () => {
const resolvers = [];
queryToolCtx.api.post = jest.fn(() => new Promise((resolve) => { resolvers.push(resolve); }));
renderToolBar(true);
await waitFor(() => expect(queryToolCtx.toolbarOptionsRef.current).not.toBeNull());
expectGated(false);
act(() => {
eventBus.fireEvent(QUERY_TOOL_EVENTS.RESTORE_TOOLBAR_OPTIONS, {auto_commit: false, auto_rollback: true});
});
await waitFor(() => expect(queryToolCtx.api.post).toHaveBeenCalledTimes(2));
await waitFor(() => expectGated(true));
// Still disabled until every option has reached the server.
await act(async () => { resolvers[0]({data: {}}); });
expectGated(true);
await act(async () => { resolvers[1]({data: {}}); });
await waitFor(() => expectGated(false));
});
it('ignores the execute shortcut until restored execute options are applied', async () => {
let resolvePost;
queryToolCtx.api.post = jest.fn(() => new Promise((resolve) => { resolvePost = resolve; }));
// F5
queryToolCtx.preferences.sqleditor.execute_script = {
alt: false, shift: false, control: false, key: {key_code: 116},
};
const triggered = jest.fn();
eventBus.registerListener(QUERY_TOOL_EVENTS.TRIGGER_EXECUTION, triggered);
const {container} = renderToolBar(true);
await waitFor(() => expect(queryToolCtx.toolbarOptionsRef.current).not.toBeNull());
act(() => {
eventBus.fireEvent(QUERY_TOOL_EVENTS.RESTORE_TOOLBAR_OPTIONS, {auto_commit: false});
});
await waitFor(() => expectGated(true));
await act(async () => { fireDomEvent.keyDown(container.firstChild, {keyCode: 116}); });
expect(triggered).not.toHaveBeenCalled();
await act(async () => { resolvePost({data: {}}); });
await waitFor(() => expectGated(false));
await act(async () => { fireDomEvent.keyDown(container.firstChild, {keyCode: 116}); });
await waitFor(() => expect(triggered).toHaveBeenCalledTimes(1));
});
it('ignores macros until restored execute options are applied', async () => {
let resolvePost;
queryToolCtx.api.post = jest.fn(() => new Promise((resolve) => { resolvePost = resolve; }));
// F6
queryToolCtx.params.macros = [{
id: 1, name: 'Test macro', sql: 'SELECT 1;',
alt: false, shift: false, control: false, key: 'F6', key_code: 117,
}];
const triggered = jest.fn();
eventBus.registerListener(QUERY_TOOL_EVENTS.TRIGGER_EXECUTION, triggered);
const {container} = renderToolBar(true);
await waitFor(() => expect(queryToolCtx.toolbarOptionsRef.current).not.toBeNull());
const macrosButton = () => document.querySelector('button[data-label="Macros"]');
expect(macrosButton()).toBeEnabled();
act(() => {
eventBus.fireEvent(QUERY_TOOL_EVENTS.RESTORE_TOOLBAR_OPTIONS, {auto_commit: false});
});
await waitFor(() => expectGated(true));
expect(macrosButton()).toBeDisabled();
await act(async () => { fireDomEvent.keyDown(container.firstChild, {keyCode: 117}); });
expect(triggered).not.toHaveBeenCalled();
await act(async () => { resolvePost({data: {}}); });
await waitFor(() => expectGated(false));
expect(macrosButton()).toBeEnabled();
await act(async () => { fireDomEvent.keyDown(container.firstChild, {keyCode: 117}); });
await waitFor(() => expect(triggered).toHaveBeenCalledWith(null, 'SELECT 1;'));
});
it('enables execution again when a restored execute option fails', async () => {
let rejectPost;
queryToolCtx.api.post = jest.fn(() => new Promise((_resolve, reject) => { rejectPost = reject; }));
renderToolBar(true);
await waitFor(() => expect(queryToolCtx.toolbarOptionsRef.current).not.toBeNull());
act(() => {
eventBus.fireEvent(QUERY_TOOL_EVENTS.RESTORE_TOOLBAR_OPTIONS, {auto_commit: false});
});
await waitFor(() => expectGated(true));
await act(async () => { rejectPost(new Error('failed')); });
await waitFor(() => expectGated(false));
expect(queryToolCtx.toolbarOptionsRef.current.auto_commit).toBe(true);
});
it('enables execution again if sending a restored execute option throws', async () => {
queryToolCtx.api.post = jest.fn(() => { throw new Error('failed'); });
renderToolBar(true);
await waitFor(() => expect(queryToolCtx.toolbarOptionsRef.current).not.toBeNull());
act(() => {
eventBus.fireEvent(QUERY_TOOL_EVENTS.RESTORE_TOOLBAR_OPTIONS, {auto_commit: false});
});
await waitFor(() => expect(queryToolCtx.api.post).toHaveBeenCalled());
await waitFor(() => expectGated(false));
expect(queryToolCtx.toolbarOptionsRef.current.auto_commit).toBe(true);
});
it('does not disable execution when no execute options need restoring', async () => {
renderToolBar(true);
await waitFor(() => expect(queryToolCtx.toolbarOptionsRef.current).not.toBeNull());
act(() => {
eventBus.fireEvent(QUERY_TOOL_EVENTS.RESTORE_TOOLBAR_OPTIONS, {auto_commit: true, explain_costs: true});
});
await waitFor(() => expect(queryToolCtx.toolbarOptionsRef.current.explain_costs).toBe(true));
expectGated(false);
expect(queryToolCtx.api.post).not.toHaveBeenCalled();
});
it('does not restore execute options for View/Edit Data', async () => {
renderToolBar(true, false);
await waitFor(() => expect(queryToolCtx.toolbarOptionsRef.current).not.toBeNull());