search

code-corps / code-corps-api

Elixir/Phoenix API for Code Corps.
https://www.codecorps.org
MIT License
234 stars 86 forks source link

Track project events #1364

Closed iref closed 6 years ago

iref commented 6 years ago

What's in this PR?

Segment plug tracker now also tracks events for project ids if id is extractable from request resource. There is more work to be done. Mostly to add acceptor property in Approved Project Membership event. This change might require passing of optional current user to SegmentTraitBuilder.build

References

Closes #1285 Progress on: #927

iref commented 6 years ago

@joshsmith please let me know if these changes match what we discussed in #1307 .

iref commented 6 years ago

I added direction to tracked events. I moved project user tracking to controller. It allows us to create events name and payloads without changes to analytics api, which wouldn't be possible if we kept tracking in plug (SegmentTraitsBuilder.build function would need action and current user as additional parameters passed from plug) these changes aren't required by other implicitly tracked events and would complicate api. It also helps with #927.

begedin commented 6 years ago

@iref That was a good call. We're actually gradually shifting away from the plug and and adding explicit tracking in controllers in other places as well.

joshsmith commented 6 years ago

@begedin you're misunderstanding the problem that led to this. Please read https://github.com/code-corps/code-corps-api/pull/1307#issuecomment-353224704

begedin commented 6 years ago

@joshsmith I assumed, incorrectly, that it would be possible to define a funnel which does not revolve around the user_id, using custom events, so that's where my confusion comes from.

From what I can tell after looking into it more deeply, that isn't possible, so I guess this is our only option.

Since, with #1351 we also have membership invites to projects, I guess we'll have to do something similar there. I'll add a comment to that PR about how I think we should do it.

iref commented 6 years ago

Did you get a change to review changes? Is there anything else I should improve before merge?

begedin commented 6 years ago

@iref sorry for the delay on this. We had some things to tace care of before merging this one. It looks good to go to me 👍

iref commented 6 years ago

No problem. I just wanted update to make sure that I didn't forget anything. :)