feat: sendHostname can works after 7 days and change GA Tracking ID #20
Conversation
|
Can one of the admins verify this patch? |
sohee-lee7
changed the title
src/js/request.js
Outdated
| */ | ||
| function isExpired(date) { | ||
| var now = new Date().getTime(); | ||
| var ms7days = 7 * 24 * 60 * 60 * 1000; |
There was a problem hiding this comment.
Extract ms7days variable to calculate this once.
src/js/request.js
Outdated
| var trackingId = 'UA-115377265-9'; | ||
| var eventCategory = 'use'; | ||
| var trackingId = trackingIdMap[applicationId] || trackingIdMap.component; | ||
| var applicationKeyForStorage = 'TOAST UI ' + applicationId + ' for ' + hostname + ': Statistics'; |
There was a problem hiding this comment.
The application name's first letter should be upper case.
There was a problem hiding this comment.
All components call 'sendHostname' with name of lower case.
|
|
||
| // skip only if the flag is defined and is set to false explicitly | ||
| // skip if the flag is defined and is set to false explicitly | ||
| if (!type.isUndefined(window.tui) && window.tui.usageStatistics === false) { |
There was a problem hiding this comment.
Can the first condition be true? This namespace tui always exist after loading tui-code-snippet.js, doesn't it?
|
[12/04] Reviewed. Good job 👍 |
src/js/request.js
Outdated
| 'chart': 'UA-129983528-1', | ||
| 'image-editor': 'UA-129999381-1', | ||
| 'component': 'UA-129987462-1' | ||
| }; |
There was a problem hiding this comment.
아이디는 여기서들고 있는것이 아니라 사용하는 쪽에서 들고 있어야 할것 같아요.
새 프로젝트가 추가되거나 ga 트래킹을 안 하던 프로젝트가 추가되게 되면 불필요하게 codesnippet도 배포해야 하니까요..
웹스토리지 키를 만드는것 때문에 필요하다면 이름도 추가하면 될것 같아요.
There was a problem hiding this comment.
이름과 아이디를 받는 형식으로 수정하겠습니다.
src/js/request.js
Outdated
| var trackingId = 'UA-115377265-9'; | ||
| var eventCategory = 'use'; | ||
| var trackingId = trackingIdMap[applicationId] || trackingIdMap.component; | ||
| var applicationKeyForStorage = 'TOAST UI ' + applicationId + ' for ' + hostname + ': Statistics'; |
There was a problem hiding this comment.
어플리케이션 별로 웹스토리지를 데이터를 만들 필요가 과연 있을까 싶어요..
2개 이상의 TOAST UI 제품을 사용한다면 모든 ga 수집에 동의하지 않을것 같아요.
There was a problem hiding this comment.
한 호스트에서 2개 이상의 TOAST UI 제품을 사용하고 로컬스토리지 1개로 판별하게 되면, 제품 중 하나만 GA를 보내게 되어 나머지는 기록에 누락될 수 있을 것 같습니다. 그래서 제품별로 로컬스토리지는 따로 갖고 있어야 할 것 같습니다.
| // skip if the flag is defined and is set to false explicitly | ||
| if (tui.usageStatistics === false) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
I'm also not sure that tui always exist. I'll revert this code.
GA 관련하여 sendHostname 함수 변경사항입니다.