[Blaze] Logic for the MyStore's View when there is no campaign #9966

hichamboushaba · 2023-10-16T12:57:09Z

Closes: #9959

Description

This PR adds the logic for the MyStore's Blaze View in the no-campaign state, it adds the following:

Logic for fetching campaigns on View launch.
Logic for observing the most recent Blaze campaign.
When there is no campaign, it handles getting the most recent product and displaying it.

Testing instructions

Please start with a Store with no products.
Launch the app, and confirm the Blaze view is hidden.
Create a single product and publish it.
Go back to the MyStore fragment, and confirm the Blaze View is shown.
Click on either the product or the Create Campaign button, and confirm it loads the WebView for Blaze campaign creation for the specified product.
Dismiss the flow, and go back to the app.
Add more products.
Go back to the MyStore fragment.
Click on Create Campaign, confirm it loads the WebView for Blaze campaign creation for the site
Go back and click on the product view, and confirm it opens the WebView for Blaze campaign creation for the specified product.

I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

hichamboushaba · 2023-10-16T13:00:15Z

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

+        }
+    }
+
+    sealed interface MyStoreBlazeCampaignState {


I changed the state to a sealed interface, this IMO simplifies the logic a bit, since there is no more duplication around the product itself, and it's also simpler for dealing when there are no active products (instead of making an additional field nullable).

Please let me know if you prefer keeping it the way it was.

hichamboushaba · 2023-10-16T13:03:03Z

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

@@ -79,32 +79,49 @@ fun MyStoreBlazeView(
                        )
                        BlazeProductItem(
                            product = state.product,
-                            onProductSelected = {},
+                            onProductSelected = state.onCreateCampaignClicked,


This might change depending on what Ann says here p1697457385386219-slack-C03L1NF1EA3

hichamboushaba · 2023-10-16T13:04:04Z

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

+        ).filterNot { it.isSampleProduct }
+        return flow {
+            emit(getCachedProducts())
+            productListRepository.fetchProductList(


I made the function's return as Flow to allow fetching products and updating the UI, let me know what you think about the logic.

wpmobilebot · 2023-10-16T13:40:34Z

📲 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	`89da3a7`
	Direct Download	`woocommerce-prototype-build-pr9966-89da3a7.apk`

This was causing issues with unit tests, probably because Mockito is still not a Kotlin library

hichamboushaba · 2023-10-17T08:41:39Z

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

-    Divider()
-    WCTextButton(
-        modifier = Modifier.padding(start = dimensionResource(id = R.dimen.major_75)),
-        onClick = onCreateCampaignClicked


@JorgeMucientes I noticed that this function was emiting multiple UI elements, so it depends on the parent Composable for defining how they will be laid out, AFAIK this is not recommended, since it's hard to predict, and maintain (any change to the parent layout will impact it, but it's not easy to figure out why).
I can't find any explicit mention of this in the official guideline, but I think it can still be inferred from the fact that Component composables need to accept a Modifier argument, so they need to emit a single UI element which will consume it 🤔, and I also found this lint rule in Slack's repo which protects against such usage.

WDYT? do you agree with this opinion? and do you think it's worth adding an explicit mention of this to our guidelines?

Just to clarify further, I updated it because I needed adding a top padding, so it was a chance to refactor it.

Good catch! We actually do have a rule for that in the guidelines, only that I missed it. In fact, now that I see this, I remember adding fun ColumnScope.CreateCampaignFooter, but since I wasn't making any use of the scope inside the function I had to remove it.

Anyway I agree with your refactor and thanks for adding the missing Modifie param 👍🏼

hichamboushaba · 2023-10-17T08:43:52Z

The instrumented tests are failling, I'll investigate it ASAP, but the rest of the PR is ready for review.

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

codecov-commenter · 2023-10-17T12:47:16Z

Codecov Report

Attention: 84 lines in your changes are missing coverage. Please review.

Comparison is base (0d26828) 43.10% compared to head (89da3a7) 43.06%.

Additional details and impacted files

@@             Coverage Diff              @@
##              trunk    #9966      +/-   ##
============================================
- Coverage     43.10%   43.06%   -0.05%     
  Complexity     4604     4604              
============================================
  Files           946      947       +1     
  Lines         50909    50962      +53     
  Branches       6596     6604       +8     
============================================
+ Hits          21943    21945       +2     
- Misses        27105    27156      +51     
  Partials       1861     1861

Files	Coverage Δ
...merce/android/ui/products/ProductListRepository.kt	`9.28% <66.66%> (+1.31%)`	⬆️
...android/ui/blaze/ObserveMostRecentBlazeCampaign.kt	`0.00% <0.00%> (ø)`
...m/woocommerce/android/ui/blaze/MyStoreBlazeView.kt	`0.00% <0.00%> (ø)`
...commerce/android/ui/blaze/MyStoreBlazeViewModel.kt	`0.00% <0.00%> (ø)`

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

wpmobilebot · 2023-10-17T13:32:52Z

Found 1 violations:

The PR caused the following dependency changes:

expand

-+--- org.wordpress:fluxc:2.50.0
-|    +--- org.wordpress:wellsql:2.0.0
-|    |    +--- androidx.annotation:annotation:1.2.0 -> 1.6.0 (*)
-|    |    \--- org.wordpress.wellsql:wellsql-annotations:2.0.0
-|    +--- org.wordpress.fluxc:fluxc-annotations:2.50.0
-|    +--- org.greenrobot:eventbus:3.3.1 (*)
-|    +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.11.0 (*)
-|    +--- com.android.volley:volley:1.1.1 -> 1.2.0
-|    +--- androidx.paging:paging-runtime:2.1.2
-|    |    +--- androidx.paging:paging-common:2.1.2
-|    |    |    +--- androidx.annotation:annotation:1.0.0 -> 1.6.0 (*)
-|    |    |    \--- androidx.arch.core:core-common:2.0.0 -> 2.1.0 (*)
-|    |    +--- androidx.arch.core:core-runtime:2.0.0 -> 2.1.0 (*)
-|    |    +--- androidx.lifecycle:lifecycle-runtime:2.0.0 -> 2.5.1 (*)
-|    |    +--- androidx.lifecycle:lifecycle-livedata:2.0.0 -> 2.5.1 (*)
-|    |    \--- androidx.recyclerview:recyclerview:1.0.0 -> 1.2.1 (*)
-|    +--- com.goterl:lazysodium-android:5.0.2
-|    +--- net.java.dev.jna:jna:5.5.0
-|    +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.20 -> 1.8.21 (*)
-|    +--- org.jetbrains.kotlin:kotlin-android-extensions-runtime:1.6.20 -> 1.8.21
-|    |    \--- org.jetbrains.kotlin:kotlin-stdlib:1.8.21 (*)
-|    +--- androidx.appcompat:appcompat:1.0.2 -> 1.4.2 (*)
-|    +--- androidx.recyclerview:recyclerview:1.0.0 -> 1.2.1 (*)
-|    +--- androidx.exifinterface:exifinterface:1.0.0 -> 1.3.3
-|    |    \--- androidx.annotation:annotation:1.2.0 -> 1.6.0 (*)
-|    +--- androidx.security:security-crypto:1.0.0 -> 1.1.0-alpha03
-|    |    +--- androidx.annotation:annotation:1.1.0 -> 1.6.0 (*)
-|    |    +--- com.google.crypto.tink:tink-android:1.5.0
-|    |    \--- androidx.collection:collection:1.1.0 -> 1.2.0 (*)
-|    +--- com.squareup.okhttp3:okhttp-urlconnection:4.9.0
-|    |    +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.11.0 (*)
-|    |    \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.4.10 -> 1.8.21 (*)
-|    +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
-|    +--- org.apache.commons:commons-text:1.10.0 (*)
-|    +--- androidx.room:room-runtime:2.4.2 -> 2.5.1 (*)
-|    +--- androidx.room:room-ktx:2.4.2 -> 2.5.1
-|    |    +--- androidx.room:room-common:2.5.1 (*)
-|    |    +--- androidx.room:room-runtime:2.5.1 (*)
-|    |    +--- org.jetbrains.kotlin:kotlin-stdlib:1.7.20 -> 1.8.21 (*)
-|    |    \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.6.4 (*)
-|    +--- com.google.dagger:dagger:2.42 -> 2.46.1
-|    |    \--- javax.inject:javax.inject:1
-|    +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.3.9 -> 1.6.4 (*)
-|    \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.3.9 -> 1.6.4 (*)
++--- org.wordpress:fluxc:trunk-3caf1058bd9384da8a134cee9855c92e31e7ff5f
+|    +--- org.wordpress:wellsql:2.0.0
+|    |    +--- androidx.annotation:annotation:1.2.0 -> 1.6.0 (*)
+|    |    \--- org.wordpress.wellsql:wellsql-annotations:2.0.0
+|    +--- org.wordpress.fluxc:fluxc-annotations:trunk-3caf1058bd9384da8a134cee9855c92e31e7ff5f
+|    +--- org.greenrobot:eventbus:3.3.1 (*)
+|    +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.11.0 (*)
+|    +--- com.android.volley:volley:1.1.1 -> 1.2.0
+|    +--- androidx.paging:paging-runtime:2.1.2
+|    |    +--- androidx.paging:paging-common:2.1.2
+|    |    |    +--- androidx.annotation:annotation:1.0.0 -> 1.6.0 (*)
+|    |    |    \--- androidx.arch.core:core-common:2.0.0 -> 2.1.0 (*)
+|    |    +--- androidx.arch.core:core-runtime:2.0.0 -> 2.1.0 (*)
+|    |    +--- androidx.lifecycle:lifecycle-runtime:2.0.0 -> 2.5.1 (*)
+|    |    +--- androidx.lifecycle:lifecycle-livedata:2.0.0 -> 2.5.1 (*)
+|    |    \--- androidx.recyclerview:recyclerview:1.0.0 -> 1.2.1 (*)
+|    +--- com.goterl:lazysodium-android:5.0.2
+|    +--- net.java.dev.jna:jna:5.5.0
+|    +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.20 -> 1.8.21 (*)
+|    +--- org.jetbrains.kotlin:kotlin-android-extensions-runtime:1.6.20 -> 1.8.21
+|    |    \--- org.jetbrains.kotlin:kotlin-stdlib:1.8.21 (*)
+|    +--- androidx.appcompat:appcompat:1.0.2 -> 1.4.2 (*)
+|    +--- androidx.recyclerview:recyclerview:1.0.0 -> 1.2.1 (*)
+|    +--- androidx.exifinterface:exifinterface:1.0.0 -> 1.3.3
+|    |    \--- androidx.annotation:annotation:1.2.0 -> 1.6.0 (*)
+|    +--- androidx.security:security-crypto:1.0.0 -> 1.1.0-alpha03
+|    |    +--- androidx.annotation:annotation:1.1.0 -> 1.6.0 (*)
+|    |    +--- com.google.crypto.tink:tink-android:1.5.0
+|    |    \--- androidx.collection:collection:1.1.0 -> 1.2.0 (*)
+|    +--- com.squareup.okhttp3:okhttp-urlconnection:4.9.0
+|    |    +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.11.0 (*)
+|    |    \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.4.10 -> 1.8.21 (*)
+|    +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
+|    +--- org.apache.commons:commons-text:1.10.0 (*)
+|    +--- androidx.room:room-runtime:2.4.2 -> 2.5.1 (*)
+|    +--- androidx.room:room-ktx:2.4.2 -> 2.5.1
+|    |    +--- androidx.room:room-common:2.5.1 (*)
+|    |    +--- androidx.room:room-runtime:2.5.1 (*)
+|    |    +--- org.jetbrains.kotlin:kotlin-stdlib:1.7.20 -> 1.8.21 (*)
+|    |    \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.6.4 (*)
+|    +--- com.google.dagger:dagger:2.42 -> 2.46.1
+|    |    \--- javax.inject:javax.inject:1
+|    +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.3.9 -> 1.6.4 (*)
+|    \--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.3.9 -> 1.6.4 (*)
-\--- org.wordpress.fluxc.plugins:woocommerce:2.50.0
-     +--- org.wordpress:wellsql:2.0.0 (*)
-     +--- org.wordpress.fluxc:fluxc-annotations:2.50.0
-     +--- androidx.room:room-ktx:2.4.2 -> 2.5.1 (*)
-     +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.20 -> 1.8.21 (*)
-     +--- org.wordpress:fluxc:2.50.0 (*)
-     +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
-     +--- com.google.dagger:dagger:2.42 -> 2.46.1 (*)
-     +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.3.9 -> 1.6.4 (*)
-     +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.3.9 -> 1.6.4 (*)
-     \--- androidx.room:room-runtime:2.4.2 -> 2.5.1 (*)
+\--- org.wordpress.fluxc.plugins:woocommerce:trunk-3caf1058bd9384da8a134cee9855c92e31e7ff5f
+     +--- org.wordpress:wellsql:2.0.0 (*)
+     +--- org.wordpress.fluxc:fluxc-annotations:trunk-3caf1058bd9384da8a134cee9855c92e31e7ff5f
+     +--- androidx.room:room-ktx:2.4.2 -> 2.5.1 (*)
+     +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.20 -> 1.8.21 (*)
+     +--- org.wordpress:fluxc:trunk-3caf1058bd9384da8a134cee9855c92e31e7ff5f (*)
+     +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
+     +--- com.google.dagger:dagger:2.42 -> 2.46.1 (*)
+     +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.3.9 -> 1.6.4 (*)
+     +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.3.9 -> 1.6.4 (*)
+     \--- androidx.room:room-runtime:2.4.2 -> 2.5.1 (*)

Please review and act accordingly

# Conflicts: # WooCommerce/src/main/kotlin/com/woocommerce/android/ui/blaze/MyStoreBlazeView.kt # WooCommerce/src/main/kotlin/com/woocommerce/android/ui/blaze/MyStoreBlazeViewModel.kt # WooCommerce/src/main/kotlin/com/woocommerce/android/ui/mystore/MyStoreFragment.kt

peril-woocommerce · 2023-10-18T11:54:44Z

	Warnings
⚠️	This PR is assigned to a milestone which is closing in less than 2 days Please, make sure to get it merged by then or assign it to a later expiring milestone

Generated by 🚫 dangerJS

JorgeMucientes · 2023-10-18T13:57:57Z

Excellent job @hichamboushaba! Code looks great and everything works as expected.

hichamboushaba added 6 commits October 16, 2023 13:57

Bump fluxc version

9ee26c1

Logic for using the appropriate state depending on last ca

4da4420

Refactor campaign state modeling

8330553

Handle showing the most recent product when no active blaze campaign

691a86a

Fix UI of the no-campaign state

47f7c06

Handle navigation to Blaze campaign creation

e10c39f

hichamboushaba force-pushed the issue/9959-blaze-no-campaign branch from 18257fd to 50ea7cf Compare October 16, 2023 12:57

hichamboushaba commented Oct 16, 2023

View reviewed changes

hichamboushaba added feature: blaze Related to the Blaze project type: enhancement A request for an enhancement. status: do not merge Dependent on another PR, ready for review but not ready for merge. labels Oct 16, 2023

hichamboushaba commented Oct 16, 2023

View reviewed changes

hichamboushaba added 2 commits October 16, 2023 14:20

Handle fetching products and update the no-campaign view

6bf5d8c

Launch blaze campaign creation when clicking on the product too.

a6700a5

hichamboushaba force-pushed the issue/9959-blaze-no-campaign branch from 50ea7cf to a6700a5 Compare October 16, 2023 13:20

hichamboushaba added 2 commits October 16, 2023 15:39

Update logic of blaze flow when clicking on the product

db96c8c

Avoid using property as default argument

229add8

This was causing issues with unit tests, probably because Mockito is still not a Kotlin library

hichamboushaba commented Oct 17, 2023

View reviewed changes

hichamboushaba requested a review from JorgeMucientes October 17, 2023 08:42

hichamboushaba marked this pull request as ready for review October 17, 2023 08:42

hichamboushaba requested a review from a team as a code owner October 17, 2023 09:39

hichamboushaba added this to the 15.9 milestone Oct 17, 2023

UI Tests: fix products endpoint matching

e753e9e

hichamboushaba force-pushed the issue/9959-blaze-no-campaign branch from d2253bd to 034431c Compare October 17, 2023 11:16

hichamboushaba commented Oct 17, 2023

View reviewed changes

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/blaze/ObserveMostRecentBlazeCampaign.kt Show resolved Hide resolved

UI Tests: add a mock for blaze campaigns endpoint

4bb0b4d

hichamboushaba force-pushed the issue/9959-blaze-no-campaign branch from 034431c to 4bb0b4d Compare October 17, 2023 12:27

hichamboushaba changed the title ~~[Blaze] Logic for the MyStore's View when there is no active campaign~~ [Blaze] Logic for the MyStore's View when there is no campaign Oct 17, 2023

Bump fluxc version

c1318f3

hichamboushaba force-pushed the issue/9959-blaze-no-campaign branch from 956e4ad to 89da3a7 Compare October 18, 2023 11:03

hichamboushaba removed the status: do not merge Dependent on another PR, ready for review but not ready for merge. label Oct 18, 2023

JorgeMucientes self-assigned this Oct 18, 2023

JorgeMucientes approved these changes Oct 18, 2023

View reviewed changes

JorgeMucientes merged commit 7d39ecb into trunk Oct 18, 2023

JorgeMucientes deleted the issue/9959-blaze-no-campaign branch October 18, 2023 13:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Blaze] Logic for the MyStore's View when there is no campaign #9966

[Blaze] Logic for the MyStore's View when there is no campaign #9966

hichamboushaba commented Oct 16, 2023 •

edited

Loading

hichamboushaba Oct 16, 2023

hichamboushaba Oct 16, 2023

hichamboushaba Oct 16, 2023

wpmobilebot commented Oct 16, 2023 •

edited

Loading

hichamboushaba Oct 17, 2023 •

edited

Loading

hichamboushaba Oct 17, 2023

JorgeMucientes Oct 18, 2023

hichamboushaba commented Oct 17, 2023

codecov-commenter commented Oct 17, 2023 •

edited

Loading

wpmobilebot commented Oct 17, 2023

peril-woocommerce bot commented Oct 18, 2023

JorgeMucientes commented Oct 18, 2023

[Blaze] Logic for the MyStore's View when there is no campaign #9966

[Blaze] Logic for the MyStore's View when there is no campaign #9966

Conversation

hichamboushaba commented Oct 16, 2023 • edited Loading

Description

Testing instructions

hichamboushaba Oct 16, 2023

Choose a reason for hiding this comment

hichamboushaba Oct 16, 2023

Choose a reason for hiding this comment

hichamboushaba Oct 16, 2023

Choose a reason for hiding this comment

wpmobilebot commented Oct 16, 2023 • edited Loading

hichamboushaba Oct 17, 2023 • edited Loading

Choose a reason for hiding this comment

hichamboushaba Oct 17, 2023

Choose a reason for hiding this comment

JorgeMucientes Oct 18, 2023

Choose a reason for hiding this comment

hichamboushaba commented Oct 17, 2023

codecov-commenter commented Oct 17, 2023 • edited Loading

Codecov Report

wpmobilebot commented Oct 17, 2023

The PR caused the following dependency changes:

peril-woocommerce bot commented Oct 18, 2023

JorgeMucientes commented Oct 18, 2023

hichamboushaba commented Oct 16, 2023 •

edited

Loading

wpmobilebot commented Oct 16, 2023 •

edited

Loading

hichamboushaba Oct 17, 2023 •

edited

Loading

codecov-commenter commented Oct 17, 2023 •

edited

Loading