diff --git a/frontend/src/metabase/visualizations/visualizations/SmartScalar.jsx b/frontend/src/metabase/visualizations/visualizations/SmartScalar.jsx index 9b52e838e1489b6f9d57a61773028f4619cbcdf6..fbd1a67a9760d59643d6c3942adc6a1f2d54f619 100644 --- a/frontend/src/metabase/visualizations/visualizations/SmartScalar.jsx +++ b/frontend/src/metabase/visualizations/visualizations/SmartScalar.jsx @@ -87,7 +87,10 @@ export default class Smart extends React.Component { return null; } - const change = formatNumber(insight["last-change"] * 100); + const lastChange = insight["last-change"]; + const previousValue = insight["previous-value"]; + + const change = formatNumber(lastChange * 100); const isNegative = (change && Math.sign(change) < 0) || false; let color = isNegative ? colors["error"] : colors["success"]; @@ -160,25 +163,32 @@ export default class Smart extends React.Component { /> )} <Box className="SmartWrapper"> - <Flex align="center" mt={1} flexWrap="wrap"> - <Flex align="center" color={color}> - <Icon name={isNegative ? "arrowDown" : "arrowUp"} /> - {changeDisplay} + {!lastChange || !previousValue ? ( + <Box + className="text-centered text-bold mt1" + color={colors["text-medium"]} + >{jt`Nothing to compare for the previous ${granularity}.`}</Box> + ) : ( + <Flex align="center" mt={1} flexWrap="wrap"> + <Flex align="center" color={color}> + <Icon name={isNegative ? "arrowDown" : "arrowUp"} /> + {changeDisplay} + </Flex> + <h4 + id="SmartScalar-PreviousValue" + className="flex align-center hide lg-show" + style={{ + color: colors["text-medium"], + }} + > + {!isFullscreen && + jt`${separator} was ${formatValue( + previousValue, + settings.column(column), + )} ${granularityDisplay}`} + </h4> </Flex> - <h4 - id="SmartScalar-PreviousValue" - className="flex align-center hide lg-show" - style={{ - color: colors["text-medium"], - }} - > - {!isFullscreen && - jt`${separator} was ${formatValue( - insight["previous-value"], - settings.column(column), - )} ${granularityDisplay}`} - </h4> - </Flex> + )} </Box> </ScalarWrapper> ); diff --git a/src/metabase/sync/analyze/fingerprint/insights.clj b/src/metabase/sync/analyze/fingerprint/insights.clj index eae4216b873b8d44f9f1b1fdb4c7a12ff3d2f0f4..2e6e4d2e0bf3d89ffea26ec455535e14c0d0debe 100644 --- a/src/metabase/sync/analyze/fingerprint/insights.clj +++ b/src/metabase/sync/analyze/fingerprint/insights.clj @@ -135,6 +135,27 @@ "We downsize UNIX timestamps to lessen the chance of overflows and numerical instabilities." #(/ % (* 1000 60 60 24))) +(defn- about= + [a b] + (< 0.9 (/ a b) 1.1)) + +(def ^:private unit->duration + {:minute (/ 1 24 60) + :hour (/ 24) + :day 1 + :week 7 + :month 30.5 + :quarter (* 30.4 3) + :year 365.1}) + +(defn- valid-period? + [from to unit] + (when (and from to) + (let [delta (- to from)] + (if unit + (about= delta (unit->duration unit)) + (some (partial about= delta) (vals unit->duration)))))) + (defn- timeseries-insight [{:keys [numbers datetimes]}] (let [datetime (first datetimes) @@ -151,21 +172,26 @@ ;; at this stage in the pipeline the value is still an int, so we can use it ;; directly. (comp (stats/somef ms->day) #(nth % x-position)))] - (apply redux/juxt (for [number-col numbers] - (redux/post-complete - (let [y-position (:position number-col) - yfn #(nth % y-position)] - (redux/juxt ((map yfn) (last-n 2)) - (stats/simple-linear-regression xfn yfn) - (best-fit xfn yfn))) - (fn [[[previous current] [offset slope] best-fit]] - {:last-value current - :previous-value previous - :last-change (change current previous) - :slope slope - :offset offset - :best-fit best-fit - :col (:name number-col)})))))) + (apply redux/juxt + (for [number-col numbers] + (redux/post-complete + (let [y-position (:position number-col) + yfn #(nth % y-position)] + (redux/juxt ((map yfn) (last-n 2)) + ((map xfn) (last-n 2)) + (stats/simple-linear-regression xfn yfn) + (best-fit xfn yfn))) + (fn [[[y-previous y-current] [x-previous x-current] [offset slope] best-fit]] + (let [show-change? (valid-period? x-previous x-current (:unit datetime))] + {:last-value y-current + :previous-value (when show-change? + y-previous) + :last-change (when show-change? + (change y-current y-previous)) + :slope slope + :offset offset + :best-fit best-fit + :col (:name number-col)}))))))) (defn- datetime-truncated-to-year? "This is hackish as hell, but we change datetimes with year granularity to strings upstream and diff --git a/test/metabase/sync/analyze/fingerprint/insights_test.clj b/test/metabase/sync/analyze/fingerprint/insights_test.clj index f3f58333f001eb0fdce47cf93166953a697c5857..634729525ac2a0c4f12ba55807aefa56a86f8621 100644 --- a/test/metabase/sync/analyze/fingerprint/insights_test.clj +++ b/test/metabase/sync/analyze/fingerprint/insights_test.clj @@ -1,6 +1,6 @@ (ns metabase.sync.analyze.fingerprint.insights-test (:require [expectations :refer :all] - [metabase.sync.analyze.fingerprint.insights :refer :all])) + [metabase.sync.analyze.fingerprint.insights :refer :all :as i])) (def ^:private cols [{:base_type :type/DateTime} {:base_type :type/Number}]) @@ -33,3 +33,45 @@ (-> (transduce identity (insights cols) [[nil nil]]) first :last-value)) + + +(defn- valid-period? + ([from to] (valid-period? from to nil)) + ([from to period] + (boolean (#'i/valid-period? (some-> from (.getTime) (#'i/ms->day)) + (some-> to (.getTime) (#'i/ms->day)) + period)))) + +(expect + true + (valid-period? #inst "2015-01" #inst "2015-02")) +(expect + true + (valid-period? #inst "2015-02" #inst "2015-03")) +(expect + false + (valid-period? #inst "2015-01" #inst "2015-03")) +(expect + false + (valid-period? #inst "2015-01" nil)) +(expect + true + (valid-period? #inst "2015-01-01" #inst "2015-01-02")) +(expect + true + (valid-period? #inst "2015-01-01" #inst "2015-01-08")) +(expect + true + (valid-period? #inst "2015-01-01" #inst "2015-04-03")) +(expect + true + (valid-period? #inst "2015" #inst "2016")) +(expect + false + (valid-period? #inst "2015-01-01" #inst "2015-01-09")) +(expect + true + (valid-period? #inst "2015-01-01" #inst "2015-04-03" :quarter)) +(expect + false + (valid-period? #inst "2015-01-01" #inst "2015-04-03" :month))