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

feat: sendHostname can works after 7 days and change GA Tracking ID #20

Merged
merged 8 commits into from
Dec 5, 2018
Merged
Show file tree
Hide file tree
Changes from 5 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
43 changes: 38 additions & 5 deletions src/js/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,28 @@

var object = require('./object');
var collection = require('./collection');
var type = require('./type');

var trackingIdMap = {
'editor': 'UA-129966929-1',
'grid': 'UA-129951906-1',
'calendar': 'UA-129951699-1',
'chart': 'UA-129983528-1',
'image-editor': 'UA-129999381-1',
'component': 'UA-129987462-1'
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아이디는 여기서들고 있는것이 아니라 사용하는 쪽에서 들고 있어야 할것 같아요.
새 프로젝트가 추가되거나 ga 트래킹을 안 하던 프로젝트가 추가되게 되면 불필요하게 codesnippet도 배포해야 하니까요..
웹스토리지 키를 만드는것 때문에 필요하다면 이름도 추가하면 될것 같아요.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이름과 아이디를 받는 형식으로 수정하겠습니다.

var ms7days = 7 * 24 * 60 * 60 * 1000;

/**
* Check if the date has passed 7 days
* @param {number} date - milliseconds
* @returns {boolean}
* @ignore
*/
function isExpired(date) {
var now = new Date().getTime();

return now - date > ms7days;
}

/**
* Send hostname on DOMContentLoaded.
Expand All @@ -19,13 +40,23 @@ function sendHostname(applicationId) {
var url = 'https://www.google-analytics.com/collect';
var hostname = location.hostname;
var hitType = 'event';
var trackingId = 'UA-115377265-9';
var eventCategory = 'use';
var trackingId = trackingIdMap[applicationId] || trackingIdMap.component;
var applicationKeyForStorage = 'TOAST UI ' + applicationId + ' for ' + hostname + ': Statistics';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The application name's first letter should be upper case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All components call 'sendHostname' with name of lower case.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

어플리케이션 별로 웹스토리지를 데이터를 만들 필요가 과연 있을까 싶어요..
2개 이상의 TOAST UI 제품을 사용한다면 모든 ga 수집에 동의하지 않을것 같아요.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

한 호스트에서 2개 이상의 TOAST UI 제품을 사용하고 로컬스토리지 1개로 판별하게 되면, 제품 중 하나만 GA를 보내게 되어 나머지는 기록에 누락될 수 있을 것 같습니다. 그래서 제품별로 로컬스토리지는 따로 갖고 있어야 할 것 같습니다.

var date = window.localStorage.getItem(applicationKeyForStorage);

// skip if the flag is defined and is set to false explicitly
if (tui.usageStatistics === false) {
return;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if using the global tui.usageStatics is reliable when installed by npm.
Is the tui namespace always exist when code-snippet is used?

Copy link
Contributor Author

@sohee-lee7 sohee-lee7 Dec 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also not sure that tui always exist. I'll revert this code.


// skip only if the flag is defined and is set to false explicitly
if (!type.isUndefined(window.tui) && window.tui.usageStatistics === false) {
// skip if not pass seven days old
if (date && !isExpired(date)) {
return;
}

window.localStorage.setItem(applicationKeyForStorage, new Date().getTime());

setTimeout(function() {
if (document.readyState === 'interactive' || document.readyState === 'complete') {
imagePing(url, {
Expand All @@ -34,7 +65,9 @@ function sendHostname(applicationId) {
tid: trackingId,
cid: hostname,
dp: hostname,
dh: applicationId
dh: applicationId,
el: applicationId,
ec: eventCategory
});
}
}, 1000);
Expand Down
43 changes: 43 additions & 0 deletions test/request.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ describe('module:request', function() {
// can not spy on imagePing. spy on appendChild instead.
spyOn(document.body, 'appendChild');
spyOn(document.body, 'removeChild');
spyOn(localStorage, 'getItem').and.returnValue(null);
});

it('should call appendChild', function(done) {
Expand All @@ -51,4 +52,46 @@ describe('module:request', function() {
}, 1500);
});
});

describe('sendHostname with localstorage', function() {
beforeEach(function() {
window.tui = window.tui || {};

// can not spy on imagePing. spy on appendChild instead.
spyOn(document.body, 'appendChild');
spyOn(document.body, 'removeChild');
});

it('should not call appendChild within 7 days', function(done) {
var now = new Date().getTime();
var ms6days = 6 * 24 * 60 * 60 * 1000;

spyOn(localStorage, 'getItem').and.returnValue(now - ms6days);

window.tui.usageStatistics = true;

request.sendHostname('editor');

setTimeout(function() {
expect(document.body.appendChild).not.toHaveBeenCalled();
done();
}, 1500);
});

it('should call appendChild after 7 days', function(done) {
var now = new Date().getTime();
var ms8days = 8 * 24 * 60 * 60 * 1000;

spyOn(localStorage, 'getItem').and.returnValue(now - ms8days);

window.tui.usageStatistics = true;

request.sendHostname('editor');

setTimeout(function() {
expect(document.body.appendChild).toHaveBeenCalled();
done();
}, 1500);
});
});
});