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

Add analytics tracking for core actions #1105

Merged
merged 1 commit into from
Oct 23, 2017

Conversation

joshsmith
Copy link
Contributor

@joshsmith joshsmith commented Oct 23, 2017

What's in this PR?

  • Assigned Task to Self
  • Assigned Task to Someone Else
  • Connected Task to Github Repo (need to track separately from just a property on the task being created)
  • Added Task Skill
  • Removed Task Skill
  • Added Project Skill
  • Removed Project Skill
  • Moved Task Between Lists
  • Reordered Task in List
  • Edited Task Title
  • Created GitHub App Installation
  • Connected GitHub App Installation to Project
  • Connected GitHub Repo to Project
  • Disconnected GitHub Repo from Project
  • Closed Task
  • Archived Task

References

Progress on #1038

@moduledoc false
@moduledoc false
@moduledoc false
@moduledoc false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

😂

@joshsmith joshsmith changed the title Add analytics tracking for added/removed TaskSkill Add analytics tracking for core actions Oct 23, 2017
@begedin
Copy link
Contributor

begedin commented Oct 23, 2017

Looks like we had a bug in our SegmentTraitsBuilder. It had a catch all function clause at the end, due to having to support a token map from the token controller, which was hiding the fact that we do not have traits defined for a bunch of our structs.

Making that clause specific to the token and fixing test failures means we need to open up several issues. I'll list them here, so I can create them later

  • check and correct traits for DonationGoal
  • check and correct traits for StripeConnectAccount
  • check and correct traits for StripeConnectPlan
  • check and correct traits for StripePlatformCard
  • check and correct traits for StripePlatformCustomer
  • check and correct traits for UserTask (added here, so can be done as part of this PR)

@begedin begedin force-pushed the 1038-add-analytics-tracking-for-core-actions branch from a32a3e0 to e931497 Compare October 23, 2017 08:57
@begedin
Copy link
Contributor

begedin commented Oct 23, 2017

Our Task.Service was not allowing connecting to a github repo during update at all. The correct behavior would be to allow this, IF the task was not previously connected, so I did some rewriting to enable this. The tracking and service tests all support that.

@begedin
Copy link
Contributor

begedin commented Oct 23, 2017

Other things we may want to consider

  • how traits for specific task events are built

@begedin
Copy link
Contributor

begedin commented Oct 23, 2017

@joshsmith

I've spent time trying to implement tracking for the case of a position change. However, due to our use of EctoOrder, I was unable to find a way to reliably determine if the change took place.

  • :position field is virtual and is nil upon initially loading the Task
  • :order field is set internally by EctoOrdered. A "rebalancing" step takes place during insert or update, which effectively can change its value even if no actual change took place
  • tracking params received from the client also isn't an option, since they could contain a value for position which doesn't actually make a change to it

For now, best I could do is add the order field to the task traits in tracking and work with "Edited Task". If you have a better idea, I'm happy to implement it.

@begedin
Copy link
Contributor

begedin commented Oct 23, 2017

On the note of GithubAppInstallation tracking

  • this PR tracks CodeCorpsWeb controller actions only, meaning

    • when user creates an installation from the client app (not actually on github yet, necessarily)
      • this installation is immediately connected to a project, so I don't think separate tracking of that makes sense in the context
    • when user clicks connect on the repo, creating a ProjectGithubRepo
  • what else do we need to track

    • when an installation is created from github - it will either just be saved, unconnected to a project, or it will be matched to an existing installation created above and then connected to a project - 2 events
    • when an installation is removed from github

The problem with this new tracking, however, is that there's no current_user within that context, so we either need to track without it, or figure out a way to provide it. In most cases, for example, we could use the sender object within the github payload to try and match to a CodeCorps user. However, there are still cases where this does not exist, so in that case we either provide a bot user as context, or we provide no context at all.-

@begedin
Copy link
Contributor

begedin commented Oct 23, 2017

@joshsmith As you can see, this was not going up to plan, exactly, and there are questions we need to resolve.

At the very least, some issues need to be created, but the PR on its own should be ready for a review.

- Added added/removed TaskSkill
- Fixed catch-all clause issue with segment traits builder, added UserTask tracking
- Added Task Github connect tracking. Fixed task service not allowing github connect in update
- Added tracking for ProjectSkill
- Added tracking for task move between lists
- Added extra task traits
- Added Task title change tracking
- Added task close tracking
- Added Task archive tracking
- Added tracking for GithubAppInstallation created
- Added ProjectGithubRepo tracking
@joshsmith joshsmith force-pushed the 1038-add-analytics-tracking-for-core-actions branch from ac78457 to 067c3fa Compare October 23, 2017 18:55
@joshsmith joshsmith merged commit d6591ee into develop Oct 23, 2017
@joshsmith joshsmith deleted the 1038-add-analytics-tracking-for-core-actions branch October 23, 2017 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants