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

Add cash tracking session event targets for POS Compliance #2567

Merged
merged 2 commits into from
Jan 22, 2025

Conversation

fatbattk
Copy link
Contributor

Background

Part of, https://github.com/Shopify/pos-next-react-native/issues/51030

Solution

  • Rename event-initiated extension targets per https://github.com/Shopify/pos-next-react-native/issues/51181
  • Add 3 cash tracking session targets for POS Compliance.
    • pos.cash-tracking-session-start.event.observe
    • pos.cash-tracking-session-cancel.event.observe
    • pos.cash-tracking-session-complete.event.observe
  • Added preliminary inputs for Cash session object.
  • Add to docs.

🎩

  • TODO: how?

Checklist

  • I have 🎩'd these changes
  • I have updated relevant documentation

This comment has been minimized.

@fatbattk fatbattk force-pushed the add-pos-compliance-extension-targets branch from c2e0fcd to 793e51a Compare January 20, 2025 15:52
@fatbattk fatbattk requested a review from vctrchu January 20, 2025 15:53
@fatbattk fatbattk force-pushed the add-pos-compliance-extension-targets branch from 793e51a to bf42c5b Compare January 21, 2025 01:34
@fatbattk fatbattk requested a review from Alex-Palad January 21, 2025 01:34
@fatbattk fatbattk force-pushed the add-pos-compliance-extension-targets branch from bf42c5b to 31b4b2c Compare January 21, 2025 11:36
Copy link
Contributor

@vctrchu vctrchu left a comment

Choose a reason for hiding this comment

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

LGTM, the naming this aligns with the POS targets sheet. We can add more properties as you expand the scope

@fatbattk fatbattk force-pushed the add-pos-compliance-extension-targets branch 2 times, most recently from 4155b9b to d1e91ea Compare January 21, 2025 18:42
Copy link
Contributor

@js-goupil js-goupil left a comment

Choose a reason for hiding this comment

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

Sorry for RC, PR looks good, just wanted to raise the alarm on a decision we're also making for the PurchaseCompleteInput as well

@fatbattk fatbattk force-pushed the add-pos-compliance-extension-targets branch from d1e91ea to f89d0e1 Compare January 22, 2025 02:58
@fatbattk fatbattk requested a review from js-goupil January 22, 2025 03:02
Copy link
Contributor

@js-goupil js-goupil left a comment

Choose a reason for hiding this comment

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

Small comments but I think we should make the input names more descriptive, makes the code more obvious and readable

@fatbattk fatbattk force-pushed the add-pos-compliance-extension-targets branch from f89d0e1 to b50a76a Compare January 22, 2025 18:43
@fatbattk fatbattk merged commit 9a3debd into unstable Jan 22, 2025
6 checks passed
@fatbattk fatbattk deleted the add-pos-compliance-extension-targets branch January 22, 2025 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants