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

[MBL-1223] Track attribution event #1961

Merged
merged 5 commits into from Mar 6, 2024
Merged

[MBL-1223] Track attribution event #1961

merged 5 commits into from Mar 6, 2024

Conversation

ifosli
Copy link
Contributor

@ifosli ifosli commented Feb 27, 2024

📲 What

Create an event attribution tracking event the first time a project page loads. This will ensure the event is sent only once per project (since we create a new view model when we configure the view controller with a new project).

Note that we can't simply send the event when the view model is configured, since we need the graphQL id of the project, which we don't have until the fresh project fetch completes.

👀 See

Jira
Spike

✅ Acceptance criteria

  • Event gets sent with empty url whenever the project page is opened from within the app
  • Event gets sent with a context page url whenever the project page is opened from a deeplink

@ifosli ifosli self-assigned this Feb 27, 2024
@ifosli ifosli marked this pull request as ready for review February 27, 2024 20:21
@ifosli ifosli requested review from a team and amy-at-kickstarter and removed request for a team February 27, 2024 20:22
props["context_page_url"] = contextPageUrl ?? ""

let propsData = try? JSONSerialization.data(withJSONObject: props)
return String(data: propsData!, encoding: .utf8)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the force unwrap here necessary? Looks like the function already returns String? so it seems like you could just test for propsData and return nil if needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! I like to force unwrap things when I'm testing but I didn't mean to leave it in. Fixed.

Copy link
Contributor

@amy-at-kickstarter amy-at-kickstarter left a comment

Choose a reason for hiding this comment

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

One nit/suggestion but otherwise lgtm

@ifosli ifosli merged commit 2c8286c into main Mar 6, 2024
5 checks passed
@ifosli ifosli deleted the trackAttributionEvent branch March 6, 2024 18:53
scottkicks added a commit that referenced this pull request Mar 11, 2024
commit ac31c78
Merge: 547a64c 0dadd81
Author: Scott Clampet <110618242+scottkicks@users.noreply.github.com>
Date:   Mon Mar 11 09:41:19 2024 -0500

    Merge branch 'main' into scott/pcp/initial-confirm-details-controller

commit 0dadd81
Author: Amy <a.dyer@kickstarter.com>
Date:   Thu Mar 7 11:04:17 2024 -0500

    Add tracking event for logins/signups that happen via OAuth

commit 547a64c
Merge: 134bcf3 2c8286c
Author: Scott Clampet <110618242+scottkicks@users.noreply.github.com>
Date:   Thu Mar 7 08:54:18 2024 -0600

    Merge branch 'main' into scott/pcp/initial-confirm-details-controller

commit 2c8286c
Author: Ingerid Fosli <i.fosli@kickstarter.com>
Date:   Wed Mar 6 11:53:04 2024 -0700

    [MBL-1223] Track attribution event (#1961)

    * Track attribution event

    * Return nil instead of force unwrapping
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants