Msi source export

[larsroettig]

Description

Provides export functionality for Stock Sources

Manual testing scenarios

1. Go to System > Export in Magento backend
2. Select Stock Sources as entity type
3. Try different filtering options and export CSV files

Contribution checklist

• Pull request has a meaningful description of its purpose
• 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)

[maghamed]

Please pay attention that Travis builds are red.
There is a failure in Static Tests - https://travis-ci.org/magento-engcom/msi/jobs/294045656

[maghamed]

Let's move this Attribute Collection building process into the separate class.
Doing so we will improve Single Responsibility as well as will introduce additional extension point for Attribute Collection building.
Thus, if 3rd party developer would like to inject additional attribute or provide own collection of attributes with filters he suppose to overwrite just this Builder

[maghamed]

do we really throw an Exception out of this method?

[vadimjustus]

Exception is thrown by an internal method.

[maghamed]

looks like there is no throwing of \Magento\Framework\Exception\LocalizedException out of current function

[vadimjustus]

In the previouse implementation this exception was thrown by an internal method. In current implementation the method really thow this exception by it self.

[maghamed]

please provide a more meaningful description. As I suppose current function is the main for the exporting process

[maghamed]

Try to avoid dynamic PHP behavior, as it most of the time leads to some magic under the hood and bugs which are really hard to debug.

What we usually do in similar cases when there are some processors (with the same contract/interfaces) and depending on the type given you need to apply specific one - we apply Strategy Pattern

In your case that will look like:

interface FilterProcessorInterface
{
     public function process($collection, $columnName, $value);
}

class FilterProcessorAggregator
{
     // accept an array configured through the DI 
     // ['handler' => 'filter/handler/class']
     //  'filter/handler/class' must be an instance of FilterProcessorInterface
     public function __construct($filterHandlers)

    // $type is one of the possible handlers  Int|String|Decimal|etc
    public function process($type, $collection, $columnName, $value)
    {
         if (!isset($this->hadlers[$type])) {
             throw new Exception();
         }
         $this->hadlers[$type]->process($collection, $columnName, $value);
    }
}

you have a dedicated class for each filter

class IntFilter implements FilterProcessorInterface
class StringFilter implements FilterProcessorInterface
class DecimalFilter implements FilterProcessorInterface

after these changes, your business logic will include just FilterProcessorAggregator instance.
and you will call:
$this->filterProcessor->($type, $collection, $columnName, $value);

Also that will give us an ability to easily add new processors later

[maghamed]

better not to use DateTime object directly

but Magento\Framework\Stdlib\DateTime instead

[maghamed]

additional blank line should be added at the end

[vadimjustus]

done

[maghamed]

Both of these refactoring applied will help you to decrease the number of external dependencies for this class

As currently we have a failure in Static Tests:
The class Sources has a coupling between objects value of 15. Consider to reduce the number of dependencies under 13.

[maghamed]

Please provide a description of what this Factory creates

[maghamed]

please provide a description of the Filter Processing in comment

[maghamed]

you can use @inheritdoc if you don't change the comment in the implementation

[maghamed]

you are still using new \DateTime but not Magento\Framework\Stdlib\DateTime

[maghamed]

Fantastic contribution!! Thanks, guys @vadimjustus and @larsroettig

This contribution really matters!
Striking out 10 Story Points out of the MSI Backlog