This project is mirrored from https://github.com/metabase/metabase.
Pull mirroring updated .
- Jun 21, 2022
-
-
metamben authored
* Add collection_preview field to report_card table * Convert bits from MySQL/MariaDB union queries into booleans
-
Cal Herries authored
* Add reply-to setting for emails * Add reply-to to SMTPSettings * Change setting UI to match spec * Remove WIP comment * Add email-from-name setting * Support comma delimited list of emails for reply-to setting * Remove extra require * Remove console.logging * Improve the setting descriptions * update email settings * For now, limit reply-to field to one email * Change back SMTPSettings to closed map * Change test to only one reply-to email * Fix SMTPSettings reply-to type * Fix email-api-test * Fix e2e email setup * Add more checks in e2e tests * Fix smoketest Co-authored-by:
Aleksandr Lesnenko <alxnddr@gmail.com>
-
Noah Moss authored
* first pass at updating parameter validation to include notebook cards and match template tags by either ID or name * fix existing tests * fix more tests * new test * more refactor and another test * Update src/metabase/query_processor/card.clj Co-authored-by:
Ngoc Khuat <qn.khuat@gmail.com> * update names to ngoc's suggestions * Update test/metabase/query_processor/card_test.clj Co-authored-by:
Cam Saul <1455846+camsaul@users.noreply.github.com> * Update src/metabase/query_processor/card.clj Co-authored-by:
Cam Saul <1455846+camsaul@users.noreply.github.com> * specific error msg for missing id on request param Co-authored-by:
Ngoc Khuat <qn.khuat@gmail.com> Co-authored-by:
Cam Saul <1455846+camsaul@users.noreply.github.com>
-
- Jun 20, 2022
-
-
metamben authored
Fix compilation of temporal arithmetic for BigQuery and Mongo 5+ * Mongo 4 doesn't support $dateAdd so the generated filters result in an exception. * Support adding field to interval too (time intervals were not allowed in the first place of an addition) * Support temporal arithmetic with more than two operands for Mongo
-
- Jun 16, 2022
-
-
dpsutton authored
MBQL goes through a different path. This is only for native queries. Also the diff is huge. Ignoring whitespace shows a very modest diff: ```diff "month" {:unit-range month-range :to-period t/months} + "quarter" {:unit-range relative-quarter-range + :to-period (comp t/months (partial * 3))} "year" {:unit-range year-range :to-period t/years}}) ... "past2months~" {:end "2016-06-30", :start "2016-04-01"} "past13months" {:end "2016-05-31", :start "2015-05-01"} + "past2quarters" {:end "2016-03-31", :start "2015-10-01"} + "past2quarters~" {:end "2016-06-30", :start "2015-10-01"} "past1years" {:end "2015-12-31", :start "2015-01-01"} "past1years~" {:end "2016-12-31", :start "2015-01-01"} ``` Helpful Testing Strategies -------------------------- Sample DB ========= `select id, created_at from orders where {{created_at}}` and set a field filter of type "Date Filter" Custom Table ============ Create a table that has entries in the middle of each quarter. ```sql esting=# create table quarters (id serial primary key not null, description text, t timestamptz); CREATE TABLE testing=# insert into quarters (description, t) values ('Q1 2022', '2022-02-01'), ('Q2 2022', '2022-05-01'), ('Q3 2022', '2022-08-01'), ('Q4 2022', '2022-11-01'); INSERT 0 4 testing=# select * from quarters; id | description | t ----+-------------+------------------------ 1 | Q1 2022 | 2022-02-01 00:00:00-06 2 | Q2 2022 | 2022-05-01 00:00:00-05 3 | Q3 2022 | 2022-08-01 00:00:00-05 4 | Q4 2022 | 2022-11-01 00:00:00-05 (4 rows) ``` Before this change ------------------ > Cannot invoke "clojure.lang.IFn.invoke(Object)" because "to_period" is null (note, if you cannot reproduce this its because you haven't gotten https://github.com/metabase/metabase/pull/23346 in your local version which ensured errors always appear as errors on the frontend. java 11 has no message for NPE so the Frontend missed it was an error and displayed no results as if the query had succeeded) This was the case for the following scenarios: - "last1quarters" - "last1quarters~" - "thisquarter" - "next1quarters" - "next1quarters~" where the ~ means to include the current quarter. After this change ----------------- Running the queries against the custom table I made (current time is Jun 15, Q2) - "last1quarters": "Q1 2022" "February 1, 2022, 6:00 AM" - "last1quarters~": "Q1 2022" "February 1, 2022, 6:00 AM" | "Q2 2022" "May 1, 2022, 5:00 AM" - "thisquarter": "Q2 2022" "May 1, 2022, 5:00 AM" - "next1quarters" "Q3 2022" "August 1, 2022, 5:00 AM" - "next1quarters~": "Q2 2022" "May 1, 2022, 5:00 AM" | "Q3 2022" "August 1, 2022, 5:00 AM" And of course added tests into the matrix for the date parsing.
-
Howon Lee authored
Pursuant to #22967. Previously the notion of the identifier in json-query in postgres assumed a singular identifier at the top-level of the Identifier record. This is not always the case in complicated binning scenarios - the identifier could also just comprise an entire tree of Identifier record maps with the actual Identifier we're looking to turn into the JSON query in some leaf somewhere. Therefore, the json-query's determination of what the identifier is now walks that identifier record tree and replaces the identifiers that have JSON field semantics with the reification of ToSql from json-query. Previous attempt at solution hacked together something at the json-query level, changing the reification, but that only worked for one level down, instead of just walking the Identifier tree and therefore getting stuff at arbitrary tree depth. And we do get nontrivial tree depth in the bucketing in #22967.
-
- Jun 15, 2022
-
-
dpsutton authored
* Ensure :error is present for frontend The frontend checks for a string at "error" to determine if there's an error or not. Any error that lacks a message (`(.getMessage e) = nil`) will report as a successful query with no results. This was hard to reproduce at first because java 14+ added a nice error message to NPEs in [JEP-358](https://openjdk.org/jeps/358). ```java ;; java 11 jshell> HashSet x = null; x ==> null jshell> x.contains(3); | Exception java.lang.NullPointerException | at (#2:1) jshell> ;; java 17 jshell> HashSet x = null; x ==> null jshell> x.contains(3); | Exception java.lang.NullPointerException: Cannot invoke "java.util.HashSet.contains(Object)" because "REPL.$JShell$11.x" is null | at (#4:1) ``` ```clojure ;; java 17 catch-exceptions=> (let [x nil] (x 3)) Execution error (NullPointerException) at metabase.query-processor.middleware.catch-exceptions/eval130385 (REPL:41). Cannot invoke "clojure.lang.IFn.invoke(Object)" because "x" is null ;; java 11 catch-exceptions=> (let [x nil] (x 3)) Execution error (NullPointerException) at metabase.driver.sql.parameters.substitution/eval118508 (REPL:17). null ``` Here are the responses to the FE edited to the relevant bits: ```javascript // objects are edited for clarity/brevity: // from java-11 { "error_type": "qp", "status": "failed", "class": "class java.lang.NullPointerException", "stacktrace": [...], "error": null, // <---- the FE ignores all of the other data and only sees this } // vs java-17 { "error_type": "qp", "status": "failed", "class": "class java.lang.NullPointerException", "stacktrace": [...], "error": "Cannot invoke \"clojure.lang.IFn.invoke(Object)\" because \"to_period\" is null",, } ``` Also, note that we were still logging > ERROR middleware.catch-exceptions :: Error processing query: null because we were looking for `(:error format-exception)` instead of `(:error formatted-exception)`. `format-exception` is a multimethod and lookup on non-associative things will happily return nil. * Tests
-
- Jun 14, 2022
-
-
metamben authored
* Use (str ...) forms to translate multi-line messages * Make H2 error messages refer to the correct field
-
- Jun 13, 2022
-
-
adam-james authored
* Migration to allow 'null' first and last names for users This is part of improving how we handle user names, related to SSO, but we also have an opportunity to make login via email simpler as well -> just loosen the requirement for names. * When only last name is given, :common_name should not have a space * WIP making optional first/last names safe in the backend. Trying to fix up some assumptions about first/last name keys in backend implementations. * Only add `:common_name` key when non-nil * Adjust setup tests to pass if use first/last name are nil * Fix up setup tests to work with null first/last names. * User API endpoints altered to handle nil names * Remove name validation test since they can be nil now * Alter JWT/SAML tests for nil user names * Remove "unknown" default in SAML * Add tests to make sure users are updating appropriately * use good naming convention in local function Co-authored-by:
Cam Saul <1455846+camsaul@users.noreply.github.com> * Simplify truthy check in test Co-authored-by:
Cam Saul <1455846+camsaul@users.noreply.github.com> * Simplify truthy check in test Co-authored-by:
Cam Saul <1455846+camsaul@users.noreply.github.com> * Indentation fix * Fix up syntax issue from github merge. * Fix missed function name changes in test * Clean up how sets are built up in the PUT user/:id endpoint * Better implementation for update-user-name * Avoid relying on 'vals' since order of keys is not guaranteed * Fixing little things, clarification in a comment * I'm tired... missed an obvious thing here. oops * indentation and clarify some testing * Change post-select implementation slightly * expected status is now an optional arg no dynamic var binding needed * 'f' is not optional, so we take it with first, then args are rest * simplify destructuring Co-authored-by:
Cam Saul <1455846+camsaul@users.noreply.github.com>
-
Jeff Bruemmer authored
-
Braden Shepherdson authored
Define identity-hash for fairly robust de-duplication when deserializing This is a fallback for fully robust de-duplication based on `entity_id` fields. All serialized models should support identity-hash, and there is a test to enforce that.
-
Ngoc Khuat authored
* add API to fetch and search card parameters * remove code to support linked filters * remove unecessary data in tests * fix from Noah's comments * some chain-filter -> param-values
-
- Jun 10, 2022
-
-
Noah Moss authored
* allow str form as the first argument to deferred-tru * add tests and update doc strings * migrate more setting descriptions * Handle multiline translation sources ```clojure enumerate=> x (deferred-tru (str "Whether an introductory modal should be shown after the next database connection is added. " "Defaults to false if any non-default database has already finished syncing for this instance.")) enumerate=> (form->string-for-translation x) "Whether an introductory modal should be shown after the next database connection is added. Defaults to false if any non-default database has already finished syncing for this instance." ``` We get the form from `(g/grasp <file> ::translate)` which returns the `x`. And then simply pick through it for either a string literal or a call to `(str <literal>+)` Co-authored-by:
dan sutton <dan@dpsutton.com>
-
Howon Lee authored
Previously we defaulted to integer and float type in all cases for json unfolding type determination, even with bigints and bigdecimals. Now if we encounter a bignum in json unfolding we call it a bignum.
-
- Jun 09, 2022
-
-
Noah Moss authored
* clear Slack cache last-updated value when resetting cache * use clear-channel-cache! function on invalid tokens as well
-
- Jun 08, 2022
-
-
dpsutton authored
* Save query executions for cached queries Previously had addressed https://github.com/metabase/metabase/pull/16720/ because we were including cache hit query executions in the running query execution time. IE, queries take 60 seconds to run but the cache hit takes 100ms. Our average thus would be (60s + 100ms)/2 ~ 30 seconds and the query duration falls below the cache threshold so we abandon the cache until the average creeps up back up again. Original issue: https://github.com/metabase/metabase/issues/15432 So the first fix was a poor one and it skipped recording the query execution at all which was way too heavy handed. ```clojure (when-not (:cached acc) (save-successful-query-execution! (:cached acc) execution-info @row-count)) ``` This approach takes the more considered approach: skip updating average execution time but record the query-execution. ```clojure ;; conditionally save average execution time (when-not (:cache_hit query-execution) (query/save-query-and-update-average-execution-time! query query-hash running-time)) ;; unconditionally (modulo having a context) save the query execution (if-not context (log/warn (trs "Cannot save QueryExecution, missing :context")) (db/insert! QueryExecution (dissoc query-execution :json_query))) ``` One question ------------ I'm unsure about one change here. What are the implications of not updating this execution time. There are two operations involved: ensuring there is a `Query` (table query) entry for a query and updating its average execution time. I'm assuming that since a query is cached it has a query table entry. But this code creates the Query if it doesn't exist and then updates its average execution time. It's not clear to me if we do in fact want to ensure a Query entry exists. But it feels harmless both in the case it is missing and also that it most likely isn't missing since we have a cached query and when it was cached this entry would have been created or updated. But adding a note just in case. * Fix test distinguish between the calls to save query execution and the calls to update average execution duration. The former should be called twice (for the uncached query and the cached query run) while the latter should only be called on the first run (aka, uncached results affect query duration stats, cached runs do not affect it) * Checksums are no more metadata became editable for models and the checksum really didn't add any value, just asserted that it had not been changed.
-
dpsutton authored
-
metamben authored
We have already had support for server authentication based on custom certificates. This change adds support for authenticating the client based on custom client key and certificate.
-
- Jun 07, 2022
-
-
Howon Lee authored
Do trivial identity for passthrough of symbols for nested field column instead of default processing (#23136) Should whack both #23026 and #23027. Previous (default) behavior of the reducible query that gets the limited contents of the JSON to break out the nested field columns is to lowercase identifiers. This is root cause of #23026 and #23027. But we want the proper case of those identifiers in order to be modifying the query correctly when we query the JSON. So we set the reducible query to just pass through the identifiers instead of default behavior.
-
- Jun 06, 2022
- Jun 03, 2022
-
-
Ngoc Khuat authored
* dashboard api to get params value should return 404 instead of 500 when param not found * on the 2nd thought, let's return 400 * update the tests too * wording
-
Ngoc Khuat authored
* add parameters to report_card and card APIs * make sure migration can work with existing cards and add migration tests * remove debug code * When parameters is empty, return template tag parameters * update schema message * update migraiton test name * minor changes in test * fix failing tests * Always use card.parameters in PublicQuestion * Add defaults and readd isEmpty check * Default to undefined/use template tags in PublicQuestion... * parameters should be in the writableProperties * Native query card: construct parameters from template tags * Separate the generation of parameter id * Add parameter_mappings to report_card (#23003) * add parameter_mappings to report_card * fix from Noah's comments * fix from Noah's comments * Update a parameter from an updated template tag * Correct the parameters construction * Also add `parameter_mappings` to writableProperties * CI test: bust the npm module cache * Revert "CI test: bust the npm module cache" This reverts commit 5a327b616f0220f43a90f7f871e0bd877ffa6f47. Co-authored-by:
Dalton Johnson <daltojohnso@users.noreply.github.com> Co-authored-by:
Ariya Hidayat <ariya@metabase.com>
-
Howon Lee authored
Adds option to turn JSON column unfolding off. This does not constitute an actual fix of boat161 but flamber wanted it
-
- Jun 01, 2022
-
-
Braden Shepherdson authored
-
- May 31, 2022
-
-
Braden Shepherdson authored
That is: collection, dimension, metric, native_query_snippet, pulse, report_card, report_dashboard, report_dashcard, segment, timeline Notably that doesn't include database, table, or field, since those all have external unique IDs that are used instead.
-
- May 30, 2022
-
-
Ngoc Khuat authored
remove param_values hydration keys
-
- May 27, 2022
-
-
Howon Lee authored
Limit the length of the row for JSON parsing. This was the simplest way to make another attack at #22636.
-
- May 25, 2022
-
-
Case Nelson authored
* Add can-manage to databases to help front-end * Only return persisted true when the state agrees * Add event for persisted-info to catch new models and turn on peristence when appropriate * Fix bad threading * Add comment about the case being detected to add the persisted-info * Fix pre-existing bug in expand-db-details where a function was used as the condition
-
metamben authored
* Indicate field responsible for database error * Implement new interface * Make error showing logic a little easier to understand * Show individual field error in database form * Add backend tests for connection errors when adding a database * Add new form error component * Make database form error message pretty * Make main database form error message field bold * Change create account database form according to feedback * Fix failed E2E tests from UI changed. * Make it easier to tree-shake "lodash" * Change according to PR review * Cleanup + remove FormError (to be included in a separate PR) Co-authored-by:
Mahatthana Nomsawadi <mahatthana.n@gmail.com>
-
- May 24, 2022
-
-
Case Nelson authored
* Add anchor time to persistence schedule Introduces a new public-setting persisted-model-refresh-anchor-time, representing the time to begin the refresh job. Defaults to midnight "00:00". Also peg the cron jobs to run in the first of: reporting timezone, system timezone, or UTC. This means that if a user needs the refresh to be anchored consistently to a certain time in a certain place, they need to update reporting timezone to match. * Add tests for anchor time * Force anchor-time to midnight if refresh hours is less than 6 * Need to check that hours was passed in * Fix ns lint * Add tests to ensure timezone is being set on trigger
-
Noah Moss authored
-
- May 23, 2022
-
-
dpsutton authored
we assert that the schedule gets randomized and it is no longer one of the default schedules. We need to make sure that it cannot return one of the default schedules in that case :). Low probability (1 in 58) but with 28 test suites run for each commit it starts to add up and becomes Flaky™
-
dpsutton authored
Settings require db access so cannot be called at read/require time. Not until we have initialized the db are setting values possible to be read.
-
- May 19, 2022
-
-
Cam Saul authored
* `defsetting` setter functions should end in `!` * Fix typo * Update .clj-kondo/hooks/metabase/models/setting.clj * Fix clj-kondo for Toucan defmodel not emitting a docstring * Remove `^:private` metadata on a couple of Settings since it makes Eastwood fussy
-
Ngoc Khuat authored
-
- May 18, 2022
-
-
Jeff Bruemmer authored
-
Cam Saul authored
-
Jeff Bruemmer authored
-
- May 17, 2022
-
-
Bryan Maass authored
* bumps outdated deps versions to be current * un-upgrade h2 and jetty * un-upgrade joda-time and kixi/stats * drop Java 8 support in circle CI config - things that used to rely on be-tests-java-8-ee now rely on be-tests-java-11-ee * remove java 8 from github health check matrix * revert toucan to 1.17.0 * revert mariadb java client to 2.7.5 * Back to 18, and handle new behavior toucan used to just look in *.models.<model-name> for models and just give up apparently. I made a feature that toucan will look in a model registry to create models rather than using the convention https://github.com/metabase/toucan/commit/762ad69defc1477423fa9423e9320ed318f7cfe7 but now we're getting errors in these tests about maps vs models. ```clojure revision_test.clj:154 Check that revisions+details pulls in user info and adds description expected: [#metabase.models.revision.RevisionInstance{:is_reversion false, :is_creation false, :message nil, :user {:id 1, :common_name "Rasta Toucan", :first_name "Rasta", :last_name "Toucan"}, :diff {:o1 nil, :o2 {:name "Tips Created by Day", :serialized true}}, :description nil}] actual: (#metabase.models.revision.RevisionInstance{:description nil, :is_creation false, :is_reversion false, :user {:id 1, :first_name "Rasta", :last_name "Toucan", :common_name "Rasta Toucan"}, :message nil, :diff {:o1 nil, :o2 #metabase.models.revision_test.FakedCardInstance{:name "Tips Created by Day", :serialized true}}}) ``` The only difference here is `:o2` is a `metabase.models.revision_test.FakedCardInstance` but still has the same keys, `:name`, and `:serialized`. So all is well, we're just able to make the model. So a few different fixes. Some are use `partial=` which doesn't care about record/map distinction. Some are just make the model, and some are turning them into maps for revision strings (which more closely mimics what the real revision stuff does): ```clojure (defn default-diff-map "Default implementation of `diff-map` which simply uses clojures `data/diff` function and sets the keys `:before` and `:after`." [_ o1 o2] (when o1 (let [[before after] (data/diff o1 o2)] {:before before :after after}))) (defn default-diff-str "Default implementation of `diff-str` which simply uses clojures `data/diff` function and passes that on to `diff-string`." [entity o1 o2] (when-let [[before after] (data/diff o1 o2)] (diff-string (:name entity) before after))) ``` So all in all this change impacts nothing in the app itself, because those models follow convention and are correct in `metabase.models.<model-name>` and are thus "modelified": ```clojure revision-test=> (revision/revisions Card 1) [#metabase.models.revision.RevisionInstance{:is_creation true, :model_id 1, :id 1, :is_reversion false, :user_id 2, :timestamp #object[java.time.OffsetDateTime "0x77e037f" "2021-10-28T15:10:19.828539Z"], :object #metabase.models.card.CardInstance {:description nil, :archived false, :collection_position nil, :table_id 5, :database_id 2, :enable_embedding false, :collection_id nil, :query_type :query, :name "ECVYUHSWQJYMSOCIFHQC", :creator_id 2, :made_public_by_id nil, :embedding_params nil, :cache_ttl 1234, :dataset_query {:database 2, :type :query, :query {:source-table 5, :aggregation [[:count]]}}, :id 1, :display :scalar, :visualization_settings {:global {:title nil}}, :dataset false, :public_uuid nil}, :message nil, :model "Card"}] ``` so the model/no-model is just arbitrary distinction in the test. All of them in the actual app are turned into models: ```clojure (defn- do-post-select-for-object "Call the appropriate `post-select` methods (including the type functions) on the `:object` this Revision recorded. This is important for things like Card revisions, where the `:dataset_query` property needs to be normalized when coming out of the DB." [{:keys [model], :as revision}] ;; in some cases (such as tests) we have 'fake' models that cannot be resolved normally; don't fail entirely in ;; those cases (let [model (u/ignore-exceptions (db/resolve-model (symbol model)))] (cond-> revision ;; this line would not find a model previously for FakedCard and ;; just return the map. But now the registry in toucan _finds_ the ;; model defintion and returns the model'd map model (update :object (partial models/do-post-select model))))) (u/strict-extend (class Revision) models/IModel (merge models/IModelDefaults {:types (constantly {:object :json}) :pre-insert pre-insert :pre-update (fn [& _] (throw (Exception. (tru "You cannot update a Revision!")))) :post-select do-post-select-for-object})) ``` * try using mssql-jdbc 10.2.1.jre11 - Important that we get off the jre8 version * various fixes that needn't be reverted * Revert "various fixes that needn't be reverted" This reverts commit 2a820db0743d0062eff63366ebe7bc78b852e81f. * go back to using circle ci's java 11 docker image * java-16 (?) -> java-17 * Revert "go back to using circle ci's java 11 docker image" This reverts commit b9b14c535a689f701d7e2541081164288c988c4e. Co-authored-by:
dan sutton <dan@dpsutton.com>
-