import-export-improvements #82 : super attribute error message improvements

tadhgbowe commented 6 years ago

Improvement to product import error handling for configurable variations column super attribute codes.

Description

If the standard product import detects that a configurable_variations attribute code is not super one the following error messages will be displayed:

Attribute code does not exist or is not in chosen attribute set. Attribute code needs to have an Input Type of "Dropdown", "Visual Swatch" or Text Swatch" Attribute code needs to have a Scope of "Global".

This covers all the reasons why it may not be a not super attribute. Displaying "Attribute with this code is not super" is not helpful enough.

Fixed Issues (if relevant)

https://github.com/magento-engcom/import-export-improvements/issues/82 Please see the above issue as there is a full comment trace.

Manual testing scenarios

Try import a configurable product with a super attribute code inside configurable_variations that:

doesn't exist
exists but is not in the target attribute set
is not global scope
Is not of type "dropdown", "visual swatch" or "text swatch"

Contribution checklist

[ x ] Pull request has a meaningful description of its purpose
[ x ] All commits are accompanied by meaningful commit messages
[ ] All new or changed code is covered with unit/integration tests (if applicable)
[ ] All automated tests passed successfully (all builds on Travis CI are green)

magento-cicd2 commented 6 years ago

All committers have signed the CLA.

magento-engcom-team commented 6 years ago

@tadhgbowe thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

dmanners commented 6 years ago

@tadhgbowe there seem to be a could of static test failures now.

1) Magento\Test\Php\LiveCodeTest::testCodeStyle
PHP Code Sniffer detected 2 violation(s): 
FILE: ...agento/ConfigurableImportExport/Model/Import/Product/Type/Configurable.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 342 | ERROR | [x] Expected 1 space after USE keyword; found 0
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------
Failed asserting that 2 matches expected 0.
/home/travis/build/magento-engcom/import-export-improvements/dev/tests/static/testsuite/Magento/Test/Php/LiveCodeTest.php:269
2) Magento\Test\Php\LiveCodeTest::testCodeMess
PHP Code Mess has found error(s):
/home/travis/build/magento-engcom/import-export-improvements/app/code/Magento/ConfigurableImportExport/Model/Import/Product/Type/Configurable.php:331   The method identifySuperAttributeError() has a Cyclomatic Complexity of 11. The configured cyclomatic complexity threshold is 10.

The first one seems like not a big problem to update. The second might take a bit of refactoring on the method to see if it can be decreased a bit. If you need some help, feel free to reach out to me on that.

tadhgbowe commented 6 years ago

@dmanners - thanks for the update earlier. Yep I noticed it too. I pushed up some enhancements (i.e. tidy up!) earlier on. Removing the foreach introduced this little beauty. There were some cyclomatic complexities too. I'm on the case... :-)

ps. hope you've been out for fresh air ;-)

dmanners commented 6 years ago

Hi @tadhgbowe looking good for the changes, I will check why the integration tests are timing out and get back to you on the next steps for this pr. Weekend involved plenty of fresh air and family time.

tadhgbowe commented 6 years ago

Morning @dmanners. #NoMagentoSunday working nicely for me. Football fever over here! Yes, I've seen the timing out thing on other pull requests. So I hope it's just a temporary blip. Grand. T

dmanners commented 6 years ago

So I have run the internal tools and it all seems fine from the testing side, think it might just be travis. Will pass this onto our QA team now @tadhgbowe and keep you updated.

tadhgbowe commented 6 years ago

@dmanners - That's great. Thanks for the update. I've got a few other issues to get on with. Cheers. T

magento-engcom / import-export-improvements