Bug 19693 - Update of an authority record creates inconsistency when the heading tag is changed
Summary: Update of an authority record creates inconsistency when the heading tag is c...
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: MARC Authority data support (show other bugs)
Version: unspecified
Hardware: All All
: P3 normal (vote)
Assignee: Janusz Kaczmarek
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-11-25 00:40 UTC by Janusz Kaczmarek
Modified: 2023-12-28 20:44 UTC (History)
6 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:
22.11.00, 22.05.08, 21.11.15


Attachments
Bug 19693: authtypefrom incorrectly set when updating auth from file (2.78 KB, patch)
2017-11-25 00:42 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Bug 19693: authtypefrom incorrectly set when updating auth from file (2.77 KB, patch)
2017-11-26 17:21 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Bug 19693: authtypefrom incorrectly set when updating auth from file (2.80 KB, patch)
2017-11-26 17:32 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Bug 19693: update of an authority record creates inconsistency when the heading tag is changed (3.51 KB, patch)
2022-07-26 09:31 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Bug 19693: Add test sub to Merge.t (7.05 KB, patch)
2022-07-26 09:32 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Bug 19693: Add test sub to Merge.t (7.10 KB, patch)
2022-09-23 13:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19693: Update of an authority record creates inconsistency when the heading tag is changed (3.57 KB, patch)
2022-09-23 13:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19693: Add test sub to Merge.t (7.20 KB, patch)
2022-10-07 09:42 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19693: Update of an authority record creates inconsistency when the heading tag is changed (3.67 KB, patch)
2022-10-07 09:42 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19693: (QA follow-up) Replace changed tag test (2.71 KB, patch)
2022-10-07 09:42 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Janusz Kaczmarek 2017-11-25 00:40:32 UTC
In those cases when authorities are updated by an external agency (or
even internally, by reviewing and correcting an exported authority file)
when the heading tag will be changed (seems odd but happens:

111 Congress ==> 110 Corporate body.Congress ;
100 Person ==> 110 Corporate body (a company named with person's name ;
151 City--object ==> 150 Object (city) etc.)

and then the authority record in Koha database will be updated with
bulkmarcimport or by calling directly ModAuthority from a custom script,
the merge function "doesn't know" that the change to the authority type 
has been made and, consequently, doesn't adequately change the tag in related fields in biblio records (as it would if two different records 
with different authtypecode were merged with Koha interface).  

This is because at the moment when merge function is being called 
by ModAuthority
Koha::Authority::Types->find($autfrom->authtypecode)
Koha::Authority::Types->find($authto->authtypecode)
both have the same value (because $autfrom == $autfrom).

Therefore the authtype guessed from the old version of the record should
be considered.

To test:
1) have an authority record used in biblio;
   export it to file;
   change 1XX heading tag to different (but reasonable) value
   and possibly change also the content of the heading
   (one can delete also 942 but it doesn't matter);
   make bulkmarcimport.pl -a -update -file <modified_auth_file> and
   see that the tag in biblio record has not been changed (whereas
   the type of authority record did change);

2) make orders in database (so that the authority type and the tag of
the field in biblio record correspond); apply the patch;

3) repeat the test from 1).
Comment 1 Janusz Kaczmarek 2017-11-25 00:42:57 UTC
Created attachment 69344 [details] [review]
Bug 19693: authtypefrom incorrectly set when updating auth  from file
Comment 2 Janusz Kaczmarek 2017-11-26 17:21:41 UTC
Created attachment 69349 [details] [review]
Bug 19693: authtypefrom incorrectly set when updating auth from file

