Skip to content

[Woo POS] Coupons: List - Enable Coupons - Wrap Up (Designer UI and Copy) #15505

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

Conversation

staskus
Copy link
Contributor

@staskus staskus commented Apr 11, 2025

Description

A wrap-up of enabling coupons setting:

  • Reload the list once the setting is enabled.
  • Loading state
  • Error handling if enabling fails
  • Disable/hide the coupon creation button when coupons cannot be created (the looks will be refined with the header task)

Steps to reproduce

Prerequisites:

  • Disable Enable the use of coupon codes in WooCommerce -> Settings
  • Enable enableCouponsInPointOfSale feature flag
  1. Open POS
  2. Open Coupons
  3. Confirm 'Coupons disabled' error is shown
  4. Confirm '+' button for adding coupons is not visible
  5. Turn off network connection
  6. Tap 'Enable'
  7. Confirm 'Coupons enabling' error is shown
  8. Turn on network connection
  9. Tap 'Retry'
  10. Confirm coupon setting is enabled and list loaded

Testing information

Tested iPad Air 11 Inch M2 iOS 18.2

Screenshots

Enabling and loading coupons

enabling.and.loading.coupons.mp4

Error enabling coupons

error.enabling.coupons.mp4

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

Reviewer (or Author, in the case of optional code reviews):

Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement:

  • The PR is small and has a clear, single focus, or a valid explanation is provided in the description. If needed, please request to split it into smaller PRs.
  • Ensure Adequate Unit Test Coverage: The changes are reasonably covered by unit tests or an explanation is provided in the PR description.
  • Manual Testing: The author listed all the tests they ran, including smoke tests when needed (e.g., for refactorings). The reviewer confirmed that the PR works as expected on all devices (phone/tablet) and no regressions are added.

@staskus staskus added type: task An internally driven task. feature: POS labels Apr 11, 2025
@staskus staskus added this to the 22.2 milestone Apr 11, 2025
@staskus staskus requested review from iamgabrielma and Copilot April 11, 2025 12:16
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

@@ -186,7 +186,9 @@ extension PointOfSaleAggregateModel {
@available(iOS 17.0, *)
extension PointOfSaleAggregateModel {
func enableCoupons() async {
await couponsController.enableCoupons()
if await couponsController.enableCoupons() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was going back and forth on whether enableCoupons should return a result, or throw an error. In the end, opted for the most straightforward solution - return a boolean. Since the error is handled within the controller, there's no point in propagating the error for the controller's caller.

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Apr 11, 2025

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

App NameWooCommerce iOS Prototype
Build Number29311
VersionPR #15505
Bundle IDcom.automattic.alpha.woocommerce
Commit55df609
Installation URL6isj52tdp64q8
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@staskus staskus requested review from joshheald and removed request for iamgabrielma April 14, 2025 15:03
@joshheald joshheald self-assigned this Apr 14, 2025
Copy link
Contributor

@joshheald joshheald left a comment

Choose a reason for hiding this comment

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

Looks good. One small question shared inline.

@@ -140,7 +140,9 @@ extension PointOfSaleAggregateModel {
@available(iOS 17.0, *)
extension PointOfSaleAggregateModel {
func enableCoupons() async {
await couponsController.enableCoupons()
if await couponsController.enableCoupons() {
await couponsController.loadItems(base: .root)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not do this inside the couponsController's enableCoupons function as well? That would make it safe to call it directly on the now-exposed coupons controller. I don't know if that's a good thing or not, so just a question.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went back and forth on this. It felt like enableCoupons would be a bit misleading since it would do more than just enable but maybe I was overthinking.

On another note, I think this whole method can be removed and we can call couponsController directly from the ItemListView. I'll update.

@staskus staskus enabled auto-merge April 14, 2025 17:23
@staskus staskus merged commit 7be476b into trunk Apr 14, 2025
13 checks passed
@staskus staskus deleted the woomob-244-woo-pos-coupons-list-disabled-state-designer-ui-and-copy branch April 14, 2025 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: POS type: task An internally driven task.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants