Skip to content

Revert the Query Tool execute option checkboxes when changing an option fails. #10600 #10621

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.

Sign up for GitHub

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

Closed
dpage wants to merge 3 commits into from
Closed

Revert the Query Tool execute option checkboxes when changing an option fails. #10600 #10621

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7eb8a50
Revert the Query Tool execute option checkboxes when changing an opti…
dpage Oct 15, 2026
83911ef
Send Query Tool execute options to the current transaction.
dpage Oct 15, 2026
41baa17
Update the Query Tool context ref in a layout effect rather than duri…
dpage Oct 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 72 additions & 19 deletions web/pgadmin/tools/sqleditor/static/js/components/sections/MainToolBar.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,14 @@ function changeQueryExecutionSettings(type, api, transId, value) {
let url = url_for(`sqleditor.${type}`, {
'trans_id': transId,
});
return api.post(url, JSON.stringify(value));
return api.post(url, JSON.stringify(value)).then((res)=>{
// The server answers 200 with status false when it cannot check the
// transaction or connection, in which case nothing was changed.
if(res?.data?.data?.status === false) {
throw new Error(res.data.data.result || gettext('Failed to change the execute option.'));
}
return res;
});
}

// Execute options that also live on the server side transaction object, so
Expand Down Expand Up @@ -84,6 +91,12 @@ export function getChangedExecutionOptions(restored, current) {
export function MainToolBar({containerRef, onFilterClick, onManageMacros, onAddToMacros}) {
const eventBus = useContext(QueryToolEventsContext);
const queryToolCtx = useContext(QueryToolContext);
/* A connection change replaces the transaction, so callbacks that outlive
a render read the current context from here rather than capturing it. */
const queryToolCtxRef = React.useRef(queryToolCtx);
React.useLayoutEffect(()=>{
queryToolCtxRef.current = queryToolCtx;
}, [queryToolCtx]);
const queryToolConnCtx = useContext(QueryToolConnectionContext);
const modalId = MODAL_DIALOGS.QT_CONFIRMATIONS;

Expand All @@ -107,6 +120,13 @@ export function MainToolBar({containerRef, onFilterClick, onManageMacros, onAddT
const [pendingExecOptions, setPendingExecOptions] = useState(null);
// The number of restored execute options still being sent to the server.
const [applyingExecOptions, setApplyingExecOptions] = useState(0);
/* Each execute option request rewrites the whole transaction object kept in
the session, so they are sent one at a time, chained on this promise, so
that a later request cannot overwrite an earlier one's change. */
const execOptionsQueueRef = React.useRef(Promise.resolve());
// Whether an execute option changed from the menu is still being sent.
const [changingExecOption, setChangingExecOption] = useState(false);
const changingExecOptionRef = React.useRef(false);
const {isSaveToolDataEnabled} = useApplicationState() ?? {};
/* Menu button refs */
const saveAsMenuRef = React.useRef(null);
Expand Down Expand Up @@ -168,22 +188,57 @@ export function MainToolBar({containerRef, onFilterClick, onManageMacros, onAddT
explain(true);
}, [explain]);

const sendExecOption = useCallback((option, value)=>{
// Start from the queue so that a synchronous failure is handled too.
// Read the context when the request is sent, as it may be queued.
const request = execOptionsQueueRef.current
.then(()=>{
const {api, params} = queryToolCtxRef.current;
return changeQueryExecutionSettings(option, api, params.trans_id, value);
});
execOptionsQueueRef.current = request.catch(()=>{/* handled by the caller */});
return request;
}, []);

const checkMenuClick = useCallback((e)=>{
setCheckedMenuItems((prev)=>{
let newVal = !prev[e.value];
if (e.value === 'server_cursor') {
queryToolCtx.updateServerCursor({server_cursor: newVal});
if(EXECUTION_OPTIONS.includes(e.value)) {
if(changingExecOptionRef.current) {
return;
}
if(e.value === 'auto_commit' || e.value === 'auto_rollback' || e.value === 'server_cursor') {
changeQueryExecutionSettings(e.value, queryToolCtx.api, queryToolCtx.params.trans_id, newVal)
.catch ((error)=>{
newVal = prev[e.value];
eventBus.fireEvent(QUERY_TOOL_EVENTS.HANDLE_API_ERROR, error, {
checkTransaction: true,
});
const option = e.value;
const oldVal = Boolean(checkedMenuItemsRef.current[option]);
const newVal = !oldVal;
const setOption = (val)=>{
setCheckedMenuItems((prev)=>({...prev, [option]: val}));
if(option === 'server_cursor') {
queryToolCtxRef.current.updateServerCursor({server_cursor: val});
}
};
changingExecOptionRef.current = true;
setChangingExecOption(true);
setOption(newVal);
sendExecOption(option, newVal)
.catch((error)=>{
setOption(oldVal);
// Save the reverted value so the tab state matches the server.
if(isSaveToolDataEnabled?.('sqleditor')) {
eventBus.fireEvent(QUERY_TOOL_EVENTS.TRIGGER_SAVE_QUERY_TOOL_DATA);
}
eventBus.fireEvent(QUERY_TOOL_EVENTS.HANDLE_API_ERROR, error, {
checkTransaction: true,
});
})
.finally(()=>{
changingExecOptionRef.current = false;
setChangingExecOption(false);
});
if(isSaveToolDataEnabled?.('sqleditor')) {
eventBus.fireEvent(QUERY_TOOL_EVENTS.TRIGGER_SAVE_QUERY_TOOL_DATA);
}

return;
}
setCheckedMenuItems((prev)=>{
let newVal = !prev[e.value];
let otherVars = {};
if (e.value === 'explain_serialize_binary' && newVal) {
otherVars = { 'explain_serialize_text': false };
Expand Down Expand Up @@ -242,9 +297,7 @@ export function MainToolBar({containerRef, onFilterClick, onManageMacros, onAddT
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))
sendExecOption(option, value)
.catch((error)=>{
setCheckedMenuItems((prev)=>({...prev, [option]: !value}));
if(option === 'server_cursor') {
Expand Down Expand Up @@ -759,11 +812,11 @@ export function MainToolBar({containerRef, onFilterClick, onManageMacros, onAddT
label={gettext('Execute Options Menu')}
>
<PgMenuItem hasCheck value="auto_commit" checked={checkedMenuItems['auto_commit']}
onClick={checkMenuClick}>{gettext('Auto commit?')}</PgMenuItem>
disabled={changingExecOption} onClick={checkMenuClick}>{gettext('Auto commit?')}</PgMenuItem>
<PgMenuItem hasCheck value="auto_rollback" checked={checkedMenuItems['auto_rollback']}
onClick={checkMenuClick}>{gettext('Auto rollback on error?')}</PgMenuItem>
disabled={changingExecOption} onClick={checkMenuClick}>{gettext('Auto rollback on error?')}</PgMenuItem>
<PgMenuItem hasCheck value="server_cursor" checked={checkedMenuItems['server_cursor']}
onClick={checkMenuClick}>{gettext('Use server cursor?')}</PgMenuItem>
disabled={changingExecOption} onClick={checkMenuClick}>{gettext('Use server cursor?')}</PgMenuItem>
</PgMenu>
<PgMenu
anchorRef={explainMenuRef}
Expand Down
164 changes: 161 additions & 3 deletions web/regression/javascript/sqleditor/MainToolBarRestore.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,15 @@ describe('MainToolBar toolbar options restore', () => {
);
queryToolCtx.params.is_query_tool = isQueryTool;
const result = render(ui(connected));
return {...result, setConnected: (conn) => result.rerender(ui(conn))};
return {
...result,
setConnected: (conn, ctx) => {
if(ctx) {
queryToolCtx = ctx;
}
result.rerender(ui(conn));
},
};
};

beforeEach(() => {
Expand Down Expand Up @@ -160,6 +168,21 @@ describe('MainToolBar toolbar options restore', () => {
expect(queryToolCtx.api.post).toHaveBeenCalledWith('/mock/sqleditor.server_cursor/1234', 'true');
});

it('sends restored execute options to the transaction current when connected', async () => {
const {setConnected} = renderToolBar(false);
await waitFor(() => expect(queryToolCtx.toolbarOptionsRef.current).not.toBeNull());

act(() => {
eventBus.fireEvent(QUERY_TOOL_EVENTS.RESTORE_TOOLBAR_OPTIONS, {auto_commit: false});
});
await waitFor(() => expect(queryToolCtx.toolbarOptionsRef.current.auto_commit).toBe(false));

// The transaction is replaced before the pending option is sent.
setConnected(true, {...queryToolCtx, params: {...queryToolCtx.params, trans_id: 5678}});
await waitFor(() => expect(queryToolCtx.api.post).toHaveBeenCalledTimes(1));
expect(queryToolCtx.api.post).toHaveBeenCalledWith('/mock/sqleditor.auto_commit/5678', 'false');
});

it('reverts and saves a restored execute option the server rejects', async () => {
queryToolCtx.api.post = jest.fn(() => Promise.reject(new Error('failed')));
const fireEvent = jest.spyOn(eventBus, 'fireEvent');
Expand Down Expand Up @@ -202,11 +225,13 @@ describe('MainToolBar toolbar options restore', () => {
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(() => expect(queryToolCtx.api.post).toHaveBeenCalledTimes(1));
await waitFor(() => expectGated(true));

// Still disabled until every option has reached the server.
// Still disabled until every option has reached the server, and the
// options are sent one at a time.
await act(async () => { resolvers[0]({data: {}}); });
await waitFor(() => expect(queryToolCtx.api.post).toHaveBeenCalledTimes(2));
expectGated(true);

await act(async () => { resolvers[1]({data: {}}); });
Expand Down Expand Up @@ -327,3 +352,136 @@ describe('MainToolBar toolbar options restore', () => {
expect(queryToolCtx.api.post).not.toHaveBeenCalled();
});
});

describe('MainToolBar execute options menu', () => {
let eventBus, queryToolCtx;
const ThemedMainToolBar = withTheme(MainToolBar);

const renderToolBar = () => {
const containerRef = React.createRef();
const ui = (ctx) => (
<QueryToolContext.Provider value={ctx}>
<QueryToolConnectionContext.Provider value={{connected: true, obtainingConn: false, connectionStatus: null}}>
<QueryToolEventsContext.Provider value={eventBus}>
<div ref={containerRef}>
<ThemedMainToolBar containerRef={containerRef} onFilterClick={()=>{/* noop */}}
onManageMacros={()=>{/* noop */}} onAddToMacros={()=>{/* noop */}} />
</div>
</QueryToolEventsContext.Provider>
</QueryToolConnectionContext.Provider>
</QueryToolContext.Provider>
);
const result = render(ui(queryToolCtx));
return {...result, setContext: (ctx) => result.rerender(ui(ctx))};
};

const menuItem = (label) => document.querySelector(`li[data-label="${label}"]`);

const openMenuAndClick = async (label) => {
if(!menuItem(label)) {
await act(async () => {
fireDomEvent.click(document.querySelector('button[data-label="Execute options"]'));
});
await waitFor(() => expect(menuItem(label)).not.toBeNull());
}
await act(async () => { fireDomEvent.click(menuItem(label)); });
};

beforeEach(() => {
eventBus = new EventBus();
queryToolCtx = {
params: {trans_id: 1234, is_query_tool: true, macros: []},
preferences: {sqleditor: {...sqleditorPrefs}, editor: {}},
api: {post: jest.fn(() => Promise.resolve({data: {data: {status: true}}}))},
modal: {},
toolbarOptionsRef: {current: null},
updateServerCursor: jest.fn(),
};
});

afterEach(() => {
jest.clearAllMocks();
});

it('sends a changed execute option to the server', async () => {
renderToolBar();
await waitFor(() => expect(queryToolCtx.toolbarOptionsRef.current).not.toBeNull());

await openMenuAndClick('Use server cursor?');
await waitFor(() => expect(queryToolCtx.api.post).toHaveBeenCalledWith('/mock/sqleditor.server_cursor/1234', 'true'));
await waitFor(() => expect(queryToolCtx.toolbarOptionsRef.current.server_cursor).toBe(true));
expect(queryToolCtx.updateServerCursor).toHaveBeenLastCalledWith({server_cursor: true});
});

it('sends execute options to the current transaction after a connection change', async () => {
const {setContext} = renderToolBar();
await waitFor(() => expect(queryToolCtx.toolbarOptionsRef.current).not.toBeNull());

// A connection change replaces the transaction with a new one.
const newCtx = {
...queryToolCtx,
params: {...queryToolCtx.params, trans_id: 5678},
updateServerCursor: jest.fn(),
};
setContext(newCtx);

await openMenuAndClick('Use server cursor?');
await waitFor(() => expect(queryToolCtx.api.post).toHaveBeenCalledWith('/mock/sqleditor.server_cursor/5678', 'true'));
expect(queryToolCtx.api.post).not.toHaveBeenCalledWith('/mock/sqleditor.server_cursor/1234', expect.anything());
expect(newCtx.updateServerCursor).toHaveBeenLastCalledWith({server_cursor: true});
expect(queryToolCtx.updateServerCursor).not.toHaveBeenCalled();
});

it('reverts an execute option when the request fails', async () => {
queryToolCtx.api.post = jest.fn(() => Promise.reject(new Error('failed')));
const fireEvent = jest.spyOn(eventBus, 'fireEvent');
renderToolBar();
await waitFor(() => expect(queryToolCtx.toolbarOptionsRef.current).not.toBeNull());

await openMenuAndClick('Use server cursor?');
await waitFor(() => {
expect(fireEvent).toHaveBeenCalledWith(QUERY_TOOL_EVENTS.HANDLE_API_ERROR, expect.any(Error), {checkTransaction: true});
});
await waitFor(() => expect(queryToolCtx.toolbarOptionsRef.current.server_cursor).toBe(false));
expect(queryToolCtx.updateServerCursor).toHaveBeenLastCalledWith({server_cursor: false});
expect(menuItem('Use server cursor?')).toHaveAttribute('data-checked', 'false');
fireEvent.mockRestore();
});

it('reverts an execute option when the server returns status false', async () => {
queryToolCtx.api.post = jest.fn(() => Promise.resolve({data: {data: {status: false, result: 'Not connected'}}}));
const fireEvent = jest.spyOn(eventBus, 'fireEvent');
renderToolBar();
await waitFor(() => expect(queryToolCtx.toolbarOptionsRef.current).not.toBeNull());

await openMenuAndClick('Auto commit?');
await waitFor(() => {
expect(fireEvent).toHaveBeenCalledWith(QUERY_TOOL_EVENTS.HANDLE_API_ERROR,
expect.objectContaining({message: 'Not connected'}), {checkTransaction: true});
});
await waitFor(() => expect(queryToolCtx.toolbarOptionsRef.current.auto_commit).toBe(true));
fireEvent.mockRestore();
});

it('disables the execute options whilst a change is being sent', async () => {
let resolvePost;
queryToolCtx.api.post = jest.fn(() => new Promise((resolve) => { resolvePost = resolve; }));
renderToolBar();
await waitFor(() => expect(queryToolCtx.toolbarOptionsRef.current).not.toBeNull());

await openMenuAndClick('Auto commit?');
await waitFor(() => expect(queryToolCtx.api.post).toHaveBeenCalledTimes(1));
['Auto commit?', 'Auto rollback on error?', 'Use server cursor?'].forEach((label) => {
expect(menuItem(label)).toHaveAttribute('aria-disabled', 'true');
});

// A click whilst the request is in flight is ignored.
await act(async () => { fireDomEvent.click(menuItem('Auto rollback on error?')); });
expect(queryToolCtx.api.post).toHaveBeenCalledTimes(1);

await act(async () => { resolvePost({data: {data: {status: true}}}); });
await waitFor(() => expect(menuItem('Auto rollback on error?')).not.toHaveAttribute('aria-disabled', 'true'));
expect(queryToolCtx.toolbarOptionsRef.current.auto_commit).toBe(false);
expect(queryToolCtx.toolbarOptionsRef.current.auto_rollback).toBe(false);
});
});
Loading