Bug 7442 - Crash when selecting an authority with 200$x or 200$y
Summary: Crash when selecting an authority with 200$x or 200$y
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: master
Hardware: All All
: P3 minor (vote)
Assignee: Fridolin Somers
QA Contact:
URL:
Keywords:
Depends on: 4838
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-13 10:20 UTC by Adrien SAURAT
Modified: 2015-12-03 22:03 UTC (History)
8 users (show)

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


Attachments
screenshot (95.59 KB, image/png)
2012-01-13 10:20 UTC, Adrien SAURAT
Details
proposed patch (1.93 KB, patch)
2012-01-13 13:46 UTC, Adrien SAURAT
Details | Diff | Splinter Review
proposed patch #2 (2.38 KB, patch)
2012-05-04 08:57 UTC, Adrien SAURAT
Details | Diff | Splinter Review
Bug 7442 - Crash when selecting an authority with 200$x or 200$y (10.52 KB, patch)
2013-07-18 16:29 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 7442 - Follow up : Authorities search on all types does not show summary (2.30 KB, patch)
2013-07-18 16:30 UTC, Fridolin Somers
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7442 - Crash when selecting an authority with 200$x or 200$y (10.89 KB, patch)
2013-07-18 18:29 UTC, Frédéric Demians
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7442 - Follow up : Authorities search on all types does not show summary (2.45 KB, patch)
2013-07-18 18:30 UTC, Frédéric Demians
Details | Diff | Splinter Review
[PASSED QA] Bug 7442 - Crash when selecting an authority with 200$x or 200$y (11.11 KB, patch)
2013-11-26 07:47 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 7442 - Follow up : Authorities search on all types does not show summary (2.51 KB, patch)
2013-11-26 07:47 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 Adrien SAURAT 2012-01-13 10:20:12 UTC
Created attachment 7133 [details]
screenshot

From a biblio record, if one wants to add a 600$a information, a pop-up appears.

On this new window, on search terms typed and validated, a table result is displayed, with a column "Get It!" allowing the selection of an authority.

From here, different cases:
1) If we have a simple authority with 200$a and 200$b subfields, a link "choose" is displayed, working correctly.

2) If the authority has different occurences of 200$a/200$b, numeric links (1 2 and so on) are displayed, one for each occurence.
In the example of my screenshot, the line with a "Paul, Korky -- Pauline, Korkette" summary possesses two links : "1" will add "Paul, Korky" whereas "2" will add "Pauline, Korkette" (couldn't come up with a better name ;)).

3) If the authority has 200$x or 200$y subfields defined, several links are also created, when it should not be the case.
In our example, "Niclausse, Paul -- Expositions" will create a link "1" for "Niclausse, Paul" and a link "2" for "Expositions".
Clicking on the 2nd link leads to the following error:
Software error:
Can't call method "subfields" on an undefined value at /home/asaurat/workspace/versions/community/authorities/blinddetail-biblio-search.pl line 86.


Only the cases 1 and 2 should be handled. The creation of links for subfields like 200$x or 200$y should be removed.
Comment 1 Adrien SAURAT 2012-01-13 13:46:34 UTC Comment hidden (obsolete)
Comment 2 Adrien SAURAT 2012-01-13 14:51:22 UTC
[MT8560]

