fix(sqllab): normalize changedOn timestamp (#24513)

superset-frontend/packages/superset-ui-core/src/time-format/index.ts

@@ -37,5 +37,6 @@ export { default as smartDateDetailedFormatter } from './formatters/smartDateDet
 export { default as smartDateVerboseFormatter } from './formatters/smartDateVerbose';
 
 export { default as normalizeTimestamp } from './utils/normalizeTimestamp';
+export { default as denormalizeTimestamp } from './utils/denormalizeTimestamp';
 
 export * from './types';

superset-frontend/packages/superset-ui-core/src/time-format/utils/denormalizeTimestamp.ts

@@ -0,0 +1,28 @@
+/*
+ * 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.
+ */
+
+import { TS_REGEX } from './normalizeTimestamp';
+
+export default function normalizeTimestamp(value: string): string {
+  const match = value.match(TS_REGEX);
+  if (match) {
+    return `${match[1]}T${match[2]}`;
+  }
+  return value;
+}

superset-frontend/packages/superset-ui-core/src/time-format/utils/normalizeTimestamp.ts

@@ -17,7 +17,7 @@
  * under the License.
  */
 
-const TS_REGEX = /(\d{4}-\d{2}-\d{2})[\sT](\d{2}:\d{2}:\d{2}\.?\d*).*/;
+export const TS_REGEX = /(\d{4}-\d{2}-\d{2})[\sT](\d{2}:\d{2}:\d{2}\.?\d*).*/;
 
 export default function normalizeTimestamp(value: string): string {
   const match = value.match(TS_REGEX);

superset-frontend/packages/superset-ui-core/test/time-format/utils/denormalizeTimestamp.test.ts

@@ -0,0 +1,43 @@
+/*
+ * 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.
+ */
+
+import denormalizeTimestamp from '../../../src/time-format/utils/denormalizeTimestamp';
+
+test('denormalizeTimestamp should normalize typical timestamps', () => {
+  expect(denormalizeTimestamp('2023-03-11 08:26:52.695 UTC')).toEqual(
+    '2023-03-11T08:26:52.695',
+  );
+  expect(
+    denormalizeTimestamp('2023-03-11 08:26:52.695 Europe/Helsinki'),
+  ).toEqual('2023-03-11T08:26:52.695');
+  expect(denormalizeTimestamp('2023-03-11T08:26:52.695 UTC')).toEqual(
+    '2023-03-11T08:26:52.695',
+  );
+  expect(denormalizeTimestamp('2023-03-11T08:26:52.695')).toEqual(
+    '2023-03-11T08:26:52.695',
+  );
+  expect(denormalizeTimestamp('2023-03-11 08:26:52')).toEqual(
+    '2023-03-11T08:26:52',
+  );
+});
+
+test('denormalizeTimestamp should return unmatched timestamps as-is', () => {
+  expect(denormalizeTimestamp('abcd')).toEqual('abcd');
+  expect(denormalizeTimestamp('03/11/2023')).toEqual('03/11/2023');
+});

superset-frontend/src/SqlLab/components/SouthPane/SouthPane.test.jsx

@@ -24,6 +24,7 @@ import SouthPane from 'src/SqlLab/components/SouthPane';
 import '@testing-library/jest-dom/extend-expect';
 import { STATUS_OPTIONS } from 'src/SqlLab/constants';
 import { initialState, table, defaultQueryEditor } from 'src/SqlLab/fixtures';
+import { denormalizeTimestamp } from '@superset-ui/core';
 
 const mockedProps = {
   queryEditorId: defaultQueryEditor.id,
@@ -61,7 +62,7 @@ const store = mockStore({
     queries: {
       LCly_kkIN: {
         cached: false,
-        changed_on: new Date().toISOString(),
+        changed_on: denormalizeTimestamp(new Date().toISOString()),
         db: 'main',
         dbId: 1,
         id: 'LCly_kkIN',
@@ -71,7 +72,7 @@ const store = mockStore({
       },
       lXJa7F9_r: {
         cached: false,
-        changed_on: new Date(1559238500401).toISOString(),
+        changed_on: denormalizeTimestamp(new Date(1559238500401).toISOString()),
         db: 'main',
         dbId: 1,
         id: 'lXJa7F9_r',
@@ -80,7 +81,7 @@ const store = mockStore({
       },
       '2g2_iRFMl': {
         cached: false,
-        changed_on: new Date(1559238506925).toISOString(),
+        changed_on: denormalizeTimestamp(new Date(1559238506925).toISOString()),
         db: 'main',
         dbId: 1,
         id: '2g2_iRFMl',
@@ -89,7 +90,7 @@ const store = mockStore({
       },
       erWdqEWPm: {
         cached: false,
-        changed_on: new Date(1559238516395).toISOString(),
+        changed_on: denormalizeTimestamp(new Date(1559238516395).toISOString()),
         db: 'main',
         dbId: 1,
         id: 'erWdqEWPm',

superset-frontend/src/SqlLab/fixtures.ts

@@ -19,7 +19,12 @@
 import sinon from 'sinon';
 import * as actions from 'src/SqlLab/actions/sqlLab';
 import { ColumnKeyTypeType } from 'src/SqlLab/components/ColumnElement';
-import { DatasourceType, QueryResponse, QueryState } from '@superset-ui/core';
+import {
+  DatasourceType,
+  denormalizeTimestamp,
+  QueryResponse,
+  QueryState,
+} from '@superset-ui/core';
 import { ISaveableDatasource } from 'src/SqlLab/components/SaveDatasetModal';
 
 export const mockedActions = sinon.stub({ ...actions });
@@ -708,7 +713,7 @@ export const mockdatasets = [...new Array(3)].map((_, i) => ({
   changed_by_name: 'user',
   kind: i === 0 ? 'virtual' : 'physical', // ensure there is 1 virtual
   changed_by: 'user',
-  changed_on: new Date().toISOString(),
+  changed_on: denormalizeTimestamp(new Date().toISOString()),
   database_name: `db ${i}`,
   explore_url: `/explore/?datasource_type=table&datasource_id=${i}`,
   id: i,

superset-frontend/src/SqlLab/reducers/sqlLab.js

@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { QueryState, t } from '@superset-ui/core';
+import { normalizeTimestamp, QueryState, t } from '@superset-ui/core';
 import getInitialState from './getInitialState';
 import * as actions from '../actions/sqlLab';
 import { now } from '../../utils/dates';
@@ -614,9 +614,10 @@ export default function sqlLabReducer(state = {}, action) {
           (state.queries[id].state !== QueryState.STOPPED &&
             state.queries[id].state !== QueryState.FAILED)
         ) {
-          const changedOn = Date.parse(changedQuery.changed_on);
+          const changedOn = normalizeTimestamp(changedQuery.changed_on);
-          if (changedOn > queriesLastUpdate) {
+          const timestamp = Date.parse(changedOn);
-            queriesLastUpdate = changedOn;
+          if (timestamp > queriesLastUpdate) {
+            queriesLastUpdate = timestamp;
           }
           const prevState = state.queries[id]?.state;
           const currentState = changedQuery.state;

superset-frontend/src/SqlLab/reducers/sqlLab.test.js

@@ -18,7 +18,6 @@
  */
 import sqlLabReducer from 'src/SqlLab/reducers/sqlLab';
 import * as actions from 'src/SqlLab/actions/sqlLab';
-import { now } from 'src/utils/dates';
 import { table, initialState as mockState } from '../fixtures';
 
 const initialState = mockState.sqlLab;
@@ -273,6 +272,8 @@ describe('sqlLabReducer', () => {
     });
   });
   describe('Run Query', () => {
+    const DENORMALIZED_CHANGED_ON = '2023-06-26T07:53:05.439';
+    const CHANGED_ON_TIMESTAMP = 1687765985439;
     let newState;
     let query;
     beforeEach(() => {
@@ -280,7 +281,8 @@ describe('sqlLabReducer', () => {
       query = {
         id: 'abcd',
         progress: 0,
-        startDttm: now(),
+        changed_on: DENORMALIZED_CHANGED_ON,
+        startDttm: CHANGED_ON_TIMESTAMP,
         state: 'running',
         cached: false,
         sqlEditorId: 'dfsadfs',
@@ -292,7 +294,8 @@ describe('sqlLabReducer', () => {
         query: {
           id: 'abcd',
           progress: 0,
-          startDttm: now(),
+          changed_on: DENORMALIZED_CHANGED_ON,
+          startDttm: CHANGED_ON_TIMESTAMP,
           state: 'running',
           cached: false,
           sqlEditorId: 'dfsadfs',
@@ -328,6 +331,19 @@ describe('sqlLabReducer', () => {
       newState = sqlLabReducer(newState, removeQueryAction);
       expect(Object.keys(newState.queries)).toHaveLength(0);
     });
+    it('should refresh queries when polling returns new results', () => {
+      newState = sqlLabReducer(
+        {
+          ...newState,
+          queries: { abcd: {} },
+        },
+        actions.refreshQueries({
+          abcd: query,
+        }),
+      );
+      expect(newState.queries.abcd.changed_on).toBe(DENORMALIZED_CHANGED_ON);
+      expect(newState.queriesLastUpdate).toBe(CHANGED_ON_TIMESTAMP);
+    });
     it('should refresh queries when polling returns empty', () => {
       newState = sqlLabReducer(newState, actions.refreshQueries({}));
     });