mirakl / sdk-php-shop

Mirakl PHP SDK for sellers
29 stars 16 forks source link

Request `OfferImportErrorReportRequest` downloads a corrupted Excel file (xlsx) #69

Closed tomloprod closed 4 months ago

tomloprod commented 4 months ago

I'm using request OfferImportErrorReportRequest to download the error report for the offers; however, the Excel file it downloads is corrupted and cannot be opened (from the internal MIrakl platform it is possible to download it).

This does not happen with request DownloadProductImportErrorReportRequest, which does download a correct and readable Excel file.

After reviewing the code of de mirakl/sdk-php-shop, everything seems ok, so maybe the issue is in the Mirakl API. Could you confirm this?

Example of code:

use Mirakl\MMP\Shop\Client\ShopApiClient;
use Mirakl\MMP\Shop\Request\Offer\Importer\OfferImportErrorReportRequest;
use Mirakl\Core\Domain\FileWrapper;

/** @var FileWrapper $result */
$result = $this->api->getOffersImportErrorReport(new OfferImportErrorReportRequest($importId));

// This works and downloads an Excel file; however, the file is corrupted and cannot be opened.
return $result->download();
amarie75 commented 4 months ago

Hi,

I have tested your code and I have valid file.

The file downloaded is a CSV, it can be opened in Excel but also with a text editor. Maybe there is a warning write by PHP in the file content. You can check this with a text editor.

Il you don't find the problem can you send use the file generated ?

Best regards, Alexandre

tomloprod commented 4 months ago

Good morning Alexandre,

When opening it with a text editor, we see the hexadecimal dump of the file, it doesn't seem to be a CSV... How could we send you the file without sharing it on GitHub?

Below is a preview of the file from which we have removed some characters in the middle to avoid leaking certain data on GitHub:

image

By the way, the filename of the downloaded file by FileWrapper download() method is offers-import-error-report-5734-41123033.xlsx.xlsx (with double xlsx extension)

Thank you very much.

amarie75 commented 4 months ago

Yes it seems it's not a CSV. The error file is in the same format as the file uploaded for the import. I have retry your code with an Excel file and in can open the generated file.

Can you try to retrieve an error file in CSV (for example the import id 41133201)

tomloprod commented 4 months ago

Could you please let me know how to specify the file format when requesting the download?

amarie75 commented 4 months ago

You can't specify the format, it's the same as the file uploaded for the import.

tomloprod commented 4 months ago

In this case, it's somewhat complex, as we've developed the system with Excel and it's a generator that's used for other integrations... Is it possible that the Mirakl API doesn't handle the error report (for offers) well when it returns an Excel file?

For the product error report, it does work.

amarie75 commented 4 months ago

I managed to reproduce the problem and search the origin

tomloprod commented 4 months ago

Thank you! We await resolution. Is it in this package or in the API?

amarie75 commented 4 months ago

I think the problem is in this package

amarie75 commented 4 months ago

We have found a fix and it will be release soon.

If you want to test it before the release, you can use this diff :

diff --git a/src/Mirakl/Core/Domain/DownloadableTrait.php b/src/Mirakl/Core/Domain/DownloadableTrait.php
index 2720b202..997b8b93 100755
--- a/src/Mirakl/Core/Domain/DownloadableTrait.php
+++ b/src/Mirakl/Core/Domain/DownloadableTrait.php
@@ -29,6 +29,10 @@ public function download()
             ob_clean();
         }

+        // Remove flags used to read CSV files because they can cause corruption with other file types (e.g. Excel)
+        // when they are retrieved from Mirakl with a content type `text/csv`
+        $this->file->setFlags(0);
+
         $this->file->rewind();
         $this->file->fpassthru();
         exit;
amarie75 commented 4 months ago

Hi,

There fix release in the version 1.20.2.

Best regards, Alexandre