Skip to content

Conversation

jaclync
Copy link
Contributor

@jaclync jaclync commented Jun 28, 2024

Part of #12998

Description

After merging #13188 and testing the WIP #13191, I realized that the order sync stopped working because the dashboard VM is still using the items: [POSItem] internal variable that isn't set anymore after extracting the item selector logic in #13188. Without the item list, order sync cannot properly set the order line items in the API request and it results in 0 order total and an empty order in core. This PR fixes it by removing the items variable and replacing its usage in order sync with itemSelectorViewModel.items.

Steps to reproduce

Prerequisite: the store is eligible for POS and the feature switch is enabled in Menu > Settings > Experimental Features > POS

  • Launch app
  • Go to Menu > Point of Sale
  • Add an item (with a non-zero price) to cart
  • Tap Checkout --> the order total should be updated to a correct, non-zero value (before this PR in trunk, the order total is always 0 after the sync)

Testing information

Screenshots

Simulator.Screen.Recording.-.iPad.10th.generation.-.2024-06-28.at.16.36.37.mp4

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

@jaclync jaclync added type: task An internally driven task. feature: POS labels Jun 28, 2024
@jaclync jaclync added this to the 19.4 milestone Jun 28, 2024
@dangermattic
Copy link
Collaborator

1 Message
📖 This PR is still a Draft: some checks will be skipped.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Collaborator

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

App NameWooCommerce iOS WooCommerce iOS
Build Numberpr13205-e0a7ed4
Version19.3
Bundle IDcom.automattic.alpha.woocommerce
Commite0a7ed4
App Center BuildWooCommerce - Prototype Builds #9812
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@jaclync jaclync marked this pull request as ready for review June 28, 2024 20:50
@jaclync jaclync enabled auto-merge June 28, 2024 21:00
@iamgabrielma iamgabrielma self-assigned this Jul 1, 2024
Copy link
Contributor

@iamgabrielma iamgabrielma 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, thanks for addressing this!

@jaclync jaclync merged commit d3ac155 into trunk Jul 1, 2024
@jaclync jaclync deleted the task/12998-fix-items-not-set-for-order-sync branch July 1, 2024 03:52
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.

4 participants