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

AD-219 Enhance Error Handling for Payment and Order Placement Steps #384

Merged
merged 6 commits into from
Apr 3, 2024

Conversation

Lukasz756
Copy link
Contributor

Description
Created ErrorHandler with basic logic to catch errors in React services. Wrapped every step with NotificationWrapper so now it displays Notification Bars on top. Added some error codes for translations. Created ErrorResponse java class to return error codes and fields containing errors (if required).

Tested scenarios
Tested error handling for:

  • wrong address
  • wrong security code for card
  • wrong cartData

Fixed issue: AD-219

@Lukasz756 Lukasz756 requested a review from a team as a code owner April 2, 2024 12:15
@Lukasz756 Lukasz756 requested a review from pjaneta April 2, 2024 12:16
@@ -105,16 +109,18 @@ public ResponseEntity<AddressData> updateDeliveryAddress(@RequestBody AddressFor

return ResponseEntity.status(HttpStatus.OK).build();
} else {
return ResponseEntity.status(HttpStatus.BAD_REQUEST).build();
ErrorResponse errorResponse = new ErrorResponse("");
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we send empty error code?

@@ -15,3 +15,6 @@ checkout.error.authorization.payment.failed=Payment failed
checkout.error.authorization.payment.rejected=Your payment was rejected
checkout.error.authorization.payment.timeout=Payment processing took longer than expected. Please check again in few minutes.
checkout.error.authorization.payment.pending=Your payment is being processed

checkout.error.default=Sorry, there was a problem, please try again later
Copy link
Collaborator

Choose a reason for hiding this comment

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

What about sample translation for DE?

@@ -21,4 +21,8 @@
margin-top: 30px;
padding: 15px;
border-radius: 12px;
}

.alert{
Copy link
Collaborator

Choose a reason for hiding this comment

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

code formatting

}
}
return noRedirect;
}

render() {
if (this.state.redirectToShippingAddress) {
return <Navigate to={routes.shippingAddress}/>
return <Navigate to={routes.shippingAddress + "/missingDataRedirect"}/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

It can be set in routes.ts as new routes


export class StepBase extends React.Component<{}, null> {

// private readonly notificationsWithRedirect: Notification[]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove all commented out code. Optionally move fetchCartData to each step as it was before

case "notifications/removeNotificationsWithoutRedirect": {
return notifications.filter((notification) => notification.isRedirect)
}
case "notifications/removeNotificationsFromList": {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't use this functionality so it might be removed

interface RemoveNotificationsWithoutRedirect extends Action<"notifications/removeNotificationsWithoutRedirect"> {
}

interface RemoveNotificationsFromList extends PayloadAction<"notifications/removeNotificationsFromList", Notification[]> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

As well as this action

checkoutStep={CheckoutSteps.SHIPPING_METHOD}><CheckoutStepWrapper><ShippingMethodStep/></CheckoutStepWrapper></NotificationWrapper></TranslationWrapper>,
},
{
path: routes.shippingMethod + "/missingDataRedirect",
Copy link
Collaborator

Choose a reason for hiding this comment

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

After creating route in routes.ts it can be used here

element: <TranslationWrapper><NotificationWrapper
checkoutStep={CheckoutSteps.SHIPPING_ADDRESS}><CheckoutStepWrapper><ShippingAddressStep/></CheckoutStepWrapper></NotificationWrapper></TranslationWrapper>,
}, {
path: routes.shippingAddress + "/missingDataRedirect",
Copy link
Collaborator

Choose a reason for hiding this comment

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

After creating route in routes.ts it can be used here

@Lukasz756 Lukasz756 merged commit 5ce94dc into develop Apr 3, 2024
2 checks passed
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.

2 participants