Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[superset-client][logger] replace ajax with SupersetClient #6133

Merged
merged 2 commits into from
Oct 23, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 51 additions & 40 deletions superset/assets/spec/javascripts/logger_spec.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import $ from 'jquery';
import sinon from 'sinon';
import fetchMock from 'fetch-mock';

import { Logger, ActionLog } from '../../src/logger';

Expand Down Expand Up @@ -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' };
Expand All @@ -59,30 +63,24 @@ describe('Logger', () => {
});

describe('.send()', () => {
beforeEach(() => {
sinon.spy($, 'ajax');
});
afterEach(() => {
$.ajax.restore();
});

const eventNames = ['test'];

function setup(overrides = {}) {
const log = new ActionLog({ eventNames, ...overrides });
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", () => {
Expand All @@ -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',
Expand All @@ -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);
},
);
});
Expand Down
23 changes: 9 additions & 14 deletions superset/assets/src/logger.js
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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 = [];
Expand All @@ -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
Expand All @@ -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 = {};

Expand Down