Bug 11170 - The pending order list should only display orders to receive
Summary: The pending order list should only display orders to receive
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 5336 11224
Blocks:
  Show dependency treegraph
 
Reported: 2013-10-30 14:35 UTC by Jonathan Druart
Modified: 2015-06-04 23:32 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 11170: The pending order list should only display orders to receive (2.38 KB, patch)
2013-10-30 14:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11170: The pending order list should only display orders to receive (2.47 KB, patch)
2013-12-16 11:08 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11170: Follow-up for typo occured in parcels.tt (1.14 KB, patch)
2013-12-16 11:09 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11170: The pending order list should only display orders to receive (2.49 KB, patch)
2013-12-20 18:03 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11170: Follow-up for typo occured in parcels.tt (1.16 KB, patch)
2013-12-20 18:04 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11170: Fix an unnecessarily convoluted logic (2.83 KB, patch)
2014-03-11 08:56 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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!