From 8f8ee7dfa23ea12c90c86d18d881d00b198aa5e8 Mon Sep 17 00:00:00 2001
From: Ryan Laurie <30528226+iethree@users.noreply.github.com>
Date: Fri, 3 Mar 2023 13:23:20 -0700
Subject: [PATCH] Test Time Fields in Action Forms (#28627)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* WIP action data type tests
* DRY up the actions tests
* lots of type tests
* make the tests work
* reorganize tests
* WIP action data type tests
* DRY up the actions tests
* lots of type tests
* make the tests work
* reorganize tests
* reduce some flakes
* better time formatting in action forms
* sort out all the timezones 🥴
* update after rebase
* rebase disaster
---
.github/workflows/e2e-tests.yml | 1 +
e2e/support/test_tables.js | 4 +-
e2e/support/test_tables_data.js | 10 +-
.../actions-on-dashboards.cy.spec.js | 114 +++++++++++++++++-
.../ActionParametersInputForm/utils.ts | 6 +-
package.json | 2 +-
6 files changed, 124 insertions(+), 13 deletions(-)
diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml
index 26fa6aea2a2..41f713ef8b5 100644
--- a/.github/workflows/e2e-tests.yml
+++ b/.github/workflows/e2e-tests.yml
@@ -71,6 +71,7 @@ jobs:
MB_SNOWPLOW_AVAILABLE: true
MB_SNOWPLOW_URL: "http://localhost:9090" # Snowplow micro
ELECTRON_EXTRA_LAUNCH_ARGS: "--remote-debugging-port=40500" # deploysentinel
+ TZ: US/Pacific # to make node match the instance tz
strategy:
fail-fast: false
matrix:
diff --git a/e2e/support/test_tables.js b/e2e/support/test_tables.js
index d50fea3d872..444130e51af 100644
--- a/e2e/support/test_tables.js
+++ b/e2e/support/test_tables.js
@@ -84,10 +84,10 @@ export const many_data_types = async dbClient => {
table.boolean("boolean");
table.date("date");
- table.dateTime("datetime");
+ table.dateTime("datetime", { useTz: false });
table.dateTime("datetimeTZ", { useTz: true });
table.time("time");
- table.timestamp("timestamp");
+ table.timestamp("timestamp", { useTz: false });
table.timestamp("timestampTZ", { useTz: true });
table.json("json");
diff --git a/e2e/support/test_tables_data.js b/e2e/support/test_tables_data.js
index 6fcd7a24a40..e6bc021181d 100644
--- a/e2e/support/test_tables_data.js
+++ b/e2e/support/test_tables_data.js
@@ -15,11 +15,11 @@ export const many_data_types_rows = [
decimal: 1.11,
boolean: true,
date: "2020-01-01",
- datetime: "2020-01-01 00:00:00",
- datetimeTZ: "2020-01-01 00:00:00",
- time: "00:00:00",
- timestamp: "2020-01-01 00:00:00",
- timestampTZ: "2020-01-01 00:00:00",
+ datetime: "2020-01-01 08:35:55",
+ datetimeTZ: "2020-01-01 08:35:55",
+ time: "08:35:55",
+ timestamp: "2020-01-01 08:35:55",
+ timestampTZ: "2020-01-01 08:35:55",
json: { a: 10, b: 20, c: [6, 7, 8], d: "foobar" },
jsonb: { a: 20, b: 30 },
enum: "beta",
diff --git a/e2e/test/scenarios/dashboard/actions-on-dashboards.cy.spec.js b/e2e/test/scenarios/dashboard/actions-on-dashboards.cy.spec.js
index 5188b25a63a..2fe419fdb2a 100644
--- a/e2e/test/scenarios/dashboard/actions-on-dashboards.cy.spec.js
+++ b/e2e/test/scenarios/dashboard/actions-on-dashboards.cy.spec.js
@@ -444,9 +444,119 @@ const MODEL_NAME = "Test Action Model";
}
});
});
+
+ it("properly loads and updates date and time fields for implicit update actions", () => {
+ createModelFromTable(TEST_COLUMNS_TABLE);
+ cy.get("@modelId").then(id => {
+ createImplicitAction({
+ kind: "update",
+ model_id: id,
+ });
+ });
+
+ createDashboardWithActionButton({
+ actionName: "Update",
+ idFilter: true,
+ });
+
+ filterWidget().click();
+ addWidgetStringFilter("1");
+
+ clickHelper("Update");
+
+ cy.wait("@executePrefetch");
+
+ const oldRow = many_data_types_rows[0];
+ const newTime = "2020-01-10T01:35:55";
+
+ modal().within(() => {
+ changeValue({
+ fieldName: "Date",
+ fieldType: "date",
+ oldValue: oldRow.date,
+ newValue: newTime.slice(0, 10),
+ });
+
+ changeValue({
+ fieldName: "Datetime",
+ fieldType: "datetime-local",
+ oldValue: oldRow.datetime.replace(" ", "T"),
+ newValue: newTime,
+ });
+
+ changeValue({
+ fieldName: "Time",
+ fieldType: "time",
+ oldValue: oldRow.time,
+ newValue: newTime.slice(-8),
+ });
+
+ changeValue({
+ fieldName: "Timestamp",
+ fieldType: "datetime-local",
+ oldValue: oldRow.timestamp.replace(" ", "T"),
+ newValue: newTime,
+ });
+
+ // only postgres has timezone-aware columns
+ // the instance is in US/Pacific so it's -8 hours
+ if (dialect === "postgres") {
+ changeValue({
+ fieldName: "Datetimetz",
+ fieldType: "datetime-local",
+ oldValue: "2020-01-01T00:35:55",
+ newValue: newTime,
+ });
+
+ changeValue({
+ fieldName: "Timestamptz",
+ fieldType: "datetime-local",
+ oldValue: "2020-01-01T00:35:55",
+ newValue: newTime,
+ });
+ }
+
+ if (dialect === "mysql") {
+ changeValue({
+ fieldName: "Datetimetz",
+ fieldType: "datetime-local",
+ oldValue: oldRow.datetimeTZ.replace(" ", "T"),
+ newValue: newTime,
+ });
+
+ changeValue({
+ fieldName: "Timestamptz",
+ fieldType: "datetime-local",
+ oldValue: oldRow.timestampTZ.replace(" ", "T"),
+ newValue: newTime,
+ });
+ }
+ cy.button("Update").click();
+ });
+
+ cy.wait("@executeAPI");
+
+ queryWritableDB(
+ `SELECT * FROM ${TEST_COLUMNS_TABLE} WHERE id = 1`,
+ dialect,
+ ).then(result => {
+ const row = result.rows[0];
+
+ // the driver adds a time to this date so we have to use .include
+ expect(row.date).to.include(newTime.slice(0, 10));
+ expect(row.time).to.equal(newTime.slice(-8));
+
+ // metabase is smart and localizes these, so all of these are +8 hours
+ const newTimeAdjusted = newTime.replace("T01", "T09");
+ // we need to use .include because the driver adds milliseconds to the timestamp
+ expect(row.datetime).to.include(newTimeAdjusted);
+ expect(row.timestamp).to.include(newTimeAdjusted);
+ expect(row.datetimeTZ).to.include(newTimeAdjusted);
+ expect(row.timestampTZ).to.include(newTimeAdjusted);
+ });
+ });
});
- },
- );
+ });
});
const createModelFromTable = tableName => {
diff --git a/frontend/src/metabase/actions/containers/ActionParametersInputForm/utils.ts b/frontend/src/metabase/actions/containers/ActionParametersInputForm/utils.ts
index 49545a930ce..fdeede63243 100644
--- a/frontend/src/metabase/actions/containers/ActionParametersInputForm/utils.ts
+++ b/frontend/src/metabase/actions/containers/ActionParametersInputForm/utils.ts
@@ -27,13 +27,13 @@ export const formatValue = (
) => {
if (!isEmpty(value)) {
if (inputType === "date" && moment(value).isValid()) {
- return moment(value).utc(false).format("YYYY-MM-DD");
+ return moment(value).utc(true).format("YYYY-MM-DD");
}
if (inputType === "datetime" && moment(value).isValid()) {
- return moment(value).utc(false).format("YYYY-MM-DDTHH:mm:ss");
+ return moment(value).utc(true).format("YYYY-MM-DDTHH:mm:ss");
}
if (inputType === "time") {
- return String(value).replace(/z/gi, "");
+ return moment(`2020-01-10 ${value}`).utc(true).format("HH:mm:ss");
}
}
return value;
diff --git a/package.json b/package.json
index 95b065d9d95..ec901e47bdf 100644
--- a/package.json
+++ b/package.json
@@ -321,7 +321,7 @@
"ci-backend": "clojure -X:dev:ee:ee-dev:drivers:drivers-dev:eastwood && clojure -X:dev:test",
"test-cypress-run": "node ./e2e/runner/run_cypress_tests.js",
"test-cypress-open": "./bin/build-for-test && yarn test-cypress-run --e2e --open",
- "test-cypress-open-qa": "yarn test-qa-dbs:up && QA_DB_ENABLED=true yarn test-cypress-open",
+ "test-cypress-open-qa": "yarn test-qa-dbs:up && QA_DB_ENABLED=true TZ='US/Pacific' yarn test-cypress-open",
"test-cypress-open-no-backend": "E2E_HOST='http://localhost:3000' yarn test-cypress-run --e2e --open",
"test-cypress": "yarn build && ./bin/build-for-test && yarn test-cypress-run",
"test-qa-dbs:up": "docker-compose -f ./e2e/test/scenarios/docker-compose.yml up -d",
--
GitLab