-
Notifications
You must be signed in to change notification settings - Fork 38
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
base: unstable
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
packages/ui-extensions/src/surfaces/point-of-sale/event/input/CashTrackingSessionInput.ts
Show resolved
Hide resolved
c2e0fcd
to
793e51a
Compare
packages/ui-extensions/src/surfaces/point-of-sale/event/input/CashTrackingSessionInput.ts
Outdated
Show resolved
Hide resolved
793e51a
to
bf42c5b
Compare
bf42c5b
to
31b4b2c
Compare
There was a problem hiding this 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
packages/ui-extensions/docs/surfaces/point-of-sale/reference/types/ExtensionTargetType.ts
Outdated
Show resolved
Hide resolved
4155b9b
to
d1e91ea
Compare
There was a problem hiding this 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
} | ||
|
||
export interface CashTrackingSessionEndInput extends BaseInput { | ||
id: number; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should move this into an object as such:
export interface CashTrackingSessionEndInput extends BaseInput {
cashTrackingSessionEnd: {
id
/// other fields
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And actually, are these same fields going to be reuseable on other targets? If so maybe we can create a more generically named type, that we can reuse for CashTrackingSesisonEndInput
, CashTrackingSessionStartInput
, etc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice, cleaner from BaseInput
, updated!
these should be limited to cash tracking sessions, will abstract out if useful elsewhere.
d1e91ea
to
f89d0e1
Compare
Background
Part of, https://github.com/Shopify/pos-next-react-native/issues/51030
Solution
pos.cash-tracking-session-start.event.observe
pos.cash-tracking-session-cancel.event.observe
pos.cash-tracking-session-complete.event.observe
🎩
Checklist