Importing and deleting product data in WordPress

Question

I am interested in how to handle errors/exceptions/returned data correctly.

For example, I have a script which creates, updates and deletes products data.

Here is method that receives data from an API, processes and decides what to create/update/delete.

private function importProducts($brand)
{
    $products = $this->dsApi->getProductsByBrand($brand);

    //returns ['create' => [], 'update' => [], 'delete' => [] ]
    $preparedProducts = $this->prepareProducts($products, $brand);

    foreach ($preparedProducts['delete'] as $key => $product) {
        $this->deleteProduct($product);
        $this->log->info(
            'Brand ' . $brand->name . '(' . $brand->id_brand . ')'
            . ' id: ' . $product . ' was deleted'
        );
    }

    foreach ($preparedProducts['create'] as $key => $product) {

        $this->importProduct($product, $brand);
        $this->log->info(
            'Brand ' . $brand->name . '(' . $brand->id_brand . ') sku: ' . $product['sku'] . ' was imported'
        );
    }

    foreach ($preparedProducts['update'] as $key => $product) {
        $this->importProduct($product, $brand, $product['idProductWc']);
        $this->log->info(
            'Brand ' . $brand->name . '(' . $brand->id_brand . ') sku: ' . $product['sku']
            . ' id: ' . $product['idProductWc'] . ' was updated'
        );
    }
}

And for example deleteProduct() method:

private function deleteProduct($product)
{
    wp_delete_post($product, true);

    $args = [
        'order' => 'ASC',
        'post_mime_type' => 'image',
        'post_parent' => (int) $product['idProductWc'],
        'post_status' => 'inherit',
        'post_type' => 'attachment',
    ];
    $child = get_children($args);
    foreach ($child as $id => $item) {
        wp_delete_attachment($id, true);
    }
}

Is the deletion method written correctly? Is it necessary to handle an error in wp_delete_post() method? How to handle it? What to write in logs in this case? Should the method return a response?

Answer 1

Looking at your importProducts() method, firstly ask if this needs to be public, private or protected see PHP visibility

Now the importProducts() method in question, I would separate and have three methods as follows:

/**
 * @param $brand string
 * @param $productsToDelete array
 */
protected function _deletePreparedProducts($brand = null, $productsToDelete = array()) {
    if(empty($brand)) {
        /* Do we need a brand? If not throw an Exception or log to your debug logger */
    }
    /* deletion logic here */
}

/**
 * @param $brand string
 * @param $productsToCreate array
 */
protected function _createPreparedProducts($brand = null, $productsToCreate = array()) { /* create logic here */ }

/**
 * @param $brand string
 * @param $productsToUpdate array
 */
protected function _updatePreparedProducts($brand = null, $productsToUpdate = array()) { /* prepare logic here */ }

I would therefore change the importProducts() method as follows (assuming that you still want this as a private method):

<?php
private function importProducts($brand)
{
    $products = $this->dsApi->getProductsByBrand($brand);

    $preparedProducts = $this->prepareProducts($products, $brand);

    if(!empty($preparedProducts['delete'])) {
        $this->_deletePreparedProducts($brand, $preparedProducts['delete']);
    }

    if(!empty($preparedProducts['create'])) {
        $this->_createPreparedProducts($brand, $preparedProducts['create']);
    }

    if(!empty($preparedProducts['update'])) {
        $this->_updatePreparedProducts($brand, $preparedProducts['update']);
    }
}

Using this approach, if you need to change the logic to update a product, you will only have to worry about your update method, rather than having lots of logic in one method. And in your main method you will be checking if there are any products to delete, update or create first rather than assuming that there is, as is the case in your original script.

If there are no products to import then why was this method called? This is where you could throw an exception, or log to a debug file, like this:

    if(empty($preparedProducts['delete'])
        && empty($preparedProducts['create'])
        && empty($preparedProducts['update'])) {
        throw new \Exception("Import products called without any products to import");
    }

I might also consider preparing the data first before calling the importProducts() method, rather than preparing that data within the method - this will depend on your dataflow.

I would finally suggest that as you are updating and deleting products, the method name may be incorrect. manageProducts() or something similar might be a better name for this method.