Fix for increment ids

[WJdeBaas]

A patch to fix the hardcoded entity type in app/code/Magento/Sales/Model/Increment.php

[airbone42]

Travis is throwing some errors. If you have a required dependency that should be injected in the constructor using the DI framework so it's always set.

[WJdeBaas]

Hi airbone42,

How about applying patch bac5bbd first, and then 0d29f9c ?
If that won't help, what should I do ?

[otoolec]

As @airbone42 pointed out, if you want this to be a required argument then it should be injected in the constructor instead of by the setEntityType method.

public function __construct(
     EavConfig $eavConfig,
     $entityType = Order::ENTITY
 ) {
     $this->eavConfig = $eavConfig;
     $this->entityType = $entityType;
 }

Please also look at the failing tests and add new test cases for injecting non-default entity types.

[vpelipenko]

@WJdeBaas, did you have a chance to look at previous comments?

[WJdeBaas]

Hi vpelipenko,

My apologies, but it's getting a bit confusing, it's not quite clear to me what is expected from me at this point about this bug and what its status exactly is.

ps: what does the CS label stands for ?

[vpelipenko]

@WJdeBaas, your fix is OK, but it leads to failed unit tests on Travis. It will be good if you fix them. CS label is used for our internal staff, it helps us to assign responsibilities.

[vpelipenko]

Internal ticket: MAGETWO-34653

[WJdeBaas]

Hi vpelipenko,

What do I have to do to make it acceptable for Travis ?

[vpelipenko]

@WJdeBaas, you've changed logic inside Magento\Sales\Model\Increment::getNextValue(). It throws exception when $this->entityType is empty. Also we have unit test dev/tests/unit/testsuite/Magento/Sales/Model/IncrementTest for testing this method. It fails now when calls original method, because $this->entityType is always empty there - look at log here https://travis-ci.org/magento/magento2/jobs/48911881.

If you are familiar with unit tests you can fix failed tests, if no, we can fix them on our side.

[vpelipenko]

@WJdeBaas, please inform us about your decision on unit tests. Do you have plans to fix it and update this PR?

[vpelipenko]

There is no response from contributor. Closed. It will be fixed internally.