Issue/10784 blaze analytics part ii #10820

JorgeMucientes · 2024-02-14T23:28:23Z

Part of: #10784

Description

Ads tracking for all the new events for Blaze campaign creation native flow

Testing instructions

Trigger Blaze creation flow
Follow the table from the tracking section for Blaze i3 PT: pe5sF9-2mb-p2 and check in the Android logcat for the following output: 🔵 Tracked: event_name, Properties: property_name:value ... to ensure all of the events from the table are fired appropriately.

Note: the last 2 events from the table _blaze_campaign_creation_success and _blaze_campaign_creation_failed are not included in this PR as the code for firing the campaign creation and handling the success/error result is not ready yet

dangermattic · 2024-02-14T23:28:57Z

	1 Message
📖	This PR contains changes to Tracks-related logic. Please ensure (author and reviewer) the following are completed: The tracks events must be validated in the Tracks system. Verify the internal Tracks spreadsheet has also been updated. Please consider registering any new events. The PR must be assigned the category: tracks label.

Generated by 🚫 Danger

wpmobilebot · 2024-02-14T23:55:27Z

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce Android
	Flavor	Jalapeno
	Build Type	Debug
	Commit	`7b5dbb8`
	Direct Download	`woocommerce-prototype-build-pr10820-7b5dbb8.apk`

…aze-analytics-part-II

codecov-commenter · 2024-02-15T09:24:28Z

Codecov Report

Attention: Patch coverage is 42.30769% with 60 lines in your changes are missing coverage. Please review.

Project coverage is 41.27%. Comparing base (c9b9256) to head (7b5dbb8).

Files	Patch %	Lines
...ze/creation/budget/BlazeCampaignBudgetViewModel.kt	0.00%	32 Missing ⚠️
...n/targets/BlazeCampaignTargetSelectionViewModel.kt	0.00%	8 Missing ⚠️
...ion/BlazeCampaignCreationAdDestinationViewModel.kt	0.00%	6 Missing ⚠️
...n/preview/BlazeCampaignCreationPreviewViewModel.kt	87.09%	2 Missing and 2 partials ⚠️
...ayment/BlazeCampaignPaymentMethodsListViewModel.kt	0.00%	3 Missing ⚠️
...on/payment/BlazeCampaignPaymentSummaryViewModel.kt	0.00%	3 Missing ⚠️
...ation/intro/BlazeCampaignCreationIntroViewModel.kt	0.00%	2 Missing ⚠️
...s/BlazeCampaignTargetLocationSelectionViewModel.kt	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##              trunk   #10820   +/-   ##
=========================================
  Coverage     41.27%   41.27%           
- Complexity     5085     5086    +1     
=========================================
  Files          1034     1034           
  Lines         59565    59632   +67     
  Branches       7991     7996    +5     
=========================================
+ Hits          24585    24614   +29     
- Misses        32792    32830   +38     
  Partials       2188     2188

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

hichamboushaba

Nice work @JorgeMucientes, all of the events work well, I left two comments, but none is blocking, we can address them if needed on separate PRs.

hichamboushaba · 2024-02-15T13:19:57Z

...main/kotlin/com/woocommerce/android/ui/blaze/creation/budget/BlazeCampaignBudgetViewModel.kt

@@ -128,6 +140,12 @@ class BlazeCampaignBudgetViewModel @Inject constructor(
            )
        }
        fetchAdForecast()