I tested with an UNIMARC db. Would it be also ok with MARC21 ? (the code about the biblio 600 field is not modified, only the authority 200$a and 200$b)
Comment 3 Chris Cormack 2012-03-03 07:41:45 UTC
I think you need to wrap this in a check for the marcflavour and only do this if it is UNIMARC
Comment 4 Jared Camins-Esakov 2012-03-17 10:26:05 UTC
(In reply to comment #3)
> I think you need to wrap this in a check for the marcflavour and only do
> this if it is UNIMARC

Agreed. I don't think there is an analogous problem in MARC21, or, if there is, I have never seen it.
Comment 5 jmbroust 2012-03-22 10:20:52 UTC
I did not succeed in testing, because searching an existing or just created authority brings no results.
tested on Biblibre sandbox1
Comment 6 Adrien SAURAT 2012-05-04 08:57:40 UTC Comment hidden (obsolete)
Comment 7 Katrin Fischer 2012-06-17 13:21:09 UTC
I tried testing this on sandbox2 - I had problems cataloguing a UNIMARC authority that would give me numbers to choose from in the auth finder. Can you perhaps attach some example records I could copy from?
Comment 8 Frédéric Demians 2012-07-09 15:58:27 UTC
Adrien, don't you think that this patch will break bug 4838?
Comment 9 Frédéric Demians 2012-07-09 16:02:55 UTC
SAUT UNIMARC authority has this summary:

[200a][, 200b][(200g)][ 200d][ ; 200c][ (200f)][ -- 200x][ -- 200z][ -- 200y]

Which drive the authority display. Have you tried modifying this
summary, and replace '--' with '-'? I think it would fix the bug, and
keep the repeated headings selection functionnality.
Comment 10 Fridolin Somers 2013-07-18 16:29:40 UTC Comment hidden (obsolete)
Comment 11 Fridolin Somers 2013-07-18 16:30:24 UTC Comment hidden (obsolete)
Comment 12 Frédéric Demians 2013-07-18 18:29:43 UTC Comment hidden (obsolete)
Comment 13 Frédéric Demians 2013-07-18 18:30:15 UTC Comment hidden (obsolete)
Comment 14 Katrin Fischer 2013-10-05 22:04:36 UTC
I had problems last time to test this, because I was not able to create authorities in my UNIMARC installation. Could you please provide some instructions?
Comment 15 Fridolin Somers 2013-10-07 14:36:29 UTC
(In reply to Katrin Fischer from comment #14)
> I had problems last time to test this, because I was not able to create
> authorities in my UNIMARC installation. Could you please provide some
> instructions?
Does it not work with a UNIMARC sandbox ?
Comment 16 Katrin Fischer 2013-10-07 14:41:53 UTC
Hi Fridolyn, I have my own UNIMARC installation with the data from the sandbox. My problem is I need to create a test case to see if this works correctly and my cataloguing skills in UNIMARC are limited.
Comment 17 Fridolin Somers 2013-10-10 14:08:11 UTC
(In reply to Katrin Fischer from comment #16)
Could you tell where you are stopped in test plan ?
Comment 18 Katrin Fischer 2013-10-10 14:11:09 UTC
Fridolyn, see comment 14 - I need instructions on how to correctly catalog a UNIMARC authority with the required fields to reproduce this problem.
Comment 19 Fridolin Somers 2013-10-11 10:46:06 UTC
(In reply to Katrin Fischer from comment #18)
> Fridolyn, see comment 14 - I need instructions on how to correctly catalog a
> UNIMARC authority with the required fields to reproduce this problem.
Don't you have existing authorities ?
Comment 20 Chris Cormack 2013-10-11 18:29:40 UTC
(In reply to Fridolyn SOMERS from comment #19)
> (In reply to Katrin Fischer from comment #18)
> > Fridolyn, see comment 14 - I need instructions on how to correctly catalog a
> > UNIMARC authority with the required fields to reproduce this problem.
> Don't you have existing authorities ?

They don't use UNIMARC in Germany, so no.
Comment 21 Fridolin Somers 2013-10-23 12:52:04 UTC
(In reply to Katrin Fischer from comment #18)
> Fridolyn, see comment 14 - I need instructions on how to correctly catalog a
> UNIMARC authority with the required fields to reproduce this problem.

> Test plan :
> - Edit an authority type summary : for example subject (heading on 250) : summary "[250a][ -- 250x]"
This authority type should exists as SNC (Sujet (nom commun), with : 
  Summary = [250a][ -- 250x][ -- 250y][ -- 250z] [(250b)]
  Auth field copied = 250

> - Create an authority A1 with one heading and a subdivision : for example a subject : 250$a "History" 250$x "20th century"
Create a new authority of type SNC : 
In existing field 250, set $a and $x
   250
      $a "History"
      $x "20th century"
Only those two subfields matter.

> - Create an authority A2 with several headings. for example a subject : 250$a "History" 250$a "Legends"
Create a new authority of type SNC : 
In existing field 250, set $a
   250
      $a "History"
Create a new field 250 and set $a
   250
      $a "Legends"

Does it help ?
Comment 22 Katrin Fischer 2013-11-26 07:47:45 UTC
Created attachment 23152 [details] [review]
[PASSED QA] Bug 7442 - Crash when selecting an authority with 200$x or 200$y

From a biblio record, if one wants to add a 600$a information, a pop-up
appears.  On this new window, on search terms typed and validated, a table
result is displayed, with a column "Get It!" allowing the selection of an
authority.  From here, different cases:

1) If we have a simple authority with 200$a and 200$b subfields, a link
   "choose" is displayed, working correctly.

2) If the authority has different occurences of 200$a/200$b, numeric links (1 2
   and so on) are displayed, one for each occurence.  In the example of my
   screenshot, the line with a "Paul, Korky -- Pauline, Korkette" summary
   possesses two links : "1" will add "Paul, Korky" whereas "2" will add
   "Pauline, Korkette" (couldn't come up with a better name ;)).

3) If the authority has 200$x or 200$y subfields defined, several links are
   also created, when it should not be the case.  In our example, "Niclausse,
   Paul -- Expositions" will create a link "1" for "Niclausse, Paul" and a link
   "2" for "Expositions".  Clicking on the 2nd link leads to the following
   error: Software error: Can't call method "subfields" on an undefined value
   at
   /home/asaurat/workspace/versions/community/authorities/blinddetail-biblio-search.pl
   line 86.  Only the cases 1 and 2 should be handled. The creation of links
   for subfields like 200$x or 200$y should be removed.

This problem is caused by the use of " -- " has separator of authorities with
several headings, but also in some heading between main part and subdivisions.
This patch corrects this by using an arry in authorities summary so that
presentation is computed in template. I've choosen to use the pipe separator
between authorities with several headings. This may be changed to be
configurable.

Test plan :

- Edit an authority type summary : for example subject (heading on 250) :
  summary "[250a][ -- 250x]"
- Create an authority A1 with one heading and a subdivision : for example a
  subject : 250$a "History" 250$x "20th century"
- Create an authority A2 with several headings. for example a subject : 250$a
  "History" 250$a "Legends"
- Rebuild Zebra queue
- Go to OPAC and click on "Authority search" and search on "History"
  => You will find A1 and A2 :
    History -- 20th century
    History | Legends
- Go to intranet autorities search and search on "History"
  => You will find A1 and A2 :
    History -- 20th century
    History | Legends
- Edit a record using this autorities type as thesaurus : for example on 606$a
- Click on thesaurus link and search on "History"
  => You will find A1 and A2 :
    History -- 20th century ; 0 times ; choose ; Edit authority
    History | Legends       ; 0 times ; 1 2    ; Edit authority
- Click on link "2" to chosse "Legends"
  => You get "Legends" in heading field : for example 606$a

Signed-off-by: Frédéric Demians <f.demians@tamil.fr>

I can confirm the problem and the solution. I have tested the patch on a large
DB with authorities having multiples headings. There is no regression on bug
4838.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes all tests and QA script.
Without the patch I couldn't choose between multiple headings
in the authority plugin, but with the patch it works as described.
Comment 23 Katrin Fischer 2013-11-26 07:47:50 UTC
Created attachment 23153 [details] [review]
[PASSED QA] Bug 7442 - Follow up : Authorities search on all types does not show summary

This patch corrects a small bug :

Authorities search on all types does not show summary because it is computed
with selected type (which is empty) instead of using found authority type.

Test plan :
- Go to intranet autorities search
- Perform a search on all authorities types
- Look at results
  => Without this patch, results rows do not display the autority summary, only
     authorized headings

Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 24 Katrin Fischer 2013-11-26 07:49:04 UTC
Hi Fridolin, thx for your hints - I think maybe using the SNC authority type was the missing piece.
Comment 25 Katrin Fischer 2013-11-26 07:50:41 UTC
Note: I couldn't reproduce the crash! But without the patches I can't pick between the 2 different headings, so I think this patch is still valid and needed.
Comment 26 Fridolin Somers 2013-11-27 11:57:20 UTC
(In reply to Katrin Fischer from comment #24)
> Hi Fridolin, thx for your hints - I think maybe using the SNC authority type
> was the missing piece.
Your welcome, its a team work.
Comment 27 Galen Charlton 2014-05-05 01:31:50 UTC
Pushed to master, along with a follow-up to apply the change to the Bootstrap theme and another follow-up to prevent a change in behavior for MARC21 authorities.  Thanks, Fridolin!
Comment 28 Fridolin Somers 2014-10-16 10:35:43 UTC
Pushed to 3.14.x, will be in 3.14.12