Add events to Google Analytics with Node #245
Conversation
93b4c47
to
df0b4ef
Compare
|
Hi @gewenyu99 I have changed the code and pushed the changes Thanks for the review comments |
node/events-to-ga/package.json
Outdated
| "license": "ISC", | ||
| "dependencies": { | ||
| "node-appwrite": "^11.0.0", | ||
| "node-fetch": "^3.3.2", |
There was a problem hiding this comment.
Can we use fetch from undici module?
You can see examples in other templates
node/events-to-ga/src/main.js
Outdated
| const ga4MeasurementId = process.env.GA4_MEASUREMENT_ID; | ||
| const ga4secret = process.env.GA4_API_SECRET; |
There was a problem hiding this comment.
Can we just use these process.env inline? Just a style thing
|
Hi @HrithikSampson can you please work on the suggested changes and request a re-review by tomorrow? We can add the hacktoberfest-accepted label then |
|
Ok I will do it by tomorrow |
692a880
to
2310fbb
Compare
node/events-to-ga/README.md
Outdated
| @@ -0,0 +1,124 @@ | |||
| <!-- Name your function --> | |||
There was a problem hiding this comment.
Can you remove the markdown comments like this?
node/events-to-ga/src/main.js
Outdated
| import { fetch } from "undici"; | ||
| import { verifyHeaders, formatIntoGoogleAnalyticsEvent } from "./utils.js"; | ||
|
|
||
| /* Appwrite function */ |
node/events-to-ga/src/main.js
Outdated
| /* Appwrite function */ | ||
|
|
||
| export default async ({ res, req, log, error }) => { | ||
| // Listen for Appwrite events |
node/events-to-ga/src/utils.js
Outdated
| */ | ||
| export function verifyHeaders(req) { | ||
| if (req.headers["x-appwrite-user-id"] == "") { | ||
| throw `x-appwrite-user-id value in req.headers is not there`; |
There was a problem hiding this comment.
Can you do throw new Error("...") here instead? and below
node/events-to-ga/src/utils.js
Outdated
| throw `x-appwrite-user-id value in req.headers is not there`; | ||
| } | ||
| if (req.headers["x-appwrite-trigger"] != "event") { | ||
| throw `Not triggered by event but by ${req.headers["x-appwrite-trigger"]}`; |
There was a problem hiding this comment.
Can you do throw new Error("...") here instead?
node/events-to-ga/package.json
Outdated
| "undici": "^5.27.0" | ||
| }, | ||
| "devDependencies": { | ||
| "prettier": "^3.0.3" |
There was a problem hiding this comment.
Can we also add a .pretterrc.json with the same settings as other template?
node/events-to-ga/src/utils.js
Outdated
| if (i % 2 == 0) oddElemArray.push(splitArray[i]); | ||
| else | ||
| wildCardArray.push([ | ||
| `${oldElemArray[oldElemArray.length - 1]}Id`, | ||
| splitArray[i], | ||
| ]); |
There was a problem hiding this comment.
Please add curly braces here, we prefer to use common syntax so the template is accessible
node/events-to-ga/src/main.js
Outdated
| user_id: `${req.headers["x-appwrite-user-id"]}`, | ||
| events: [ | ||
| { | ||
| // Event names must have all characters as alphanumeric. |
2310fbb
to
79551ff
Compare
de93988
to
55edded
Compare
7ea30ce
to
4098807
Compare
|
Testing Video: https://drive.google.com/file/d/1Guw74H-xbZm1Xl2mlIlNBnMYVUBuhrAr/view?usp=sharing Hi @Haimantika, I have requested for rereview |
node/events-to-ga/src/utils.js
Outdated
| if (i % 2 == 0) oddElemArray.push(splitArray[i]); | ||
| else | ||
| wildCardArray.push([ | ||
| `${oddElemArray[oddElemArray.length - 1]}Id`, |
There was a problem hiding this comment.
Hmm this will result in something like databasesId, right? It would be better if it was singular to match how we typically have it.
There was a problem hiding this comment.
Also, this algorithm won't work for events like teams.*.memberships.*.update.status
There was a problem hiding this comment.
is it because of no 's' in update because that I can resolve but I cant find out a way to get [status,prefs,name,email,password] unless I consider them that these will not go to the wildcard array
There was a problem hiding this comment.
Can I assume the length of the words in wildCard array is more than 12 or somethiing to distinguish it
There was a problem hiding this comment.
is it because of no 's' in update because that I can resolve but I cant find out a way to get [status,prefs,name,email,password] unless I consider them that these will not go to the wildcard array
Because not every odd element is an ID. Although...I guess it's fine that status ends up in the params...
There was a problem hiding this comment.
Can I assume the length of the words in wildCard array is more than 12 or somethiing to distinguish it
No, an ID isn't necessarily more than 12 chars.
4098807
to
887c231
Compare
|
|
887c231
to
a549fcd
Compare
a549fcd
to
d3d4895
Compare
|
Hey there! There were a lot of big PRs during this Hacktoberfest, and we wanted to give everyone ample time to collaborate with our engineering team. If you were able to merge your PRs during October, amazing. If it’s still not merged, don’t worry about it either. Either way, we’ve got your Hacktoberfest swag minted and ready to ship. Please comment with your Discord username here so we can contact you about your shipping information to deliver your Hacktoberfest swag. |
|
Hi, |
|
Be in touch soon! |
What does this PR do?
The function created will be triggered by configured Appwrite events and report these events to Google Analytics
Test Plan
I have checked the functionality by creating a dummy webapp and triggering event in Google Analytics by login and logout
Video Link: https://drive.google.com/file/d/1CONrh9BpSRaEv3JcdshtexRKQDRCUOU5/view?usp=sharing
Related PRs and Issues
Have you read the Contributing Guidelines on issues?
Yes