-
Notifications
You must be signed in to change notification settings - Fork 371
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:
|
Test | Artifacts | |
---|---|---|
Assisted Service Module > Customer Support Agent - Emulation > should checkout as customer |
Test Replay
Screenshots
Video
|
ssr/pages.core-e2e.cy.ts • 3 flaky tests • SSR
Test | Artifacts | |
---|---|---|
SSR > should render homepage |
Test Replay
Screenshots
Video
|
|
SSR > should render PLP |
Test Replay
Screenshots
Video
|
|
SSR > should render PDP |
Test Replay
Screenshots
Video
|
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