http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13244 --- Comment #1 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 33517 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33517&action=edit Bug 13244: Merge GetOrders and GetCancelledOrders These two subroutines did the same job (same select, same join, etc.) Test plan: Go on the basket list page and verify you see the pending and the cancelled baskets. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13244 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33517|0 |1 is obsolete| | --- Comment #2 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 33518 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33518&action=edit Bug 13244: Merge GetOrders and GetCancelledOrders These two subroutines did the same job (same select, same join, etc.) Test plan: Go on the basket list page and verify you see the pending and the cancelled baskets. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13244 --- Comment #4 from Jonathan Druart <jonathan.druart@biblibre.com> --- s/are/is! -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13244 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl Status|Needs Signoff |Failed QA --- Comment #6 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Running the test (adjust tests to 87): not ok 14 - GetOrder gets an order with the right number of fields # Failed test 'GetOrder gets an order with the right number of fields' # at t/db_dependent/Acquisition.t line 392. # got: '60' # expected: '59' not ok 16 - GetOrder gets an order with no unexpected fields # Failed test 'GetOrder gets an order with no unexpected fields' # at t/db_dependent/Acquisition.t line 399. # got: 'cancelledby' # expected: '' etc. # Looks like you failed 8 tests of 87. Please verify if you have the same .. Parking in FQA now. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13244 --- Comment #8 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #7)

(In reply to M. de Rooy from comment #6)

...

Running the test (adjust tests to 87):

not ok 14 - GetOrder gets an order with the right number of fields # Failed test 'GetOrder gets an order with the right number of fields' # at t/db_dependent/Acquisition.t line 392. # got: '60' # expected: '59'

not ok 16 - GetOrder gets an order with no unexpected fields # Failed test 'GetOrder gets an order with no unexpected fields' # at t/db_dependent/Acquisition.t line 399. # got: 'cancelledby' # expected: ''

etc.

# Looks like you failed 8 tests of 87.

Please verify if you have the same .. Parking in FQA now.

Does they pass on master? Maybe you have an extra DB field in your aqorders table (check the var $test_extra_fields l.392). The tests pass here.

Sorry, it does not pass on master too. Will have a look what fields were added when testing something else :) -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13244 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #10 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- OK. The test passes now. Overall, it looks good. One small question: This does not look good? $query .= $cancelled ? q| aqorders_transfers.ordernumber_from AS transferred_to, aqorders_transfers.timestamp AS transferred_to_timestamp | : q| aqorders_transfers.ordernumber_from AS transferred_from, aqorders_transfers.timestamp AS transferred_from_timestamp |; Did you mixup to and from in the first line? Please compare with the lines you removed: - aqorders_transfers.ordernumber_to AS transferred_to, - aqorders_transfers.timestamp AS transferred_to_timestamp Parking in FQA -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13244 --- Comment #12 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to M. de Rooy from comment #10)

OK. The test passes now. Overall, it looks good. One small question: This does not look good?

$query .= $cancelled ? q| aqorders_transfers.ordernumber_from AS transferred_to, aqorders_transfers.timestamp AS transferred_to_timestamp | : q| aqorders_transfers.ordernumber_from AS transferred_from, aqorders_transfers.timestamp AS transferred_from_timestamp |;

Did you mixup to and from in the first line?

Erk, yes I did! Thanks for catching that Marcel. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13244 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33519|0 |1 is obsolete| | Attachment #35582|0 |1 is obsolete| | Attachment #35583|0 |1 is obsolete| | --- Comment #13 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 35584 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=35584&action=edit Bug 13244: Merge GetOrders and GetCancelledOrders These two subroutines did the same job (same select, same join, etc.) Test plan: Go on the basket list page and verify you see the pending and the cancelled baskets. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Two small things are adjusted in separate follow-ups. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13244 --- Comment #15 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 35586 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=35586&action=edit Bug 13244: (follow-up) fix mixup to and from in the sql query Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13244 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13244 --- Comment #17 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 35625 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=35625&action=edit [PASSED QA] Bug 13244: Follow-up for unneeded redeclaration Resolves: FAIL acqui/basket.pl OK critic OK forbidden patterns OK pod FAIL valid "my" variable @results masks earlier declaration in same scope Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13244 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Patch doesn't apply CC| |tomascohen@gmail.com --- Comment #19 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Unfortunately this one is not cleanly applying after I pushed 12976 and 12970. Please rebase. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13244 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #35624|0 |1 is obsolete| | Attachment #35625|0 |1 is obsolete| | Attachment #35626|0 |1 is obsolete| | --- Comment #20 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 36034 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=36034&action=edit Bug 13244: Merge GetOrders and GetCancelledOrders These two subroutines did the same job (same select, same join, etc.) Test plan: Go on the basket list page and verify you see the pending and the cancelled baskets. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Two small things are adjusted in separate follow-ups. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13244 --- Comment #22 from Jonathan Druart <jonathan.druart@biblibre.com> --- Rebased, second patch skipped (not used anymore). -- You are receiving this mail because: You are watching all bug changes.