[Koha-bugs] [Bug 10096] Add a Z39.50 interface for authority searching

[bugzilla-daemon at bugs.koha-community.org]

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10096

--- Comment #25 from David Cook <dcook at prosentient.com.au> ---
(In reply to Katrin Fischer from comment #23)
> Hi David,
> 
> some very first observations from reading the code and the QA script:
> 
> The license on cataloguing/z3950_auth_search.pl is GPL2+ - maybe you could
> consider making it GPL3+?
> 
> There are some .pl files with SQL statements in them - I think it would be
> nicer to move them out to an existing or new module. I know that most of it
> probably is kind of a preexisting problem stemming from how we handle
> bibliographic records.
> 
> For the changes in Breeding.pm some unit tests should be provided.
> 
> In your updatedatabase statement could you specify after which column the
> new column should be added (little pet peeve of mine I guess :) )?

Hi Katrin:

1) Sure. I've changed the license on cataloguing/z3950_auth_search.pl to GPL3+.

2) It is a pre-existing problem, so perhaps a separate bug should be opened
instead for removing SQL from the .pl files and putting them in an existing or
new module? (It's funny that we removed the defunct Z39.50 module since it
wasn't being used and now it seems that we could use a new Z39.50 module :P). 

That'll actually be quite a large re-write, I think. 

3) As for the unit tests...I can't think of any way to really create a unit
test for this, since "Z3950SearchAuth" is too tightly tied to the template and
an actual logged in user. The same goes for the regular "Z3950Search".

There should probably be a better separation of code and template with the
Z3950 searching, but that also seems like a pre-existing problem. I just copied
the code and made it work for authorities instead of bibliographic records. 

I imagine fixing that would also be better as a separate bug. 

4) Fixed updatedatabase to specify an "AFTER" column (as well as removing MySQL
backticks). 

5) I've removed the FIXMEs and TODOs from the parts of Breeding.pm that I've
added. (They weren't "real" FIXMEs.)

-- 
You are receiving this mail because:
You are watching all bug changes.