Skip to content
Snippets Groups Projects
This project is mirrored from https://github.com/metabase/metabase. Pull mirroring updated .
  1. Dec 11, 2023
  2. Dec 09, 2023
  3. Dec 08, 2023
  4. Dec 07, 2023
    • Mark Bastian's avatar
      Refactoring formatting code for common utility (#36490) · 58f22594
      Mark Bastian authored
      * Apply column formatting to CSV exports
      
      This PR applies the same formatting logic to CSV exports as it does to pulse bodies (e.g. HTML).
      
      Formerly, there were two related formatting paths:
      - `metabase.query-processor.streaming.common/format-value`, which is from a protocol that takes an object and returns a string. It is what was used to export csv data. It applies no actual formatting, only converts objects to strings.
      - `metabase.pulse.render.body/get-format`, which builds a formatter from an optional time zone, column metadata, and visualization settings. This formatter takes a string and formats it. It was only used for rendering inline artifacts, such as embedded HTML in emails.
      
      The first function is insufficient to format row data as it is unaware of the column metadata and viz settings. We need to use that data everywhere data is exported in a uniform way.
      
      The solution is to lift `get-format` from `metabase.pulse.render.body` to a more common location (`metabase.pulse.render.common` in this PR step, but needs to be moved out of the pulse code to be a more shared concern) and use it everywhere artifacts are generated.
      
      For csv export, this was achieved as follows in `metabase.query-processor.streaming.csv`:
      
      ```clojure
      (defmethod qp.si/streaming-results-writer :csv
        [_ ^OutputStream os]
        (let [writer     (BufferedWriter. (OutputStreamWriter. os StandardCharsets/UTF_8))
              formatters (atom {})]
          (reify qp.si/StreamingResultsWriter
            (begin! [_ {{:keys [ordered-cols results_timezone]} :data} viz-settings]
              (swap! formatters (constantly (zipmap
                                              ordered-cols
                                              (map (fn [col]
                                                     (p.common/get-format results_timezone col viz-settings))
                                                   ordered-cols))))
              (csv/write-csv writer [(map (some-fn :display_name :name) ordered-cols)])
              (.flush writer))
      
            (write-row! [_ row _row-num cols {:keys [output-order]}]
              (let [[ordered-row
                     ordered-cols] (if output-order
                                     (let [row-v  (into [] row)
                                           cols-v (into [] cols)]
                                       [(for [i output-order] (row-v i))
                                        (for [i output-order] (cols-v i))])
                                     [row cols])]
                (csv/write-csv writer [(map (fn [c r]
                                              (let [formatter (@formatters c)]
                                                (formatter (common/format-value r))))
                                            ordered-cols ordered-row)])
                (.flush writer)))
      
            (finish! [_ _]
              ;; TODO -- not sure we need to flush both
              (.flush writer)
              (.flush os)
              (.close writer)))))
      ```
      
      The formatters for each column are build in the `begin!` function and then looked up in each `write-row!`.  The existing `format-value` is used to produce a string then passed into our looked up column formatter.
      
      Note that the new unit tests simulate a pulse and grab the provided temp files as attachments and analyzes those for correctness. This should work in a CI environment so long as the test process has permission to both write attachments to the temp directory and read those attachments back out. Also note that these tests can be slow (but not terribly so).
      
      Primary changes:
      - `metabase.email.messages` - fix spelling
      - `metabase.pulse.render.body` - move `get-format` out of this ns
      - `metabase.pulse.render.common` - move `get-format` into this ns
      - `metabase.query-processor.streaming.csv` - new logic to apply pulse renderer formatting to csv exports
      - `metabase.pulse.pulse-integration-test` - adding unit tests
      
      One TODO before a final commit of this PR is to move the `get-format` logic out of a pulse ns into something more general. Ultimately, it would be nice if this was a common capability used by both BE and FE.
      
      * Refactoring formatting code for common utility
      
      This PR refactors `metabase.pulse.render.common` to `metabase.formatter` as this is code we want applied to several locations, not just in pulses. It also updates references to these nses and the consistent alias.
      
      A key observation of this formatting code, and reason for the refactor, is that it is "metabase-aware" in that it takes into account metadata columns and visualization settings when building formatters rather than just being a simple generic date or number formatter. This is a common code path that should be used any time we are rendering static assets and could potentially be used as common FE code with future development.
      
      Moves:
      - `metabase.pulse.render.common` to `metabase.formatter`
      - `metabase.pulse.render.datetime` to `metabase.formatter.datetime`
      - `metabase.pulse.render.common-test` to `metabase.formatter-test`
      - `metabase.pulse.render.datetime-test` to `metabase.formatter.datetime-test`
      
      * Ordering consistent aliases in kondo config
      
      * Rebase fix to use formatter ns in streaming.csv
      
      * Adding `metabase.formatter` require.
      
      * Updating require alias on new test.
      Unverified
      58f22594
    • lbrdnk's avatar
      Update server side relative datetime generation honeysql (#36528) · 930bbc5a
      lbrdnk authored
      * Use date instead of timestamp
      
      * Update logging and add code for local testing
      
      * Rename datetime to date
      Unverified
      930bbc5a
    • Ryan Laurie's avatar
      Use stretch variant on switch (#36503) · f5d6697a
      Ryan Laurie authored
      Unverified
      f5d6697a
    • Anton Kulyk's avatar
      Add icon search to Storybook (#36547) · bbdbf09b
      Anton Kulyk authored
      Unverified
      bbdbf09b
    • Anton Kulyk's avatar
      Add "refresh_downstream" icon (#36546) · b3e6f9c6
      Anton Kulyk authored
      Unverified
      b3e6f9c6
    • Uladzimir Havenchyk's avatar
    • Anton Kulyk's avatar
    • shaun's avatar
      Fix settings jwt (#36189) · 83bf74cf
      shaun authored
      * fix types
      * swap unit test step order
      Unverified
      83bf74cf
    • Nicolò Pretto's avatar
      [Help link] feature branch (#36172) · f79a58ed
      Nicolò Pretto authored
      feat:: white labeling option for help link
      Unverified
      f79a58ed
    • Case Nelson's avatar
      [MLv2] drop-stage-if-empty (#36500) · 3281075f
      Case Nelson authored
      * [MLv2] drop-stage-if-empty
      
      * Addressing PR comments
      Unverified
      3281075f
    • Mark Bastian's avatar
      Apply column formatting to CSV exports (#36484) · 356c4986
      Mark Bastian authored
      * Apply column formatting to CSV exports
      
      This PR applies the same formatting logic to CSV exports as it does to pulse bodies (e.g. HTML).
      
      Formerly, there were two related formatting paths:
      - `metabase.query-processor.streaming.common/format-value`, which is from a protocol that takes an object and returns a string. It is what was used to export csv data. It applies no actual formatting, only converts objects to strings.
      - `metabase.pulse.render.body/get-format`, which builds a formatter from an optional time zone, column metadata, and visualization settings. This formatter takes a string and formats it. It was only used for rendering inline artifacts, such as embedded HTML in emails.
      
      The first function is insufficient to format row data as it is unaware of the column metadata and viz settings. We need to use that data everywhere data is exported in a uniform way.
      
      The solution is to lift `get-format` from `metabase.pulse.render.body` to a more common location (`metabase.pulse.render.common` in this PR step, but needs to be moved out of the pulse code to be a more shared concern) and use it everywhere artifacts are generated.
      
      For csv export, this was achieved as follows in `metabase.query-processor.streaming.csv`:
      
      ```clojure
      (defmethod qp.si/streaming-results-writer :csv
        [_ ^OutputStream os]
        (let [writer     (BufferedWriter. (OutputStreamWriter. os StandardCharsets/UTF_8))
              formatters (atom {})]
          (reify qp.si/StreamingResultsWriter
            (begin! [_ {{:keys [ordered-cols results_timezone]} :data} viz-settings]
              (swap! formatters (constantly (zipmap
                                              ordered-cols
                                              (map (fn [col]
                                                     (p.common/get-format results_timezone col viz-settings))
                                                   ordered-cols))))
              (csv/write-csv writer [(map (some-fn :display_name :name) ordered-cols)])
              (.flush writer))
      
            (write-row! [_ row _row-num cols {:keys [output-order]}]
              (let [[ordered-row
                     ordered-cols] (if output-order
                                     (let [row-v  (into [] row)
                                           cols-v (into [] cols)]
                                       [(for [i output-order] (row-v i))
                                        (for [i output-order] (cols-v i))])
                                     [row cols])]
                (csv/write-csv writer [(map (fn [c r]
                                              (let [formatter (@formatters c)]
                                                (formatter (common/format-value r))))
                                            ordered-cols ordered-row)])
                (.flush writer)))
      
            (finish! [_ _]
              ;; TODO -- not sure we need to flush both
              (.flush writer)
              (.flush os)
              (.close writer)))))
      ```
      
      The formatters for each column are build in the `begin!` function and then looked up in each `write-row!`.  The existing `format-value` is used to produce a string then passed into our looked up column formatter.
      
      Note that the new unit tests simulate a pulse and grab the provided temp files as attachments and analyzes those for correctness. This should work in a CI environment so long as the test process has permission to both write attachments to the temp directory and read those attachments back out. Also note that these tests can be slow (but not terribly so).
      
      Primary changes:
      - `metabase.email.messages` - fix spelling
      - `metabase.pulse.render.body` - move `get-format` out of this ns
      - `metabase.pulse.render.common` - move `get-format` into this ns
      - `metabase.query-processor.streaming.csv` - new logic to apply pulse renderer formatting to csv exports
      - `metabase.pulse.pulse-integration-test` - adding unit tests
      
      One TODO before a final commit of this PR is to move the `get-format` logic out of a pulse ns into something more general. Ultimately, it would be nice if this was a common capability used by both BE and FE.
      
      * Updated formatter state
      
      The rows in qp.si/StreamingResultsWriter are ordered, so there should be no need to do a lookup by col. We should just be able to map the ordered-formatters (using the same order as the cols) with the ordered row data.
      
      * Updating tests in `metabase.query-processor.streaming-test` to reflect new csv formatting.
      
      * Updated fomatter for tests and consistency
      
      This updates several tests (metabase.query-processor.streaming.csv-test) to reflect the new and correct behavior of using a common csv formatter as well as update the formatting code to correctly handle relational types and datetimes.
      
      * require fmt
      
      * Updating metabase.pulse.render.body-test to reflect `:type/DateTime` formatting.
      
      * Updating sqlite test
      
      * Updating report-timezone-test to reflect new CSV rendering
      
      * Fixing e2e test
      Unverified
      356c4986
    • Jerry Huang's avatar
    • Braden Shepherdson's avatar
      [MLv2] Unskip FE integration tests for automatic-insights drills (#36529) · 25ce6e4c
      Braden Shepherdson authored
      This was missed from #36443.
      
      Fixes #33558.
      Unverified
      25ce6e4c
    • metamben's avatar
      Enable converting simple value expressions from pMBQL to legacy (#36493) · e97a889f
      metamben authored
      Fixes #36459.
      
      Also makes sure that legacy to pMBQL works and that simple value expressions can be named.
      Unverified
      e97a889f
    • Anton Kulyk's avatar
      Fix dashboard filter width in edit mode (#36483) · 8b0fe5b6
      Anton Kulyk authored
      * Use `minWidth` instead of `width`
      
      * Reserve left padding for the settings icon
      Unverified
      8b0fe5b6
    • Gustavo Saiani's avatar
      Tweak code block in doc (#36501) · 79074fa7
      Gustavo Saiani authored
      Unverified
      79074fa7
    • Alexander Solovyov's avatar
  5. Dec 06, 2023
Loading