Skip to content
Snippets Groups Projects
Unverified Commit bac53b24 authored by adam-james's avatar adam-james Committed by GitHub
Browse files

User Parameter Values Are Unique Per Dashboard (#45064)


* User Parameter Values Are Unique Per Dashboard

Fixes: #43001

Might be related: #44858

User Parameter Values previously stored just the `parameter-id` and `user-id` as data to look up the last used
parameter value. This isn't sufficient as the parameter id is not guaranteed to be unique, in particular this is true
when a dashboard is duplicated: the parameters are copied to the new dashboard without changing the parameter id at
all.

This means that we need to also store the dashboard-id in the user_parameter_value table and use that to update/get
the last used value.

The migration removes existing entries to the user_parameter_value table because I want a non-nullable constraint on
the dashboard_id column, and existing entries will not have a value. The only way to backfill those values would be to
look through every dashboard containing parameters, and for every parameter check for a matching ID. Even if you can
do this, there's no way to disambiguate if the same paramter exists on 2 dashboards anyway, so one of them will be
wrong. I think it's not worth the trouble considering that removing the entries in this table doesn't break anything;
it just becomes a mild inconvenience that a user has to select a filter value again (since the dashboard will use the
default value).

* alter test to check for uniqueness across dashboard

This test makes 2 dashboards with parameters of the same ID and asserts that changing the value on dashboard-a does
not change the value on dashboard-b.

* adjust migration to pass linter rules

* remove the extra rollback on migration

* Update src/metabase/models/user_parameter_value.clj

Co-authored-by: default avatarbryan <bryan.maass@gmail.com>

* Update src/metabase/models/user_parameter_value.clj

Co-authored-by: default avatarbryan <bryan.maass@gmail.com>

* adjust parameters in test so we don't get logged warnings

* put update!/insert! in a transaction to avoid any race conditions

---------

Co-authored-by: default avatarbryan <bryan.maass@gmail.com>
parent 42b167b8
Branches
Tags
No related merge requests found
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment