Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[OrderItemUnit][ShippingUnit][InventoryUnit] Clean up #6112

Merged
merged 5 commits into from
Sep 15, 2016

Conversation

GSadee
Copy link
Member

@GSadee GSadee commented Sep 14, 2016

Q A
Bug fix? no
New feature? no
BC breaks? yes
Related tickets
License MIT
  • remove inventoryState from InventoryUnit
  • remove shippingState from ShipmentUnit
  • remove inventoryState and shippingState from OrderItemUnit
  • remove OrderInventoryListener
  • remove OrderItemInventoryListener
  • remove InventoryHandler

@koemeet
Copy link
Contributor

koemeet commented Sep 14, 2016

@GSadee This PR may be somehow related to a fix I proposed in #6064.

@GSadee GSadee changed the title [OrderItemUnit] Clean up OrderItemUnit [WIP][OrderItemUnit] Clean up OrderItemUnit Sep 14, 2016
@GSadee GSadee changed the title [WIP][OrderItemUnit] Clean up OrderItemUnit [OrderItemUnit][ShippingUnit][InventoryUnit] Clean up Sep 15, 2016
@@ -62,7 +57,7 @@ public function getId()
}

/**
* {@inheritdoc}
* {@inheritd
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whaat?!

@michalmarcinkowski michalmarcinkowski added the BC Break PRs introducing BC breaks (do not even try to merge). label Sep 15, 2016
@michalmarcinkowski michalmarcinkowski merged commit 6066868 into Sylius:master Sep 15, 2016
@michalmarcinkowski
Copy link
Contributor

Good job Grzesiu!

@tchapi
Copy link
Contributor

tchapi commented Sep 15, 2016

Just a quick question on this PR. Before, in order to tell that an order was shipped programmatically (think of a mp3 download, for instance), I was doing that :

// ...
foreach ($item->getUnits() as $unit) {
        $unit->setShippingState(ShipmentInterface::STATE_SHIPPED);
        $unit->getShipment()->setState(ShipmentInterface::STATE_SHIPPED);
 }
// ...

Now, I just remove the $unit->setShippingState() call, is that right ? And the setState() of the Shipment will do the rest ?

Thanks for your light on this !

@koemeet
Copy link
Contributor

koemeet commented Sep 15, 2016

@tchapi I think so yeah. From what I can tell the shipment now says if it's shipped or not. And I guess when you want to do partial shipping, you just remove some units of a shipment and put it on a new Shipment and transition it to shipped and you have partially shipped an order.

In your case you can just set the shipment to shipped, but make sure to use the state machine so any needed actions can be taken (I saw you didn't do that in your code example, but maybe you did in on purpose idk).

@tchapi
Copy link
Contributor

tchapi commented Sep 15, 2016

hmmm ... that makes sense, right. A little bit of testing gave me the same result, so I guess I'll go this way. Thanks again Steffen !

@GSadee GSadee deleted the cleanup-order-item-unit branch September 22, 2017 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BC Break PRs introducing BC breaks (do not even try to merge).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants