Bug 6720 - Saved authorities always show as 'Default'
Summary: Saved authorities always show as 'Default'
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: MARC Authority data support (show other bugs)
Version: 3.8
Hardware: All All
: P5 - low normal (vote)
Target Milestone: ---
Assignee: Jared Camins-Esakov
QA Contact: Jonathan Druart
URL: /cgi-bin/koha/authorities/detail.pl?a...
Whiteboard:
Keywords:
Depends on:
Blocks: 8212
  Show dependency treegraph
 
Reported: 2011-08-14 15:59 UTC by Katrin Fischer
Modified: 2013-12-05 20:06 UTC (History)
5 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
screenshot (32.81 KB, image/png)
2011-08-14 15:59 UTC, Katrin Fischer
Details
Bug 6720: Display the authority type in OPAC (1.73 KB, patch)
2012-06-13 23:27 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6720: Display the authority type in OPAC (1.84 KB, patch)
2012-06-15 14:24 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 6720: show authority type in auth details (832 bytes, patch)
2012-06-15 14:40 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6720: show authority type in auth details (918 bytes, patch)
2012-06-15 14:43 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 6720: Display the authority type in OPAC (2.38 KB, patch)
2012-06-20 12:49 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 6720: Display the authority type in OPAC (2.44 KB, patch)
2012-06-23 19:31 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 6720 follow-up: use authtypetext instead of summary (1.38 KB, patch)
2012-07-15 13:54 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 6720 follow-up: fix error when auth has been deleted (1.19 KB, patch)
2012-08-02 13:19 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2011-08-14 15:59:00 UTC
Created attachment 4999 [details]
screenshot

To test:
- add a new personal name or other authority
- search for your authority
- Bug: detail page will show it as 'Default'
- edit screen shows authority type correctly

I suspect a template problem but wasn't able to spot it.
Comment 1 Owen Leonard 2011-08-15 16:36:22 UTC
If this is a template error then it's not one I can see either. I'm reclassifying this as an Authorities bug.
Comment 2 Jared Camins-Esakov 2012-06-13 23:27:03 UTC Comment hidden (obsolete)
Comment 3 Katrin Fischer 2012-06-15 14:24:08 UTC Comment hidden (obsolete)
Comment 4 Katrin Fischer 2012-06-15 14:25:54 UTC
Small note: Bug should remain open after pushing this patch. The problem also still exists on the authority detail page in staff.
Comment 5 Jared Camins-Esakov 2012-06-15 14:40:00 UTC Comment hidden (obsolete)
Comment 6 Katrin Fischer 2012-06-15 14:43:40 UTC Comment hidden (obsolete)
Comment 7 Katrin Fischer 2012-06-15 14:44:45 UTC
Thx Jared for providing an additional patch for the staff side problem!
Comment 8 Ian Walls 2012-06-15 17:13:57 UTC
Patch 1:  adds new template param to get the Auth Type properly, then displays it in the template.  Passed QA.

Patch 2:  replaces an empty string with the actual Auth Type.  Marking Passed QA.
Comment 9 Paul Poulain 2012-06-20 08:15:19 UTC
The 1st patch does not apply anymore, the 2nd one is OK, so will push the 2nd one now, please rebase the 1st one
Comment 10 Paul Poulain 2012-06-20 08:15:44 UTC
Comment on attachment 10297 [details] [review]
[SIGNED-OFF] Bug 6720: show authority type in auth details

