Skip to content
Snippets Groups Projects
This project is mirrored from https://github.com/metabase/metabase. Pull mirroring updated .
  1. Aug 24, 2022
  2. Aug 23, 2022
    • Bryan Maass's avatar
      [Actions] disallow is_write outside of actions api (#24712) · e91f4fce
      Bryan Maass authored
      * disallow writable queries outside of actions api
      
      - handles cards, dashboards, embedding and pulses
      - tests for disallowing writable query execution
      - fix namespace decl
      
      * fix ns decl
      
      * use `false?` to avoid any nil cards causing a 405
      
      * check that the card is there
      
      * Revert "check that the card is there"
      
      This reverts commit 5b56e2d00291bd21c0461a0567537769b5f73a83.
      
      * Revert "use `false?` to avoid any nil cards causing a 405"
      
      This reverts commit 72569d353ee5830f37d1a14c46289afbe511841b.
      Unverified
      e91f4fce
    • Cam Saul's avatar
      [Toucan 2 Prep] Replace the `IObjectPermissions` protocol with multimethods (#24917) · 241f2179
      Cam Saul authored
      * [Toucan 2 prep] Don't invoke Toucan models as functions
      
      * Some fixes
      
      * Test fixes
      
      * Test fix
      
      * [Toucan 2 prep] Don't call `type` or `class` on Toucan models
      
      * Test fixes
      
      * More test fixes :wrench:
      
      * Replace perms protocol with multimethods; derive models from perms policy keywords
      
      * Test fixes :wrench:
      
      * Appease Eastwood
      
      * Fix errors now that App has been merged in
      
      * Empty commit to trigger CI
      Unverified
      241f2179
    • metamben's avatar
      Hydrate app_id for collections with apps attached (#24950) · 02eb6982
      metamben authored
      * Hydrate app_id for collections with apps attached
      
      Addresses #24941, part of #24861
      Unverified
      02eb6982
    • Cal Herries's avatar
      BE fix for 23505: Models should not allow variables (#24902) · 0fddfe48
      Cal Herries authored
      
      * Add validation to card PUT and POST endpoints to invalidate saving models with variables
      
      * Fix validation for snippets and saved question CTEs
      
      * Move validation to pre-update and pre-insert instead of API endpoints
      
      * Fix lint warning
      
      * Replace hard-coded id
      
      * Optimize DB calls in card pre-update
      
      * Accept formatting suggestion
      
      Co-authored-by: default avatarNgoc Khuat <qn.khuat@gmail.com>
      
      Co-authored-by: default avatarNgoc Khuat <qn.khuat@gmail.com>
      Unverified
      0fddfe48
    • Mahatthana (Kelvin) Nomsawadi's avatar
      Show data point values on static viz (#24813) · df02d46c
      Mahatthana (Kelvin) Nomsawadi authored
      * Remove non-existent route
      
      * Make some internal routes easier to access
      
      * Show data point value on static bar chart
      
      * Show data point values on static line chart
      
      * Show data point values on static area chart
      
      * Remove unused function
      
      * Improve multiple series area chart data point value legibility
      
      * Render stacked area chart data point values
      
      * Automatically format long values with compact format
      
      * Sample data point values on dense chart
      
      * Improve bar chart values legibility
      Unverified
      df02d46c
  3. Aug 22, 2022
  4. Aug 18, 2022
    • Aleksandr Lesnenko's avatar
      fix static viz waterfall colors (#24852) · 8c225418
      Aleksandr Lesnenko authored
      
      * fix static viz waterfall colors
      
      * tests
      
      * Pass colors into StaticChart as third argument
      
      This might not be the best design, so changes are welcome, but I added this to properly consider the case where the
      application-colors are different, but the chart has no viz-settings: application-colors were being ignored (as they
      were passed into options, but not pulled out of options into the colors arg).
      
      * review
      
      Co-authored-by: default avatarAdam James <adam.vermeer2@gmail.com>
      Unverified
      8c225418
  5. Aug 17, 2022
    • Cal Herries's avatar
      Fix for #24715: "Remember Me" functionality doesn't work (#24744) · 4532d07e
      Cal Herries authored
      * Separate setting the timeout cookie's expires attribute from the session cookie's max-age
      
      * Add back public-settings/session-cookies tests and add docstrings
      
      * Fix setting cookies with full-app-embedding
      
      * Add docstring
      
      * Change logout to always delete the session
      
      * Remove extra code
      
      * Change session-cookie-name from multimethod to simple case expression
      
      * Refactor: move use-permanent-cookies? closer to usage
      
      * Fix FE unit test
      Unverified
      4532d07e
  6. Aug 16, 2022
    • dpsutton's avatar
      Preserve metadata visibility-type (#24754) · 4c7791eb
      dpsutton authored
      * Preserve metadata visibility-type
      
      Was being saved but not preserved and merged correctly. There are two
      points we need to preserve this information: in annotating the result
      metadata, we need to "flow" them. This is the combine metadata function
      which uses `preserve-keys`.
      
      And finally that stuff needs to be selected to be saved in
      `results_metadata`. That plucks off the `cols` map from the query run,
      merges it with insights and saves it.
      
      Missing this metadata in either location means we drop it: we have to
      flow it through the metadata and then ensure we select it for the final
      saving.
      
      * mt/mbql-query wants a symbol not a keyword
      
      * Update test to include visibility type when present
      Unverified
      4c7791eb
    • Ngoc Khuat's avatar
      Fix incorrect hash for Advanced fieldvalues (#24740) · 8a3e16b0
      Ngoc Khuat authored
      * fix incorrectly hash a fieldvalues for field that is not the field we use to define sandbox rule
      
      * appease clj-kondo
      
      * make the doc clearer
      
      * remove the _id destructring
      Unverified
      8a3e16b0
  7. Aug 15, 2022
  8. Aug 12, 2022
    • Cam Saul's avatar
      Enable Kondo for tests part 2: enable `:unused-binding` linter and fix warnings (#24748) · adf45182
      Cam Saul authored
      * Fix some small things
      
      * Add Kondo to deps.edn to be able to debug custom hooks from REPL
      
      * Fix macroexpansion hook for with-temp* without values
      
      * Test config (WIP)
      
      * More misc fixes
      
      * Disable :inline-def for tests
      
      * More misc fixes
      
      * Fix $ids and mbql-query kondo hooks.
      
      * Fix with-temporary-setting-values with namespaced symbols
      
      * More misc fixes
      
      * Fix the rest of the easy ones
      
      * Fix hook for mt/dataset
      
      * Horrible hack to work around https://github.com/clj-kondo/clj-kondo/issues/1773 . Custom linter for mbql-query macro
      
      * Fix places calling mbql-query with a keyword table name
      
      * Fix the last few errors in test/
      
      * Fix errors in enterprise/test and shared/test
      
      * Fix driver test errors
      
      * Enable linters on CI
      
      * Enable unresolved-namespace linter for tests
      
      * Appease the namespace linter again
      
      * Test fixes
      
      * Enable unused-binding linter for test/ => 293 warnings
      
      * 259 warnings
      
      * 234 warnings
      
      * => 114 warnings
      
      * Fix the rest of the unused binding warnings in test/
      
      * Fix unused binding errors in enterprise/backend/test
      
      * Fix unused binding lint errors in driver tests
      
      * Test fix :wrench:
      
      * Assure Kondo that something is in fact used
      Unverified
      adf45182
    • Cam Saul's avatar
      Enable Kondo for tests (part 1) (#24736) · bc4acbd2
      Cam Saul authored
      * Fix some small things
      
      * Add Kondo to deps.edn to be able to debug custom hooks from REPL
      
      * Fix macroexpansion hook for with-temp* without values
      
      * Test config (WIP)
      
      * More misc fixes
      
      * Disable :inline-def for tests
      
      * More misc fixes
      
      * Fix $ids and mbql-query kondo hooks.
      
      * Fix with-temporary-setting-values with namespaced symbols
      
      * More misc fixes
      
      * Fix the rest of the easy ones
      
      * Fix hook for mt/dataset
      
      * Horrible hack to work around https://github.com/clj-kondo/clj-kondo/issues/1773 . Custom linter for mbql-query macro
      
      * Fix places calling mbql-query with a keyword table name
      
      * Fix the last few errors in test/
      
      * Fix errors in enterprise/test and shared/test
      
      * Fix driver test errors
      
      * Enable linters on CI
      
      * Enable unresolved-namespace linter for tests
      
      * Appease the namespace linter again
      
      * Test fixes
      Unverified
      bc4acbd2
  9. Aug 11, 2022
    • Case Nelson's avatar
      [Actions] Add emitter-usages hydration (#24640) · b4033491
      Case Nelson authored
      * Move writeback migrations to 45
      
      * Empty commit to trigger GitHub Actions
      
      * [Actions] Simplify emitter schema model
      
      emitter_action was dropped since emitters just have a singular action
      and the join table was unecessary.
      
      emitter_action.action_id columns moved onto emitter table.
      
      Dropped CardEmitter and DashboardEmitter pre-insert, pre-update,
      pre-delete since they were used in tests only and normal operation
      would see the emitter inserted first.
      
      Since previous code may have 'orphaned' emitters without an action, we
      delete emitters without action to be safe.
      
      * Handle flakiness with geojson java.net.UnknownHostException errors (#24523)
      
      * Handle flakiness with geojson java.net.UnknownHostException errors
      
      In CI seems like we are getting errant errors:
      
      ```clojure
      geojson.clj:62
      It validates URLs and files appropriately
      http://0xc0000200
      expected: (valid? geojson)
        actual: #error {
       :cause "Invalid IP address literal: 0xc0000200"
       :via
       [{:type clojure.lang.ExceptionInfo
         :message "Invalid GeoJSON file location: must either start with http:// or https:// or be a relative path to a file on the classpath. URLs referring to hosts that supply internal hosting metadata are prohibited."
         :data {:status-code 400, :url "http://0xc0000200"}
         :at [metabase.api.geojson$valid_url_QMARK_ invokeStatic "geojson.clj" 62]}
        {:type java.net.UnknownHostException
         :message "0xc0000200"
         :at [java.net.InetAddress getAllByName "InetAddress.java" 1340]}
        {:type java.lang.IllegalArgumentException
         :message "Invalid IP address literal: 0xc0000200"
         :at [sun.net.util.IPAddressUtil validateNumericFormatV4 "IPAddressUtil.java" 150]}]
      ```
      
      Not clear if this change has a hope of fixing it: if it doesn't resolve
      once its possible it is cached somewhere in the network stack, or it
      won't resolve if you ask again.
      
      But gonna give it a shot.
      
      Set the property `"networkaddress.cache.negative.ttl"` to `"0"`
      
      > networkaddress.cache.negative.ttl (default: 10)
      >    Indicates the caching policy for un-successful name lookups from the name service. The value is specified as an integer to indicate the number of seconds to cache the failure for un-successful lookups.
      
      >    A value of 0 indicates "never cache". A value of -1 indicates "cache forever".
      
      From
      https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/net/InetAddress.html
      
      
      in the hopes that we can try multiple times. Restores the original value
      after the test completes so we don't inadvertently change behavior
      elsewhere.
      
      If we get an error of java.net.UnknownHostException we try again if we
      have attempts remaining. If we get a boolean it means the ip resolution
      worked so we can rely on the response (checking if it resolves locally
      or not)
      
      * add a delay
      
      * comment out test
      
      * [Actions] Add emitter-usages hydration
      
      This allows the front end to inform users of dashboards or cards that
      will be affected before deleting actions.
      
      Adds an `emitter-usages` hydration that shows emitters that reference
      a specific action or `is_write` card.
      
      The shape of `emitter-usages` is `[{:type "dashboard" :id 1 :name
      "Dashboard Name"} {:type "card" :id 2 :name "Card Name"}]`.
      
      * Add tests for new hydrations
      
      * add docstrings to 2 public emitter usage functions
      
      Co-authored-by: default avatarCam Saul <github@camsaul.com>
      Co-authored-by: default avatardpsutton <dan@dpsutton.com>
      Co-authored-by: default avatarBryan Maass <bryan.maass@gmail.com>
      Unverified
      b4033491
    • dpsutton's avatar
      Apply main args from bootstrap (#24730) · 5edfbdb6
      dpsutton authored
      Unverified
      5edfbdb6
    • Cam Saul's avatar
      Enable clj-kondo for driver namespaces (#24728) · 047a336b
      Cam Saul authored
      * Enable clj-kondo for driver namespaces
      
      * Fix CI command
      
      * Fix CI config (?)
      
      * Try hardcoding the driver directories in CI config
      
      * Fix some busted stuff
      
      * Enable the redundant fn wrapper linter
      
      * Appease the namespace linter
      Unverified
      047a336b
  10. Aug 10, 2022
    • Case Nelson's avatar
      [Actions] Simplify emitter schema model (#24570) · 98bbb001
      Case Nelson authored
      * Move writeback migrations to 45
      
      * Empty commit to trigger GitHub Actions
      
      * [Actions] Simplify emitter schema model
      
      emitter_action was dropped since emitters just have a singular action
      and the join table was unecessary.
      
      emitter_action.action_id columns moved onto emitter table.
      
      Dropped CardEmitter and DashboardEmitter pre-insert, pre-update,
      pre-delete since they were used in tests only and normal operation
      would see the emitter inserted first.
      
      Since previous code may have 'orphaned' emitters without an action, we
      delete emitters without action to be safe.
      
      * Handle flakiness with geojson java.net.UnknownHostException errors (#24523)
      
      * Handle flakiness with geojson java.net.UnknownHostException errors
      
      In CI seems like we are getting errant errors:
      
      ```clojure
      geojson.clj:62
      It validates URLs and files appropriately
      http://0xc0000200
      expected: (valid? geojson)
        actual: #error {
       :cause "Invalid IP address literal: 0xc0000200"
       :via
       [{:type clojure.lang.ExceptionInfo
         :message "Invalid GeoJSON file location: must either start with http:// or https:// or be a relative path to a file on the classpath. URLs referring to hosts that supply internal hosting metadata are prohibited."
         :data {:status-code 400, :url "http://0xc0000200"}
         :at [metabase.api.geojson$valid_url_QMARK_ invokeStatic "geojson.clj" 62]}
        {:type java.net.UnknownHostException
         :message "0xc0000200"
         :at [java.net.InetAddress getAllByName "InetAddress.java" 1340]}
        {:type java.lang.IllegalArgumentException
         :message "Invalid IP address literal: 0xc0000200"
         :at [sun.net.util.IPAddressUtil validateNumericFormatV4 "IPAddressUtil.java" 150]}]
      ```
      
      Not clear if this change has a hope of fixing it: if it doesn't resolve
      once its possible it is cached somewhere in the network stack, or it
      won't resolve if you ask again.
      
      But gonna give it a shot.
      
      Set the property `"networkaddress.cache.negative.ttl"` to `"0"`
      
      > networkaddress.cache.negative.ttl (default: 10)
      >    Indicates the caching policy for un-successful name lookups from the name service. The value is specified as an integer to indicate the number of seconds to cache the failure for un-successful lookups.
      
      >    A value of 0 indicates "never cache". A value of -1 indicates "cache forever".
      
      From
      https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/net/InetAddress.html
      
      
      in the hopes that we can try multiple times. Restores the original value
      after the test completes so we don't inadvertently change behavior
      elsewhere.
      
      If we get an error of java.net.UnknownHostException we try again if we
      have attempts remaining. If we get a boolean it means the ip resolution
      worked so we can rely on the response (checking if it resolves locally
      or not)
      
      * add a delay
      
      * comment out test
      
      Co-authored-by: default avatarCam Saul <github@camsaul.com>
      Co-authored-by: default avatardpsutton <dan@dpsutton.com>
      Unverified
      98bbb001
    • dpsutton's avatar
      Sql autocomplete options (#24693) · 65b1b771
      dpsutton authored
      
      * `sql-editor-autocomplete-match-style` options for perf
      
      Large instances with 100ks of fields are choking on the select name from
      fields where name like "%search-term%`. Version in 43 was
      acceptable. This adds back the behavior in 43: prefix searching.
      
      Also adds ability to turn off if even the prefix search is burdensome.
      
      * Rename autocomplete-suggestion param `search` -> `substring`
      
      * support sql-editor-autocomplete-match-style on the FE
      
      * fix substring shows placeholder
      
      * Ensure using prefix style matching in e2e test
      
      * Make autocomplete style public
      
      * Rename defsetting to have "native-query" in name, not sql
      
      generally true. Consider mongo which has a native editor but no notion
      of SQL
      
      Co-authored-by: default avatarAleksandr Lesnenko <alxnddr@gmail.com>
      Unverified
      65b1b771
  11. Aug 09, 2022
  12. Aug 08, 2022
  13. Aug 05, 2022
    • dpsutton's avatar
      Fix in-memory logger (#24616) · df165ba1
      dpsutton authored
      * Fix in-memory logger
      
      Our Admin > Troubleshooting > Logs page broke, just showing a spinner
      and never showing the logs.
      
      Don't quite understand why this fixes it. Javadocs are
      https://logging.apache.org/log4j/2.x/log4j-api/apidocs/org/apache/logging/log4j/LogManager.html#getContext-boolean-
      
      ```clojure
      logger=> (log/warn "test")
      nil
      logger=> (count @messages*)
      0
      ;; no in-memory logs so page is empty
      ;; change `(LogManager/getContext true)` in the momoized ns-logger fn
      ;; and then observe:
      logger=> (log/warn "test")
      nil
      logger=> (count @messages*)
      4
      ```
      
      Some explorations that might shine some light:
      
      ```clojure
      logger=> (into {} (.getAppenders (.getLogger (LogManager/getContext false) (str *ns*))))
      {}
      logger=> (into {} (.getAppenders (.getLogger (LogManager/getContext true) (str *ns*))))
      {"metabase-appender" #object[metabase.logger.proxy$org.apache.logging.log4j.core.appender.AbstractAppender$ff19274a
                                   "0x4d680247"
                                   "metabase-appender"]}
      ```
      
      So something is not hooked up quite right.
      
      * Add tests for metabase.logger
      
      * Ensure `api/util/logs` returns logs
      
      * tests
      
      * Check for presence of `MetabaseLoggerFactory` rather than whole str
      
      When in a namespace with a record, `Foo resolves to ns.Foo. But outside
      it resolves to ns/Foo. When running tests from the command line *ns* is
      user so it gets more complicated.
      
      * Kinda playing whackamole™ with `(LogManager/getContext true)`
      
      * Remove custom memoizing logger
      
      History:
      
      39.2 we set `Multi-Release: true` in our manifest file and query speed
      drops like a stone. Jeff was able to track this to our logger calls in
      tight loops.
      
      We revert the multi-release and keep seeing the warning on startup
      
      > WARNING: sun.reflect.Reflection.getCallerClass is not supported. This will impact performance.
      
      Benchmarking on 39.2
      (bench (log/trace "hi")) -> 15383 ns
      
      So we freaked out and set up a memoizing logger factory
      
      (bench (log/trace "hi")) -> 141 ns
      
      What a win.
      
      But we never noticed that the default *logger-factory* being picked up
      was slf4j ( `(log.impl/name log/*logger-factory*)` -> "org.slf4j" ). On
      39.2 if you set the factory to the log4j2 version you get back to a
      great number: `(bench (log/trace "hi"))` -> 25 ns
      
      And thus ensuring that our logger is the default log4j2 version is even
      faster than our memoizing logger-factory.
      
      Memoizing factory: 141 ns
      slf4j factory: 2269 ns
      log4j2 factory: 31 ns
      
      What does `(LogManager/getContext false)` mean versus using `true`? We
      only need and want a single context. But log4j2 by default uses a
      context selector called `ClassLoaderContextSelector`. We could put all
      of this behind us if we used a context selector type
      `BasicContextSelector` but this is surprisingly hard to do: you have to
      set a system property. And since all of this stuff gets initialized in
      static initializers, we don't have an opportunity to do this
      programmatically. The way around this is to require people to pass this
      system property on startup which is not acceptable.
      
      So getContext true checks for a threadlocal context in a specific static
      variable and falls back to a Default context. getContext false looks at
      classloaders and ends up at a different context. BUT: the log.impl
      version uses a closure over getContext false instead of getting it each
      time. And I suspect that when it does this there is only one so it is
      the default and continues to use this one. In our LoggerFactory
      implementation we were looking up the context each time. This still
      seems to work and everything is playing nice in our application
      classloader but its totally possible that our drivers are not hitting
      this. I'll have to investigate this.
      
      Verification:
      - build the uberjar locally (`bin/build`)
      - copy to some temp directory and also copy criterium.jar
      
      ```shell
      MB_JETTY_PORT=4000 java "$(socket-repl 4001)" -cp locally-built.jar:criterium.jar metabase.core
      ```
      
      ```clojure
      /tmp/locally-built via :coffee: v17.30 on :cloud:  metabase-query
      ❯ nc localhost 4001
      user=> (doto 'metabase.logger require in-ns)
      metabase.logger
      metabase.logger=> (require '[criterium.core :refer [bench]])
      nil
      metabase.logger=> (bench (log/trace "hi"))
      Evaluation count : 1686535500 in 60 samples of 28108925 calls.
                   Execution time mean : 22.487972 ns
          Execution time std-deviation : 0.101004 ns
         Execution time lower quantile : 22.326806 ns ( 2.5%)
         Execution time upper quantile : 22.648368 ns (97.5%)
                         Overhead used : 6.924761 ns
      nil
      metabase.logger=> (count (messages))
      358
      metabase.logger=>
      ```
      
      Verifies that we are on the order of 22 nanoseconds and the in-memory
      logger has messages in it.
      
      * Appease our namespace linters
      
      * I'll unplug you ns linter
      
      * Better tests and ns docstring
      
      * Bootstrap to set system properties
      
      New entrypoint for the application: metabase.bootstrap
      
      sets two properties for logging (context selector, log4j2 factory) and
      ensures those properties are set before any logging code is loaded.
      
      * docstrings and clean ns
      
      * metabase.logger ns docstring cleanup
      
      * docstring
      
      * rename a test now that there's no memoization
      
      * add logger properties to :dev profile
      
      * Revert "add logger properties to :dev profile"
      
      This reverts commit 4f09fa3b631f882a3c5edcab4508769ffb20d4fa.
      
      * deps
      Unverified
      df165ba1
  14. Aug 04, 2022
    • metamben's avatar
      Recursively nest expressions (#24404) · d6bc5e93
      metamben authored
      Unverified
      d6bc5e93
    • metamben's avatar
    • Ryan Laurie's avatar
      Add Support for Case-insensitive `contains` dashboard filters (#24582) · 5b0b592a
      Ryan Laurie authored
      
      * Dashboard Card String Filters are now case-insensitive
      
      This PR is a draft because while it solves the problem of string filters being case sensitive, it doesn't necessarily
      do it in the best way.
      
      This isn't necessarily
      a bug, but it seems that there is no way for the frontend to set the :case-sensitive true/false option anyway.
      
      For the purposes of this initial attempt at a solution, I have modified the endpoint
      ` POST "/:dashboard-id/dashcard/:dashcard-id/card/:card-id/query"` to automatically include an option map containing
      :case-sensitive false.
      
      The machinery to take this option into consideration already exists with the default :sql ->honeysql function in the `
      metabase.driver.sql.query-processor` namespace. See the `like-clause` private function in particular.
      
      Since the query processor is a bit opaque to me at present, I was unable to figure out if there is a proper way that
      the frontend could send an options map (or key-value pair) all the way through the qp middleware to the query building
      stage. I discovered that if you conj a map `{:case-sensitive false}` to the output of the `to-clause` function in
      `metabase.driver.common.parameters.operators`, you will get the desired case-insensitive behavior. So, I modified the
      to-clause function in this PR to appropriately conj an options map if one exists.
      
      What I'd like to know:
      
      - is there a super-simple way to pass an option in already that I just missed? (eg. I thought perhaps in the `[:field
      13 nil]` that 'nil' could be an options map, but I couldn't get that to work for me)
      
      - is there a middleware approach that I should consider?
      
      - any other options to appropriately hanlde this?
      
      * Revert the endpoint.
      
      If the frontend sends an options map on an options key inside the parameter, this endpoint will pass that on, so no
      change is needed.
      
      * include parameter options in datasetQuery
      
      Co-authored-by: default avatarAdam James <adam.vermeer2@gmail.com>
      Unverified
      5b0b592a
    • Cam Saul's avatar
      Fix SSH tunnel leaks when testing connections (#24446) · cc632e81
      Cam Saul authored
      * Fix SSH tunnel leaks when testing connections
      
      * Appease kondo =(
      
      * Add some more NOTES
      Unverified
      cc632e81
    • Braden Shepherdson's avatar
      Serdes v2: Portable `:visualization_settings` fields on (dash)cards. (#24606) · 5d40fa4f
      Braden Shepherdson authored
      These fields hold JSON, which can contain field IDs in a few places.
      
      Particularly nasty is the `:column_settings` subfield, which is a map
      whose *keys* are JSON strings with field IDs.
      Unverified
      5d40fa4f
    • dpsutton's avatar
      Ensure to use original query not persisted query when converting (#24580) · ea9baa51
      dpsutton authored
      If you click "view sql" on a cached model you see the substituted query
      rather than the "real" query.
      
      The query based on cache:
      
      ```sql
      SELECT "source"."login" AS "login", "source"."count" AS "count"
      FROM (select "login", "count" from "metabase_cache_e449f_19"."model_4112_issue_assi") "source"
      LIMIT 1048575
      ```
      
      The real underlying query:
      
      ```sql
      SELECT "source"."login" AS "login", "source"."count" AS "count"
      FROM (SELECT "github_raw"."issue_events__issue__assignees"."login" AS "login", count(*) AS "count" FROM "github_raw"."issue_events__issue__assignees"
      GROUP BY "github_raw"."issue_events__issue__assignees"."login"
      ORDER BY "count" DESC, "github_raw"."issue_events__issue__assignees"."login" ASC) "source"
      LIMIT 1048575
      ```
      
      If you click the "convert to sql question" you will end up with a sql
      question that is not based on the original question but the Metabase
      managed table.
      Unverified
      ea9baa51
    • Ngoc Khuat's avatar
  15. Aug 03, 2022
    • Noah Moss's avatar
      Set `CookieSpecs/STANDARD` on HttpClient for Snowplow (#24579) · 1786fffa
      Noah Moss authored
      * set CookieSpecs/STANDARD on Apache HttpClient for Snowplow
      
      * add a comment in the code pointing to the PR
      
      * set cookie spec via RequestConfig instead
      Unverified
      1786fffa
    • Howon Lee's avatar
      22831 fix (group-by time bucketing on replacements for json aliases) (#24545) · 1fd2c151
      Howon Lee authored
      * rewrite here...
      
      * no bq
      
      * thinking
      
      * conjunction of alias thing...
      
      * make sure we dont wanna see it
      
      * nit
      
      * multi-arity func
      
      * fiddly bit on test
      
      * add bit to see order by works right
      
      * no default breakout true
      Unverified
      1fd2c151
    • Braden Shepherdson's avatar
      Serdes v2: Handle other embedded MBQL fragments (#24537) · cb9e9aed
      Braden Shepherdson authored
      This PR handles the other JSON-encoded MBQL snippets I was able to find.
      
      These snippets contain `Table` and `Field` IDs, and so are not portable. These
      fields are expanded during serialization and the IDs replaced with portable
      references, then converted back in deserialization.
      
      Note that the referenced field must already be loaded before it has a valid ID.
      `serdes-dependencies` defines this order, therefore each entity depends on those
      tables and fields referenced in its MBQL fragments.
      
      The complete set of fields I found to convert:
      
      - `Metric.definition`
      - `Segment.definition`
      - `DashboardCard.parameter_mappings`
      - `Card.parameter_mappings`
      Unverified
      cb9e9aed
    • dpsutton's avatar
      Remove deprecated friend library (#24543) · 4069ac4f
      dpsutton authored
      * Remove deprecated friend library
      
      - friend has two functions we used: bcrypt and bcrypt-verify. Easy to
      lift them into our own namespace with attribution
      - uses simple interop on org.mindrot.jbcrypt.BCrypt to achieve these
      - also brings in other stuff we don't need
      
      ```
      com.cemerick/friend 0.2.3
        X org.mindrot/jbcrypt 0.3m :use-top <- all we care about
        X org.clojure/core.cache 0.6.3 :superseded
          X org.clojure/data.priority-map 0.0.2 :parent-omitted
        . org.openid4java/openid4java-nodeps 0.9.6
          X commons-logging/commons-logging 1.1.1 :older-version
          . net.jcip/jcip-annotations 1.0
        . com.google.inject/guice 2.0
          . aopalliance/aopalliance 1.0
      ```
      
      And we already declare a dependency on 0.4 of this lib
      
      ```
      org.mindrot/jbcrypt 0.4
      ```
      
      This means we can remove openid4, google.inject/guice, aopalliance, etc
      and just keep using the same `BCrypt` java class we have been using this
      whole time. Behavior and classfiles are identical. So very low risk
      
      Want to call out a use of
      
      ```clojure
          (when-not api/*is-superuser?*
            (api/checkp (u.password/bcrypt-verify (str (:password_salt user) old_password) (:password user))
              "old_password"
              (tru "Invalid password")))
      ```
      
      This has the same signature of an existing function in `u.password/verify-password`:
      
      ```clojure
      (defn verify-password
        "Verify if a given unhashed password + salt matches the supplied hashed-password. Returns `true` if matched, `false`
        otherwise."
        ^Boolean [password salt hashed-password]
        ;; we wrap the friend/bcrypt-verify with this function specifically to avoid unintended exceptions getting out
        (boolean (u/ignore-exceptions
                   (bcrypt-verify (str salt password) hashed-password))))
      ```
      
      I did not replace it in this PR so that the diff is essentially
      `creds/<fn>` -> `u.password/<fn>` and very easy to structually see what
      is going on.
      
      But totally makes sense to clean up the usages of these in another pass
      
      * sort ns
      
      * simple tests
      Unverified
      4069ac4f
  16. Aug 02, 2022
  17. Aug 01, 2022
Loading