akeneo / pim-community-dev

[Community Development Repository] The open source Product Information Management (PIM)
http://www.akeneo.com
Other
950 stars 512 forks source link

Quick export: Number of columns inconsistent #3595

Closed Hocdoc closed 8 years ago

Hocdoc commented 8 years ago

Although the export button in the product grid in Akeneo 1.4.9 is labeled "Quick export/CSV (All attributes)" the number of exported attributes depends on the selected products.

It seems to me, that if in an attribute group all attribute values are empty, all attributes of the attribute group will not get exported. So when we are exporting different products, we have different columns in the CSV files which is more difficult to handle and confusing.

LaureBrosseau commented 8 years ago

Hello Hocdoc,

sorry for the delay to answer, this was a big topic for us. We have made some changes on the 1.4.12, regarding imports and exports, and changes made on families. Before when you were adding/removing an attribute at the family level, you had to save the product(s) to have the attributes updated, now it's done automatically.

Regarding empty attributes that are not exported, I could not reproduce the issue on my side, many empty attributes were exported in my quick export.

It should work fine now, can you upgraded to the latest version (1.4.12) and let us know if it's ok on your side.

Thanks a lot,

Laure

Hocdoc commented 8 years ago

I've just tried it with 1.4.12, the problem is still there.

Steps to reproduce:

  1. Create two products with the same family (with some attribute groups).
  2. Save the first product just with the SKU.
  3. Save the second product with values in all attributes.
  4. Make a quick export of the first product.
  5. Make a quick export of the second product.

=> In the CSV of the first product export there are fewer columns than in the second export (attributes are missing).

LaureBrosseau commented 8 years ago

Hello,

Thanks for your feedback, I have followed your steps and tested on the 1.4.13 and it is working fine for me: I have the exact same number of attributes in both quick exports.

I'll still try to reproduce it on our demo.akeneo.com (once new versions will be deployed).

Thanks.

Hocdoc commented 8 years ago

I've just tried it on demo.akeneo.com with Akeneo 1.4.13 and still have the problem:

  1. Create a product with the family Loudspeakers, leave all fields empty and return to the grid.
  2. Make the quick export for just this product -> The CSV contains 16 columns (A-P)
  3. Export the product with the SKU 10699783 (Sony SRS-BTV25) which is also a loudspeaker. -> The CSV contains 17 columns (A-Q).

The quick export of the first empty product is missing the weight-unit column, I don't know why. In our production system there are often missing many columns for nearly-empty products.

Hocdoc commented 8 years ago

The issue is still there in 1.4.17.

I think the problem is that missing product values get converted to a single empty-string-field in Pim\Bundle\TransformBundle\Normalizer\Flat\ProductValueNormalizer (data is null) regardless of the attribute type. So we are missing f.ex. the -unit-columns for empty metrics.

ProductToFlatArrayProcessor.process() calls a addMissingProductValues(), it should also call addMissingAssociations() as otherwise the association columns are missing if there were no associations in all exported products.

I hesitate to write a pull request, as I'm not sure if there are side effects when I fix these missing-columns-issue.

nidup commented 8 years ago

Hi @Hocdoc

Thank you for this very well qualified bug report!

Functionally speaking, the quick export has been designed to export all attributes belonging to selected products in the grid.

In 1.4.12, we fixed this quick export with the ticket PIM-5215 to ensure that even when the product values are not yet stored in the database (what happen when you create a product with a family without saving it in the product edit form later), we export these attributes as empty columns.

I double checked with our beloved product owner and we don't expect to export associations here. By the way, we have a story in our backlog to enhance/create another export to be able to export only what is displayed in the grid (not all attributes).

Regarding the missing weight-unit column in this case, you're right it's a bug. One extra difficulty is that for prices and metrics we maintain a legacy double format, these attributes can be read from split columns or single column (weight column with "12 KG" as value is importable for instance).

I took a look and I do agree with your analysis, data is null in case of metric attribute for a very new created product. I would be very happy if you would do a small PR to test the fix!

About side effects, we have a bunch of behat scenario and i will be glad to run them to check together if we introduce a regression. BTW, we're enhancing our Distributed Continuous Integration to improve the build time (~30 min to run the ~1600 behat scenarios vs 15h on a local env) and also easier to use for contribution (more integrated to github).

Hocdoc commented 8 years ago

Thanks about your always-helpful information and the details about the metrics columns. I've created a mini-PR that fixes this issue (at least for our data).

QuickExport also seems to export the associations if they are set.

In the future maybe it's also possible to refactor the classes from Pim\Bundle\EnrichBundle\Connector\Processor\QuickExport\ProductToFlatArrayProcessor (used for the QuickExport) and Pim\Bundle\BaseConnectorBundle\Processor\ProductToFlatArrayProcessor (used for the normal Export) together as they share some code.

nidup commented 8 years ago

thank you for the PR! (a core member will handle it quickly). agree about the refactoring we should simplify those, it seems there are small differences about the behavior of these two exports. we continuously improve the import / export part of the PIM and there is still a lot to do :)

nidup commented 8 years ago

thank you again for the PR which has been merged https://github.com/akeneo/pim-community-dev/pull/3967 I close the issue. don't hesitate to open issue or PR if you encounter other bugs :bug: