Skip to content
Snippets Groups Projects
This project is mirrored from https://github.com/metabase/metabase. Pull mirroring updated .
  1. Aug 03, 2021
    • Cam Saul's avatar
      :race_car::race_car::race_car: test runner improvements :race_car::race_car::race_car: (#17286) · e1e815f8
      Cam Saul authored
      * Add some dox
      
      * Rename test-runner & cloverage -> metabase.test-runner & metabase.cloverage-runner
      
      * Move JUnit-related test runner code to separate namespace
      
      * Bespoke JUnit-output code
      
      * Delete unused .lein-classpath file
      
      * Do JUnit writes in a cached thread pool
      
      * Add failing test to make sure JUnit output works as expected...
      
      * Move bad-test to metabase.bad-test so the FAILING tests actually run
      
      * FASTER TEST LOADING :race_car:
      
      * Test CircleCI :system-out
      
      * Final cleanup (hopefully)
      
      * Sort some namespaces; allow initializing :plugins when namespaces get loaded for now.
      
      * Need those [[tags]] in docstrings
      
      * Fix bad docstring
      e1e815f8
    • Dalton's avatar
      Add the ability to verify/unverify questions (#17030) · b7fa2f34
      Dalton authored
      * rmv old bucm icons and remove verified fill color
      
      * add moderation action section to sidebar
      
      * add moderation review icon to the saved question header button
      
      * hide moderation section when is not a moderator
      
      * add UI for ModerationReviewBanner
      
      * Backend for moderation-review
      
      - create table moderation_review. Same as before but also has a
        "most_recent" boolean flag for the most recent moderation for easy
        lookup
      - POST /moderation-review/ . Status can be "verified" or nil
      - must be an admin to post
      - No PUT or edit route yet. Not sure if this is even
        necessary. _MAYBE_ to edit the text, but certainly not for the
        status, ids, etc. If there's to be history, let's build some history
      - Ensure we never have more than 10 reviews. Adding a new review will
        delete the older ones, mark all old ones as not `most_recent`, and
        add the newest one as `most_recent true`
      - Ensure the card actually exists before creating the mod review
      - Since admin only at this time, don't need to check moderate
        permission or view permission
      - When hydrating ensure reviews are ordered by id desc. Should mimic
        the created_at desc
      
      * fix moderation review banner tooltip offset
      
      * disable verification button when already verified
      
      * rmv iconOnly prop because it seems to do nothing
      
      * update getLatestModerationReview to rely on most_recent boolean
      
      * Return 400 on invalid status to post /moderation-review
      
      the schema was using keywords on the left hand side rather than the
      symbols. Required a change to the docstring generator, when it made a
      docstring for enums, it would call (sort (:vs enum)) and need to
      string em.
      
      * Add ModerationReview model to models.clj and copy infra
      
      * hydrate moderation reviews on cards
      
      * clean up + wire up to BE + ensure mod buttons don't show for normal users
      
      * rmv unused moderation redux logic from QuestionDetailsSidebarPanel
      
      * finish writing unit tests for FE
      
      * ensure getIconForReview returns an object
      
      * enable/disable verify button tooltip when unverified/verified
      
      * add e2e tests
      
      * fix tests
      
      * styling tweaks
      
      * more styling on moderationReviewBanner
      
      * add function for abbreviated timestamp
      
      * increase fontsize of timestamp back to 12
      
      * fix tooltip offset
      
      * ensure custom locale is separate from 'en' and not used for other languages
      
      * Deletion moderation reviews when deleting cards
      
      i had actually thought this was a much larger problem. But it turns
      out we almost never delete cards (thanks comment!). And so we won't
      really generate a lot of garbage.
      
      I was worried that since we aren't using actual foreign keys but just
      `moderated_item_type "card"` and `moderated_item_id 2` we would have
      deleted cards with these moderation reviews but that is not the case
      as the cards aren't deleted.
      
      * hide verify disabled button when a question is verified
      
      * update test to use queryByTestId
      
      * Hydrate moderation reviews on cards on ordered cards
      
      * Handle mysql's lack of offset functionality
      
      mysql cannot handle just a `offset` clause, it also needs a limit
      
      clause grammar from
      https://dev.mysql.com/doc/refman/8.0/en/select.html:
      
      [LIMIT {[offset,] row_count | row_count OFFSET offset}]
      
      select id, name from metabase_field offset 5;         -- errors
      select id, name from metabase_field limit 2 offset 5; -- works
      
      Since our numbers are so small here there is no worry and just do the
      offset in memory rather than jump through hoops for different dbs.
      
      * Batch hydrate moderation reviews
      
      * Don't let /api/user/:userId failure conceal moderation banner
      
      * fix moderation cy tests
      
      * work around possible bug in toucan hydration
      
      dashboards hydrate ordered cards
      (hydrate [:ordered_cards [:card :moderation_reviews] :series])
      
      Ordered_cards are dashboard_cards which have an optional card_id. But
      toucan hydration doesn't filter out the nils as they go down. It seems
      toucan returns a nil card, and then when hydrating the
      moderation_review, passes the collection of all "cards" including the
      nil ones into the hydration function for moderation_reviews
      
      This feels like a bug to me
      
      * Cleanup moderation warnings
      
      * Docstring in moderation review
      
      * include hoisted moderated_status on cards in collections api
      
      * Expect unverified in test :wrench:
      
      
      
      Co-authored-by: default avatardan sutton <dan@dpsutton.com>
      Co-authored-by: default avatarMaz Ameli <maz@metabase.com>
      Co-authored-by: default avataralxnddr <alxnddr@gmail.com>
      b7fa2f34
  2. Aug 02, 2021
  3. Jul 30, 2021
  4. Jul 29, 2021
    • Cam Saul's avatar
      Run Eastwood linter against test namespaces. Remove reflection-warning script (#17193) · 89382bae
      Cam Saul authored
      * Fix some Eastwood failures
      
      * Fix a lot of Eastwood errors now that it runs against test namespaces [ci skip]
      
      * Run Eastwood against test namespaces [WIP]
      
      * Bump Eastwood version to fix some errors in Eastwood itself
      
      * Fix another lint warning
      
      * Fix test failure
      
      * More test fixes :wrench:
      
      * Fix all the warnings!
      
      * Test fix :wrench:
      
      * Bump Eastwood GH action timeout to 20 minutes (!)
      89382bae
  5. Jul 28, 2021
  6. Jul 23, 2021
    • Jeff Evans's avatar
      Implement JDBC based Presto driver (#16194) · 80b46b1f
      Jeff Evans authored
      Implement JDBC based Presto driver
      
      Adding new Presto JDBC driver using the PrestoDB JDBC driver from `https://github.com/prestodb/presto`
      
      Marking the old Presto driver as being `superseded-by` the new one
      
      Pulling out common Presto code into new presto-common driver (modeled after the relationship between, ex: `googleanalytics` and `google)`
      
      Putting common QP/HoneySQL logic into the new (abstract) :presto-common driver
      
      Updating :presto driver to extend from the new common driver and only adding HTTP/REST API related methods
      
      Adding implementation of Presto JDBC driver, named :presto-jdbc, extending from :presto-common and :sql-jdbc
      
      Using com.facebook.presto/presto-jdbc as underlying JDBC driver dependency (since this is explicitly for Presto clusters, as opposed to Trino)
      
      Adapting code from the existing Presto driver where appropriate
      
      Adding new dependency-satisfied? implementation for :env-var, to allow for a plugin to require an env var be set, and making the Presto JDBC driver depend on that (specifically: `mb-enable-presto-jdbc-driver`)
      
      Adding CircleCI configuration to run against the newer Presto (0.254) Docker image
      
      Adding explicit ordering in a few tests where it was missing
      
      Fixing presto-type->base-type (timestamps were being synced as :type/Time because the regex pattern was wrong)
      
      Add tx/format-name test implementation for :presto-jdbc to lowercase table name
      
      Make modified test Oracle friendly
      
      Fixing bug parsing the `[:zone :time]` case within `metabase.util.date-2.parse/parse-with-formatter`; the offset is nil so it can't be passed directly in this case, so use the `standard-offset` fn (which was moved from `date-2` to `common` to get a standard offset for that zone
      
      Fixing more test failures by adding explicit ordering
      
      Changing sync to check whether the driver supports foreign keys before attempting to sync those (since drivers might throw an exception if attempting to check)
      
      Moving some common test dataset functionality between :presto and :presto-jdbc to a new test.data ns for :presto-common
      
      Adding HoneySQL form for :count-where, since we have to explicitly give a higher precision Decimal in order for Presto to not reduce the precision in results
      
      Put limit within subquery for `expression-using-aggregation-test` (since ordering from subquery is not guaranteed in Presto)
      
      Adding impls for ->prepared-substitution to handle substitutions for native query params
      
      Adding HoneySQL impls for `mod` (to do as "mod(x,y)" and `timestamp` (since it's a function with no parens to invoke) functions
      
      Adding various `sql.qp/date` impls that use the `AT TIME ZONE` operator to account for report tz, and make bucketing tests happy
      80b46b1f
    • Jeff Evans's avatar
      Add support for driver deprecation (#17028) · 3ddcb263
      Jeff Evans authored
      
      # Backend changes
      
      Introducing new `superseded-by` property to plugin manifest YAML, which will indicate the driver that is to eventually replace this one (and will drive UI/UX behavior).  If a driver declares this property, then it's considered to be deprecated in favor of the specified one.
      
      Adding top level `test_modules` directory (with the same structure as modules) for the sole purpose of module/plugin testing of YAML files, which will not be included with the driver build
      
      Updating `driver-plugin-manifest` to look for the new `test_modules` directory in addition to `modules`, when loading the driver manifest
      
      # Frontend changes
      
      Calculate `supersededBy` and supersedes maps from the "superseded-by" property for each engine
      
      Change the options for the engine field to use a function to dynamically show the legacy driver if allowed by rules (either the new driver is selected, or the legacy driver was already selected for an existing DB, or the driver is not superseded by anything)
      
      Add new `DriverWarning` component to show these warnings based on supersede status
      
      Co-authored-by: default avatarAnton Kulyk <kuliks.anton@gmail.com>
      3ddcb263
  7. Jul 21, 2021
  8. Jul 20, 2021
  9. Jul 19, 2021
    • dpsutton's avatar
      offset is a reserved word in mariadb as of 10.6 (#17101) · 64062e87
      dpsutton authored
      * offset is a reserved word in mariadb as of 10.6
      
      - https://mariadb.com/kb/en/reserved-words/
      - https://github.com/MariaDB/server/commit/299b935320
      
      I think it is related to https://jira.mariadb.org/browse/MDEV-23908
      "Implement SELECT ... OFFSET ... FETCH ..."
      
      * Handle new names for utf8 in mariadb
      
      https://mariadb.com/kb/en/unicode/
      
      - utf8: Until MariaDB 10.5, this was a UTF-8 encoding using one to
      three bytes per character. Basic Latin letters, numbers and
      punctuation use one byte. European and Middle East letters mostly fit
      into 2 bytes. Korean, Chinese, and Japanese ideographs use 3-bytes. No
      supplementary characters are stored. From MariaDB 10.6, utf8 is an
      alias for utf8mb3, but this can changed to ut8mb4 by changing the
      default value of the old_mode system variable.
      
      - utf8mb3: UTF-8 encoding using one to three bytes per
      character. Basic Latin letters, numbers and punctuation use one
      byte. European and Middle East letters mostly fit into 2
      bytes. Korean, Chinese, and Japanese ideographs use 3-bytes. No
      supplementary characters are stored. Until MariaDB 10.5, this was an
      alias for utf8. From MariaDB 10.6, utf8 is by default an alias for
      utf8mb3, but this can changed to ut8mb4 by changing the default value
      of the old_mode system variable.
      64062e87
  10. Jul 15, 2021
  11. Jul 14, 2021
    • Nemanja Glumac's avatar
      Remove db paging (#16990) (#17054) · c43a7828
      Nemanja Glumac authored
      
      I thought (and assured dpsutton, lol) that the default behavior of the pagination spliced in was just not paginating, if there wasn't a limit passed in. Unfortunately, it appears that I tested this on the endpoint without pagination spliced in... so the default behavior isn't that way, it paginates if you don't pass in stuff.
      
      So rip it out of the DB endpoint (only) for now and put it back in when we paginate
      
      Authored-by: default avatarHowon Lee <hlee.howon@gmail.com>
      c43a7828
    • dpsutton's avatar
      Smartscalars (#17020) · 90435d6d
      dpsutton authored
      * Smartscalars
      
      * smartscalar html styling
      
      * Docstring, ns, prevent top-level trs/tru
      90435d6d
  12. Jul 09, 2021
    • dpsutton's avatar
      Render/scalar (#16955) · c04e9a63
      dpsutton authored
      * Scalar rendering as text in slack
      
      * Fix tests for pulse
      
      * Tests for slack scalars
      c04e9a63
  13. Jul 08, 2021
  14. Jul 06, 2021
  15. Jun 30, 2021
    • dpsutton's avatar
      Chain filters cpu utilization (#16817) · 8e684e3f
      dpsutton authored
      * Breadth first search on graph for chain filters
      
      - abandons at max-depth and looks in breadth first fashion rather than
      descending as far as possible on each possible path
      
      * Add loop detection
      
      not actually fooled by loops since we abandon after a certain path
      length. But will remove some false starts before they have to be
      killed by the path length check.
      
      * Add seen optimization of nodes
      
      We reach nodes in the shortest path possible, so keep a list of nodes
      visited and don't revisit them: we got to them as fast as possible,
      and once there, there's no new nodes that we can see from that vantage
      point.
      
      This allows a serious speedup and should hopefully fully resolve the
      cpu issues seen by some.
      
      * alignment
      8e684e3f
  16. Jun 28, 2021
  17. Jun 23, 2021
    • dpsutton's avatar
      Cache results dont affect average execution time (#16720) · d4121d04
      dpsutton authored
      * Only save successful-query-execution if it wasnt a cached result
      
      the only thing this function does is update the rolling average query
      execution time
      
      * Failing tests for cache execution stats
      
      These tests fail, but they shouldn't.
      
      The save-execution information should not be called twice, and the
      average execution duration should remain the same. What's happening is
      that it is getting called twice just in the cache call, and only one
      of those has the new key `:metrics/ignore-execution-time`. This
      remains a mystery to me
      
      * Don't call completion arity from step arity
      
      the `(rf acc)` call violated the terms of transduction. If it does
      need to be there, perhaps it could be used with reduced
      
      wary of #12207 (which fixes #12165) and seeing if that crops up again.
      
      - i had introduced :metrics/ignore-execution-time but this was because
      i thought that we were running a simple query (limit 1) to get col
      metadata. Now that i know we were just being a bit careless with `(rf
      acc)` this extra information is no longer needed, and we can just
      react to cached data.
      
      * hoist closing paren
      d4121d04
    • Cam Saul's avatar
      :wave: `get-id` (#16730) · f3847cb8
      Cam Saul authored
      f3847cb8
    • Noah Moss's avatar
  18. Jun 22, 2021
  19. Jun 18, 2021
    • Howon Lee's avatar
      Fix issue with the collections appearing in the list page (#16562) · 5156d881
      Howon Lee authored
      I thought the collections appearing in the list page problem was solved, but this turned out to be a bug with the ttest. Upon actual inspection the lists remained.
      
      To fix this without changing all sorts of crap that depended upon collections api contract (we futzed with the output but the futzing of the output is transparent to the frontend because of the entity thing they got going on), needed a way to elicit null response from collections endpoint. shoved one in. then renamed it because the first name, "dummy", makes it all sound kind of jank, so 'no-model' for explicitly asking for no model without handing in an empty set, which turns into the set of all models.
      
      This is an 80% unjankening. Full unjankening would entail whacking this special case.
      5156d881
    • Anton Kulyk's avatar
      Fix can't unarchive question without data access (#16590) · fd9d2816
      Anton Kulyk authored
      * Fix can't unarchive question without data access
      
      * Test API checks perms when unarchiving a card
      fd9d2816
  20. Jun 17, 2021
  21. Jun 16, 2021
  22. Jun 15, 2021
    • dpsutton's avatar
      Don't compute only on non-zero scores (#16598) · 0f5de39b
      dpsutton authored
      * Don't compute only on non-zero scores
      
      I was removing a helper function when refactoring the scoring. That
      had a filter on :score?. I'm not sure why i translated this to only
      interpret the denominator of scoring to only include scores that were
      positive. This is obviously wrong.
      
      Also, previously, all databases would be returned in the archived
      query which seems obviously wrong. Databases cannot be archived and so
      should never appear when searching for archived items.
      
      * Add some tests
      
      - had been asserting that ee-score = os-score if not in an official
      collection. This is actually hard to do under the current scheme, and
      actually not important. The important bit is that the relative
      ordering is the same. It had been adding 0 score 2 weight previously,
      and removing all scores with a score of 0. This created a bad way to
      score, as something with 1/2, 0/3, 0/4, 0/10 (net score 1/2=0.5) would
      appear higher than something with 1/2, 1/3, 1/4, 3/10 (net score
      6/19=0.315) and it really shouldn't. The former really should
      be (1/19=0.05). And since we are adding in a denominator of
      2 (official collection weight) all of the scores are actually smaller,
      but that's ok, as they are all proportionately smaller.
      0f5de39b
Loading