Skip to content
Snippets Groups Projects
This project is mirrored from https://github.com/metabase/metabase. Pull mirroring updated .
  1. Jul 18, 2022
  2. Jul 15, 2022
    • GitStart's avatar
      Visualization: Bar chart not showing 0 values unlike line and area chart (#23626) · c718ed3d
      GitStart authored
      
      * Visualization: Bar chart not showing 0 values unlike line and area chart
      
      Co-authored-by: default avatargitstart <gitstart@gitstart.com>
      Co-authored-by: default avatargitstart <gitstart@users.noreply.github.com>
      Co-authored-by: default avatarNitesh Singh <nitesh.singh@gitstart.dev>
      Co-authored-by: default avatarGh0sT <coolmagnas@gmail.com>
      Co-authored-by: default avatarRubens Rafael <70234898+RubensRafael@users.noreply.github.com>
      Co-authored-by: default avatarGitStart <1501599+gitstart@users.noreply.github.com>
      Co-authored-by: default avatarBenjamin Mayanja <vibenjamin6@gmail.com>
      Co-authored-by: default avatarJúlio Piubello da Silva Cabral <julio.piubello@gitstart.dev>
      
      * Visualization: Bar chart not showing 0 values unlike line and area chart
      
      Co-authored-by: default avatargitstart <gitstart@gitstart.com>
      Co-authored-by: default avatargitstart <gitstart@users.noreply.github.com>
      Co-authored-by: default avatarGitStart-Chalktalk <nitesh.singh@gitstart.dev>
      Co-authored-by: default avatarGh0sT <coolmagnas@gmail.com>
      Co-authored-by: default avatarRubens Rafael <70234898+RubensRafael@users.noreply.github.com>
      Co-authored-by: default avatarGitStart <1501599+gitstart@users.noreply.github.com>
      Co-authored-by: default avatarBenjamin Mayanja <vibenjamin6@gmail.com>
      Co-authored-by: default avatarJúlio Piubello da Silva Cabral <julio.piubello@gitstart.dev>
      
      * remove 0 filter for bar chart
      
      Co-authored-by: default avatargitstart <gitstart@gitstart.com>
      Co-authored-by: default avatargitstart <gitstart@users.noreply.github.com>
      Co-authored-by: default avatarGitStart-Chalktalk <nitesh.singh@gitstart.dev>
      Co-authored-by: default avatarGh0sT <coolmagnas@gmail.com>
      Co-authored-by: default avatarRubens Rafael <70234898+RubensRafael@users.noreply.github.com>
      Co-authored-by: default avatarGitStart <1501599+gitstart@users.noreply.github.com>
      Co-authored-by: default avatarBenjamin Mayanja <vibenjamin6@gmail.com>
      Co-authored-by: default avatarJúlio Piubello da Silva Cabral <julio.piubello@gitstart.dev>
      
      Co-authored-by: default avatargitstart <gitstart@users.noreply.github.com>
      Co-authored-by: default avatargitstart <gitstart@gitstart.com>
      Co-authored-by: default avatarNitesh Singh <nitesh.singh@gitstart.dev>
      Co-authored-by: default avatarGh0sT <coolmagnas@gmail.com>
      Co-authored-by: default avatarRubens Rafael <70234898+RubensRafael@users.noreply.github.com>
      Co-authored-by: default avatarBenjamin Mayanja <vibenjamin6@gmail.com>
      Co-authored-by: default avatarJúlio Piubello da Silva Cabral <julio.piubello@gitstart.dev>
      Unverified
      c718ed3d
    • Ryan Laurie's avatar
      update modal header (#24001) · 8889384e
      Ryan Laurie authored
      Unverified
      8889384e
    • Natalie's avatar
    • metamben's avatar
      Retain source alias for aggregates (#23771) · 6858dcef
      metamben authored
      Unverified
      6858dcef
    • Nick Fitzpatrick's avatar
      Fixing viewheader unit test (#24016) · 956bfd7b
      Nick Fitzpatrick authored
      * Fixing viewheader unit test
      
      * Fixing SavedQuestionHeaderButton unit test
      Unverified
      956bfd7b
    • dpsutton's avatar
      Remove google socket factory (#23976) · 50c31876
      dpsutton authored
      The presence of this lib was breaking BigQuery: see
      https://github.com/metabase/metabase/issues/23895.
      
      Running the RC-1 (or running bin/build and making your own) would error
      on bigquery. Removing the dep restores bigquery access.
      Unverified
      50c31876
    • dpsutton's avatar
      Check email test errors for javax.mail.AuthenticationFailedException (#23921) · 52336b97
      dpsutton authored
      * Check email test errors for javax.mail.AuthenticationFailedException
      
      Authing with Office365 led to a less-than-great user experience. If you
      type in the wrong username or password, we would display an unhelpful
      message:
      
      > Sorry, something went wrong. Please try again. Error::Exception
      reading response.Read timed out.
      
      This is an error from an inner nested exception which looks like:
      ```clojure
      (javax.mail.AuthenticationFailedException.
       "" ;; Office365 returns auth exception with no message so we only saw "Read timed out" prior
       (javax.mail.MessagingException.
        "Exception reading response"
        (java.net.SocketTimeoutException. "Read timed out")))
      ```
      
      We didn't get any message from the AuthenticationFailedException so our
      humanization strategy only went on the underlying "Read timed out". Now
      we can check against the error class and use that information.
      
      * Little bit of clarity
      
      better function name (`check` -> `match-error`), better arg name (
      `regex|exception-class` -> `regex-or-exception-class`), full argument
      names `m` -> `message` and `es` -> `exceptions`
      Unverified
      52336b97
    • dpsutton's avatar
      Handle email password obfuscation (#23925) · 5e49508d
      dpsutton authored
      The frontend receives obfuscated values for settings marked sensitive:
      
      ```clojure
      (defsetting email-smtp-password
        (deferred-tru "SMTP password.")
        :sensitive? true)
      ```
      
      and over the wire:
      ```javascript
          {
              "key": "email-smtp-password",
              "value": "**********ig",
              "is_env_setting": false,
              "env_name": "MB_EMAIL_SMTP_PASSWORD",
              "description": "SMTP password.",
              "default": null
          },
      ```
      
      So the frontend form never has a valid password in it. When you edit
      email settings, we check if those are valid, and if so, commit the
      settings. But with the wrong password email settings are almost always
      wrong. You have to re-type the email password just to change other
      settings, like the friendly name, reply-to email address, etc.
      
      So we recognize that we've received the obfuscated value, swap in the
      real value for the email connection test, and then obfuscate the
      password in the response. If we do not receive an obfuscated value, just
      leave it alone and return the value anyways (they typed it in, so seems
      safe to send it back).
      
      Note that there is a function in models.setting called
      `obfuscated-value?` that checks strings against a regex of
      `#"^\*{10}.{2}$"`. This is used to never set settings to an obfuscated
      value. But its a bit less sensitive than our purposes need. We have a
      real value and an obfuscated value so we can check if the obfuscated
      value is based on the real value. (obfuscate-value "password") ->
      "**********rd" . Whereas we might recognize "**********AA" as the
      obfuscated value if we reused that helper function.
      
      NOTE this could be weird if anyone's password changes from "password" to
      "**********rd" but the chances of that happening are miniscule.
      
      Had thought to have the FE not send a value at all if it is unchanged
      but this had some far-reaching implications that we don't want to tackle
      so close to the release. There's an associated issue for LDAP settings
      that is largely the same as this:
      https://github.com/metabase/metabase/issues/16708 But it is not clear to
      me if these are the only two places or if there could be others. Until
      that research is done we can just accept this patch as is and come up
      with a systematic approach in the future.
      
      But it does seem only three settings are sensitive:
      ```clojure
      setting=> (->> (vals @registered-settings)
                     (filter :sensitive?)
                     (map :name))
      (:saml-keystore-password :email-smtp-password :ldap-password)
      ```
      
      The direct setting apis won't write setting values which appear as
      obfuscated, but we have other endpoints that take collections of
      settings as a unit (for instance, the endpoint /api/email that takes all
      of the email settings here to test if they work together).
      Unverified
      5e49508d
    • dpsutton's avatar
      Async card metadata (#23672) · 21aa0053
      dpsutton authored
      
      * Lets use the main thread
      
      * Strip out channel stuff and rename
      
      * 202 -> 200 response
      
      When returning a channel we return a 202. A map is just a 200. Since we
      no longer need to have the main stuff async (as opposed to the metadata
      stuff) we can just return the map with a 200 instead of this long
      running channel stuff and a 202.
      
      * Last test
      
      * renames, logging, ensure query is the same before saving metadata
      
      * Sandbox test 202 -> 200
      
      * Another 202 -> 200
      
      * Put timeout on async metadata saving
      
      timeout of 15 minutes before we give up on the async metadata saving. It
      is possible this cuts things off but hard to tell if work is still being
      done at that point.
      
      * outdated comment
      
      * Return json error message, not text/plain
      
      this is a subtle one that I'm not happy about. Our error handling will
      return text/plain if you throw an `(ex-info "something" {:status-code
      400})`.
      
      ```shell
      ❯ http post localhost:3000/api/timeline-event/ name=some-name description=Bob timestamp=2022 timezone=America/Central time_matters:=false timeline_id:=1629  Cookie:$COOKIE
      HTTP/1.1 404 Not Found
      Content-Type: text/plain
      
      Timeline with id 1,629 not found
      ```
      
      But if you add extra information to the map to the ex-info, you get
      json!
      
      ```clojure
      (defmethod api-exception-response Throwable
        [^Throwable e]
        (let [{:keys [status-code], :as info} (ex-data e)
              other-info                      (dissoc info :status-code :schema :type)
              body                            (cond
                                                (and status-code (empty? other-info))
                                                ;; If status code was specified but other data wasn't, it's something like a
                                                ;; 404. Return message as the (plain-text) body.
                                                (.getMessage e)
      
                                                ;; if the response includes `:errors`, (e.g., it's something like a generic
                                                ;; parameter validation exception), just return the `other-info` from the
                                                ;; ex-data.
                                                (and status-code (:errors other-info))
                                                other-info
      
                                                ;; Otherwise return the full `Throwable->map` representation with Stacktrace
                                                ;; and ex-data
                                                :else
                                                (merge
                                                 (Throwable->map e)
                                                 {:message (.getMessage e)}
                                                 other-info))]
          {:status  (or status-code 500)
           :headers (mw.security/security-headers)
           :body    body}))
      ```
      
      So this fix is a _very_ subtle way to get to what we want, although it
      does add a bunch of extra junk to our response.
      
      ```javascript
      {
       ...
       "message": "Invalid Field Filter: Field 26 \"PRODUCTS\".\"CATEGORY\"
      belongs to Database 1 \"Sample Database\", but the query is against
      Database 990 \"copy of sample dataset\"",
       "data": {
         "status-code": 400,
         "query-database": 990,
         "field-filter-database": 1}
       ...
      }
      ```
      
      Reminder of what we want: the frontend is saving a card. We added a
      field filter on a database and then changed the source database of the
      query. So the backend needs to reject the field filter as being on the
      wrong db. The FE expects a json response with a message and will then
      show that message in the save/edit modal.
      
      Why did this come up now: these endpoints for saving and editing a card
      used to always return 202 streaming responses. This means they would
      have to tuck any errors inside of an already open 202 response. Which is
      why you get a message as a json response. But now that they are sync,
      the api can just return a proper 400 with a reason. But we still want
      that to be a json response for the FE.
      
      * error layout fix
      
      * Several Cleanups
      
      - make sure numbers have units at the end (-ms)
      - use (u/minutes->ms 15) rather than a more opaque (* 15 60 1000)
      - move the scheduled metadata saving into its own function
      
      This last bit is actually a bit more than that. I was previously
      throwing everything in a thread submitted to the pooled executor. I'm
      now using a `future` but with a caveat: All of the waiting for the
      timeout, checking if we got metadata is done in a simple `a/go` block
      now, and the thread does just the last bit of IO. We have to select the
      card as it is now to ensure the query is the same and then save.
      
      Refresher on why we have to check if the query is the same. We have up
      to 15 minutes to wait for the query metadata to come back. Completely
      possible for them to have edited the query and then our metadata is
      useless.
      
      Co-authored-by: default avatarAleksandr Lesnenko <alxnddr@gmail.com>
      Unverified
      21aa0053
    • Nemanja Glumac's avatar
      [E2E] Fix broken `collections` tests (#24010) · 69fd5b73
      Nemanja Glumac authored
      * Fix broken collection tests
      
      * Merge related tests together
      
      - #16555 is not relevant anymore
      - add repro for #20716
      Unverified
      69fd5b73
    • dpsutton's avatar
      Fix constantly checking token on error (#23815) · e7b4dc62
      dpsutton authored
      * Fix constantly checking token on error
      
      We were catching `clojure.lang.ExceptionInfo` which only catches a small
      subset of errors and not any related to what might be thrown in network
      calls.
      
      The reason this affected us is that the cache will cache this value but
      keep trying what is throwing the error. So each time we look at token
      features it attempts again and throws an error.
      
      If we instead do what the code _meant_ to do and we return a value
      indicating there was an error, we are all good.
      
      Example:
      
      ```clojure
      premium-features=> (defn is-odd? [x]
                           (println "computing for " x)
                           (if (odd? x) true (throw (ex-info "boom" {}))))
      premium-features=> (def modd (memoize/ttl is-odd? :ttl/threshold 30000))
      premium-features=> (modd 3)
      computing for  3
      true
      premium-features=> (modd 5)
      computing for  5
      true
      premium-features=> (modd 3)
      true
      premium-features=> (modd 2)
      computing for  2
      Execution error (ExceptionInfo) at metabase.public-settings.premium-features/is-odd? (REPL:289).
      boom
      premium-features=> (modd 2)
      computing for  2
      Execution error (ExceptionInfo) at metabase.public-settings.premium-features/is-odd? (REPL:289).
      boom
      ```
      
      Note that `(modd 2)` keeps attempting to compute for 2 since it throws
      an error rather than returning a value indicating an error.
      
      When the token check throws an error:
      
      ```clojure
      premium-features=> (fetch-token-status (apply str (repeat 64 "b")))
      Execution error (UnknownHostException) at java.net.Inet6AddressImpl/lookupAllHostAddr (Inet6AddressImpl.java:-2).
      store.metabase.com: nodename nor servname provided, or not known
      ```
      
      When the token check returns a value indicating an error:
      
      ```clojure
      premium-features=> (fetch-token-status (apply str (repeat 64 "b")))
      {:valid false,
       :status "Unable to validate token",
       :error-details "store.metabase.com: nodename nor servname provided, or not known"}
      ```
      
      (both instances were with wifi turned off locally)
      
      * add test
      
      * Assert we only call the api once when it throws an error
      
      before this PR we would make a call for each setting (perhaps
      more). Each time trying to hit the endpoint. Now it catches those errors
      and reports not a valid token
      Unverified
      e7b4dc62
    • Nick Fitzpatrick's avatar
    • Nemanja Glumac's avatar
      [E2E] Generate Mochawesome reports from within a Cypress runner (#23972) · c88aad8c
      Nemanja Glumac authored
      * Run mochawesome from within a custom Cypress runner
      
      * Remove Mochawesome report from GitHub E2E workflows
      Unverified
      c88aad8c
    • Anton Kulyk's avatar
      Tiny i18n fix (#23997) · 2f216e72
      Anton Kulyk authored
      * Wrap string in `t`
      
      * Don't translate email string
      Unverified
      2f216e72
    • Mahatthana (Kelvin) Nomsawadi's avatar
      Make collection dashboard question buttons consistent (#23967) · 8bae7d35
      Mahatthana (Kelvin) Nomsawadi authored
      * Make collection and dashboard header buttons consistent
      
      * Make question buttons consistent
      
      * Standardize header button margin for collection and dashboard
      
      * Use Maz's spacing
      
      * Fix conflicts with Maz's spacing on dashboards
      
      * Change spacing between collection header buttons to use gap
      
      * Match dashboard and question divider spacing
      
      * Refactor question header spacing to use gap
      Unverified
      8bae7d35
    • Maz Ameli's avatar
      Restyle EntityMenu (#23991) · 38574278
      Maz Ameli authored
      * text-dark and a new hover state
      
      * change min width
      Unverified
      38574278
    • Jeff Bruemmer's avatar
      note on GitHub raw links (#24000) · 37d96fbd
      Jeff Bruemmer authored
      Unverified
      37d96fbd
    • Ngoc Khuat's avatar
      Store linked-filter fieldvalues (#23699) · e8d98d95
      Ngoc Khuat authored
      * first pass storing linked-filter fieldvalues
      
      * limit linked-filter to not explode
      
      * no check perms for fields when getting params values on dashboard
      Unverified
      e8d98d95
    • Nick Fitzpatrick's avatar
    • Nemanja Glumac's avatar
    • Mahatthana (Kelvin) Nomsawadi's avatar
      fix inconsistent dashboard action buttons hover effects (#23833) · 0cdc2e4e
      Mahatthana (Kelvin) Nomsawadi authored
      
      * Refactor code to make it more concise
      
      * Make dashboard buttons hover effect more consistent
      
      * Make EntityMenu style more consistent
      
      * Fix collection event button tab selection
      
      * Add missing tooltip to dashboard info button
      
      * styled cleanup
      
      * Fix failed E2E tests
      
      * Address Anton's feedback
      
      * Fix question buttons style clashing with #23827
      
      * Address Alexander's feedback
      
      * maz spacing and sizing tweaks
      
      Co-authored-by: default avatarMaz Ameli <mazameli@gmail.com>
      Unverified
      0cdc2e4e
    • Aleksandr Lesnenko's avatar
      Unverified
      0816799d
    • Alexander Polyankin's avatar
    • Alexander Polyankin's avatar
    • Alexander Polyankin's avatar
      Fix color pills in Safari (#23946) · d31df486
      Alexander Polyankin authored
      Unverified
      d31df486
  3. Jul 14, 2022
Loading