This patch has been pushed
Comment 11 Jared Camins-Esakov 2012-06-20 12:49:37 UTC Comment hidden (obsolete)
Comment 12 Jared Camins-Esakov 2012-06-20 12:50:36 UTC
Another signoff for the first patch now that it's been rebased would be super keen.
Comment 14 Jared Camins-Esakov 2012-06-22 02:19:12 UTC
(In reply to comment #13)
> Jared, is 
> http://git.koha-community.org/gitweb/?p=koha.git;a=blobdiff;f=authorities/
> detail.pl;h=a7d7d8f21882b7bcace75c5eb862eaebad8dfdeb;
> hp=d4f2c6730c76f19fb0aeefebdfd7fe5558066eff;
> hb=f809e47fd1644314bbd63542f3021382eece642f;
> hpb=f2ca89b30e8d48b987782ff3f9f7c8e8240b87a4
> 
> Safe to cherry-pick now, or best to wait until the second patch is pushed to
> master also?

You can cherry-pick that patch, and cherry-pick the next once it's been pushed to master.
Comment 15 Chris Cormack 2012-06-22 02:22:58 UTC
Done, thanks for the quick reply
Comment 16 Mirko Tietgen 2012-06-23 19:31:00 UTC
Created attachment 10444 [details] [review]
Bug 6720: Display the authority type in OPAC

Due to a peculiarity in the way SearchAuthorities worked, the authority
search results on the OPAC always listed the authority type that the
user searched for, rather than the authority type of the results.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works nicely, result list for OPAC search now shows the authority type
for each record.

Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Rebased 20 June 2012.

Signed-off-by: Mirko Tietgen <5p4m@gmx.de>
Works as expected.
Comment 17 Jonathan Druart 2012-07-05 11:58:47 UTC
Comment on attachment 10380 [details] [review]
Bug 6720: Display the authority type in OPAC

This one seems to be obsoleted
Comment 18 Jonathan Druart 2012-07-05 12:17:34 UTC
QA Comments:

my $thisauthtype = GetAuthType(GetAuthTypeCode($authid));
This line is very scabby. It does 2 selects on the auth_header table.
But I don't find an other way to do that in our API.

Considering the dirty code into the SearchAuthorities routine, I mark as passed QA this patch.
Comment 19 Paul Poulain 2012-07-06 09:56:30 UTC
mmm... the code says:
+                $newline{authtype}     = defined ($thisauthtype) ?
+                                            $thisauthtype->{'summary'} :
+                                            GetAuthType($authtypecode)->{'summary'};

"summary" is not the authtype description, but how the authority display is built. I tried the patch anyway, and got a -suspected- wrong result: 

The "type of heading" column now show:
[200a][, 200b][ 200d][ ; 200c][ (200f)]

:\\\
Comment 20 Jared Camins-Esakov 2012-07-15 13:49:22 UTC
(In reply to comment #19)
> mmm... the code says:
> +                $newline{authtype}     = defined ($thisauthtype) ?
> +                                            $thisauthtype->{'summary'} :
> +                                           
> GetAuthType($authtypecode)->{'summary'};
> 
> "summary" is not the authtype description, but how the authority display is
> built. I tried the patch anyway, and got a -suspected- wrong result: 
> 
> The "type of heading" column now show:
> [200a][, 200b][ 200d][ ; 200c][ (200f)]
> 
> :\\\

This definitely does not sound right. Follow-up coming shortly.
Comment 21 Jared Camins-Esakov 2012-07-15 13:54:06 UTC
Created attachment 10861 [details] [review]
Bug 6720 follow-up: use authtypetext instead of summary

Although auth_types.summary contains a string describing the type of
authority in MARC21, it does not in UNIMARC. auth_types.authtypetext
contains the proper label in both marcflavours.
Comment 22 Paul Poulain 2012-07-17 13:30:58 UTC
The 3 patches on branch new/bug_6720 can be cherry picked to 3.8.x.
Comment 23 Chris Cormack 2012-07-18 08:01:29 UTC
String changes, held for 3.8.4
Comment 24 Chris Cormack 2012-07-21 08:39:41 UTC
Patches do not apply on 3.8.x
Comment 25 Paul Poulain 2012-08-02 13:12:52 UTC
I think I've found a big problem with this patch.
Under certain circumstances (I think it's because zebra & mysql are unsynched, this can happen when you just deleted an authority, and it hasn't been deleted on zebra) I get:

Can't use an undefined value as a HASH reference at /home/paul/koha.dev/koha-community/C4/AuthoritiesMarc.pm line 355.
 at /home/paul/koha.dev/koha-community/authorities/authorities-home.pl line 203

git blame tells me the culprit is here. Investigating a little bit, I see that:
+   GetAuthType($authtypecode)->{'summary'};
will fail if GetAuthType() returns nothing
Changing this line to 
 ( GetAuthType($authtypecode) ? $_->{'authtypetext'}:'');

seems to fix the problem. Jared, could you check, confirm & provide a patch, i'll sign it immediately

Another problem, maybe not related = the authorities in the list are now displayed  HASH(0xb67ed88)
Comment 26 Jared Camins-Esakov 2012-08-02 13:19:21 UTC
Created attachment 11296 [details] [review]
Bug 6720 follow-up: fix error when auth has been deleted

Thanks to Paul Poulain for finding the problem and proposing a solution.
Comment 27 Jared Camins-Esakov 2012-08-02 13:20:31 UTC
Your suggested fix worked perfectly, as far as I could tell.

(In reply to comment #25)
> Another problem, maybe not related = the authorities in the list are now
> displayed  HASH(0xb67ed88)

See bug 8520 for a patch.
Comment 28 Paul Poulain 2012-08-02 13:40:48 UTC
follow-up signed-off, passed QA & pushed in one go
Comment 29 Chris Cormack 2012-08-02 22:34:57 UTC
The three patches here don't apply cleanly to 3.8.x

<<<<<<< HEAD
=======
                my $thisauthtype = GetAuthType(GetAuthTypeCode($authid));
                $newline{authtype}     = defined ($thisauthtype) ?
                                            $thisauthtype->{'authtypetext'} :
                                            (GetAuthType($authtypecode) ? $_->{'authtypetext'} : '');
>>>>>>> 983012d... Bug 6720 follow-up: fix error when auth has been deleted
Comment 30 Chris Cormack 2013-04-24 07:32:20 UTC
Was released in 3.10.0, patches attached don't apply to 3.8.x, closing