{"action":"created","issue":{"url":"https://api.github.com/repos/solidusio/solidus/issues/1004","repository_url":"https://api.github.com/repos/solidusio/solidus","labels_url":"https://api.github.com/repos/solidusio/solidus/issues/1004/labels{/name}","comments_url":"https://api.github.com/repos/solidusio/solidus/issues/1004/comments","events_url":"https://api.github.com/repos/solidusio/solidus/issues/1004/events","html_url":"https://github.com/solidusio/solidus/pull/1004","id":143042346,"number":1004,"title":"Encapsulate the logic for when an item is shippable.","user":{"login":"eric1234","id":43233,"avatar_url":"https://avatars.githubusercontent.com/u/43233?v=3","gravatar_id":"","url":"https://api.github.com/users/eric1234","html_url":"https://github.com/eric1234","followers_url":"https://api.github.com/users/eric1234/followers","following_url":"https://api.github.com/users/eric1234/following{/other_user}","gists_url":"https://api.github.com/users/eric1234/gists{/gist_id}","starred_url":"https://api.github.com/users/eric1234/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/eric1234/subscriptions","organizations_url":"https://api.github.com/users/eric1234/orgs","repos_url":"https://api.github.com/users/eric1234/repos","events_url":"https://api.github.com/users/eric1234/events{/privacy}","received_events_url":"https://api.github.com/users/eric1234/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":5,"created_at":"2016-03-23T18:25:45Z","updated_at":"2016-04-01T14:08:39Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/solidusio/solidus/pulls/1004","html_url":"https://github.com/solidusio/solidus/pull/1004","diff_url":"https://github.com/solidusio/solidus/pull/1004.diff","patch_url":"https://github.com/solidusio/solidus/pull/1004.patch"},"body":"Transitioning an item (InventoryUnit) to `shipped` is restricted [allowing an item to ask itself if it is shippable before it is marked as shipped](https://github.com/solidusio/solidus/blob/master/core/app/models/spree/inventory_unit.rb#L58). This questioning is implemented by [verifying the item is on-hand](https://github.com/solidusio/solidus/blob/master/core/app/models/spree/inventory_unit.rb#L118-L120).\r\n\r\nBut when the overall shipment is checking to see if the item is shippable and [it does it's own interrogation of the items in the shipment rather than asking the items itself](https://github.com/solidusio/solidus/blob/master/core/app/models/spree/shipment.rb#L84).\r\n\r\nWorse is that the implementation differs from the implementation in InventoryUnit. InventoryUnit questions if the item is \"on-hand\" while in Shipment it questions if the item is not backordered. Unsure if it is a real issue but it might mean we could get a case where the inventory unit is in neither state allowing the shipment to think it can ship while the inventory unit thinks it can't resulting in an error.\r\n\r\nThis commit encapulates the decision entirely in InventoryUnit allowing the item to determine if it is ready to ship. This logically should be the same as before only we now have better encapsulation.\r\n\r\nThe additional benefit (and primary motivation) is that the condition can customized now that it is properly encapsulated. I am working on an app with on-demand manufactoring with no real inventory. An item cannot be shipped until it is built. We can customize InventoryUnit as follows:\r\n\r\n    Spree::InventoryUnit.class_eval do\r\n      module CheckBuiltForShipping do\r\n        def allow_ship?\r\n          super && built?\r\n        end\r\n      end\r\n      prepend CheckBuiltForShipping\r\n    end\r\n\r\nPrior to this commit, the system would allow the user to attempt to ship the item (since it is not \"backordered\") but the ship would fail since our customization made it not shippable. With this commit the shipment it now not shippable also meaning the user isn't even presented with the option.\r\n\r\nIn theory this commit should have required no test changes as it is just refactoring implementation details. Unfortunately due to the abuse of mocks there are some adjustments to the test to get things passing."},"comment":{"url":"https://api.github.com/repos/solidusio/solidus/issues/comments/204410175","html_url":"https://github.com/solidusio/solidus/pull/1004#issuecomment-204410175","issue_url":"https://api.github.com/repos/solidusio/solidus/issues/1004","id":204410175,"user":{"login":"eric1234","id":43233,"avatar_url":"https://avatars.githubusercontent.com/u/43233?v=3","gravatar_id":"","url":"https://api.github.com/users/eric1234","html_url":"https://github.com/eric1234","followers_url":"https://api.github.com/users/eric1234/followers","following_url":"https://api.github.com/users/eric1234/following{/other_user}","gists_url":"https://api.github.com/users/eric1234/gists{/gist_id}","starred_url":"https://api.github.com/users/eric1234/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/eric1234/subscriptions","organizations_url":"https://api.github.com/users/eric1234/orgs","repos_url":"https://api.github.com/users/eric1234/repos","events_url":"https://api.github.com/users/eric1234/events{/privacy}","received_events_url":"https://api.github.com/users/eric1234/received_events","type":"User","site_admin":false},"created_at":"2016-04-01T14:08:39Z","updated_at":"2016-04-01T14:08:39Z","body":"Commits have been squashed and rebased to master."}}