Skip to content

Conversation

@iamgabrielma
Copy link
Contributor

@iamgabrielma iamgabrielma commented Feb 20, 2025

Closes: #15149

Description

This PR improves conformance by splitting POS-related events from shared CollectOrderPaymentAnalyticsTracking protocol to a separate POSCollectOrderPaymentAnalyticsTracking protocol, which holds the POS-specific functions, clearing up the shared protocol to have empty implementations.

Changes

  • Create a POSCollectOrderPaymentAnalyticsTracking protocol that contains POS-related events
  • Make a new POSCollectOrderPaymentAnalytics, that conforms to POSCollectOrderPaymentAnalyticsTracking
  • Make a new MockPOSCollectOrderPaymentAnalytics for tests
  • Inject the specific dependency into POS

Testing information

  • There are no functional changes. CI should pass.

Question:

When updating the test with the new mock, I've realized the test removeAllItemsFromCart_when_tapped_then_tracks_event fails. This is expected since we've updated this functionality on #15200 , however, CI never failed and we could merge this normally. I've removed the test in this PR but I'm wondering if there are differences we should be aware off in POS (eg: iOS17?)


  • 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.

@dangermattic
Copy link
Collaborator

dangermattic commented Feb 20, 2025

2 Warnings
⚠️ View files have been modified, but no screenshot or video is included in the pull request. Consider adding some for clarity.
⚠️ This PR is assigned to the milestone 21.8. This milestone is due in less than 2 days.
Please make sure to get it merged by then or assign it to a milestone with a later deadline.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Feb 20, 2025

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 Numberpr15214-33b40e9
Version21.7
Bundle IDcom.automattic.alpha.woocommerce
Commit33b40e9
App Center BuildWooCommerce - Prototype Builds #13068
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@iamgabrielma iamgabrielma changed the title Split POS-related CollectOrderPaymentAnalyticsTracking conformance to… [Woo POS] Improve POS tracking conformance Feb 20, 2025
@iamgabrielma iamgabrielma added type: enhancement A request for an enhancement. type: task An internally driven task. feature: POS labels Feb 20, 2025
@iamgabrielma iamgabrielma added this to the 21.8 milestone Feb 20, 2025
@iamgabrielma iamgabrielma marked this pull request as ready for review February 20, 2025 10:31
@iamgabrielma iamgabrielma requested a review from staskus February 20, 2025 10:50
Copy link
Contributor

@staskus staskus 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!

@iamgabrielma iamgabrielma merged commit 0af088f into trunk Feb 20, 2025
31 of 33 checks passed
@iamgabrielma iamgabrielma deleted the issue/15149-improve-conformance branch February 20, 2025 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: POS type: enhancement A request for an enhancement. type: task An internally driven task.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Woo POS] MVP analytics: improve POSCollectOrderPaymentAnalyticsTracking conformance

5 participants