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

Issue/6585 onboarding sl banner display logic #6620

Merged
merged 13 commits into from
Jun 1, 2022

Conversation

JorgeMucientes
Copy link
Contributor

@JorgeMucientes JorgeMucientes commented May 27, 2022

Part of: #6585

Description

This PR adds the logic to determine if the WC Shipping banner should be displayed in order detail screen. The established criteria for displaying the banner is:

  • WCShip is not installed or not enabled.
  • The store is located in US.
  • The oerder currency is USD.
  • The products are eligible for SL (not virtual/downloadable).
  • The order is not eligible for IPP.

Additionally there is a feature flag in place that I've moved to OrderDetailFragment to be able to unit tests things without depending on its value.

Testing instructions

Using a site eligible for shipping labels, disable the WC Shipping plugin from wc-core and test that the banner is shown if this conditions are met:

  • The store is located in US.
  • The order currency is USD.
  • The products are eligible for SL (not virtual/downloadable).
  • The order is not eligible for IPP.

Test that if any of those conditions for the given order is not satisfied, the banner is not displayed.

I've created a new repository to handle the banner's visibility as I'll need to reuse some of its logic for Settings screen in this task.

Images/gif

The banner should appear below products section:

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

@peril-woocommerce
Copy link

peril-woocommerce bot commented May 27, 2022

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@JorgeMucientes JorgeMucientes added feature: order details Related to order details. feature: shipping labels Related to creating, ordering, or printing shipping labels. labels May 27, 2022
@JorgeMucientes JorgeMucientes changed the base branch from trunk to issue/6585-onboarding-sl-banner-ui May 27, 2022 10:07
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented May 27, 2022

You can test the changes on this Pull Request by downloading an installable build, or scanning this QR code:

…oarding-sl-banner-display-logic

# Conflicts:
#	WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/details/OrderDetailFragment.kt
#	WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/details/OrderDetailViewModel.kt
#	WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt
@JorgeMucientes JorgeMucientes added the type: enhancement A request for an enhancement. label May 30, 2022
@codecov-commenter
Copy link

codecov-commenter commented May 30, 2022

Codecov Report

Merging #6620 (0f20123) into trunk (36f501b) will increase coverage by 0.01%.
The diff coverage is 78.26%.

@@             Coverage Diff              @@
##              trunk    #6620      +/-   ##
============================================
+ Coverage     45.27%   45.29%   +0.01%     
- Complexity     2890     2897       +7     
============================================
  Files           517      518       +1     
  Lines         28181    28196      +15     
  Branches       3706     3711       +5     
============================================
+ Hits          12760    12770      +10     
- Misses        14242    14243       +1     
- Partials       1179     1183       +4     
Impacted Files Coverage Δ
...rders/details/ShippingLabelOnboardingRepository.kt 68.75% <68.75%> (ø)
.../android/ui/orders/details/OrderDetailViewModel.kt 70.14% <100.00%> (-0.08%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 36f501b...0f20123. Read the comment docs.

@JorgeMucientes JorgeMucientes marked this pull request as ready for review May 30, 2022 13:46
@JorgeMucientes JorgeMucientes added the status: do not merge Dependent on another PR, ready for review but not ready for merge. label May 30, 2022
@JorgeMucientes JorgeMucientes requested review from nbradbury and removed request for malinajirka May 31, 2022 05:44
@nbradbury nbradbury assigned nbradbury and unassigned nbradbury May 31, 2022
@anitaa1990 anitaa1990 self-assigned this Jun 1, 2022
Copy link
Contributor

@anitaa1990 anitaa1990 left a comment

Choose a reason for hiding this comment

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

Works great :shipit: Thanks for moving the onboarding logic to a different repository and adding unit tests ❤️ It looks much nicer!!

Base automatically changed from issue/6585-onboarding-sl-banner-ui to trunk June 1, 2022 09:32
@JorgeMucientes JorgeMucientes removed the status: do not merge Dependent on another PR, ready for review but not ready for merge. label Jun 1, 2022
@JorgeMucientes JorgeMucientes merged commit c398383 into trunk Jun 1, 2022
@JorgeMucientes JorgeMucientes deleted the issue/6585-onboarding-sl-banner-display-logic branch June 1, 2022 09:34
@peril-woocommerce
Copy link

Warnings
⚠️ PR is not assigned to a milestone.

Generated by 🚫 dangerJS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: order details Related to order details. feature: shipping labels Related to creating, ordering, or printing shipping labels. type: enhancement A request for an enhancement.
Projects
Development

Successfully merging this pull request may close these issues.

5 participants