OBPIH-6369 Receiving : Receipt not complete after using "Edit" to change item#4901
OBPIH-6369 Receiving : Receipt not complete after using "Edit" to change item#4901awalkowiak merged 3 commits intodevelopfrom
Conversation
…supporting partial receiving
alannadolny
requested review from
awalkowiak,
drodzewicz,
ewaterman and
kchelstowski
github-actions
bot
added
the
domain: backend
|
|
||
| void setupSpec() { | ||
| mockDomain Receipt | ||
| mockDomain Shipment |
There was a problem hiding this comment.
for future reference, you can do mockDomains(Receipt, Shipment)
| void 'savePartialReceiptEvent should create RECEIVED event when partial receiving is not supported'() { | ||
| given: | ||
| Location location = Stub(Location) { | ||
| supports(_ as ActivityCode) >> false |
There was a problem hiding this comment.
Is there any reason not to specify the actual enum value here? It makes things a bit more clear in my opinion and also guarantees that we're entering the proper flow.
Same for all the other tests.
| supports(_ as ActivityCode) >> false | |
| supports(ActivityCode.PARTIAL_RECEIVING) >> false |
| shipment.destination) | ||
| } | ||
| } else { | ||
| if (!shipment.wasReceived() && (!shipment.destination.supports(ActivityCode.PARTIAL_RECEIVING) || shipment.isFullyReceived())) { |
There was a problem hiding this comment.
why are we even allowing the flow to continue here if partial receiving is disabled for the location? Shouldn't we error instead?
There was a problem hiding this comment.
I don't know why we call this feature "partial" everywhere, because all those functions simultaneously support partial / not partial receiving. That's the reason for allowing it here; if partial receiving is disabled, we can receive it only once.
| import spock.lang.Unroll | ||
|
|
||
| @Unroll | ||
| class ReceiptServiceSpec extends Specification implements ServiceUnitTest<ReceiptService>, DataTest { |
There was a problem hiding this comment.
Great job writing tests!
It'd probably also be worth testing these scenarios:
- partial receiving is enabled, it was partially received before, and now it's fully received (received event should happen)
- partial receiving is enabled, it was partially received before, and it still is (no events should happen)
- partial receiving is enabled, it was fully received before, and it still is (no events should happen)
| shipment.destination) | ||
| } | ||
| } else { | ||
| if (!shipment.wasReceived() && (!shipment.destination.supports(ActivityCode.PARTIAL_RECEIVING) || shipment.isFullyReceived())) { |
There was a problem hiding this comment.
also, I'm not totally sure about this new flow. The old flow looked correct to me. Mainly:
// We're fully receiving
if (fully received now) {
if (it was not already in fully received status) {
create fully received event
}
}
// We're partially receiving
else if (it was not already in partially received status) {
create partial received event
}
so I'm wondering if there's an issue not with this class but with shipment.isFullyReceived(). Or maybe I'm just misunderstanding. I haven't had a chance to dig much deeper into the code
3 participants
No description provided.