Merge branch 'master' into current-period-instrumentation

frontend/src/metabase/pulse/components/PulseCardPreview.jsx

@@ -22,6 +22,7 @@ export default class PulseCardPreview extends Component {
         onRemove: PropTypes.func.isRequired,
         fetchPulseCardPreview: PropTypes.func.isRequired,
         attachmentsEnabled: PropTypes.bool,
+        trackPulseEvent: PropTypes.func.isRequired
     };
 
     componentWillMount() {
@@ -46,8 +47,12 @@ export default class PulseCardPreview extends Component {
         const { card, onChange } = this.props;
         if (this.hasAttachment()) {
             onChange({ ...card, include_csv: false, include_xls: false })
+
+            this.props.trackPulseEvent("RemoveAttachment")
         } else {
             onChange({ ...card, include_csv: true })
+
+            this.props.trackPulseEvent("AddAttachment", 'csv')
         }
     }
 

frontend/src/metabase/pulse/components/PulseEditCards.jsx

@@ -38,10 +38,17 @@ export default class PulseEditCards extends Component {
         });
     }
 
+    trackPulseEvent = (eventName: string, eventValue: string) => {
+        MetabaseAnalytics.trackEvent(
+            (this.props.pulseId) ? "PulseEdit" : "PulseCreate",
+            eventName,
+            eventValue
+        );
+    }
+
     addCard(index, cardId) {
         this.setCard(index, { id: cardId })
-
-        MetabaseAnalytics.trackEvent((this.props.pulseId) ? "PulseEdit" : "PulseCreate", "AddCard", index);
+        this.trackPulseEvent("AddCard", index);
     }
 
     removeCard(index) {
@@ -51,7 +58,7 @@ export default class PulseEditCards extends Component {
             cards: [...pulse.cards.slice(0, index), ...pulse.cards.slice(index + 1)]
         });
 
-        MetabaseAnalytics.trackEvent((this.props.pulseId) ? "PulseEdit" : "PulseCreate", "RemoveCard", index);
+        this.trackPulseEvent("RemoveCard", index);
     }
 
     getNotices(card, cardPreview, index) {
@@ -61,7 +68,7 @@ export default class PulseEditCards extends Component {
         if (hasAttachment) {
             notices.push({
                 head: t`Attachment`,
-                body: <AttachmentWidget card={card} onChange={(card) => this.setCard(index, card)} />
+                body: <AttachmentWidget card={card} onChange={(card) => this.setCard(index, card)} trackPulseEvent={this.trackPulseEvent} />
             });
         }
         if (cardPreview) {
@@ -134,6 +141,7 @@ export default class PulseEditCards extends Component {
                                             onRemove={this.removeCard.bind(this, index)}
                                             fetchPulseCardPreview={this.props.fetchPulseCardPreview}
                                             attachmentsEnabled={this.props.attachmentsEnabled}
+                                            trackPulseEvent={this.trackPulseEvent}
                                         />
                                     :
                                         <CardPicker
@@ -155,7 +163,7 @@ export default class PulseEditCards extends Component {
 
 const ATTACHMENT_TYPES = ["csv", "xls"];
 
-const AttachmentWidget = ({ card, onChange }) =>
+const AttachmentWidget = ({ card, onChange, trackPulseEvent }) =>
     <div>
         { ATTACHMENT_TYPES.map(type =>
             <span
@@ -166,6 +174,8 @@ const AttachmentWidget = ({ card, onChange }) =>
                     for (const attachmentType of ATTACHMENT_TYPES) {
                       newCard["include_" + attachmentType] = type === attachmentType;
                     }
+
+                    trackPulseEvent("AttachmentTypeChanged", type);
                     onChange(newCard)
                 }}
             >
@@ -176,5 +186,6 @@ const AttachmentWidget = ({ card, onChange }) =>
 
 AttachmentWidget.propTypes = {
     card: PropTypes.object.isRequired,
-    onChange: PropTypes.func.isRequired
+    onChange: PropTypes.func.isRequired,
+    trackPulseEvent: PropTypes.func.isRequired
 }

frontend/test/query_builder/qb_drillthrough.integ.spec.js

@@ -71,7 +71,6 @@ describe("QueryBuilder", () => {
                     }
                 }));
 
-
                 click(qb.find(RunButton));
                 await store.waitForActions([QUERY_COMPLETED]);
 
@@ -79,7 +78,9 @@ describe("QueryBuilder", () => {
                 const firstRowCells = table.find("tbody tr").first().find("td");
                 expect(firstRowCells.length).toBe(2);
 
-                expect(firstRowCells.first().text()).toBe("4  –  6");
+                // NOTE: Commented out due to the randomness involved in sample dataset generation
+                // which sometimes causes the cell value to be different
+                // expect(firstRowCells.first().text()).toBe("4  –  6");
 
                 const countCell = firstRowCells.last();
                 expect(countCell.text()).toBe("2");
@@ -187,6 +188,10 @@ describe("QueryBuilder", () => {
                 // Should have visualization type set to the previous visualization
                 const card = getCard(store.getState())
                 expect(card.display).toBe("bar");
+
+                // Some part of visualization seems to be asynchronous, causing a cluster of errors
+                // about missing query results if this delay isn't present
+                await delay(100)
             });
         })
     })

src/metabase/driver/googleanalytics/query_processor.clj

@@ -63,10 +63,10 @@
 
 ;;; ### breakout
 
-(defn- first-aggregation [{aggregations :aggregation}]
+(defn- aggregations [{aggregations :aggregation}]
   (if (every? sequential? aggregations)
-    (first aggregations)
-    aggregations))
+    aggregations
+    [aggregations]))
 
 (defn- unit->ga-dimension
   [unit]
@@ -170,7 +170,7 @@
                                 :descending "-")
                               (cond
                                 (instance? DateTimeField field) (unit->ga-dimension (:unit field))
-                                (instance? AgFieldRef field)    (second (first-aggregation query)) ; aggregation is of format [ag-type metric-name]; get the metric-name
+                                (instance? AgFieldRef field)    (second (nth (aggregations query) (:index field))) ; aggregation is of format [ag-type metric-name]; get the metric-name
                                 :else                           (->rvalue field)))))}))
 
 ;;; ### limit
@@ -249,11 +249,12 @@
 
 (defn- built-in-metrics
   [{query :query}]
-  (let [[aggregation-type metric-name] (first-aggregation query)]
-    (when (and aggregation-type
-               (= :metric (qputil/normalize-token aggregation-type))
-               (string? metric-name))
-      metric-name)))
+  (if-not (empty? (aggregations query))
+    (s/join "," (for [[aggregation-type metric-name] (aggregations query)
+                      :when (and aggregation-type
+                                 (= :metric (qputil/normalize-token aggregation-type))
+                                 (string? metric-name))]
+                  metric-name))))
 
 (defn- handle-built-in-metrics [query]
   (-> query