Bug 8829

Summary: can't import authorities
Product: Koha Reporter: Nicole C. Engard <nengard>
Component: CatalogingAssignee: Galen Charlton <gmcharlt>
Status: CLOSED FIXED QA Contact: Mason James <mtj>
Severity: critical    
Priority: P1 - high CC: chris, jcamins, m.de.rooy, mtj, paul.poulain
Version: 3.10   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8818
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 8829: Fix authority importing
[SIGNED-OFF] Bug 8829: Fix authority importing

Description Nicole C. Engard 2012-09-26 14:53:39 UTC
I cannot import authority files via the staff client, but can import bib records.

----------

FROM IRC:

[10:45]  <nengard> jcamins, i'm testing the authority import in master and it's frozen
[10:45]  <nengard> and the list of processes on my server doesn't show it running anymore
[10:45]  <nengard> it did work the first time I tested it … last week I think
[10:45]  <jcamins> nengard: your javascript was out of date. Go to the manage MARC import screen.
[10:45]  <nengard> it's not there
[10:46]  <jcamins> And it's not running?
[10:46]  <jcamins> Weird.
[10:46]  <nengard> but the one I did on the 20th is there
[10:46]  <nengard> It is stuck at processing
[10:46]  <nengard> on the browser
[10:46]  <nengard> and the process is not running anymore on the server
[10:46]  <nengard> what has changed for me since the 20th is that I upgraded to mountain lion … but don't think that's it
[10:46]  <jcamins> When I imported authorities yesterday it worked.
[10:47]  <nengard> hmmm
[10:47]  <jcamins> Actually, it was two days ago.
[10:47]  <jcamins> Monday.
[10:47]  * jcamins can eventually figure out what the calendar says.
[10:48]  <nengard> heh
[10:48]  <nengard> well i did pull down new updates to master yesterday
[10:48]  <nengard> so maybe something broke it yesterday
[10:48]  <jcamins> Right, but I didn't think there was anything new since Monday.
[10:49]  <nengard> here's what happens … 
[10:49]  <nengard> 1. browse for file
[10:49]  <nengard> 2. change type to authority
[10:49]  <nengard> 3. don't look for matches
[10:49]  <nengard> 4. click button
[10:49]  <nengard> server says :  www-data  1915  0.0  3.3 109904 24300 ?        R    10:43   0:00 /usr/bin/perl /home/nengard/kohaclone/tools/background-job-progress.pl 
[10:49]  <nengard> 1 minute later that line is gone from the processes, but the browser still says it's processing
[10:50]  <nengard> same process with a bib record finishes in no time
Comment 1 Jared Camins-Esakov 2012-09-26 20:37:55 UTC Comment hidden (obsolete)
Comment 2 Nicole C. Engard 2012-09-27 01:39:21 UTC
Created attachment 12541 [details] [review]
[SIGNED-OFF] Bug 8829: Fix authority importing

A subroutine was not being imported by C4::ImportBatch (ironic, no?)
so this patch makes the call fully-qualified. This patch also cleans
up two warnings in C4::Auth that are raised when logged in as the
database user.

Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com>
Comment 3 Mason James 2012-10-03 22:06:46 UTC
(In reply to comment #2)
> Created attachment 12541 [details] [review]
> [SIGNED-OFF] Bug 8829: Fix authority importing
> 
> A subroutine was not being imported by C4::ImportBatch (ironic, no?)
> so this patch makes the call fully-qualified. This patch also cleans
> up two warnings in C4::Auth that are raised when logged in as the
> database user.
> 
> Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com>

patch work well, passing QA...

$ koha-qa.pl -c 1

testing 1 commit(s) (applied to commit 50be765)
 * 09b39f1 Bug 8829: Fix authority importing
      C4/Auth.pm
      C4/ImportBatch.pm

* C4/Auth.pm                                                               OK
* C4/ImportBatch.pm                                                        OK
Comment 4 Paul Poulain 2012-10-05 15:07:05 UTC
I agree with the description and the fact that it's critical.

But isn't it caused by the fact that GetAuthorizedHeading isn't exported in C4/AuthoritiesMarc.pm ?
Shouldn't we add this sub to expose it (in the .pm) instead of adding a fully qualified name (in ImportBatch) ? 

(reminder = in Perl, you can't access a sub in a package unless:
 - it's exported
 - you fully qualify it [that's why purists says there's not real OO in Perl, because you can't have a really private method in you object]
)
Comment 5 Jared Camins-Esakov 2012-10-05 15:17:51 UTC
(In reply to comment #4)
> I agree with the description and the fact that it's critical.
> 
> But isn't it caused by the fact that GetAuthorizedHeading isn't exported in
> C4/AuthoritiesMarc.pm ?
> Shouldn't we add this sub to expose it (in the .pm) instead of adding a
> fully qualified name (in ImportBatch) ? 
> 
> (reminder = in Perl, you can't access a sub in a package unless:
>  - it's exported
>  - you fully qualify it [that's why purists says there's not real OO in
> Perl, because you can't have a really private method in you object]
> )

I prefer the fully-qualified name because really GetAuthorizedHeading shouldn't be exported. It should be an object method on Koha::Authority. BuildSummary also should not be exported. However, changing that is a bit more than a bugfix, and I'd prefer not to make master unstable this close to 3.10.
Comment 6 Paul Poulain 2012-10-08 16:19:35 UTC
OK, thx for the explanation, patch pushed
Comment 7 Chris Cormack 2013-04-20 23:32:59 UTC
Released in 3.10.0