Make mergeBasket Conditional More Robust

[bendvc]

Description

This PR fixes #1043.

Previously the condition for calling merge was to ensure the customer is now "registered" indicating that the customer has logged in. This was good, however if you create a new customer by registering this merge logic is also called as you are logged in after registering. In this scenario you do not need to call merge as it is done automatically.

Bonus

To keep the network less chatty, you don't want to call mergeBasket if you don't have any items in your guest basket that need merging. So we additionally check to see if you current basket has items before calling.

Bonus #2

Previously if you refreshed the browser as a registered user, a call to merger baskets was made as the customer auth type transitions from undefined to registered. I've added logic to ensure it only happens when transitioning from guest to registered.

Types of Changes

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Documentation update
• Breaking change (could cause existing functionality to not work as expected)
• Other changes (non-breaking changes that does not fit any of the above)

Changes

• Add new conditions before calling mergeBasket

How to Test-Drive This PR

• Run the template-retail-react-app in this branch.
• In chrome's network tab, add the filter merge.
• Test the following scenarios:

1. Login as a pre-existing customer with an empty guest basket. You should not see a network request being made to merge.
2. Login as a pre-existing customer with items in your guest basket. You should see a network request being made to merge.
3. Register a new customer with an empty guest basket. You should not see a network request being made to merge.
4. Register a new customer with items in your guest basket. You should see a network request being made to merge. You should also see that your current basket has those items previously added to your guest basket

Checklists

[shethj]

LGTM!

[wjhsf]

Suggested change

	const lastLoginTimeDate = new Date(Date.parse(customer.lastLoginTime))
	const creationDate = new Date(Date.parse(customer.creationDate))
	const lastLoginTimeDate = Date.parse(customer.lastLoginTime)
	const creationDate = Date.parse(customer.creationDate)

Don't convert from number to Date if all you're doing is calculating a number. 😉

[bendvc]

True. I'll fix this outside of this suggestion as my OCD is telling me to rename those variables as they would no longer be Date but rather TimeStamps.

[wjhsf]

Suggested change

	}, [customer.authType])
	}, [customer.authType, customer.isNewlyRegistered, basket.itemCount])

Are these needed? (React hooks are still confusing!)

[bendvc]

No. If we added those values it just means that the hooks didUpdate logic would be run when those addition values change, which isn't necessarily what we want to be doing. I know the hook with the guard in it is ugly, there are alternatives but they are equally unsightly.

[wjhsf]

"Grace period" is confusing - is this just a workaround for "just registered and logged in"?

[bendvc]

Is this is a work around for determining if a user has just registered. Unfortunately there isn't a property on the customer object that tells you that directly. There is a host of data type values, but they unfortunately cant be used to derive whether or not a user is new.

I was struggling with that name, I'm open to suggestions on other names, or solutions.