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

Update EventClient following event spec r0.1 #2

Merged
merged 13 commits into from
Nov 6, 2020
Merged

Conversation

ebongzzang
Copy link
Contributor

1. 변경 내용 요약

  • 이벤트 메소드 이름을 수정했습니다. sendUnenrollPreferences -> sendUnenrollPreference
  • Init 이벤트를 추가해서 GTM에서 ridi-event-tracker가 적절한때에 초기화되도록 수정했습니다.
  • 이벤트 전송시 event_params 내에 uId를 기본으로 포함하도록 수정했습니다.
  • PageView이벤트를 수정했습니다.
    • 클라이언트에서 href, referrer, pageMeta 를 보내는 대신 GTM 내에서 기본 변수인 href, referrer를 활용해 계산하도록 바꿨습니다.
  • ts 가 잘못된 값으로 넘어가고 있던 문제를 수정했습니다.

2. 그 외

  • ruid, pvid
    • event-client에서 이 두개를 직접 만들어 보내는 대신 ridi-event-tracker 에서 자동으로 ruid, pvid를 생성해주려고 합니다.
    • 기존에 beacon tracker에서 ruidpvid를 자동으로 생성해 보내주는 부분이 있어서 이를 활용하면 될 것 같아요.

3. 중점적으로 리뷰받고 싶은 사항

3. 배포 후

README.md Outdated

eventClient.initialize().then((_) => {

Choose a reason for hiding this comment

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

만일 이벤트 함수를 특정 순간에 찍는다면 그때마다 eventClient.initialize().then(...) 내부에서 찍어주어야 하는지 궁금합니다!
만일 이미 initialize가 이미 호출되었다면 이벤트 함수를 바로 호출할 수는 없을까요?
(내부적으로 initialized 플래그가 있다던지..??)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

gtm 스크립트는 async / sync 차이가 없어서 그냥 initialize 메소드 자체를 없앴습니다.

boridevna
boridevna previously approved these changes Nov 6, 2020
README.md Outdated Show resolved Hide resolved
boridevna
boridevna previously approved these changes Nov 6, 2020
@ebongzzang ebongzzang merged commit efb97b7 into main Nov 6, 2020
@ebongzzang ebongzzang deleted the dev/ridi-event/0.1 branch November 8, 2020 05:31
@ebongzzang ebongzzang mentioned this pull request Nov 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants