Bug 11170

Summary: The pending order list should only display orders to receive
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: AcquisitionsAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: fridolin.somers, gmcharlt, m.de.rooy
Version: Main   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11398
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 5336, 11224    
Bug Blocks:    
Attachments: Bug 11170: The pending order list should only display orders to receive
Bug 11170: The pending order list should only display orders to receive
Bug 11170: Follow-up for typo occured in parcels.tt
Bug 11170: The pending order list should only display orders to receive
Bug 11170: Follow-up for typo occured in parcels.tt
Bug 11170: Fix an unnecessarily convoluted logic

Description Jonathan Druart 2013-10-30 14:35:22 UTC
The parcel page should only display status "ordered" and "partial".
Comment 1 Jonathan Druart 2013-10-30 14:36:21 UTC Comment hidden (obsolete)
Comment 2 Marcel de Rooy 2013-12-16 11:08:58 UTC Comment hidden (obsolete)
Comment 3 Marcel de Rooy 2013-12-16 11:09:13 UTC Comment hidden (obsolete)
Comment 4 Kyle M Hall 2013-12-20 18:03:24 UTC
Created attachment 23728 [details] [review]
Bug 11170: The pending order list should only display orders to receive

The order status ordered is set when the basket is closed.
The parcel page should only display status "ordered" and "partial".

Test plan:
- create a basket.
- create an order.
- verify the order is not listed on the parcel page (i.e. you cannot
  receive it).
- close the basket.
- verify the order is listed on the parcel page.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 5 Kyle M Hall 2013-12-20 18:04:15 UTC
Created attachment 23729 [details] [review]
Bug 11170: Follow-up for typo occured in parcels.tt

Found this typo while testing bug 11170.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 6 Galen Charlton 2014-03-10 19:54:52 UTC
Comment on attachment 23728 [details] [review]
Bug 11170: The pending order list should only display orders to receive

Review of attachment 23728 [details] [review]:
-----------------------------------------------------------------

Failing on account of the lack of documentation and unit test coverage for the new parameter to SearchOrders().

::: C4/Acquisition.pm
@@ +1665,4 @@
>      my $basketgroupname = $params->{basketgroupname};
>      my $owner = $params->{owner};
>      my $pending = $params->{pending};
> +    my $ordered = $params->{ordered};

There is neither POD nor unit test coverage of this new option.

@@ +1695,5 @@
> +        $query .= q{ AND (quantity > quantityreceived OR quantityreceived is NULL)};
> +    }
> +    if ( $ordered ) {
> +        $query .= q{ AND (quantity > quantityreceived OR quantityreceived is NULL)}
> +            unless $pending;

The logic here is unnecessarily convoluted.  Please restructure so that only one Boolean test controls whether the "AND (quantity > quantityreceived OR quantityreceived is NULL)" clause gets added.
Comment 7 Jonathan Druart 2014-03-11 08:56:03 UTC
Created attachment 26049 [details] [review]
Bug 11170: Fix an unnecessarily convoluted logic

This patch also adds POD and UT.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 8 Jonathan Druart 2014-03-11 08:57:36 UTC
(In reply to Galen Charlton from comment #6)
> Failing on account of the lack of documentation and unit test coverage for
> the new parameter to SearchOrders().

POD and UT added.

> The logic here is unnecessarily convoluted.

Yes, true! Fixed.

Switch back to needs SO if you think it is needed.
Comment 9 Galen Charlton 2014-03-11 15:23:24 UTC
Pushed to master.  Thanks, Jonathan!