Ga4/prod #173

Shivam4415 · 2020-12-17T08:18:58Z

Description of the change

Description here

Type of change

Bug fix (non-breaking change that fixes an issue)
New feature (non-breaking change that adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

Related issues

Fix #1

Checklists

Development

Lint rules pass locally
The code changed/added as part of this pull request has been covered with tests
All tests related to the changed code pass in development

Code review

This pull request has a descriptive title and information useful to a reviewer. There may be a screenshot or screencast attached
"Ready for review" label attached to the PR and reviewers mentioned in a comment
Changes have been reviewed by at least one other engineer
Issue from task tracker has a link to this pull request

This change is

add intg name

sayan-mitra · 2020-12-17T09:16:22Z

integrations/GA4/browser.js

+  }
+  /* utility functions --- Ends here ---  */
+
+  isReservedName(eventName) {


add the helper methods to GA4/utils ?

sayan-mitra · 2020-12-17T09:16:40Z

integrations/GA4/browser.js

+    window.gtag("event", event, props);
+  }
+
+  getDestinationEvent(event) {


this too to utils

sayan-mitra · 2020-12-17T09:16:48Z

integrations/GA4/browser.js

+    return eventName.find((p) => p.src.includes(event.toLowerCase()));
+  }
+
+  getDestinationEventProperties(props, destParameter) {


same as above

As method doc like what is the input and output format with an example

better to use a library that does this? can you check once like get-value we use for transformer?

better to use a library that does this? can you check once like get-value we use for transformer?

There is a library : " loadash " using that we can do, but loading some external library means loading extra js, isn't if we use our own implementation where we can modify code as required. which means we can move above function to global and make more generic. One of the way I can think is going through loadash code and provide same implementation in our utils.
What do you say, should I use external library or implement on our own .....

we won't be loading a js but yes our total JS SDK size will increase. We can leave it
Can you add the examples for the input and output like
/*
props: {key: val, key2:val2}
destParameter: {..}
output: {...}
*/
getDestinationEventProperties(props, destParameter) {...}

sayan-mitra · 2020-12-17T09:17:01Z

integrations/GA4/browser.js

+    return destinationProperties;
+  }
+
+  getDestinationItemProperties(products, item) {


same as above

sayan-mitra · 2020-12-17T09:20:24Z

integrations/integration_cname.js

+  "Google Analytics 4": "GA4",
+  GoogleAnalytics4: "GA4",
+  GA4: "GA4",
+>>>>>>> 23bdbcd... GA4 sdk implementation


resolve conflict

sayan-mitra · 2020-12-17T09:20:47Z

integrations/client_server_name.js

@@ -20,8 +20,12 @@ const clientToServerNames = {
  OPTIMIZELY: "Optimizely",
  FULLSTORY: "Fullstory",
  TVSQUUARED: "TVSquared",
+<<<<<<< HEAD


resolve conflict

sayan-mitra · 2020-12-17T09:24:29Z

integrations/GA4/browser.js

+    window.gtag(
+      "event",
+      "page_view",
+      (rudderElement.message.context && rudderElement.message.context.page) ||


https://support.google.com/analytics/answer/9234069
let's try to map some of our params to the params mentioned in this link,
For ex: page_view ==>

page_location

page_referrer

page_title
(So that we don't blow up number of unique params supported by GA..there is a limit there I think of unique params https://developers.google.com/analytics/devguides/collection/ga4/tag-guide#how_do_events_and_parameters_work_with_google_analytics_4_properties)
For extra params let's take a boolean input from config like send extra_params for page view and then send them. Also use rudderElement.message.properties instead of rudderElement.message.context.page

Also check if GA4 allows sending campaign info present under rudderElement.message.context.campaign

Added function to support GA4 params and added config to decide if need to send extra params

sayan-mitra · 2020-12-17T09:26:29Z

integrations/GA4/browser.js

+  identify(rudderElement) {
+    if (this.sendUserId && rudderElement.message.userId) {
+      const userId = this.analytics.userId || this.analytics.anonymousId;
+      window.gtag("config", this.measurementId, {


this works even if identify is not the first call?

Yes, user is identified even if no identify call is made. As soon as ga scripts load he gets identified in google analytics.

sayan-mitra · 2020-12-17T09:31:01Z

integrations/GA4/browser.js

+      throw Error("Cannot call un-named/reserved named track event");
+    }
+
+    const obj = this.getDestinationEvent(event);


better name like destEventName

sayan-mitra · 2020-12-17T09:32:27Z

integrations/GA4/browser.js

+  /* utility functions --- Ends here ---  */
+
+  isReservedName(eventName) {
+    const reservedName = [


reservedEventNames

sayan-mitra · 2020-12-17T09:32:50Z

integrations/GA4/ECommerceEventConfig.js

@@ -0,0 +1,83 @@
+const eventName = [


eventNamesConfigArray

sayan-mitra · 2020-12-17T09:32:59Z

integrations/GA4/ECommerceEventConfig.js

+  // --------
+];
+
+const eventParameter = [


similar as above

sayan-mitra · 2020-12-17T09:33:07Z

integrations/GA4/ECommerceEventConfig.js

+  { src: "checkout_id", dest: ["transaction_id"] },
+];
+
+const itemParameter = [


similar as above

sayan-mitra · 2020-12-18T04:36:49Z

use prettier and eslint in vs code. It will take the config from .prettierrc and .eslintrc present in the repo.
This will help resolve the new files as much as possible.
Can you try that @Shivam4415

sayan-mitra · 2020-12-18T05:11:28Z

integrations/index.js

@@ -17,8 +17,12 @@ import * as Optimizely from "./Optimizely";
 import * as Bugsnag from "./Bugsnag";
 import * as Fullstory from "./Fullstory";
 import * as TVSquared from "./TVSquared";
+<<<<<<< HEAD


missed this?

sayan-mitra · 2020-12-18T05:14:07Z

integrations/GA4/browser.js

+  }
+
+  page(rudderElement) {
+    const page =


change to pageProps
get the value from message.properties. Reason: for page call to Rudder SDK ==> this is the syntax:
page(, {}, ...)
here properties can be any custom param, which is not present un message.context.page (the contextual info only contains standard params like referrer, url, title etc)

sayan-mitra · 2020-12-18T05:22:47Z

integrations/GA4/utility.js

+    );
+  }
+
+  function getDestinationEventProperties(props, destParameter) {


destParameter ==> destParameterConfig

sayan-mitra · 2020-12-18T05:23:17Z

integrations/GA4/utility.js

+    return reservedEventNames.includes(name);
+  }
+
+  function getDestinationEventName(events) {


events ==> event

sayan-mitra · 2020-12-18T05:40:22Z

integrations/GA4/browser.js

+  }
+  /* utility functions --- Ends here ---  */
+
+  getDestinationItemProperties(products, item) {


forgot to remove from here ?

now removed

sayan-mitra · 2020-12-18T05:42:00Z

integrations/GA4/browser.js

+    const items = [];
+    let obj = {};
+    products.forEach((p) => {
+      obj = {


better name than obj

obj -> eventMappingObj

sayan-mitra · 2020-12-18T05:56:08Z

integrations/GA4/ECommerceEventConfig.js

+  { src: "affiliation", dest: ["affiliation"] },
+  { src: "shipping", dest: ["shipping"] },
+  { src: "tax", dest: ["tax"] },
+  { src: "affiliation", dest: ["affiliation"] },


remove duplicate

sayan-mitra · 2020-12-18T06:22:04Z

add a way to check and abort if item_id/item_name, promotion_id/promotion_name, search_term are not present in the final destination payload.
ref: https://developers.google.com/gtagjs/reference/ga4-events#view_promotion

sayan-mitra · 2020-12-18T07:27:02Z

Update getDestinationEventProperties logic as discussed
formatting fixes
method definition examples and comments
Wishlist and share(product and cart) mapping
Utility to Utils
Move tests under GA folder to keep it localised and keep on completing it

sayan-mitra · 2020-12-18T11:23:02Z

integrations/GA4/ECommerceEventConfig.js

+
+const eventParametersConfigArray = [
+  { src: "query", dest: ["search_term"] },
+  { src: "list_id", dest: ["item_list_id", "items.item_list_id"] },


instead of array, use a flag like storeAlsoInItems: true/false
that will make the parsing simpler?

okkk looking into it

sayan-mitra · 2020-12-21T12:23:28Z

integrations/GA4/ECommerceEventConfig.js

    ],
-    hasItem: true,
+    hasMultiplePayload: true,


do we need this?
can we do this:
{
src: ["payment info entered"],
dest: "add_payment_info",
hasItem: false,
onlyIncludeParams: includeParams.PaymentInfo,
},

{
src: ["payment info entered"],
dest: "add_shipping_info",
hasItem: false,
onlyIncludeParams: includeParams.ShippingInfo,
},

and filter multiple events by same src name?

we can do this as well

…dMultiplepayload flag. Also includeList instead of onlyparams for Payment Info Entered.

…of two values

sayan-mitra and others added 4 commits December 2, 2020 21:43

Merge pull request #168 from rudderlabs/am-name

58837d4

add intg name

ga4 first commit

923a803

GA4 sdk comment updated

222b325

Added GA4 ecommerce Event

a821868

Shivam4415 requested a review from sayan-mitra December 17, 2020 08:18

sayan-mitra reviewed Dec 17, 2020

View reviewed changes

Shivam4415 added 2 commits December 17, 2020 19:47

resolved conflicts

2bb0b3b

review commits fixes

a036105

sayan-mitra reviewed Dec 18, 2020

View reviewed changes

review commits fixes 2

720b4ac

fixed wrong indentation

366d838

sayan-mitra requested a review from prabrishac December 18, 2020 11:08

sayan-mitra reviewed Dec 18, 2020

View reviewed changes

Shivam4415 and others added 9 commits December 18, 2020 22:27

updated logic, eventConfig and added comments

5ee6d8c

Logic Changes according to comments and improved readibility

8848083

Added comments

313dd45

Removed test file to GA4 folder

a6cc5bc

add changes related to share etc

5804064

Updated code with new modification as discussed

217e8c9

Fixed Prettier prob

2d2d9c5

Fixed extra param getting added in payload

0953dad

Added comments for future scope

4c1b1f0

sayan-mitra reviewed Dec 21, 2020

View reviewed changes

Shivam4415 and others added 3 commits December 21, 2020 18:30

Replaced Find with Filter in getDestinationEventname() and removed ha…

d912d0b

…dMultiplepayload flag. Also includeList instead of onlyparams for Payment Info Entered.

Updated utils getDestinationEventName() to return only array instead …

8f7eab6

…of two values

add comments

7996e57

sayan-mitra changed the base branch from production to production-staging December 21, 2020 16:14

merge staging to this

15b7f7f

sayan-mitra merged commit b84af1b into production-staging Dec 21, 2020

akashrpo deleted the ga4/prod branch March 5, 2022 10:28

psrikanth88 mentioned this pull request Jan 10, 2024

[Snyk] Security upgrade serialize-javascript from 6.0.1 to 6.0.2 #1585

Closed

Ga4/prod #173

Ga4/prod #173

Conversation

Shivam4415 commented Dec 17, 2020 • edited by SumanthPuram Loading

Description of the change

Type of change

Related issues

Checklists

Development

Code review

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

Choose a reason for hiding this comment

sayan-mitra Dec 17, 2020 • edited Loading

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

sayan-mitra commented Dec 18, 2020 • edited Loading

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

sayan-mitra commented Dec 18, 2020

sayan-mitra commented Dec 18, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sayan-mitra Dec 21, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Shivam4415 commented Dec 17, 2020 •

edited by SumanthPuram

Loading

sayan-mitra Dec 17, 2020 •

edited

Loading

sayan-mitra commented Dec 18, 2020 •

edited

Loading

sayan-mitra Dec 21, 2020 •

edited

Loading