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 transaction complete event types #2577

Merged
merged 1 commit into from
Jan 22, 2025

Conversation

vctrchu
Copy link
Contributor

@vctrchu vctrchu commented Jan 22, 2025

Resolves https://github.com/Shopify/pos-next-react-native/issues/51358

Background

This PR adds proper typing to each property for a transaction complete input. It also encapsulates all the properties in a object to improve usability and seperation from the BaseInput properties. You can find the file for POS CheckoutState here

Before:
Screenshot 2025-01-21 at 4 17 06 PM
After:
Screenshot 2025-01-21 at 4 16 56 PM

Solution

I just copied a lot of the checkout types over. We will need to refactor and remove some definitions. Please help me align with what exact properties we want to define in each of our types of TaxLines, ShippingLine and PaymentMethod

This comment has been minimized.

@@ -34,6 +34,7 @@ export interface LineItem {
vendor?: string;
properties: {[key: string]: string};
isGiftCard: boolean;
attributedUserId?: number;
Copy link
Contributor Author

@vctrchu vctrchu Jan 22, 2025

Choose a reason for hiding this comment

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

attributedStaff is known as attributedUserId as defined in the POS model CheckoutLineItem

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, we'll have to map this value over for the cart api as well just fyi

@@ -107,3 +108,84 @@ export interface Address {
provinceCode?: string;
countryCode?: CountryCode;
}

export type ShippingLine =
Copy link
Contributor Author

Choose a reason for hiding this comment

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

POS model of ShippingLine

Retail = 'RETAIL',
}

export interface TaxLine {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

POS model of TaxLine

@vctrchu vctrchu marked this pull request as draft January 22, 2025 00:28
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.

Looks good, just minor comments and moving a couple things into some other files.

@vctrchu vctrchu marked this pull request as ready for review January 22, 2025 18:09
@vctrchu vctrchu requested review from js-goupil and fatbattk January 22, 2025 18:09
Copy link
Contributor

@fatbattk fatbattk left a comment

Choose a reason for hiding this comment

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

code lgtm!

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 fixes then ship it!

lineItems: LineItem[];
orderId: number;
paymentMethods: PaymentMethod[];
shippingLines: ShippingLine;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is also optional

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add optional values for


interface CreditPayment extends BasePayment {
type: 'CreditCard';
lastDigits: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

let's remove this field until we're sure we need it. cc @Alex-Palad

Add changeset

Fix optionals and remove last digits
@vctrchu vctrchu force-pushed the vic/transaction-complete-types branch from 90f01bf to 5074ca0 Compare January 22, 2025 18:54
@vctrchu vctrchu merged commit a3a9109 into unstable Jan 22, 2025
6 checks passed
@vctrchu vctrchu deleted the vic/transaction-complete-types branch January 22, 2025 18:58
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.

3 participants