-
Notifications
You must be signed in to change notification settings - Fork 386
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
feat/CXSPA-7301: Allow Billing Address + Guest checkout - in Digital Payments #18892
Conversation
4 flaky tests on run #44014 ↗︎
Details:
regression/asm/asm.emulation.core-e2e.cy.ts • 1 flaky test • B2C
ssr/pages.core-e2e.cy.ts • 3 flaky tests • SSR
Review all test suite changes for PR #18892 ↗︎ |
Looks good when looking at the changes related to the core project |
const numbers = getAddressNumbers(address, textPhone, textMobile); | ||
|
||
return { | ||
textBold: address.firstName + ' ' + address.lastName, |
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.
Small but better to use template literals:
textBold: ${address.firstName} ${address.lastName}
address.line1, | ||
address.line2, | ||
address.town + ', ' + region + address.country?.isocode, | ||
address.postalCode, |
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.
Template literal here too:
${address.town}, ${region}${address.country?.isocode}
} | ||
|
||
isBillingAddressSameAsDeliveryAddress(): boolean { | ||
if (this.billingAddress === undefined) { |
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.
Without using if statements, I think we can rewrite this as:
return this.billingAddress !== undefined;
this.launchDialogService.closeDialog(reason); | ||
} | ||
|
||
continue() { |
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 would change to something like continueNavigation(). continue is a reserved word in JS
clickAddNewPayment(); | ||
cy.wait('@getDigitalPaymentsRequest'); | ||
fillBillingAddress(my_user.billingAddress); | ||
cy.get('button.btn.btn-block.btn-secondary') |
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 assume some of these could be moved to a helper class but it's nothing urgent
@@ -0,0 +1,211 @@ | |||
<!-- BILLING --> |
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.
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.
done
https://jira.tools.sap/browse/CXSPA-7301
The following is a checklist for new epics or features acceptance for Spartacus, based on our Definition of Done document
General
Audits/reviews
New Library
If your epic will be introduced or introduces a new library:
Sample data
If the new feature requires new or updated sample data for a specific Commerce backend version:
Documentation