mirror of
https://github.com/apache/superset.git
synced 2026-05-12 19:35:17 +00:00
fix: save button was enabled even no changes were made to the dashboard (#35817)
This commit is contained in:
committed by
GitHub
parent
6701d0ae0c
commit
467b008f36
@@ -711,3 +711,134 @@ test('should call setShowUnsavedChangesModal(false) on cancel', async () => {
|
||||
|
||||
expect(setShowModal).toHaveBeenCalledWith(false);
|
||||
});
|
||||
|
||||
test('should clear history and unsaved changes when entering edit mode', () => {
|
||||
const clearDashboardHistory = jest.fn();
|
||||
|
||||
jest.spyOn(redux, 'bindActionCreators').mockImplementation(() => ({
|
||||
addSuccessToast,
|
||||
addDangerToast,
|
||||
addWarningToast,
|
||||
onUndo,
|
||||
onRedo,
|
||||
setEditMode,
|
||||
setUnsavedChanges,
|
||||
fetchFaveStar,
|
||||
saveFaveStar,
|
||||
savePublished,
|
||||
fetchCharts,
|
||||
updateDashboardTitle,
|
||||
updateCss,
|
||||
onChange,
|
||||
onSave,
|
||||
setMaxUndoHistoryExceeded,
|
||||
maxUndoHistoryToast,
|
||||
logEvent,
|
||||
setRefreshFrequency,
|
||||
onRefresh,
|
||||
dashboardInfoChanged,
|
||||
dashboardTitleChanged,
|
||||
clearDashboardHistory,
|
||||
}));
|
||||
|
||||
const canEditState = {
|
||||
dashboardInfo: {
|
||||
...initialState.dashboardInfo,
|
||||
dash_edit_perm: true,
|
||||
},
|
||||
};
|
||||
|
||||
setup(canEditState);
|
||||
|
||||
const editButton = screen.getByText('Edit dashboard');
|
||||
userEvent.click(editButton);
|
||||
|
||||
expect(clearDashboardHistory).toHaveBeenCalledTimes(1);
|
||||
expect(setUnsavedChanges).toHaveBeenCalledWith(false);
|
||||
});
|
||||
|
||||
test('should mark theme change as unsaved when in edit mode', async () => {
|
||||
const testStore = createStore(
|
||||
{
|
||||
...initialState,
|
||||
...editableState,
|
||||
dashboardInfo: {
|
||||
...editableState.dashboardInfo,
|
||||
theme: 'LIGHT',
|
||||
},
|
||||
},
|
||||
reducerIndex,
|
||||
);
|
||||
|
||||
render(
|
||||
<div className="dashboard">
|
||||
<Header />
|
||||
</div>,
|
||||
{
|
||||
useRedux: true,
|
||||
useTheme: true,
|
||||
store: testStore,
|
||||
},
|
||||
);
|
||||
|
||||
expect(setUnsavedChanges).not.toHaveBeenCalledWith(true);
|
||||
|
||||
testStore.dispatch({
|
||||
type: 'DASHBOARD_INFO_UPDATED',
|
||||
newInfo: {
|
||||
theme: 'DARK',
|
||||
},
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(setUnsavedChanges).toHaveBeenCalledWith(true);
|
||||
});
|
||||
});
|
||||
|
||||
test('should not mark initial theme as unsaved change', () => {
|
||||
setup({
|
||||
...editableState,
|
||||
dashboardInfo: {
|
||||
...editableState.dashboardInfo,
|
||||
theme: 'LIGHT',
|
||||
},
|
||||
});
|
||||
|
||||
expect(setUnsavedChanges).not.toHaveBeenCalledWith(true);
|
||||
});
|
||||
|
||||
test('should sync theme ref when navigating between dashboards', async () => {
|
||||
const testStore = createStore(
|
||||
{
|
||||
...initialState,
|
||||
dashboardInfo: {
|
||||
...initialState.dashboardInfo,
|
||||
theme: 'LIGHT',
|
||||
},
|
||||
},
|
||||
reducerIndex,
|
||||
);
|
||||
|
||||
render(
|
||||
<div className="dashboard">
|
||||
<Header />
|
||||
</div>,
|
||||
{
|
||||
useRedux: true,
|
||||
useTheme: true,
|
||||
store: testStore,
|
||||
},
|
||||
);
|
||||
|
||||
testStore.dispatch({
|
||||
type: 'DASHBOARD_INFO_UPDATED',
|
||||
newInfo: {
|
||||
id: 2,
|
||||
theme: 'DARK',
|
||||
},
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(setUnsavedChanges).toHaveBeenCalledTimes(0);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -218,6 +218,7 @@ const Header = () => {
|
||||
const refreshTimer = useRef(0);
|
||||
const ctrlYTimeout = useRef(0);
|
||||
const ctrlZTimeout = useRef(0);
|
||||
const previousThemeRef = useRef(dashboardInfo.theme);
|
||||
|
||||
const dashboardTitle = layout[DASHBOARD_HEADER_ID]?.meta?.text;
|
||||
const { slug } = dashboardInfo;
|
||||
@@ -324,11 +325,12 @@ const Header = () => {
|
||||
startPeriodicRender(refreshFrequency * 1000);
|
||||
}, [refreshFrequency, startPeriodicRender]);
|
||||
|
||||
// Ensure theme changes are tracked as unsaved changes
|
||||
// Track theme changes as unsaved changes, and sync ref when navigating between dashboards
|
||||
useEffect(() => {
|
||||
if (editMode && dashboardInfo.theme !== undefined) {
|
||||
if (editMode && dashboardInfo.theme !== previousThemeRef.current) {
|
||||
boundActionCreators.setUnsavedChanges(true);
|
||||
}
|
||||
previousThemeRef.current = dashboardInfo.theme;
|
||||
}, [dashboardInfo.theme, editMode, boundActionCreators]);
|
||||
|
||||
useEffect(() => {
|
||||
@@ -559,6 +561,12 @@ const Header = () => {
|
||||
[boundActionCreators],
|
||||
);
|
||||
|
||||
const handleEnterEditMode = useCallback(() => {
|
||||
toggleEditMode();
|
||||
boundActionCreators.clearDashboardHistory?.();
|
||||
boundActionCreators.setUnsavedChanges(false);
|
||||
}, [toggleEditMode, boundActionCreators]);
|
||||
|
||||
const NavExtension = extensionsRegistry.get('dashboard.nav.right');
|
||||
|
||||
const editableTitleProps = useMemo(
|
||||
@@ -710,10 +718,7 @@ const Header = () => {
|
||||
{userCanEdit && (
|
||||
<Button
|
||||
buttonStyle="secondary"
|
||||
onClick={() => {
|
||||
toggleEditMode();
|
||||
boundActionCreators.clearDashboardHistory?.(); // Resets the `past` as an empty array
|
||||
}}
|
||||
onClick={handleEnterEditMode}
|
||||
data-test="edit-dashboard-button"
|
||||
className="action-button"
|
||||
css={editButtonStyle}
|
||||
@@ -736,6 +741,7 @@ const Header = () => {
|
||||
emphasizeUndo,
|
||||
handleCtrlY,
|
||||
handleCtrlZ,
|
||||
handleEnterEditMode,
|
||||
hasUnsavedChanges,
|
||||
overwriteDashboard,
|
||||
redoLength,
|
||||
|
||||
Reference in New Issue
Block a user