rusackas commented on code in PR #45589:
URL: https://github.com/apache/superset/pull/45589#discussion_r4366140737
##########
superset-frontend/src/features/home/ChartTable.tsx:
##########
@@ -232,10 +242,10 @@ function ChartTable({
hasPerm={hasPerm}
showThumbnails={showThumbnails}
bulkSelectEnabled={bulkSelectEnabled}
- refreshData={refreshData}
+ refreshData={handleRefreshData}
Review Comment:
Added a Mine-tab delete test: the DELETE is held open, `onActivityRefresh`
is asserted not called, then called once after it resolves. Reverting to the
raw `refreshData` makes it fail.
##########
superset-frontend/src/features/home/ActivityTable.tsx:
##########
@@ -155,7 +157,7 @@ export default function ActivityTable({
isMounted = false;
setIsFetchingEditedCards(false);
};
- }, [activeChild, user.userId]);
+ }, [activeChild, user.userId, refreshKey]);
Review Comment:
Added a Home-level test that selects Edited, refreshes with a changed
edited-objects response, and asserts the removed card disappears. Dropping
`refreshKey` from the deps makes it fail.
##########
superset-frontend/src/pages/Home/Home.refresh.test.tsx:
##########
@@ -0,0 +1,178 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+// Imported first so the antd mock is defined before anything requires 'antd'.
+import { mockAntdWithDesktopBreakpoint } from 'spec/helpers/mobileTestUtils';
+import fetchMock from 'fetch-mock';
+import { SupersetClient } from '@superset-ui/core';
+import {
+ render,
+ screen,
+ userEvent,
+ waitFor,
+} from 'spec/helpers/testing-library';
+import Welcome from 'src/pages/Home';
+import { LocalStorageKeys, setItem } from 'src/utils/localStorageHelpers';
+
+const chartsEndpoint = 'glob:*/api/v1/chart/?*';
+const dashboardsEndpoint = 'glob:*/api/v1/dashboard/?*';
+const savedQueryEndpoint = 'glob:*/api/v1/saved_query/?*';
+const recentActivityEndpoint = 'glob:*/api/v1/log/recent_activity/*';
+
+jest.mock('antd', () => mockAntdWithDesktopBreakpoint());
+
+// The delete flow lives in the tables; Home only receives `onActivityRefresh`.
+// Stub the tables with a button that simulates a completed delete.
+jest.mock('src/features/home/DashboardTable', () => ({
+ __esModule: true,
+ default: ({ onActivityRefresh }: { onActivityRefresh?: () => void }) => (
+ <button type="button" onClick={onActivityRefresh}>
+ simulate delete
+ </button>
+ ),
+}));
+jest.mock('src/features/home/ChartTable', () => ({
+ __esModule: true,
+ default: () => <div />,
+}));
+
+const user = {
+ username: 'alpha',
+ firstName: 'alpha',
+ lastName: 'alpha',
+ createdOn: '2016-11-11T12:34:17',
+ userId: 5,
+ email: '[email protected]',
+ isActive: true,
+ isAnonymous: false,
+ permissions: {},
+ roles: { sql_lab: [['can_read', 'SavedQuery']] },
+};
+
+const recentItem = (title: string, id: number) => ({
+ action: 'dashboard',
+ item_title: title,
+ item_type: 'dashboard',
+ item_url: `/superset/dashboard/${id}/`,
+ time: 1741644942130 + id,
+ time_delta_humanized: 'a day ago',
+});
+
+let recentResponder: () => unknown;
+
+beforeAll(() => {
+ SupersetClient.configure().init();
+});
+
+beforeEach(() => {
+ localStorage.clear();
+ // Start with the Recents and Dashboards panels expanded.
+ setItem(LocalStorageKeys.HomepageCollapseState, ['recents', 'dashboards']);
+ fetchMock.get(chartsEndpoint, { result: [] });
+ fetchMock.get(dashboardsEndpoint, { result: [] });
+ fetchMock.get(savedQueryEndpoint, { result: [] });
+ fetchMock.get(recentActivityEndpoint, () => recentResponder());
+});
+
+afterEach(() => {
+ fetchMock.removeRoutes();
+ fetchMock.clearHistory();
+});
+
+const renderHome = async () => {
+ render(<Welcome user={user} />, { useRedux: true, useRouter: true });
+ return screen.findByRole('button', { name: 'simulate delete' });
+};
+
+test('delete refetches recents without resetting the selected tab', async ()
=> {
+ recentResponder = () => ({ result: [recentItem('Viewed dash', 1)] });
+ const deleteButton = await renderHome();
+ expect(await screen.findByText('Viewed dash')).toBeInTheDocument();
+ expect(fetchMock.callHistory.calls(recentActivityEndpoint)).toHaveLength(1);
+ const dashboardCallsBefore =
+ fetchMock.callHistory.calls(dashboardsEndpoint).length;
+
+ // Switch away from the default (Viewed) tab; a refresh must not undo this.
+ await userEvent.click(screen.getByText('Created'));
+ await waitFor(() =>
+ expect(screen.queryByText('Viewed dash')).not.toBeInTheDocument(),
+ );
+
+ // Drop the persisted tab so only the in-memory selection survives; a
+ // refresh that re-ran the first-load tab restore would flip back to Viewed.
+ localStorage.removeItem(LocalStorageKeys.HomepageActivityFilter);
+
+ recentResponder = () => ({ result: [recentItem('Fresh dash', 2)] });
+ await userEvent.click(deleteButton);
+
+ await waitFor(() =>
+
expect(fetchMock.callHistory.calls(recentActivityEndpoint)).toHaveLength(2),
+ );
+ await waitFor(() =>
+ expect(
+ fetchMock.callHistory.calls(dashboardsEndpoint).length,
+ ).toBeGreaterThan(dashboardCallsBefore),
+ );
+ // Still on the Created tab: neither the old nor the refreshed Viewed items
+ // are shown, and the collapse panels were not reset.
Review Comment:
Reworked it to collapse Dashboards (open by default) while Recents stays
open, clear the persisted state, then refresh and assert both panels keep their
state. Removing the `refreshKey === 0` guard now fails it. The fallback
defaults leave Recents collapsed, so Recents alone could not catch a reset.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
