Bug 33342 - Fix framework handling in manage-marc-import.pl
Summary: Fix framework handling in manage-marc-import.pl
Status: RESOLVED DUPLICATE of bug 33412
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: Main
Hardware: All All
: P5 - low major (vote)
Assignee: David Gustafsson
QA Contact: Testopia
URL:
Keywords:
: 33692 (view as bug list)
Depends on:
Blocks:
 
Reported: 2023-03-27 13:03 UTC by David Gustafsson
Modified: 2023-05-05 20:36 UTC (History)
7 users (show)

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


Attachments
Bug 33342: Assigment in if clause sets invalid marc framework (1.10 KB, patch)
2023-03-27 13:11 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 33342: Fix framework handling in manage-marc-import.pl (1.20 KB, patch)
2023-03-28 13:25 UTC, David Gustafsson
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Gustafsson 2023-03-27 13:03:19 UTC
In the last patch in bug 15869 a bug is introduced when assignment (=) is mistakenly used instead of "eq", resulting in an invalid marc framework value being stored for replaced records, which in turn makes them non editable in the staff interface. Considering this has been backported to to 22.11.03 and could affect current users would be good to get this fixed rather quickly.
Comment 1 David Gustafsson 2023-03-27 13:11:52 UTC
Created attachment 148766 [details] [review]
Bug 33342: Assigment in if clause sets invalid marc framework

In a follow up in bug 15869 an if clause is added where "=" is
used instead of "eq" in effect setting an invalid marc famework
for the record. Fix this by using the correct operator.
Comment 2 Andreas Jonsson 2023-03-28 07:48:28 UTC
Obviously, the intention must be comparison; not assignment.

But I don't see how this can introduce an invalid framework code.

When I test the parameter "overlay_framework" is simply ignored.  The expression $overlay_framework = '_USE_ORIG' should evaluate to TRUE, so the body of the if-statement, $overlay_framework = undef, should always be executed.

Also, in manage-marc-import.tt the option name seems to have an underscore at the end as well:

<option value="_USE_ORIG_">Keep original framework</option>

So, only replacing assignment with comparison operator actually introduces the incorrect framework codes.  We also need to fix the code value.
Comment 3 Stefan Berndtsson 2023-03-28 08:29:28 UTC
Yes, of course. Thanks for that. That explains things (from the other perspective).

I think our experience (having received "_USE"-frameworkcodes in the database) comes from us fixing the comparison, but not the dropdown, so now when the comparison works properly, it gets the wrong value, as you say.

This patch should be updated before going forward. Setting it to "In Discussion" until it is properly fixed.
Comment 4 David Gustafsson 2023-03-28 13:25:31 UTC
Created attachment 148822 [details] [review]
Bug 33342: Fix framework handling in manage-marc-import.pl

In a follow up in bug 15869 an if clause is added where "=" is
used instead of "eq" resulting in framwork always being set to undef.
Fix this by using the correct operator. Also fix the name of
the framework placeholder in manage-marc-import.pl
which differed from the template.
Comment 5 David Gustafsson 2023-03-28 13:31:21 UTC
@Andreas Jonsson, thanks for spotting this! Have now fixed the patch and changed the description as the previous one did not correctly describe the current behavior. Since the assignment evaluates to true in a boolean context $overlay_framework as far as I can tell is always set to undef (which should now be fixed).
Comment 6 David Nind 2023-04-01 20:26:05 UTC
If you could add a test plan, I would be more than happy to test.
Comment 7 Phil Ringnalda 2023-05-05 20:27:36 UTC
*** Bug 33692 has been marked as a duplicate of this bug. ***
Comment 8 Phil Ringnalda 2023-05-05 20:36:49 UTC

*** This bug has been marked as a duplicate of bug 33412 ***