+        analyticsTrackerWrapper.track(


This gets called every time the duration slider is moved, which seems like a bug to me, I started a discussion about it here p1708002828270189-slack-C03L1NF1EA3

hichamboushaba · 2024-02-15T13:22:23Z

...ommerce/android/ui/blaze/creation/destination/BlazeCampaignCreationAdDestinationViewModel.kt

@@ -64,6 +67,7 @@ class BlazeCampaignCreationAdDestinationViewModel @Inject constructor(
    }

    fun onDestinationUrlChanged(destinationUrl: String) {
+        analyticsTrackerWrapper.track(stat = BLAZE_CREATION_EDIT_DESTINATION_SAVE_TAPPED)


There is a small difference between iOS and Android here, and I think we might need to have different events because of it, in iOS the Save button is for the whole screen, and saves both the targetUrl and the parameters, while here this gets called when the targetUrl is changed, and not the parameters, WDYT?

Good point, @hichamboushaba. I actually had thoughts of initiating a discussion precisely about this here p1707952047198469-slack-C03L1NF1EA3 😅. However, I refrained from it because I didn't think the difference was that relevant. But after reading your comment I guess I should reconsider it :)

I agree we should have different events here as the frequency in which the save button is clicked will likely be very different between platforms.

Wdyt about a new event for Android: blaze_creation_destination_url_changed replacing the actual blaze_creation_edit_destination_save_tapped, and leaving the latter only on iOS?

If agreed, I can proceed and update the P2 with this slight difference between platforms

Wdyt about a new event for Android: blaze_creation_destination_url_changed replacing the actual blaze_creation_edit_destination_save_tapped, and leaving the latter only on iOS?

Sounds good, will you add one other event for the URL parameters?

I don't mind. Is a really minor change, but do you think we need that level of detail? Are we going to use that event?

Nah, let's add it just in case; it won't harm. Although I have doubts about how useful it will be.

IMO this is the part that most users will use, the majority will be happy about the product URL as destination, so they probably won't change.

On another note, just an idea, if we want to keep the same event as iOS, we can probably just fire it when the user taps back on the screen, we can check if the data has changed, and if yes, we consider it as a save button call, it's a minor point, just wanted to share it, feel free to ignore it though if you already added the new events.

Yeah good point, I like that idea better, especially because it keeps event parity between platforms. Applied it here: 5a9474c

# Conflicts: # WooCommerce/src/main/kotlin/com/woocommerce/android/ui/blaze/creation/ad/BlazeCampaignCreationEditAdViewModel.kt # WooCommerce/src/main/kotlin/com/woocommerce/android/ui/blaze/creation/payment/BlazeCampaignPaymentSummaryViewModel.kt # WooCommerce/src/main/kotlin/com/woocommerce/android/ui/blaze/creation/preview/BlazeCampaignCreationPreviewViewModel.kt

… bottom sheet

hichamboushaba · 2024-02-16T11:12:25Z

...main/kotlin/com/woocommerce/android/ui/blaze/creation/budget/BlazeCampaignBudgetViewModel.kt

@@ -219,11 +223,13 @@ class BlazeCampaignBudgetViewModel @Inject constructor(
        val dailySpending: String,
        val forecast: ForecastUi,
        val durationInDays: Int,
+        val sliderDurationValue: Float,


I know @JorgeMucientes you are still working on this, just wondering, do we need an additional property in the State for this, when I was planning to work on this I was think we can just make this a UI detail, and handle it using Compose's functions, so we'll just initialize a State using rememberSaveable in the BottomSheet to keep the state of the duration slider, then when apply is clicked, we pass this State's value as a callback to our ViewModel, WDYT?

In addition this change adds a repository cache for ai ad suggestions to avoid requesting suggestions twice when entering edit ad screen

JorgeMucientes · 2024-02-16T12:13:43Z

Hey @hichamboushaba, the PR is ready for another round. Thanks for all the provided feedback 👍🏼. Here's a summary of the changes applied:

Update only campaign duration once the Apply button is clicked: 177c827 and then followed the suggestion to simplify the code: 7deed09
Track blaze_creation_edit_destination_save_tapped when navigating back from the destination screen and the url or any of its params has changed: 5a9474c
Fixes bug. When changing the campaign duration, the total budget value was not updated to fit in between the allowed minimum and maximum daily spend: b1f3293
Added missing property is_ai_suggested_ad_content:true/false to the event _blaze_creation_confirm_details_tapped. While adding this, I realized we were fetching AI suggestions every time we entered the preview screen and again when opening the edit ad screen. I've added an in-memory cache to avoid this and to have ai suggestions accessible to set the correct value for the is_ai_suggested_ad_content property: 20bf87d

Sorry for the many changes added, but I think applying them in this PR made sense because that enabled firing the tracking events at the right time.

hichamboushaba

Nice work @JorgeMucientes 👏, thanks for addressing those issues and the new ones.

I left some np comments, but please check the first one before merging.

hichamboushaba · 2024-02-16T14:06:31Z

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/blaze/BlazeRepository.kt

 import kotlin.math.roundToInt
 import kotlin.time.Duration.Companion.days

+@Singleton


np, Do we need to make this @Singleton for this? this will cause the data to be cached for the whole lifetime of the app, and if someone forgets to pass forceRefresh, we could be reusing wrong suggestions.

WDYT about saving the suggestions in the Preview ViewModel, and pass them as an argument to Edit Ad ViewModel?

Fair enough, the reason I went for this approach initially is because it was easier and required a bit less file changes. But its true that it has that risk. So I used nav args instead and completely removed the repository cache.
Good suggestion 👍🏼

Thanks @JorgeMucientes, and sorry for nitpicking on this.
One point just for context, no need to change unless you agree with me, I suggested caching the suggestion at the ViewModel level, and not the CampaignDetails, because adding them as a parameter to the CampaignDetails will cause them to be saved to the SavedState, while we don't need it really, as they can be re-fetched if the screen ViewModel gets re-created.

Makes sense. I've extracted it out of the CampaignDetails model to a class field 👍🏼

hichamboushaba · 2024-02-16T14:07:56Z

...main/kotlin/com/woocommerce/android/ui/blaze/creation/budget/BlazeCampaignBudgetViewModel.kt

+        }
+    }
+
+    private fun calculateDailySpending(duration: Int): Float {


Nice find, thanks for fixing this.

hichamboushaba · 2024-02-16T14:09:14Z

...main/kotlin/com/woocommerce/android/ui/blaze/creation/budget/BlazeCampaignBudgetViewModel.kt

+        return if (dailySpend < CAMPAIGN_MINIMUM_DAILY_SPEND) {
+            CAMPAIGN_MINIMUM_DAILY_SPEND
+        } else if (dailySpend > CAMPAIGN_MAXIMUM_DAILY_SPEND) {
+            CAMPAIGN_MAXIMUM_DAILY_SPEND
+        } else {
+            dailySpend
        }


np, we can simplify this logic using coerceIn.

Nice! Way simpler TIL

# Conflicts: # WooCommerce/src/main/kotlin/com/woocommerce/android/ui/blaze/BlazeRepository.kt # WooCommerce/src/main/kotlin/com/woocommerce/android/ui/blaze/creation/destination/BlazeCampaignCreationAdDestinationViewModel.kt # WooCommerce/src/main/kotlin/com/woocommerce/android/ui/blaze/creation/payment/BlazeCampaignPaymentSummaryViewModel.kt

JorgeMucientes added 8 commits February 14, 2024 18:54

Track when Learn mor is clicked from Blaze intro screen

283228e

Track campaign preview screen displayed

6eb6034

Track events for campaign preview screen

38d2e7f

Track events for edit Ad screen

a3192e1

Track events for budget screen

dabea04

Track events for saved target options

078fd23

Track events for saving campaign targeting options

6092646

Track events for payment screens

093a5f5

JorgeMucientes changed the base branch from trunk to issue/10784-blaze-analytics-part-I February 14, 2024 23:28

JorgeMucientes added the feature: blaze Related to the Blaze project label Feb 14, 2024

JorgeMucientes added this to the 17.4 milestone Feb 14, 2024

JorgeMucientes added the status: do not merge Dependent on another PR, ready for review but not ready for merge. label Feb 14, 2024

JorgeMucientes mentioned this pull request Feb 14, 2024

Blaze Analytics #10784

Closed

JorgeMucientes requested review from 0nko and hichamboushaba February 15, 2024 08:50

JorgeMucientes marked this pull request as ready for review February 15, 2024 08:50

JorgeMucientes added 2 commits February 15, 2024 09:51

Merge branch 'issue/10784-blaze-analytics-part-I' into issue/10784-bl…

5c32a17

…aze-analytics-part-II

Fix detekt: remove unused import

26524a6

hichamboushaba self-assigned this Feb 15, 2024

Base automatically changed from issue/10784-blaze-analytics-part-I to trunk February 15, 2024 12:16

hichamboushaba approved these changes Feb 15, 2024

View reviewed changes

JorgeMucientes added 6 commits February 15, 2024 18:31

Update only campaign duration once the apply button is clicked

177c827

Track duration applied event only after clicking on apply button from…

68c6667

… bottom sheet

Fix bug, ensure total budget value is within min and max ranges

b1f3293

Merge branch 'trunk' into issue/10784-blaze-analytics-part-II

073a80f

Track edit destination event tapped event only if destination changes

5a9474c

Track edit destination event tapped event only if destination changes

6584264

hichamboushaba reviewed Feb 16, 2024

View reviewed changes

JorgeMucientes added 2 commits February 16, 2024 12:33

Use compose property to keep track of the duration slider value

7deed09

Track if AI content was used for the creating the ad

20bf87d

In addition this change adds a repository cache for ai ad suggestions to avoid requesting suggestions twice when entering edit ad screen

JorgeMucientes requested a review from hichamboushaba February 16, 2024 12:13

JorgeMucientes added feature: analytics In-app store analytics category: tracks Related to analytics, including Tracks Events. and removed status: do not merge Dependent on another PR, ready for review but not ready for merge. feature: analytics In-app store analytics labels Feb 16, 2024

hichamboushaba approved these changes Feb 16, 2024

View reviewed changes

Simplify code by using ext function coerceIn

9a8add9

spencertransier modified the milestones: 17.4, 17.5 Feb 16, 2024

JorgeMucientes added 3 commits February 16, 2024 17:58

Remove ai ad suggestions cached and pass them as nav args

c726565

Fix detekt issues after merge with trunk

64a1ff2

JorgeMucientes enabled auto-merge February 16, 2024 17:32

JorgeMucientes mentioned this pull request Feb 16, 2024

Blaze: Fix a crash when no ad suggestions #10853

Merged

Fix unit tests

7da9c7e

spencertransier modified the milestones: 17.5, 17.6 Feb 23, 2024

JorgeMucientes added 3 commits February 26, 2024 10:58

Merge branch 'trunk' into issue/10784-blaze-analytics-part-II

ca81beb

Remove ai suggestions from Campaign details model¥

ae11eef

Fix redundant qualifiers and wrong imports

7b5dbb8

JorgeMucientes disabled auto-merge February 26, 2024 10:56

JorgeMucientes merged commit 24ffc7e into trunk Feb 26, 2024
14 checks passed

JorgeMucientes deleted the issue/10784-blaze-analytics-part-II branch February 26, 2024 11:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue/10784 blaze analytics part ii #10820

Issue/10784 blaze analytics part ii #10820

JorgeMucientes commented Feb 14, 2024 •

edited

Loading

dangermattic commented Feb 14, 2024 •

edited

Loading

wpmobilebot commented Feb 14, 2024 •

edited

Loading

codecov-commenter commented Feb 15, 2024 •

edited

Loading

hichamboushaba left a comment

hichamboushaba Feb 15, 2024

hichamboushaba Feb 15, 2024

JorgeMucientes Feb 15, 2024

hichamboushaba Feb 15, 2024

JorgeMucientes Feb 15, 2024 •

edited

Loading

hichamboushaba Feb 15, 2024

JorgeMucientes Feb 16, 2024

hichamboushaba Feb 16, 2024

JorgeMucientes commented Feb 16, 2024

hichamboushaba left a comment

hichamboushaba Feb 16, 2024

JorgeMucientes Feb 16, 2024

hichamboushaba Feb 16, 2024

JorgeMucientes Feb 26, 2024

hichamboushaba Feb 16, 2024

hichamboushaba Feb 16, 2024

JorgeMucientes Feb 16, 2024

Issue/10784 blaze analytics part ii #10820

Issue/10784 blaze analytics part ii #10820

Conversation

JorgeMucientes commented Feb 14, 2024 • edited Loading

Description

Testing instructions

dangermattic commented Feb 14, 2024 • edited Loading

wpmobilebot commented Feb 14, 2024 • edited Loading

codecov-commenter commented Feb 15, 2024 • edited Loading

Codecov Report

hichamboushaba left a 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

JorgeMucientes Feb 15, 2024 • 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

JorgeMucientes commented Feb 16, 2024

hichamboushaba left a 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

JorgeMucientes commented Feb 14, 2024 •

edited

Loading

dangermattic commented Feb 14, 2024 •

edited

Loading

wpmobilebot commented Feb 14, 2024 •

edited

Loading

codecov-commenter commented Feb 15, 2024 •

edited

Loading

JorgeMucientes Feb 15, 2024 •

edited

Loading