A corrected version.  It is more safe to use the GuessAuthTypeCode function only if really needed, i.e. when merging to itself (especially when using some non standard authority types that for some reason are not covered in GuessAuthTypeCode).
Comment 3 Janusz Kaczmarek 2017-11-26 17:32:26 UTC
Created attachment 69350 [details] [review]
Bug 19693: authtypefrom incorrectly set when updating auth  from file
Comment 4 Marcel de Rooy 2018-02-28 13:09:26 UTC
Not really trivial
Comment 5 Mark Tompsett 2018-05-01 02:46:07 UTC
t/db_dependent/Authority/Merge.t triggers this code change fully. However, there is no new test that fails before this patch, and succeeds after. Marking Failed QA.
Comment 6 Janusz Kaczmarek 2022-07-26 09:31:37 UTC
Created attachment 138121 [details] [review]
Bug 19693: update of an authority record creates inconsistency when the heading tag is changed

Since after a few years the problem is still unresolved here comes a new attempt to address it:

In those rare cases when authorities are updated by an external agency (or
even internally, by reviewing and correcting an exported authority file)
when the heading tag will be changed (seems odd but happens:

111 Congress ==> 110 Corporate body.Congress ;
100 Person ==> 110 Corporate body (a company named with person's name ;
151 City--object ==> 150 Object (city) etc.)

and then the authority record in Koha database will be updated with
bulkmarcimport or by calling directly ModAuthority from a custom script,
the merge function "doesn't know" that the change to the authority type
has been made and, consequently, doesn't adequately change the tag in
related fields in biblio records (as it would if two different records
with different authtypecode were merged with Koha interface).

This is because at the moment when merge function is being called
by ModAuthority:
Koha::Authority::Types->find($autfrom->authtypecode)
Koha::Authority::Types->find($authto->authtypecode)
both have the same value (because $mergefrom == $mergeto).

Therefore in case when $mergefrom == $mergeto and the heading tag changes,
$authtypefrom and $authfrom calculated in merge in the ordinar way are
misleading and should not be taken unto consideration.

Test plan:
==========

1. run t/db_dependent/Authority/Merge.t
2. you should see problems in "Test update A with modified heading tag
   (changing authtype)"
3. apply the patch
4. run t/db_dependent/Authority/Merge.t again
5. the test should pass

Alternatively:

1. have an authority record used in biblio;
   export it to file;
   change 1XX heading tag to a different (but reasonable) value
   and possibly change also the content of the heading
   (one can delete also 942 but it doesn't matter);
   make bulkmarcimport.pl -a -update -file <modified_auth_file> and
   see that the tag in biblio record has not been changed (whereas
   the type of authority record did change);

2. make orders in database (so that the authority type and the tag of
   the field in biblio record correspond); apply the patch;

3. repeat the test from 1.
Comment 7 Janusz Kaczmarek 2022-07-26 09:32:30 UTC
Created attachment 138122 [details] [review]
Bug 19693: Add test sub to Merge.t
Comment 8 Martin Renvoize 2022-09-23 13:57:03 UTC
Created attachment 140915 [details] [review]
Bug 19693: Add test sub to Merge.t

In those rare cases when authorities are updated by an external agency (or
even internally, by reviewing and correcting an exported authority file)
when the heading tag will be changed (seems odd but happens:

111 Congress ==> 110 Corporate body.Congress ;
100 Person ==> 110 Corporate body (a company named with person's name ;
151 City--object ==> 150 Object (city) etc.)

and then the authority record in Koha database will be updated with
bulkmarcimport or by calling directly ModAuthority from a custom script,
the merge function "doesn't know" that the change to the authority type
has been made and, consequently, doesn't adequately change the tag in
related fields in biblio records (as it would if two different records
with different authtypecode were merged with Koha interface).

This is because at the moment when merge function is being called
by ModAuthority:
Koha::Authority::Types->find($autfrom->authtypecode)
Koha::Authority::Types->find($authto->authtypecode)
both have the same value (because $mergefrom == $mergeto).

Therefore in case when $mergefrom == $mergeto and the heading tag changes,
$authtypefrom and $authfrom calculated in merge in the ordinar way are
misleading and should not be taken unto consideration.

Test plan:
==========

1. run t/db_dependent/Authority/Merge.t
2. you should see problems in "Test update A with modified heading tag
   (changing authtype)"
3. apply the patch
4. run t/db_dependent/Authority/Merge.t again
5. the test should pass

Alternatively:

1. have an authority record used in biblio;
   export it to file;
   change 1XX heading tag to a different (but reasonable) value
   and possibly change also the content of the heading
   (one can delete also 942 but it doesn't matter);
   make bulkmarcimport.pl -a -update -file <modified_auth_file> and
   see that the tag in biblio record has not been changed (whereas
   the type of authority record did change);

2. make orders in database (so that the authority type and the tag of
   the field in biblio record correspond); apply the patch;

3. repeat the test from 1.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Martin Renvoize 2022-09-23 13:57:07 UTC
Created attachment 140916 [details] [review]
Bug 19693: Update of an authority record creates inconsistency when the heading tag is changed

In those rare cases when authorities are updated by an external agency (or
even internally, by reviewing and correcting an exported authority file)
when the heading tag will be changed (seems odd but happens:

111 Congress ==> 110 Corporate body.Congress ;
100 Person ==> 110 Corporate body (a company named with person's name ;
151 City--object ==> 150 Object (city) etc.)

and then the authority record in Koha database will be updated with
bulkmarcimport or by calling directly ModAuthority from a custom script,
the merge function "doesn't know" that the change to the authority type
has been made and, consequently, doesn't adequately change the tag in
related fields in biblio records (as it would if two different records
with different authtypecode were merged with Koha interface).

This is because at the moment when merge function is being called
by ModAuthority:
Koha::Authority::Types->find($autfrom->authtypecode)
Koha::Authority::Types->find($authto->authtypecode)
both have the same value (because $mergefrom == $mergeto).

Therefore in case when $mergefrom == $mergeto and the heading tag changes,
$authtypefrom and $authfrom calculated in merge in the ordinar way are
misleading and should not be taken unto consideration.

Test plan:
==========

1. run t/db_dependent/Authority/Merge.t
2. you should see problems in "Test update A with modified heading tag
   (changing authtype)"
3. apply the patch
4. run t/db_dependent/Authority/Merge.t again
5. the test should pass

Alternatively:

1. have an authority record used in biblio;
   export it to file;
   change 1XX heading tag to a different (but reasonable) value
   and possibly change also the content of the heading
   (one can delete also 942 but it doesn't matter);
   make bulkmarcimport.pl -a -update -file <modified_auth_file> and
   see that the tag in biblio record has not been changed (whereas
   the type of authority record did change);

2. make orders in database (so that the authority type and the tag of
   the field in biblio record correspond); apply the patch;

3. repeat the test from 1.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 10 Martin Renvoize 2022-09-23 13:57:40 UTC
Works as described and the code makes sense.. also nice looking unit tests as a quick glance.

Signing off.
Comment 11 Marcel de Rooy 2022-10-07 06:32:56 UTC
QAing
Comment 12 Marcel de Rooy 2022-10-07 07:21:23 UTC
Great catch, Janusz.
I know the code of merge very well, although time has passed :)
Have the feeling that we are cheating a bit here, but I tend to agree with your solution direction.
Tempted to polish it a bit further..
Comment 13 Marcel de Rooy 2022-10-07 09:42:07 UTC
Created attachment 141480 [details] [review]
Bug 19693: Add test sub to Merge.t

In those rare cases when authorities are updated by an external agency (or
even internally, by reviewing and correcting an exported authority file)
when the heading tag will be changed (seems odd but happens:

111 Congress ==> 110 Corporate body.Congress ;
100 Person ==> 110 Corporate body (a company named with person's name ;
151 City--object ==> 150 Object (city) etc.)

and then the authority record in Koha database will be updated with
bulkmarcimport or by calling directly ModAuthority from a custom script,
the merge function "doesn't know" that the change to the authority type
has been made and, consequently, doesn't adequately change the tag in
related fields in biblio records (as it would if two different records
with different authtypecode were merged with Koha interface).

This is because at the moment when merge function is being called
by ModAuthority:
Koha::Authority::Types->find($autfrom->authtypecode)
Koha::Authority::Types->find($authto->authtypecode)
both have the same value (because $mergefrom == $mergeto).

Therefore in case when $mergefrom == $mergeto and the heading tag changes,
$authtypefrom and $authfrom calculated in merge in the ordinar way are
misleading and should not be taken unto consideration.

Test plan:
==========

1. run t/db_dependent/Authority/Merge.t
2. you should see problems in "Test update A with modified heading tag
   (changing authtype)"
3. apply the patch
4. run t/db_dependent/Authority/Merge.t again
5. the test should pass

Alternatively:

1. have an authority record used in biblio;
   export it to file;
   change 1XX heading tag to a different (but reasonable) value
   and possibly change also the content of the heading
   (one can delete also 942 but it doesn't matter);
   make bulkmarcimport.pl -a -update -file <modified_auth_file> and
   see that the tag in biblio record has not been changed (whereas
   the type of authority record did change);

2. make orders in database (so that the authority type and the tag of
   the field in biblio record correspond); apply the patch;

3. repeat the test from 1.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 14 Marcel de Rooy 2022-10-07 09:42:11 UTC
Created attachment 141481 [details] [review]
Bug 19693: Update of an authority record creates inconsistency when the heading tag is changed

In those rare cases when authorities are updated by an external agency (or
even internally, by reviewing and correcting an exported authority file)
when the heading tag will be changed (seems odd but happens:

111 Congress ==> 110 Corporate body.Congress ;
100 Person ==> 110 Corporate body (a company named with person's name ;
151 City--object ==> 150 Object (city) etc.)

and then the authority record in Koha database will be updated with
bulkmarcimport or by calling directly ModAuthority from a custom script,
the merge function "doesn't know" that the change to the authority type
has been made and, consequently, doesn't adequately change the tag in
related fields in biblio records (as it would if two different records
with different authtypecode were merged with Koha interface).

This is because at the moment when merge function is being called
by ModAuthority:
Koha::Authority::Types->find($autfrom->authtypecode)
Koha::Authority::Types->find($authto->authtypecode)
both have the same value (because $mergefrom == $mergeto).

Therefore in case when $mergefrom == $mergeto and the heading tag changes,
$authtypefrom and $authfrom calculated in merge in the ordinar way are
misleading and should not be taken unto consideration.

Test plan:
==========

1. run t/db_dependent/Authority/Merge.t
2. you should see problems in "Test update A with modified heading tag
   (changing authtype)"
3. apply the patch
4. run t/db_dependent/Authority/Merge.t again
5. the test should pass

Alternatively:

1. have an authority record used in biblio;
   export it to file;
   change 1XX heading tag to a different (but reasonable) value
   and possibly change also the content of the heading
   (one can delete also 942 but it doesn't matter);
   make bulkmarcimport.pl -a -update -file <modified_auth_file> and
   see that the tag in biblio record has not been changed (whereas
   the type of authority record did change);

2. make orders in database (so that the authority type and the tag of
   the field in biblio record correspond); apply the patch;

3. repeat the test from 1.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 15 Marcel de Rooy 2022-10-07 09:42:15 UTC
Created attachment 141482 [details] [review]
Bug 19693: (QA follow-up) Replace changed tag test

The test
  $MARCfrom->field('1..', '2..'))[0]->tag ne ($MARCto->field('1..', '2..'))[0]->tag
is not completely consistent with following code in sub merge.
I decided to get the authtype code from the old record that comes
from Koha and should include the type.
Remaining changes refer to indentation/comments.

Test plan:
Run t/db_dependent/Authority/Merge.t
Note: The test actually could be extended a bit for mocking MARC
flavor, but needs some additional framework support to work.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 16 Tomás Cohen Arazi 2022-10-24 20:23:12 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 17 Lucas Gass 2022-12-05 23:00:28 UTC
Backported to 22.05.x for upcoming 22.05.08
Comment 18 Arthur Suzuki 2022-12-16 15:54:39 UTC
applied to 21.11 for 21.11.15
Comment 19 wainuiwitikapark 2023-03-15 01:05:54 UTC
Not backported to 21.05.x