Adrienne / Rudderstack integration #9013

adrienne-deriv · 2023-06-14T09:24:16Z

Changes:

Removed rudderstack-store from root store
Added rudderstack integration to shared package

Please provide a summary of the change.

Screenshots:

Please provide some screenshots of the change.

vercel · 2023-06-14T09:24:19Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
deriv-app	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 5, 2023 9:48am

github-actions · 2023-06-14T09:53:51Z

A production App ID was automatically generated for this PR. (log)

PR: Adrienne / Rudderstack integration #9013
URLs:
- w/ App ID + Server: https://deriv-app-git-fork-adrienne-deriv-rudderstack-integration-hook.binary.sx?qa_server=red.binaryws.com&app_id=24096
- Original: https://deriv-app-git-fork-adrienne-deriv-rudderstack-integration-hook.binary.sx
App ID: 24096

Click here to copy & paste above information.

- **PR**: [https://github.com/binary-com/deriv-app/pull/9013](https://github.com/binary-com/deriv-app/pull/9013)
- **URLs**:
    - **w/ App ID + Server**: https://deriv-app-git-fork-adrienne-deriv-rudderstack-integration-hook.binary.sx?qa_server=red.binaryws.com&app_id=24096
    - **Original**: https://deriv-app-git-fork-adrienne-deriv-rudderstack-integration-hook.binary.sx
- **App ID**: `24096`

github-actions · 2023-06-14T09:54:18Z

🚨 Lighthouse report for the changes in this PR:

Category	Score
🔺 Performance	20
🟧 Accessibility	75
🟢 Best practices	92
🟧 SEO	85
🟢 PWA	90

Lighthouse ran with https://deriv-app-git-fork-adrienne-deriv-rudderstack-integration-hook.binary.sx/

farzin-deriv

Nice work man 🚀

I think we can go with a simpler implementation, We basically just want to add types around RudderStack SDK 🤔 What do you think? 🤔

class RudderStack {
    constructor() {
        this.init();
    }

    init = () => {
        // Abort if RudderStack key or url is not set in .env
        if (!process.env.RUDDERSTACK_KEY || !process.env.RUDDERSTACK_URL) return;

        rudderanalytics.load(process.env.RUDDERSTACK_KEY, process.env.RUDDERSTACK_URL);
    };

    identify = (user_id: string, payload: TEvents['identify']) => rudderanalytics.identify(user_id, payload);

    track = <T extends keyof TEvents>(event: T, payload: TPayload[T]) => rudderanalytics.track(event, payload);

    page = (name: string) => rudderanalytics.page('Deriv App', name);

    reset = () => rudderanalytics.reset();
}

farzin-deriv · 2023-06-14T09:57:52Z

packages/core/src/Stores/rudderstack-store.js

farzin-deriv · 2023-06-14T09:57:58Z

packages/core/src/index.html

farzin-deriv · 2023-06-14T10:34:24Z

packages/shared/src/utils/analytics/rudderstack.ts

+    pageView() {
+        const current_page = window.location.hostname + window.location.pathname;
+


Suggested change

pageView() {

const current_page = window.location.hostname + window.location.pathname;

pageView(current_page: string) {

@adrienne-deriv Let's make it prop instead of using window.

farzin-deriv · 2023-06-14T10:35:00Z

packages/shared/package-lock.json

@adrienne-deriv This file shouldn't be in the changes 👀

farzin-deriv · 2023-06-14T10:36:02Z

packages/shared/src/index.ts

@@ -1,4 +1,5 @@
 export * from './services';
+export * from './utils/analytics';


@adrienne-deriv I would still recommend creating a new package like @deriv/analytics and move this implementation there instead of @deriv/shared 👀

cc: @yashim-deriv

Absolutely. I think this would be a great thing to abstract out, as we will surely use it in multiple projects as we abstract out the projects.

farzin-deriv · 2023-06-14T10:38:16Z

packages/shared/src/utils/analytics/rudderstack.ts

+    current_page = '';
+
+    constructor() {
+        const write_key = isProduction() ? process.env.RUDDERSTACK_PRODUCTION_KEY : process.env.RUDDERSTACK_STAGING_KEY;


Suggested change

const write_key = isProduction() ? process.env.RUDDERSTACK_PRODUCTION_KEY : process.env.RUDDERSTACK_STAGING_KEY;

const write_key = process.env.RUDDERSTACK_KEY;

This should be handled in the CI, We should expose the different environment variables there 🤔

cc: @ali-hosseini-deriv @yashim-deriv

with our current circleci setup this is not possible.
what we can do is to decide based on the circle ci job name.
I believe @adrienne-deriv is implementing it now in that way.

farzin-deriv · 2023-06-14T10:41:03Z

packages/shared/src/utils/analytics/rudderstack.ts

+    track<EventType extends keyof RSEvents, EventPayload extends RSEvents[EventType]>(
+        event_type: EventType,
+        payload: EventPayload
+    ) {


Suggested change

track<EventType extends keyof RSEvents, EventPayload extends RSEvents[EventType]>(

event_type: EventType,

payload: EventPayload

) {

track<T extends keyof TEvents>(event: T, payload: TPayload[T]) {

@adrienne-deriv Let's name things simpler 🙇🏻

farzin-deriv · 2023-06-14T11:08:22Z

packages/shared/src/utils/analytics/rudderstack.ts

+    error_message?: string;
+};
+
+type TradeTypesFormAction =


@adrienne-deriv Let's do some type improvements together when you are free if you like 🙇🏻

Yeaah I need some feedback on these types 😅

shayan-deriv · 2023-06-14T16:13:49Z

packages/core/src/App/Containers/Layout/app-contents.jsx

+            rudderstack.identifyEvent(user_id, {
+                language: getLanguage().toLowerCase() || 'en',
+            });
+            rudderstack.pageView();


why do we need to call pageView() again? because It's being called inside identyfyEvent 🤔

and correct me if I'm wrong, we are calling identyfyEvent inside client-store after user is logged in. so here when we call it again, it's not going to do anything because has_initialized is already set to true , right?

Actually I've added rudderstack.reset() when the user logs out, so this resets the state has_identified to false

Actually I'll remove the pageView call inside identifyEvent, it should be called separately

shayan-deriv

One more thing. I agree with Farzin and Mark about abstracting these things in a separate package so It can be used across all of our projects.

packages/core/src/Stores/client-store.js

niloofar-deriv · 2023-06-15T14:54:10Z

packages/shared/package-lock.json

@@ -0,0 +1,6373 @@
+{


Please remove this package-lock and run npm run bootstrap:dev in the root 🙏

…riv/deriv-app into rudderstack-integration-hook

maryia-deriv

👍🏻

coveralls · 2023-07-05T07:43:58Z

coverage: 8.902% (+0.03%) from 8.875% when pulling 212bb46 on adrienne-deriv:rudderstack-integration-hook into f48ed0d on binary-com:master.

…riv/deriv-app into rudderstack-integration-hook

ali-hosseini-deriv · 2023-07-05T08:33:25Z

packages/analytics/src/rudderstack.ts

+    }
+
+    init() {
+        const isProduction = process.env.CIRCLE_JOB === 'release_production';


please write it in a way to make sure CIRCLE_JOB is either production or staging otherwise you need to skip it.
lets say on test links or localhost, we need to exclude them, right?

jim-deriv

LGTM

sonarcloud · 2023-07-05T09:07:21Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
1.8% Duplication

ali-hosseini-deriv · 2023-07-05T11:01:20Z

✨ PR has been merged by Paimon the Release Bot

adrienne-deriv added 4 commits June 12, 2023 14:56

feat: integrated rudderstack hook

32c86bd

feat: added test cases for use-rudderstack hook

ce1d752

refactor: moved rudderstack out of store and into utils

4a13cbe

chore: updated conditions

967f9db

adrienne-deriv requested a review from yashim-deriv as a code owner June 14, 2023 09:24

boring-cyborg bot added Account Core P2P Shared Trader labels Jun 14, 2023

adrienne-deriv and others added 5 commits June 14, 2023 17:29

chore: removed rudderstack script html

3af0282

chore: removed test functions

e046141

chore: removed test functions

1218319

chore: removed old changes

92ceeb4

Merge branch 'master' into rudderstack-integration-hook

69a7391

vercel bot had a problem deploying to Preview June 14, 2023 10:04 Failure

farzin-deriv reviewed Jun 14, 2023

View reviewed changes

shayan-deriv reviewed Jun 14, 2023

View reviewed changes

adrienne-deriv commented Jun 15, 2023

View reviewed changes

packages/core/src/Stores/client-store.js Outdated Show resolved Hide resolved

chore: refactored code based on reviews

8afb3fd

vercel bot deployed to Preview June 15, 2023 08:37 View deployment

chore: test commit

58463a6

vercel bot deployed to Preview June 15, 2023 10:13 View deployment

niloofar-deriv reviewed Jun 15, 2023

View reviewed changes

Merge branch 'master' into rudderstack-integration-hook

7bbf22a

vercel bot deployed to Preview June 16, 2023 00:28 View deployment

vercel bot deployed to Preview June 23, 2023 04:20 View deployment

adrienne-deriv added 3 commits June 23, 2023 14:38

chore: reverted changes from reviews

f4063b8

Merge branch 'master' into rudderstack-integration-hook

0b616a3

Merge branch 'master' into rudderstack-integration-hook

824ebc4

vercel bot had a problem deploying to Preview June 23, 2023 06:54 Failure

adrienne-deriv added 2 commits June 23, 2023 15:31

chore: incorporated code review changes

2dcf1d2

Merge branch 'rudderstack-integration-hook' of github.com:adrienne-de…

666b5d1

…riv/deriv-app into rudderstack-integration-hook

vercel bot had a problem deploying to Preview June 23, 2023 07:47 Failure

farzin-deriv previously approved these changes Jun 23, 2023

View reviewed changes

maryia-deriv approved these changes Jun 23, 2023

View reviewed changes

Merge branch 'master' into rudderstack-integration-hook

bca879e

vercel bot deployed to Preview June 26, 2023 04:01 View deployment

jim-deriv previously approved these changes Jun 26, 2023

View reviewed changes

Merge branch 'master' into rudderstack-integration-hook

0746185

vercel bot deployed to Preview June 30, 2023 09:58 View deployment

Merge branch 'master' into rudderstack-integration-hook

768c79a

vercel bot deployed to Preview July 5, 2023 07:57 View deployment

adrienne-deriv added 2 commits July 5, 2023 16:27

fix: separated env variables for ruddetstack

b3dee8f

Merge branch 'rudderstack-integration-hook' of github.com:adrienne-de…

9820005

…riv/deriv-app into rudderstack-integration-hook

adrienne-deriv dismissed stale reviews from jim-deriv and farzin-deriv via 9820005 July 5, 2023 08:29

ali-hosseini-deriv reviewed Jul 5, 2023

View reviewed changes

refactor: updated conditions

212bb46

jim-deriv approved these changes Jul 5, 2023

View reviewed changes

ali-hosseini-deriv approved these changes Jul 5, 2023

View reviewed changes

vercel bot deployed to Preview July 5, 2023 09:48 View deployment

ali-hosseini-deriv merged commit 37b843d into deriv-com:master Jul 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adrienne / Rudderstack integration #9013

Adrienne / Rudderstack integration #9013

adrienne-deriv commented Jun 14, 2023 •

edited

Loading

vercel bot commented Jun 14, 2023 •

edited

Loading

github-actions bot commented Jun 14, 2023 •

edited

Loading

github-actions bot commented Jun 14, 2023 •

edited

Loading

farzin-deriv left a comment

farzin-deriv Jun 14, 2023

farzin-deriv Jun 14, 2023

farzin-deriv Jun 14, 2023

farzin-deriv Jun 14, 2023

farzin-deriv Jun 14, 2023

markw-deriv Jun 14, 2023

farzin-deriv Jun 14, 2023

ali-hosseini-deriv Jul 5, 2023

farzin-deriv Jun 14, 2023

farzin-deriv Jun 14, 2023

adrienne-deriv Jun 15, 2023

shayan-deriv Jun 14, 2023

shayan-deriv Jun 14, 2023

adrienne-deriv Jun 15, 2023

adrienne-deriv Jun 15, 2023 •

edited

Loading

shayan-deriv left a comment •

edited

Loading

niloofar-deriv Jun 15, 2023

maryia-deriv left a comment

coveralls commented Jul 5, 2023 •

edited

Loading

ali-hosseini-deriv Jul 5, 2023

jim-deriv left a comment

sonarcloud bot commented Jul 5, 2023

ali-hosseini-deriv commented Jul 5, 2023

		pageView() {
		const current_page = window.location.hostname + window.location.pathname;

	pageView() {
	const current_page = window.location.hostname + window.location.pathname;
	pageView(current_page: string) {

		@@ -1,4 +1,5 @@
		export * from './services';
		export * from './utils/analytics';

	const write_key = isProduction() ? process.env.RUDDERSTACK_PRODUCTION_KEY : process.env.RUDDERSTACK_STAGING_KEY;
	const write_key = process.env.RUDDERSTACK_KEY;

Adrienne / Rudderstack integration #9013

Adrienne / Rudderstack integration #9013

Conversation

adrienne-deriv commented Jun 14, 2023 • edited Loading

Changes:

Screenshots:

vercel bot commented Jun 14, 2023 • edited Loading

github-actions bot commented Jun 14, 2023 • edited Loading

github-actions bot commented Jun 14, 2023 • edited Loading

farzin-deriv left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adrienne-deriv Jun 15, 2023 • edited Loading

Choose a reason for hiding this comment

shayan-deriv left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maryia-deriv left a comment

Choose a reason for hiding this comment

coveralls commented Jul 5, 2023 • edited Loading

Choose a reason for hiding this comment

jim-deriv left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Jul 5, 2023

ali-hosseini-deriv commented Jul 5, 2023

adrienne-deriv commented Jun 14, 2023 •

edited

Loading

vercel bot commented Jun 14, 2023 •

edited

Loading

github-actions bot commented Jun 14, 2023 •

edited

Loading

github-actions bot commented Jun 14, 2023 •

edited

Loading

adrienne-deriv Jun 15, 2023 •

edited

Loading

shayan-deriv left a comment •

edited

Loading

coveralls commented Jul 5, 2023 •

edited

Loading