Skip to content
Snippets Groups Projects
Commit 749dc3d3 authored by Allen Gilliland's avatar Allen Gilliland
Browse files

Merge pull request #2234 from metabase/simplify-fk-modeling 

Simplify FK modeling
parents 3073be7a 1ba1f0c0
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
Showing
with 151 additions and 215 deletions
frontend/src/admin/datamodel/components/database/ColumnItem.jsx
+ 2
− 2
View file @ 749dc3d3
......@@ -55,7 +55,7 @@ export default class Column extends Component {
}
onTargetChange(target_field) {
this.props.field.target_id = target_field.id;
this.props.field.fk_target_field_id = target_field.id;
this.props.updateFieldTarget(this.props.field);
}
......@@ -66,7 +66,7 @@ export default class Column extends Component {
<Select
className="TableEditor-field-target block"
placeholder="Select a target"
value={this.props.field.target && _.find(this.props.idfields, (field) => field.id === this.props.field.target.id)}
value={this.props.field.fk_target_field_id && _.find(this.props.idfields, (field) => field.id === this.props.field.fk_target_field_id)}
options={this.props.idfields}
optionNameFn={(field) => field.table.schema && field.table.schema !== "public" ? titleize(humanize(field.table.schema))+"."+field.displayName : field.displayName}
onChange={this.onTargetChange}
......
frontend/src/admin/datamodel/datamodel.controllers.js
+ 4
− 31
View file @ 749dc3d3
......@@ -11,8 +11,8 @@ angular
'metabase.directives',
'metabase.forms'
])
.controller('MetadataEditor', ['$scope', '$route', '$routeParams', '$location', '$q', '$timeout', 'databases', 'Metabase', 'ForeignKey', 'Segment', 'Metric',
function($scope, $route, $routeParams, $location, $q, $timeout, databases, Metabase, ForeignKey, Segment, Metric) {
.controller('MetadataEditor', ['$scope', '$route', '$routeParams', '$location', '$q', '$timeout', 'databases', 'Metabase', 'Segment', 'Metric',
function($scope, $route, $routeParams, $location, $q, $timeout, databases, Metabase, Segment, Metric) {
// inject the React component to be rendered
$scope.MetadataEditor = MetadataEditor;
......@@ -145,15 +145,9 @@ function($scope, $route, $routeParams, $location, $q, $timeout, databases, Metab
// If we are changing the field from a FK to something else, we should delete any FKs present
if (field.target && field.target.id != null && field.special_type !== "fk") {
// we have something that used to be an FK and is now not an FK
// Let's delete its foreign keys
try {
await deleteAllFieldForeignKeys(field);
} catch (e) {
console.warn("Errpr deleting foreign keys", e);
}
// clean up after ourselves
field.target = null;
field.target_id = null;
field.fk_target_field_id = null;
}
// save the field
return $scope.updateField(field);
......@@ -161,30 +155,9 @@ function($scope, $route, $routeParams, $location, $q, $timeout, databases, Metab
$scope.updateFieldTarget = async function(field) {
// This function notes a change in the target of the target of a foreign key
// If there is already a target, we should delete that FK and create a new one
// This is meant to be transitional until we add an FK modify function to the API
// If there was not a target, we should create a new FK
try {
await deleteAllFieldForeignKeys(field);
} catch (e) {
console.warn("Error deleting foreign keys", e);
}
var result = await Metabase.field_addfk({
"db": $scope.databaseId,
"fieldId": field.id,
'target_field': field.target_id,
"relationship": "Mt1"
}).$promise;
field.target = result.destination;
$scope.updateField(field);
};
async function deleteAllFieldForeignKeys(field) {
var fks = await Metabase.field_foreignkeys({ 'fieldId': field.id }).$promise;
return await Promise.all(fks.map(function(fk) {
return ForeignKey.delete({ 'fkID': fk.id }).$promise;
}));
}
$scope.onRetireSegment = async function(segment) {
await Segment.delete(segment).$promise;
MetabaseAnalytics.trackEvent("Data Model", "Retire Segment");
......
frontend/src/services.js
+ 0
− 26
View file @ 749dc3d3
......@@ -395,17 +395,6 @@ CoreServices.factory('Slack', ['$resource', function($resource) {
});
}]);
CoreServices.factory('ForeignKey', ['$resource', '$cookies', function($resource, $cookies) {
return $resource('/api/foreignkey/:fkID', {}, {
delete: {
method: 'DELETE',
params: {
fkID: '@fkID'
}
}
});
}]);
CoreServices.factory('Metabase', ['$resource', '$cookies', 'MetabaseCore', function($resource, $cookies, MetabaseCore) {
return $resource('/api/meta', {}, {
db_list: {
......@@ -583,21 +572,6 @@ CoreServices.factory('Metabase', ['$resource', '$cookies', 'MetabaseCore', funct
fieldId: '@id'
}
},
field_foreignkeys: {
url: '/api/field/:fieldId/foreignkeys',
method: 'GET',
params: {
fieldId: '@fieldId'
},
isArray: true
},
field_addfk: {
url: '/api/field/:fieldId/foreignkeys',
method: 'POST',
params: {
fieldId: '@fieldId'
}
},
dataset: {
url: '/api/dataset',
method: 'POST'
......
resources/migrations/031_add_field_fk_target.yaml 0 → 100644
+ 15
− 0
View file @ 749dc3d3
databaseChangeLog:
- changeSet:
id: 31
author: agilliland
changes:
- addColumn:
tableName: metabase_field
columns:
- column:
name: fk_target_field_id
type: int
constraints:
nullable: true
deferrable: false
initiallyDeferred: false
resources/migrations/liquibase.json
+ 2
− 1
View file @ 749dc3d3
......@@ -28,6 +28,7 @@
{"include": {"file": "migrations/027_add_dashcardseries_table.yaml"}},
{"include": {"file": "migrations/028_add_user_is_qbnewb.yaml"}},
{"include": {"file": "migrations/029_add_pulse_channel_schedule_frame.yaml"}},
{"include": {"file": "migrations/030_add_field_visibility_type.yaml"}}
{"include": {"file": "migrations/030_add_field_visibility_type.yaml"}},
{"include": {"file": "migrations/031_add_field_fk_target.yaml"}}
]
}
src/metabase/api/field.clj
+ 17
− 35
View file @ 749dc3d3
......@@ -6,8 +6,7 @@
[metabase.db.metadata-queries :as metadata]
(metabase.models [hydrate :refer [hydrate]]
[field :refer [Field] :as field]
[field-values :refer [FieldValues create-field-values-if-needed field-should-have-field-values?]]
[foreign-key :refer [ForeignKey] :as fk])))
[field-values :refer [FieldValues create-field-values-if-needed field-should-have-field-values?]])))
(defannotation FieldSpecialType
"Param must be a valid `Field` special type."
......@@ -24,12 +23,6 @@
[symb value :nillable]
(checkp-contains? field/field-types symb (keyword value)))
(defannotation ForeignKeyRelationship
"Param must be a valid `ForeignKey` relationship: one of `1t1` (one-to-one)m
`Mt1` (many-to-one), or `MtM` (many-to-many)."
[symb value :nillable]
(checkp-contains? fk/relationships symb (keyword value)))
(defendpoint GET "/:id"
"Get `Field` with ID."
[id]
......@@ -39,19 +32,29 @@
(defendpoint PUT "/:id"
"Update `Field` with ID."
[id :as {{:keys [special_type visibility_type description display_name], :as body} :body}]
[id :as {{:keys [special_type visibility_type fk_target_field_id description display_name], :as body} :body}]
{special_type FieldSpecialType
visibility_type FieldVisibilityType
display_name NonEmptyString}
(let-404 [field (Field id)]
(write-check field)
(let [special_type (if (contains? body :special_type) special_type (:special_type field))
visibility_type (or visibility_type (:visibility_type field))]
(let [special_type (if (contains? body :special_type) special_type (:special_type field))
visibility_type (or visibility_type (:visibility_type field))
fk_target_field_id (when (= :fk special_type)
;; only let target field be set for :fk type fields,
;; and if it's not in the payload then leave the current value
(if (contains? body :fk_target_field_id)
fk_target_field_id
(:fk_target_field_id field)))]
(check-400 (field/valid-metadata? (:base_type field) (:field_type field) special_type visibility_type))
;; validate that fk_target_field_id is a valid Field within the same database as our field
(when fk_target_field_id
(checkp (exists? Field :id fk_target_field_id) :fk_target_field_id "Invalid target field"))
;; update the Field. start with keys that may be set to NULL then conditionally add other keys if they have values
(check-500 (m/mapply upd Field id (merge {:description description
:special_type special_type
:visibility_type visibility_type}
(check-500 (m/mapply upd Field id (merge {:description description
:special_type special_type
:visibility_type visibility_type
:fk_target_field_id fk_target_field_id}
(when display_name {:display_name display_name}))))
(Field id))))
......@@ -64,27 +67,6 @@
[:distincts (metadata/field-distinct-count field)]]))
(defendpoint GET "/:id/foreignkeys"
"Get `ForeignKeys` whose origin is `Field` with ID."
[id]
(read-check Field id)
(-> (sel :many ForeignKey :origin_id id)
(hydrate [:origin :table] [:destination :table])))
(defendpoint POST "/:id/foreignkeys"
"Create a new `ForeignKey` relationgship with `Field` with ID as the origin."
[id :as {{:keys [target_field relationship]} :body}]
{target_field Required, relationship [Required ForeignKeyRelationship]}
(write-check Field id)
(write-check Field target_field)
(-> (ins ForeignKey
:origin_id id
:destination_id target_field
:relationship relationship)
(hydrate [:origin :table] [:destination :table])))
(defendpoint GET "/:id/values"
"If `Field`'s special type is `category`/`city`/`state`/`country`, or its base type is `BooleanField`, return
all distinct values of the field, and a map of human-readable values defined by the user."
......
src/metabase/api/foreignkey.clj deleted 100644 → 0
+ 0
− 15
View file @ 3073be7a
(ns metabase.api.foreignkey
"/api/foreignkey endpoints."
(:require [compojure.core :refer [DELETE]]
[metabase.api.common :refer :all]
[metabase.db :as db]
(metabase.models [foreign-key :refer [ForeignKey]])))
(defendpoint DELETE "/:id"
"Delete a `ForeignKey`."
[id]
(write-check ForeignKey id)
(db/cascade-delete ForeignKey :id id)
{:success true})
(define-routes)
src/metabase/api/routes.clj
+ 0
− 2
View file @ 749dc3d3
......@@ -8,7 +8,6 @@
[dataset :as dataset]
[email :as email]
[field :as field]
[foreignkey :as fk]
[metric :as metric]
[notify :as notify]
[pulse :as pulse]
......@@ -40,7 +39,6 @@
(context "/dataset" [] (+auth dataset/routes))
(context "/email" [] (+auth email/routes))
(context "/field" [] (+auth field/routes))
(context "/foreignkey" [] (+auth fk/routes))
(GET "/health" [] (if ((resolve 'metabase.core/initialized?))
{:status 200 :body {:status "ok"}}
{:status 503 :body {:status "initializing" :progress ((resolve 'metabase.core/initialization-progress))}}))
......
src/metabase/api/table.clj
+ 10
− 6
View file @ 749dc3d3
......@@ -6,7 +6,6 @@
[metabase.db :refer :all]
(metabase.models [hydrate :refer :all]
[field :refer [Field]]
[foreign-key :refer [ForeignKey]]
[table :refer [Table] :as table])
[metabase.driver :as driver]))
......@@ -80,11 +79,16 @@
(defendpoint GET "/:id/fks"
"Get all `ForeignKeys` whose destination is a `Field` that belongs to this `Table`."
[id]
(read-check Table id)
(let-404 [field-ids (sel :many :id Field :table_id id :visibility_type [not= "retired"])]
(-> (sel :many ForeignKey :destination_id [in field-ids])
;; TODO - it's a little silly to hydrate both of these table objects
(hydrate [:origin [:table :db]] [:destination :table]))))
(let-404 [table (Table id)]
(read-check table)
(let [field-ids (sel :many :id Field :table_id id :visibility_type [not= "retired"])]
(for [origin-field (sel :many Field :fk_target_field_id [in field-ids])]
;; it's silly to be hydrating some of these tables/dbs
{:relationship :Mt1
:origin_id (:id origin-field)
:origin (hydrate origin-field [:table :db])
:destination_id (:fk_target_field_id origin-field)
:destination (hydrate (Field (:fk_target_field_id origin-field)) :table)}))))
(defendpoint POST "/:id/sync"
"Re-sync the metadata for this `Table`."
......
src/metabase/db/migrations.clj
+ 10
− 0
View file @ 749dc3d3
......@@ -170,3 +170,13 @@
(k/where {:id (:id bad-card)}))
(when more
(recur more (+ row-target (:sizeY bad-card)))))))))
;; migrate FK information from old ForeignKey model to Field.fk_target_field_id
(defmigration migrate-fk-metadata
(when (> 1 (:cnt (first (k/select Field (k/aggregate (count :*) :cnt) (k/where (not= :fk_target_field_id nil))))))
(when-let [fks (not-empty (db/sel :many ForeignKey))]
(doseq [{:keys [origin_id destination_id]} fks]
(k/update Field
(k/set-fields {:fk_target_field_id destination_id})
(k/where {:id origin_id}))))))
src/metabase/driver/query_processor/annotate.clj
+ 4
− 5
View file @ 749dc3d3
......@@ -4,8 +4,7 @@
[medley.core :as m]
[metabase.db :refer [sel]]
[metabase.driver.query-processor.interface :as i]
(metabase.models [field :refer [Field], :as field]
[foreign-key :refer [ForeignKey]])
[metabase.models.field :refer [Field]]
[metabase.util :as u]))
;; Fields should be returned in the following order:
......@@ -195,9 +194,9 @@
;; Fetch the ForeignKey objects whose origin is in the returned Fields, create a map of origin-field-id->destination-field-id
([fields fk-ids]
(when (seq fk-ids)
(fk-field->dest-fn fields fk-ids (sel :many :field->field [ForeignKey :origin_id :destination_id]
:origin_id [in fk-ids]
:destination_id [not= nil]))))
(fk-field->dest-fn fields fk-ids (sel :many :field->field [Field :id :fk_target_field_id]
:id [in fk-ids]
:fk_target_field_id [not= nil]))))
;; Fetch the destination Fields referenced by the ForeignKeys
([fields fk-ids id->dest-id]
(when (seq id->dest-id)
......
src/metabase/driver/query_processor/resolve.clj
+ 3
− 3
View file @ 749dc3d3
......@@ -9,7 +9,6 @@
[metabase.driver.query-processor.interface :refer :all]
(metabase.models [database :refer [Database]]
[field :as field]
[foreign-key :refer [ForeignKey]]
[table :refer [Table]])
[metabase.util :as u])
(:import (metabase.driver.query_processor.interface DateTimeField
......@@ -210,8 +209,9 @@
fk-field-id->field-name (sel :many :id->field [field/Field :name], :id [in fk-field-ids], :table_id source-table-id, :special_type "fk")
;; Build a map of join table PK field IDs -> source table FK field IDs
pk-field-id->fk-field-id (sel :many :field->field [ForeignKey :destination_id :origin_id],
:origin_id [in (set (keys fk-field-id->field-name))])
pk-field-id->fk-field-id (sel :many :field->field [field/Field :fk_target_field_id :id],
:id [in (set (keys fk-field-id->field-name))]
:fk_target_field_id [not= nil])
;; Build a map of join table ID -> PK field info
join-table-id->pk-field (let [pk-fields (sel :many :fields [field/Field :id :table_id :name], :id [in (set (keys pk-field-id->fk-field-id))])]
......
src/metabase/driver/sync.clj
+ 3
− 9
View file @ 749dc3d3
......@@ -17,7 +17,6 @@
(metabase.models [common :as common]
[field :refer [Field] :as field]
[field-values :as field-values]
[foreign-key :refer [ForeignKey]]
[table :refer [Table], :as table])
[metabase.util :as u]))
......@@ -474,14 +473,9 @@ infer-field-special-type
(when-let [dest-table-id (sel :one :field [Table :id], :db_id (:db_id table) :name (:name dest-table) :schema (:schema dest-table))]
(when-let [dest-column-id (sel :one :id Field, :table_id dest-table-id, :name dest-column-name, :parent_id nil)]
(log/debug (u/format-color 'green "Marking foreign key '%s.%s' -> '%s.%s'." (:name table) fk-column-name (:name dest-table) dest-column-name))
(when-not (exists? ForeignKey :origin_id fk-column-id, :destination_id dest-column-id)
(ins ForeignKey
:origin_id fk-column-id
:destination_id dest-column-id
;; TODO: do we even care about this?
;:relationship (determine-fk-type {:id fk-column-id, :table (delay table)}) ; fake a Field instance
:relationship :Mt1))
(upd Field fk-column-id :special_type :fk))))))))))
(upd Field fk-column-id
:special_type :fk
:fk_target_field_id dest-column-id))))))))))
;; ## Analyze Table
......
test/metabase/api/database_test.clj
+ 2
− 0
View file @ 749dc3d3
......@@ -246,6 +246,7 @@
:created_at $
:base_type "BigIntegerField"
:visibility_type "normal"
:fk_target_field_id $
:parent_id nil
:values []})
(match-$ (Field (id :categories :name))
......@@ -264,6 +265,7 @@
:created_at $
:base_type "TextField"
:visibility_type "normal"
:fk_target_field_id $
:parent_id nil
:values []})]
:segments []
......
test/metabase/api/field_test.clj
+ 36
− 0
View file @ 749dc3d3
......@@ -57,6 +57,7 @@
:preview_display true
:created_at $
:base_type "TextField"
:fk_target_field_id nil
:parent_id nil})
((user->client :rasta) :get 200 (format "field/%d" (id :users :name))))
......@@ -68,6 +69,7 @@
;; ## PUT /api/field/:id
(defn simple-field-details [field]
(select-keys field [:name :display_name :description :visibility_type :special_type]))
......@@ -118,6 +120,40 @@
updated-val
(simple-field-details (db/sel :one Field :id field-id))]))))))
;; when we set the special-type from :fk to something else, make sure fk_target_field_id is set to nil
(expect
[true
nil]
(tu/with-temp Database [{database-id :id} {:name "Field Test"
:engine :yeehaw
:details {}
:is_sample false}]
(tu/with-temp Table [{table-id :id} {:name "Field Test"
:db_id database-id
:active true}]
(tu/with-temp Field [{field-id1 :id} {:table_id table-id
:name "Target Field"
:base_type :TextField
:field_type :info
:special_type :id
:active true
:preview_display true
:position 1}]
(tu/with-temp Field [{field-id :id} {:table_id table-id
:name "Field Test"
:base_type :TextField
:field_type :info
:special_type :fk
:fk_target_field_id field-id1
:active true
:preview_display true
:position 1}]
(let [original-val (boolean (db/sel :one :field [Field :fk_target_field_id] :id field-id))]
;; unset the :fk special-type
((user->client :crowberto) :put 200 (format "field/%d" field-id) {:special_type :name})
[original-val
(db/sel :one :field [Field :fk_target_field_id] :id field-id)]))))))
;; check that you can't set a field to :timestamp_seconds if it's not of a proper base_type
(expect
["Invalid Request."
......
test/metabase/api/foreignkey_test.clj deleted 100644 → 0
+ 0
− 51
View file @ 3073be7a
(ns metabase.api.foreignkey-test
"tests for /api/foreignkey api endpoints"
(:require [clojure.tools.macro :refer [symbol-macrolet]]
[expectations :refer :all]
[metabase.db :as db]
(metabase [http-client :as http]
[middleware :as middleware])
(metabase.models [database :refer [Database]]
[field :refer [Field]]
[foreign-key :refer :all]
[table :refer [Table]])
[metabase.test.util :as tu]
[metabase.test.data.users :refer :all]))
;; ## /api/segment/* AUTHENTICATION Tests
;; We assume that all endpoints for a given context are enforced by the same middleware, so we don't run the same
;; authentication test on every single individual endpoint
(expect (get middleware/response-unauthentic :body) (http/client :delete 401 "foreignkey/123"))
;; ## DELETE /api/foreignkey/:id
;; test security. requires superuser perms
(expect "You don't have permissions to do that."
((user->client :rasta) :delete 403 "foreignkey/1"))
(expect
[{:success true}
nil]
(tu/with-temp Database [{database-id :id} {:name "FK Test"
:engine :yeehaw
:details {}
:is_sample false}]
(tu/with-temp Table [{table-id :id} {:name "FK Test"
:db_id database-id
:active true}]
(tu/with-temp Field [{field-id :id} {:table_id table-id
:name "FK Test"
:base_type :TextField
:field_type :info
:active true
:preview_display true
:position 1}]
(tu/with-temp ForeignKey [{:keys [id]} {:destination_id field-id
:origin_id field-id
:relationship "whoot"}]
[((user->client :crowberto) :delete 200 (format "foreignkey/%d" id))
(db/sel :one ForeignKey :id id)])))))
test/metabase/api/table_test.clj
+ 18
− 9
View file @ 749dc3d3
......@@ -5,7 +5,6 @@
(metabase [http-client :as http]
[middleware :as middleware])
(metabase.models [field :refer [Field]]
[foreign-key :refer [ForeignKey]]
[table :refer [Table]])
[metabase.test.data :refer :all]
(metabase.test.data [dataset-definitions :as defs]
......@@ -108,6 +107,7 @@
:created_at $
:base_type "BigIntegerField"
:visibility_type "normal"
:fk_target_field_id $
:parent_id nil})
(match-$ (Field (id :categories :name))
{:description nil
......@@ -124,6 +124,7 @@
:created_at $
:base_type "TextField"
:visibility_type "normal"
:fk_target_field_id $
:parent_id nil})]
((user->client :rasta) :get 200 (format "table/%d/fields" (id :categories))))
......@@ -153,6 +154,7 @@
:created_at $
:base_type "BigIntegerField"
:visibility_type "normal"
:fk_target_field_id $
:parent_id nil})
(match-$ (Field (id :categories :name))
{:description nil
......@@ -170,6 +172,7 @@
:created_at $
:base_type "TextField"
:visibility_type "normal"
:fk_target_field_id $
:parent_id nil})]
:field_values {}
:rows 75
......@@ -229,6 +232,7 @@
:created_at $
:base_type "BigIntegerField"
:visibility_type "normal"
:fk_target_field_id $
:parent_id nil})
(match-$ (sel :one Field :id (id :users :last_login))
{:description nil
......@@ -246,6 +250,7 @@
:created_at $
:base_type "DateTimeField"
:visibility_type "normal"
:fk_target_field_id $
:parent_id nil})
(match-$ (sel :one Field :id (id :users :name))
{:description nil
......@@ -263,6 +268,7 @@
:created_at $
:base_type "TextField"
:visibility_type "normal"
:fk_target_field_id $
:parent_id nil})
(match-$ (sel :one Field :table_id (id :users) :name "PASSWORD")
{:description nil
......@@ -280,6 +286,7 @@
:created_at $
:base_type "TextField"
:visibility_type "sensitive"
:fk_target_field_id $
:parent_id nil})]
:rows 15
:updated_at $
......@@ -335,6 +342,7 @@
:created_at $
:base_type "BigIntegerField"
:visibility_type "normal"
:fk_target_field_id $
:parent_id nil})
(match-$ (Field (id :users :last_login))
{:description nil
......@@ -352,6 +360,7 @@
:created_at $
:base_type "DateTimeField"
:visibility_type "normal"
:fk_target_field_id $
:parent_id nil})
(match-$ (Field (id :users :name))
{:description nil
......@@ -369,6 +378,7 @@
:created_at $
:base_type "TextField"
:visibility_type "normal"
:fk_target_field_id $
:parent_id nil})]
:rows 15
:updated_at $
......@@ -439,15 +449,12 @@
;; ## GET /api/table/:id/fks
;; We expect a single FK from CHECKINS.USER_ID -> USERS.ID
(expect-let [checkins-user-field (sel :one Field :table_id (id :checkins) :name "USER_ID")
users-id-field (sel :one Field :table_id (id :users) :name "ID")]
[(match-$ (sel :one ForeignKey :destination_id (:id users-id-field))
{:id $
:origin_id (:id checkins-user-field)
(expect
(let [checkins-user-field (sel :one Field :table_id (id :checkins) :name "USER_ID")
users-id-field (sel :one Field :table_id (id :users) :name "ID")]
[{:origin_id (:id checkins-user-field)
:destination_id (:id users-id-field)
:relationship "Mt1"
:created_at $
:updated_at $
:origin (match-$ checkins-user-field
{:id $
:table_id $
......@@ -462,6 +469,7 @@
:field_type "info"
:active true
:special_type "fk"
:fk_target_field_id $
:created_at $
:updated_at $
:table (match-$ (Table (id :checkins))
......@@ -493,6 +501,7 @@
:field_type "info"
:active true
:special_type "id"
:fk_target_field_id $
:created_at $
:updated_at $
:table (match-$ (Table (id :users))
......@@ -508,7 +517,7 @@
:active true
:id $
:db_id $
:created_at $})})})]
:created_at $})})}])
((user->client :rasta) :get 200 (format "table/%d/fks" (id :users))))
......
test/metabase/driver/generic_sql_test.clj
+ 0
− 1
View file @ 749dc3d3
......@@ -4,7 +4,6 @@
[metabase.driver :as driver]
[metabase.driver.generic-sql :refer :all]
(metabase.models [field :refer [Field]]
[foreign-key :refer [ForeignKey]]
[table :refer [Table]])
[metabase.test.data :refer :all]
[metabase.test.util :refer [resolve-private-fns]]
......
test/metabase/driver/sync_test.clj
+ 25
− 19
View file @ 749dc3d3
......@@ -9,7 +9,6 @@
(metabase.models [database :refer [Database]]
[field :refer [Field]]
[field-values :refer [FieldValues]]
[foreign-key :refer [ForeignKey]]
[hydrate :refer :all]
[table :refer [Table]])
(metabase.test [data :refer :all]
......@@ -94,7 +93,8 @@
:preview_display true,
:display_name "Id",
:base_type :IntegerField,
:visibility_type :normal}
:visibility_type :normal
:fk_target_field_id nil}
{:description nil,
:special_type nil,
:name "studio",
......@@ -105,7 +105,8 @@
:preview_display true,
:display_name "Studio",
:base_type :TextField,
:visibility_type :normal}
:visibility_type :normal
:fk_target_field_id nil}
{:description nil,
:special_type nil,
:name "title",
......@@ -116,7 +117,8 @@
:preview_display true,
:display_name "Title",
:base_type :TextField,
:visibility_type :normal}]}
:visibility_type :normal
:fk_target_field_id nil}]}
{:schema nil
:name "studio"
:display_name "Studio"
......@@ -136,7 +138,8 @@
:preview_display true,
:display_name "Name",
:base_type :TextField,
:visibility_type :normal}
:visibility_type :normal
:fk_target_field_id nil}
{:description nil,
:special_type :id,
:name "studio",
......@@ -147,7 +150,8 @@
:preview_display true,
:display_name "Studio",
:base_type :TextField,
:visibility_type :normal}]}]
:visibility_type :normal
:fk_target_field_id nil}]}]
(tu/with-temp Database [fake-db {:name "sync-test"
:engine :sync-test
:details {}}]
......@@ -181,7 +185,8 @@
:preview_display true,
:display_name "Id",
:base_type :IntegerField,
:visibility_type :normal}
:visibility_type :normal
:fk_target_field_id nil}
{:description nil,
:special_type nil,
:name "studio",
......@@ -192,7 +197,8 @@
:preview_display true,
:display_name "Studio",
:base_type :TextField,
:visibility_type :normal}
:visibility_type :normal
:fk_target_field_id nil}
{:description nil,
:special_type nil,
:name "title",
......@@ -203,7 +209,8 @@
:preview_display true,
:display_name "Title",
:base_type :TextField,
:visibility_type :normal}]}
:visibility_type :normal
:fk_target_field_id nil}]}
(tu/with-temp Database [fake-db {:name "sync-test"
:engine :sync-test
:details {}}]
......@@ -285,27 +292,26 @@
;; Check that Foreign Key relationships were created on sync as we expect
(expect (id :venues :id)
(sel :one :field [ForeignKey :destination_id] :origin_id (id :checkins :venue_id)))
(sel :one :field [Field :fk_target_field_id] :id (id :checkins :venue_id)))
(expect (id :users :id)
(sel :one :field [ForeignKey :destination_id] :origin_id (id :checkins :user_id)))
(sel :one :field [Field :fk_target_field_id] :id (id :checkins :user_id)))
(expect (id :categories :id)
(sel :one :field [ForeignKey :destination_id] :origin_id (id :venues :category_id)))
(sel :one :field [Field :fk_target_field_id] :id (id :venues :category_id)))
;; Check that sync-table! causes FKs to be set like we'd expect
(expect [[:fk true]
[nil false]
[:fk true]]
(expect [{:special_type :fk, :fk_target_field_id true}
{:special_type nil, :fk_target_field_id false}
{:special_type :fk, :fk_target_field_id true}]
(let [field-id (id :checkins :user_id)
get-special-type-and-fk-exists? (fn []
[(sel :one :field [Field :special_type] :id field-id)
(exists? ForeignKey :origin_id field-id)])]
(-> (sel :one :fields [Field :special_type :fk_target_field_id] :id field-id)
(update :fk_target_field_id #(exists? Field :id %))))]
[ ;; FK should exist to start with
(get-special-type-and-fk-exists?)
;; Clear out FK / special_type
(do (del ForeignKey :origin_id field-id)
(upd Field field-id :special_type nil)
(do (upd Field field-id :special_type nil, :fk_target_field_id nil)
(get-special-type-and-fk-exists?))
;; Run sync-table and they should be set again
(let [table (Table (id :checkins))]
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment