Bug 17510 - MARC modification templates ignore subfield $0
Summary: MARC modification templates ignore subfield $0
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Andreas Roussos
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 26894
  Show dependency treegraph
 
Reported: 2016-10-27 12:24 UTC by Hugo Agud
Modified: 2021-06-14 21:31 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:
20.05.00
Circulation function:


Attachments
Bug 17510: enable use of subfield 0 in MMT actions (5.16 KB, patch)
2020-02-14 20:22 UTC, Andreas Roussos
Details | Diff | Splinter Review
Bug 17510: update MMT-related unit tests (8.09 KB, patch)
2020-02-14 20:22 UTC, Andreas Roussos
Details | Diff | Splinter Review
Bug 17510: update MMT-related unit tests (8.04 KB, patch)
2020-02-14 20:32 UTC, Andreas Roussos
Details | Diff | Splinter Review
Bug 17510: enable use of subfield 0 in MMT actions (5.11 KB, patch)
2020-02-14 20:51 UTC, Andreas Roussos
Details | Diff | Splinter Review
Bug 17510: update MMT-related unit tests (8.04 KB, patch)
2020-02-14 20:51 UTC, Andreas Roussos
Details | Diff | Splinter Review
Bug 17510: update MMT-related unit tests (8.16 KB, patch)
2020-02-14 21:57 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 17510: update MMT-related unit tests (8.16 KB, patch)
2020-02-14 21:58 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 17510: enable use of subfield 0 in MMT actions (5.22 KB, patch)
2020-02-14 21:59 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 17510: update MMT-related unit tests (8.15 KB, patch)
2020-02-21 08:29 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 17510: enable use of subfield 0 in MMT actions (5.21 KB, patch)
2020-02-21 08:30 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Hugo Agud 2016-10-27 12:24:20 UTC
We have realized that Koha MArc modification templates ignore subfield 0, it is not very common used subfield, it is working with subfields greater than 0

We have create a template for adding 773$0 (host biblionumber in analytics) but the template doesn't get the subfield and only stores main tag 773.

Checked in:


Koha 3.22 & 16.05.05 Debian 8 and plack enabled

TEST
Go to Tools --> MARC modification templates 

Create one rule.
Add/Update field 773 0 with value 'demo'
save it

you can check that subfield is not appearing in the descrption, if you edit it appears, but when you try to execute thah template, and try to previwe the marc you get internal error and get the following log.

This action is not implemented yet at /usr/share/koha/lib/Koha/SimpleMARC.pm line 169.
Comment 1 Marjorie Barry-Vila 2017-04-18 14:34:27 UTC
Still valid in 16.11.

Marjorie
Comment 2 Donna 2018-10-05 13:27:51 UTC
This is still an issue in 17.11.  Library is trying to remove subject headings using marc modification templates, specifically in the 650$0.
Comment 3 Katrin Fischer 2020-01-11 15:57:34 UTC
Haven't confirmed in newer version yet.
Comment 4 Sarah Cornell 2020-01-13 18:54:22 UTC
Confirmed still an issue in 19.05.
Comment 5 Katrin Fischer 2020-01-13 19:14:10 UTC
(In reply to Sarah Cornell from comment #4)
> Confirmed still an issue in 19.05.

Thx, Sarah!
Comment 6 Andreas Roussos 2020-02-01 13:25:09 UTC
As Hugo pointed out, this bug has two aspects.

1) Firstly, there's the issue of the subfield not appearing in the MARC Modification Template (MMT) action description when its value is 0 (even though it will appear when editing the action). These are the relevant lines of code from marc_modification_templates.tt:

136 [% ActionsLoo.from_field | html %][% IF ( ActionsLoo.from_subfield ) %]$[% ActionsLoo.from_subfield | html %][% END %] 

143 to [% ActionsLoo.to_field | html %][% IF ( ActionsLoo.to_subfield ) %]$[% ActionsLoo.to_subfield | html %][% END %]

154 [% ActionsLoo.conditional_field | html %][% IF ( ActionsLoo.conditional_subfield ) %]$[% ActionsLoo.conditional_subfield | html %][% END %]

The problem lies with the "[% IF ( ... ) %]" statements: when the from/to/conditional subfield value is set to 0 the IF clause will evaluate to false and therefore the value will not be printed.

2) Secondly, we have an error message coming up in the Plack log file ("This action is not implemented yet"), and this relates to commit c7d862b (from Bug 11319). In that commit, among other things, the update_field() subroutine was added to SimpleMARC.pm, the most relevant lines of which are:

167 if ( not $subfieldName or $subfieldName eq '' ) {
168     # FIXME I'm not sure the actual implementation is correct.
169     die "This action is not implemented yet";
170     #_update_field({ record => $record, field => $fieldName, values => \@values });
171 } else {
172     _update_subfield({ record => $record, field => $fieldName, subfield => $subfieldName, values => \@values, field_numbers => $field_numbers });
173 }

The first condition in the 'if' clause above results in the expression evaluating to true when a subfield value of 0 (integer zero) or '0' (string zero) is entered -- hence the error message.
Comment 7 Andreas Roussos 2020-02-14 20:22:06 UTC Comment hidden (obsolete)
Comment 8 Andreas Roussos 2020-02-14 20:22:09 UTC Comment hidden (obsolete)
Comment 9 Andreas Roussos 2020-02-14 20:32:07 UTC Comment hidden (obsolete)
Comment 10 Andreas Roussos 2020-02-14 20:48:36 UTC
I've just realised that the URL for the git-bz test bug here on Bugzilla
(Bug 6473) got included at the end of my previous commits. I tried to
attach my patches to the test bug before attaching them here (just to be
on the safe side), but must have messed up somewhere. I will re-attach
them now... Sorry for any confusion.
Comment 11 Andreas Roussos 2020-02-14 20:51:38 UTC
Created attachment 99034 [details] [review]
Bug 17510: enable use of subfield 0 in MMT actions

