superset-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ccwilli...@apache.org
Subject [incubator-superset] branch master updated: [superset-client][logger] replace ajax with SupersetClient (#6133)
Date Tue, 23 Oct 2018 05:42:33 GMT
This is an automated email from the ASF dual-hosted git repository.

ccwilliams pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 8573fde  [superset-client][logger] replace ajax with SupersetClient (#6133)
8573fde is described below

commit 8573fdec122c6e8eab077b90a01d1f2dfb03d3c3
Author: Chris Williams <williaster@users.noreply.github.com>
AuthorDate: Mon Oct 22 22:42:23 2018 -0700

    [superset-client][logger] replace ajax with SupersetClient (#6133)
    
    * [superset-client][logger] replace ajax with SupersetClient
    
    * [superset-client][logger] don't parse log response
---
 superset/assets/spec/javascripts/logger_spec.js | 91 ++++++++++++++-----------
 superset/assets/src/logger.js                   | 23 +++----
 2 files changed, 60 insertions(+), 54 deletions(-)

diff --git a/superset/assets/spec/javascripts/logger_spec.js b/superset/assets/spec/javascripts/logger_spec.js
index 4b85759..36e2923 100644
--- a/superset/assets/spec/javascripts/logger_spec.js
+++ b/superset/assets/spec/javascripts/logger_spec.js
@@ -1,5 +1,4 @@
-import $ from 'jquery';
-import sinon from 'sinon';
+import fetchMock from 'fetch-mock';
 
 import { Logger, ActionLog } from '../../src/logger';
 
@@ -43,10 +42,15 @@ describe('ActionLog', () => {
     log.addEvent(eventName, eventBody);
     expect(log.events[eventName]).toHaveLength(1);
     expect(log.events[eventName][0]).toMatchObject(eventBody);
+    Logger.end(log);
   });
 });
 
 describe('Logger', () => {
+  const logEndpoint = 'glob:*/superset/log/*';
+  fetchMock.post(logEndpoint, 'success');
+  afterEach(fetchMock.resetHistory);
+
   it('should add events when .append(eventName, eventBody) is called', () => {
     const eventName = 'testEvent';
     const eventBody = { test: 'event' };
@@ -59,13 +63,6 @@ describe('Logger', () => {
   });
 
   describe('.send()', () => {
-    beforeEach(() => {
-      sinon.spy($, 'ajax');
-    });
-    afterEach(() => {
-      $.ajax.restore();
-    });
-
     const eventNames = ['test'];
 
     function setup(overrides = {}) {
@@ -73,16 +70,17 @@ describe('Logger', () => {
       return log;
     }
 
-    it('should POST an event to /superset/log/ when called', () => {
+    it('should POST an event to /superset/log/ when called', (done) => {
       const log = setup();
       Logger.start(log);
       Logger.append(eventNames[0], { test: 'event' });
       expect(log.events[eventNames[0]]).toHaveLength(1);
       Logger.end(log);
-      expect($.ajax.calledOnce).toBe(true);
-      const args = $.ajax.getCall(0).args[0];
-      expect(args.url).toBe('/superset/log/');
-      expect(args.method).toBe('POST');
+
+      setTimeout(() => {
+        expect(fetchMock.calls(logEndpoint)).toHaveLength(1);
+        done();
+      }, 0);
     });
 
     it("should flush the log's events", () => {
@@ -97,7 +95,7 @@ describe('Logger', () => {
 
     it(
       'should include ts, start_offset, event_name, impression_id, source, and source_id
in every event',
-      () => {
+      (done) => {
         const config = {
           eventNames: ['event1', 'event2'],
           impressionId: 'impress_me',
@@ -111,39 +109,52 @@ describe('Logger', () => {
         Logger.append('event2', { foo: 'bar' });
         Logger.end(log);
 
-        const args = $.ajax.getCall(0).args[0];
-        const events = JSON.parse(args.data.events);
-
-        expect(events).toHaveLength(2);
-        expect(events[0]).toMatchObject({
-          key: 'value',
-          event_name: 'event1',
-          impression_id: config.impressionId,
-          source: config.source,
-          source_id: config.sourceId,
-        });
-        expect(events[1]).toMatchObject({
-          foo: 'bar',
-          event_name: 'event2',
-          impression_id: config.impressionId,
-          source: config.source,
-          source_id: config.sourceId,
-        });
-        expect(typeof events[0].ts).toBe('number');
-        expect(typeof events[1].ts).toBe('number');
-        expect(typeof events[0].start_offset).toBe('number');
-        expect(typeof events[1].start_offset).toBe('number');
+        setTimeout(() => {
+          const calls = fetchMock.calls(logEndpoint);
+          expect(calls).toHaveLength(1);
+          const options = calls[0][1];
+          const events = JSON.parse(options.body.get('events'));
+
+          expect(events).toHaveLength(2);
+
+          expect(events[0]).toMatchObject({
+            key: 'value',
+            event_name: 'event1',
+            impression_id: config.impressionId,
+            source: config.source,
+            source_id: config.sourceId,
+          });
+
+          expect(events[1]).toMatchObject({
+            foo: 'bar',
+            event_name: 'event2',
+            impression_id: config.impressionId,
+            source: config.source,
+            source_id: config.sourceId,
+          });
+
+          expect(typeof events[0].ts).toBe('number');
+          expect(typeof events[1].ts).toBe('number');
+          expect(typeof events[0].start_offset).toBe('number');
+          expect(typeof events[1].start_offset).toBe('number');
+
+          done();
+        }, 0);
       },
     );
 
     it(
       'should send() a log immediately if .append() is called with sendNow=true',
-      () => {
+      (done) => {
         const log = setup();
         Logger.start(log);
         Logger.append(eventNames[0], { test: 'event' }, true);
-        expect($.ajax.calledOnce).toBe(true);
-        Logger.end(log);
+
+        setTimeout(() => {
+          expect(fetchMock.calls(logEndpoint)).toHaveLength(1);
+          Logger.end(log); // flush logs
+          done();
+        }, 0);
       },
     );
   });
diff --git a/superset/assets/src/logger.js b/superset/assets/src/logger.js
index afd33d8..abc6339 100644
--- a/superset/assets/src/logger.js
+++ b/superset/assets/src/logger.js
@@ -1,6 +1,5 @@
 /* eslint no-console: 0 */
-
-import $ from 'jquery';
+import { SupersetClient } from '@superset-ui/core';
 
 // This creates an association between an eventName and the ActionLog instance so that
 // Logger.append calls do not have to know about the appropriate ActionLog instance
@@ -41,13 +40,13 @@ export const Logger = {
 
   send(log) {
     const { impressionId, source, sourceId, events } = log;
-    let url = '/superset/log/';
+    let endpoint = '/superset/log/?explode=events';
 
     // backend logs treat these request params as first-class citizens
     if (source === 'dashboard') {
-      url += `?dashboard_id=${sourceId}`;
+      endpoint += `&dashboard_id=${sourceId}`;
     } else if (source === 'slice') {
-      url += `?slice_id=${sourceId}`;
+      endpoint += `&slice_id=${sourceId}`;
     }
 
     const eventData = [];
@@ -63,14 +62,10 @@ export const Logger = {
       });
     }
 
-    $.ajax({
-      url,
-      method: 'POST',
-      dataType: 'json',
-      data: {
-        explode: 'events',
-        events: JSON.stringify(eventData),
-      },
+    SupersetClient.post({
+      endpoint,
+      postPayload: { events: eventData },
+      parseMethod: null,
     });
 
     // flush events for this logger
@@ -88,7 +83,7 @@ export class ActionLog {
     this.impressionId = impressionId;
     this.source = source;
     this.sourceId = sourceId;
-    this.eventNames = eventNames;
+    this.eventNames = eventNames || [];
     this.sendNow = sendNow || false;
     this.events = {};
 


Mime
View raw message