sadpandajoe commented on code in PR #45560:
URL: https://github.com/apache/superset/pull/45560#discussion_r4342746900
##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditor.test.tsx:
##########
@@ -213,6 +213,84 @@ test('can delete columns', async () => {
await waitFor(() => expect(limitedProps.onChange).toHaveBeenCalled());
});
+const renderWithOneColumn = async () => {
+ const baseProps = createProps();
+ const limitedProps = {
+ ...baseProps,
+ onChange: jest.fn(),
+ datasource: {
+ ...baseProps.datasource,
+ table_name: 'Vehicle Sales +',
+ columns: baseProps.datasource.columns
+ .slice(0, 1)
+ .map(column => ({ ...column })),
+ },
+ };
+ fastRender(limitedProps);
+ await dismissDatasourceWarning();
+ await userEvent.click(await screen.findByTestId('collection-tab-Columns'));
+ return limitedProps;
+};
+
+const syncColumnsFromSource = async (
+ testProps: Awaited<ReturnType<typeof renderWithOneColumn>>,
+) => {
+ const [{ column_name, type, is_dttm }] = testProps.datasource.columns;
+ fetchMock.removeRoute(DATASOURCE_ENDPOINT);
+ fetchMock.get(DATASOURCE_ENDPOINT, [{ column_name, type, is_dttm }], {
+ name: DATASOURCE_ENDPOINT,
+ });
+ testProps.onChange.mockClear();
+ await userEvent.click(screen.getByText(/sync columns from source/i));
+ await waitFor(() =>
+ expect(fetchMock.callHistory.called(DATASOURCE_ENDPOINT)).toBe(true),
+ );
+ await waitFor(() => expect(testProps.onChange).toHaveBeenCalled());
+ return testProps.onChange.mock.lastCall![0].columns;
+};
+
+test('syncing columns keeps column edits made in the session', async () => {
+ const testProps = await renderWithOneColumn();
+ const [toggle] = await screen.findAllByRole('button', {
+ name: /expand row/i,
+ });
+ await userEvent.click(toggle);
+ fireEvent.change(screen.getByPlaceholderText('Label'), {
+ target: { value: 'edited_label' },
+ });
+ await waitFor(() =>
+ expect(testProps.onChange.mock.lastCall?.[0].columns[0].verbose_name).toBe(
+ 'edited_label',
+ ),
+ );
+
+ const columns = await syncColumnsFromSource(testProps);
Review Comment:
The fix reads the columns through refs so a sync request still in flight
sees edits made after it started, but this test finishes the label edit before
clicking sync. If someone later reads `databaseColumns` straight from the
callback closure instead of the ref, the edit is already in state when the
callback is rebuilt, so this test still passes while an edit made during the
request would be reverted again. Could a variant defer the metadata response,
click sync, edit the label, then resolve the response and assert the emitted
column keeps that label and its id?
--
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]