When adding MARC modification template (MMT) actions, if a subfield
value of 0 (zero) is entered it will not show up in the action's
description upon saving. Also, if you try to modify an authority or
biblio record using actions that refer to subfield 0 the procedure
will fail.

This patch fixes that.

Test plan:
0) Create a MARC modification template and add some actions to it,
   ideally testing all action types (Delete/Add new/Update existing
   or add new/Move/Copy/Copy and replace); make sure you input a 0
   (integer zero) in the text box for the subfield value(s).
1) As you save each action, observe that subfield 0 ($0) is missing
   from the action description.
2) Home > Tools > Batch record modification: try to modify a biblio-
   graphic record using the modification template you just created.
   Observe that you get an error in the system logs if you click on
   'Show MARC' or 'Modify selected records'.
3) Apply the patch.
4) Repeat steps 0-2. The subfield value ($0) should be displayed in
   the action description and the Batch record modification should
   work without problems.
Comment 12 Andreas Roussos 2020-02-14 20:51:41 UTC
Created attachment 99035 [details] [review]
Bug 17510: update MMT-related unit tests

The unit tests related to MARC modification templates need to be
updated with extra tests specific to actions involving subfield 0.

Test plan:
0) Apply the patch.
1) Run the updated unit tests, they should pass without any errors:
   $ prove t/SimpleMARC.t
   $ prove t/db_dependent/MarcModificationTemplates.t
Comment 13 Lisette Scheer 2020-02-14 21:57:44 UTC
Created attachment 99050 [details] [review]
Bug 17510: update MMT-related unit tests

The unit tests related to MARC modification templates need to be
updated with extra tests specific to actions involving subfield 0.

Test plan:
0) Apply the patch.
1) Run the updated unit tests, they should pass without any errors:
   $ prove t/SimpleMARC.t
   $ prove t/db_dependent/MarcModificationTemplates.t


Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org>
Comment 14 Lisette Scheer 2020-02-14 21:58:22 UTC
Created attachment 99051 [details] [review]
Bug 17510: update MMT-related unit tests

The unit tests related to MARC modification templates need to be
updated with extra tests specific to actions involving subfield 0.

Test plan:
0) Apply the patch.
1) Run the updated unit tests, they should pass without any errors:
   $ prove t/SimpleMARC.t
   $ prove t/db_dependent/MarcModificationTemplates.t

Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org>

Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org>
Comment 15 Lisette Scheer 2020-02-14 21:59:01 UTC
Created attachment 99052 [details] [review]
Bug 17510: enable use of subfield 0 in MMT actions

When adding MARC modification template (MMT) actions, if a subfield
value of 0 (zero) is entered it will not show up in the action's
description upon saving. Also, if you try to modify an authority or
biblio record using actions that refer to subfield 0 the procedure
will fail.

This patch fixes that.

Test plan:
0) Create a MARC modification template and add some actions to it,
   ideally testing all action types (Delete/Add new/Update existing
   or add new/Move/Copy/Copy and replace); make sure you input a 0
   (integer zero) in the text box for the subfield value(s).
1) As you save each action, observe that subfield 0 ($0) is missing
   from the action description.
2) Home > Tools > Batch record modification: try to modify a biblio-
   graphic record using the modification template you just created.
   Observe that you get an error in the system logs if you click on
   'Show MARC' or 'Modify selected records'.
3) Apply the patch.
4) Repeat steps 0-2. The subfield value ($0) should be displayed in
   the action description and the Batch record modification should
   work without problems.


Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org>
Comment 16 Martin Renvoize (ashimema) 2020-02-21 08:29:59 UTC
Created attachment 99356 [details] [review]
Bug 17510: update MMT-related unit tests

The unit tests related to MARC modification templates need to be
updated with extra tests specific to actions involving subfield 0.

Test plan:
0) Apply the patch.
1) Run the updated unit tests, they should pass without any errors:
   $ prove t/SimpleMARC.t
   $ prove t/db_dependent/MarcModificationTemplates.t

Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 17 Martin Renvoize (ashimema) 2020-02-21 08:30:03 UTC
Created attachment 99357 [details] [review]
Bug 17510: enable use of subfield 0 in MMT actions

When adding MARC modification template (MMT) actions, if a subfield
value of 0 (zero) is entered it will not show up in the action's
description upon saving. Also, if you try to modify an authority or
biblio record using actions that refer to subfield 0 the procedure
will fail.

This patch fixes that.

Test plan:
0) Create a MARC modification template and add some actions to it,
   ideally testing all action types (Delete/Add new/Update existing
   or add new/Move/Copy/Copy and replace); make sure you input a 0
   (integer zero) in the text box for the subfield value(s).
1) As you save each action, observe that subfield 0 ($0) is missing
   from the action description.
2) Home > Tools > Batch record modification: try to modify a biblio-
   graphic record using the modification template you just created.
   Observe that you get an error in the system logs if you click on
   'Show MARC' or 'Modify selected records'.
3) Apply the patch.
4) Repeat steps 0-2. The subfield value ($0) should be displayed in
   the action description and the Batch record modification should
   work without problems.

Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 18 Martin Renvoize (ashimema) 2020-02-21 08:30:24 UTC
Obvious fix, now well covered by tests.. Thanks

Passing QA
Comment 19 Martin Renvoize (ashimema) 2020-02-21 16:08:21 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 20 Joy Nelson 2020-03-13 00:53:48 UTC
enhancement not backported to 19.11.x