Bug 9105 - Moving Z3950 search code to Breeding.pm
Summary: Moving Z3950 search code to Breeding.pm
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Z39.50 / SRU / OpenSearch Servers (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Marcel de Rooy
QA Contact: Katrin Fischer
URL:
Keywords:
: 8886 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-11-19 14:28 UTC by Marcel de Rooy
Modified: 2014-05-26 21:04 UTC (History)
6 users (show)

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


Attachments
Patch for 9105 (33.62 KB, patch)
2012-11-21 14:50 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Followup for 9105 (16.81 KB, patch)
2012-11-21 14:51 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9105: Move Z3950 search code to Breeding module (33.67 KB, patch)
2012-11-21 15:30 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9105: Housekeeping followup (16.83 KB, patch)
2012-11-21 15:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Second (housekeeping) patch (2.96 KB, patch)
2012-11-22 09:23 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9105: Second housekeeping followup (3.63 KB, patch)
2012-11-22 09:50 UTC, Marcel de Rooy
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 9105: Move Z3950 search code to Breeding module (33.72 KB, patch)
2012-12-07 16:27 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 9105: Housekeeping followup (16.67 KB, patch)
2012-12-07 16:27 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 9105: Second housekeeping followup (3.64 KB, patch)
2012-12-07 16:28 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9105: Move Z3950 search code to Breeding module (33.72 KB, patch)
2012-12-21 14:39 UTC, Paul Poulain
Details | Diff | Splinter Review
Bug 9105: Housekeeping followup (16.67 KB, patch)
2012-12-21 14:39 UTC, Paul Poulain
Details | Diff | Splinter Review
Bug 9105: Second housekeeping followup (3.70 KB, patch)
2012-12-21 14:39 UTC, Paul Poulain
Details | Diff | Splinter Review
Followup for closing connections (861 bytes, patch)
2013-01-07 13:10 UTC, Marcel de Rooy
Details | Diff | Splinter Review
[SIGNED-OFF] 9105: Followup for closing Zoom connections (872 bytes, patch)
2013-01-18 15:15 UTC, Kyle M Hall
Details | Diff | Splinter Review
9105: Followup for closing Zoom connections (963 bytes, patch)
2013-02-11 07:34 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2012-11-19 14:28:03 UTC
In order to realize the goals in bug 6536, this patch first moves the z3950 search code in acquisition and cataloging to one place, the Breeding module.
Comment 1 Marcel de Rooy 2012-11-21 14:50:57 UTC Comment hidden (obsolete)
Comment 2 Marcel de Rooy 2012-11-21 14:51:56 UTC Comment hidden (obsolete)
Comment 3 Marcel de Rooy 2012-11-21 14:54:43 UTC
TEST PLAN: Just check some z3950 searches without this patch and with this patch. There should be no behavior changes.
Note that the patch touches two places: Cataloging/Z3950 Search and Acquisition, open a basket, add a line with From an external source.
Comment 4 Jonathan Druart 2012-11-21 15:30:30 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2012-11-21 15:31:11 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2012-11-21 15:32:20 UTC
Great work Marcel!

Another SO is welcomed :)
Comment 7 Marcel de Rooy 2012-11-22 09:23:06 UTC Comment hidden (obsolete)
Comment 8 Marcel de Rooy 2012-11-22 09:35:27 UTC
Comment on attachment 13625 [details] [review]
Second (housekeeping) patch

Still see something. Please wait..
Comment 9 Marcel de Rooy 2012-11-22 09:50:50 UTC Comment hidden (obsolete)
Comment 10 Marcel de Rooy 2012-11-22 09:55:18 UTC
(In reply to comment #6)
> Another SO is welcomed :)

Thanks Jonathan for your signing off as Unimarc user. Although moving the code now not really relates to marc flavour, another marc21 signoff is welcome.
For future QA it would be better not to squash the three patches.
Comment 11 Kyle M Hall 2012-12-07 16:27:30 UTC Comment hidden (obsolete)
Comment 12 Kyle M Hall 2012-12-07 16:27:47 UTC Comment hidden (obsolete)
Comment 13 Kyle M Hall 2012-12-07 16:28:03 UTC Comment hidden (obsolete)
Comment 14 Kyle M Hall 2012-12-07 16:28:53 UTC
MARC21 user here, looks good to me!
Comment 15 Paul Poulain 2012-12-21 14:39:15 UTC
QA comment:
 * not really an enhancement, changing to "normal" bugfix
 * koha-qa.pl is OK
 * I've grepped the moved subs and checked there is a use C4::Breeding instead of C4::Z3950

passed QA
Comment 16 Paul Poulain 2012-12-21 14:39:40 UTC Comment hidden (obsolete)
Comment 17 Paul Poulain 2012-12-21 14:39:44 UTC Comment hidden (obsolete)
Comment 18 Paul Poulain 2012-12-21 14:39:47 UTC Comment hidden (obsolete)
Comment 19 Jared Camins-Esakov 2012-12-22 21:18:12 UTC
This patch has been pushed to master.

There is, however, a problem: we do not destroy the ZOOM connection or recordset. A follow-up destroying those would be greatly appreciated.
Comment 20 Marcel de Rooy 2013-01-07 13:08:59 UTC
(In reply to comment #19)
> This patch has been pushed to master.
> 
> There is, however, a problem: we do not destroy the ZOOM connection or
> recordset. A follow-up destroying those would be greatly appreciated.

Patch forthcoming.
Comment 21 Marcel de Rooy 2013-01-07 13:10:11 UTC Comment hidden (obsolete)
Comment 22 Kyle M Hall 2013-01-18 15:15:18 UTC Comment hidden (obsolete)
Comment 23 Katrin Fischer 2013-02-11 07:34:38 UTC
Created attachment 15222 [details] [review]
9105: Followup for closing Zoom connections

Housekeeping: close the results sets and connections from Z3950 searches.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
No regression found, all tests and QA script pass.
Comment 24 Jared Camins-Esakov 2013-02-12 13:50:16 UTC
This patch has been pushed to master.
Comment 25 Mark Tompsett 2013-04-11 02:35:44 UTC
*** Bug 8886 has been marked as a duplicate of this bug. ***