Skip to content
This repository has been archived by the owner on Apr 22, 2019. It is now read-only.

AVS-421_75 - Refactor to prevent errors when product is deleted #107

Conversation

rsisco
Copy link

@rsisco rsisco commented Feb 1, 2018

- Refactor code to use nulls when product is missing
Copy link
Contributor

@erikhansen erikhansen left a comment

Choose a reason for hiding this comment

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

@rsisco I added two optional changes. You can merge this now or decide to make the changes.

{
$data = [];
if ($product) {
$data['itemCode'] = $this->taxClassHelper->getItemCodeOverride($product);
Copy link
Contributor

Choose a reason for hiding this comment

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

I would have created the $data array like this. However I'd say it's an optional refactor.

$data [
    'itemCode' => $this->taxClassHelper->getItemCodeOverride($product),
    'taxCode' => $this->taxClassHelper->getAvataxTaxCodeForProduct($product, $storeId),
    'productRef1' => $this->taxClassHelper->getRef1ForProduct($product),
    'productRef2' => $this->taxClassHelper->getRef2ForProduct($product)
]

// Using null values for these parameters since the product can no longer be found; they're null by default
// and only have values if explicitly defined in the configuration. Using nulls won't prevent submission to
// Avalara and will only raise an issue if the product had these values before being deleted.
$data['itemCode'] = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Same feedback as previous comment.

@rsisco rsisco merged commit b8c8656 into develop Feb 8, 2018
@rsisco rsisco deleted the feature/AVS-421_75-refactor-to-prevent-error-when-product-is-deleted branch February 8, 2018 20:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants