From cb7ad5bc95ad505f92269b9ce816fa21c55ac411 Mon Sep 17 00:00:00 2001
From: Simon Belak <simon@metabase.com>
Date: Sat, 29 Aug 2020 12:53:25 +0200
Subject: [PATCH] Field ordering: ignore non-active fields when validating
(#13182)
---
src/metabase/models/table.clj | 5 ++-
test/metabase/models/table_test.clj | 56 +++++++++++++++++++++++++++++
2 files changed, 60 insertions(+), 1 deletion(-)
create mode 100644 test/metabase/models/table_test.clj
diff --git a/src/metabase/models/table.clj b/src/metabase/models/table.clj
index 01a8e8a7bc9..c03184de550 100644
--- a/src/metabase/models/table.clj
+++ b/src/metabase/models/table.clj
@@ -102,7 +102,10 @@
(defn- valid-field-order?
"Field ordering is valid if all the fields from a given table are present and only from that table."
[table field-ordering]
- (= (db/select-ids Field :table_id (u/get-id table)) (set field-ordering)))
+ (= (db/select-ids Field
+ :table_id (u/get-id table)
+ :active true)
+ (set field-ordering)))
(defn custom-order-fields!
"Set field order to `field-order`."
diff --git a/test/metabase/models/table_test.clj b/test/metabase/models/table_test.clj
new file mode 100644
index 00000000000..a1f7b972308
--- /dev/null
+++ b/test/metabase/models/table_test.clj
@@ -0,0 +1,56 @@
+(ns metabase.models.table-test
+ (:require [clojure.java.jdbc :as jdbc]
+ [clojure.test :refer :all]
+ [metabase
+ [sync :as sync]
+ [test :as mt]]
+ [metabase.models.table :as table]
+ [metabase.test.data.one-off-dbs :as one-off-dbs]))
+
+(deftest valud-field-order?-test
+ (testing "A valid field ordering is a set IDs of all active fields in a given table"
+ (is (#'table/valid-field-order? (mt/id :venues)
+ [(mt/id :venues :name)
+ (mt/id :venues :category_id)
+ (mt/id :venues :latitude)
+ (mt/id :venues :longitude)
+ (mt/id :venues :price)
+ (mt/id :venues :id)])))
+ (testing "Field ordering is invalid if some fields are missing"
+ (is (false? (#'table/valid-field-order? (mt/id :venues)
+ [(mt/id :venues :category_id)
+ (mt/id :venues :latitude)
+ (mt/id :venues :longitude)
+ (mt/id :venues :price)
+ (mt/id :venues :id)]))))
+ (testing "Field ordering is invalid if some fields are from a differnt table"
+ (is (false? (#'table/valid-field-order? (mt/id :venues)
+ [(mt/id :venues :name)
+ (mt/id :venues :category_id)
+ (mt/id :venues :latitude)
+ (mt/id :venues :longitude)
+ (mt/id :venues :price)
+ (mt/id :checkins :id)]))))
+ (testing "Only active fields should be considerd when checking field order"
+ (one-off-dbs/with-blank-db
+ (doseq [statement [ ;; H2 needs that 'guest' user for QP purposes. Set that up
+ "CREATE USER IF NOT EXISTS GUEST PASSWORD 'guest';"
+ ;; Keep DB open until we say otherwise :)
+ "SET DB_CLOSE_DELAY -1;"
+ ;; create table & load data
+ "DROP TABLE IF EXISTS \"BIRDS\";"
+ "CREATE TABLE \"BIRDS\" (\"SPECIES\" VARCHAR PRIMARY KEY, \"EXAMPLE_NAME\" VARCHAR);"
+ "GRANT ALL ON \"BIRDS\" TO GUEST;"
+ (str "INSERT INTO \"BIRDS\" (\"SPECIES\", \"EXAMPLE_NAME\") VALUES "
+ "('House Finch', 'Marshawn Finch'), "
+ "('California Gull', 'Steven Seagull'), "
+ "('Chicken', 'Colin Fowl');")]]
+ (jdbc/execute! one-off-dbs/*conn* [statement]))
+ (sync/sync-database! (mt/db))
+ (is (#'table/valid-field-order? (mt/id :birds)
+ [(mt/id :birds :species)
+ (mt/id :birds :example_name)]))
+ (jdbc/execute! one-off-dbs/*conn* ["ALTER TABLE \"BIRDS\" DROP COLUMN \"EXAMPLE_NAME\";"])
+ (sync/sync-database! (mt/db))
+ (is (#'table/valid-field-order? (mt/id :birds)
+ [(mt/id :birds :species)])))))
--
GitLab