[Koha-bugs] [Bug 7817] Items Permanent location (shelving location) is set to NULL when item is edited

[bugzilla-daemon at bugs.koha-community.org]

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7817

M. de Rooy <m.de.rooy at rijksmuseum.nl> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Signed Off                  |In Discussion

--- Comment #23 from M. de Rooy <m.de.rooy at rijksmuseum.nl> ---
QA Comment:

I looked at the first signed patch and also the signed counterpatch.
I have a strong preference for the original patch. I think that the reason for
failing QA on that patch was not really valid. If failed because "If a library
decides to map permanent_location to a marc field, they will be unable to set
it to a null value." A library should never decide that.

Someone who maps permanent_location to a marc field, is doing something very
weird! This item field is really a Koha internal for e.g. the CART functions.
Nobody should use it for another reason (same for a few other item fields). We
could hide such fields from using them in admin/koha2marclinks.pl and
admin/marc_subfields_structure.pl (new report?)

I think that one of the reasons for this problem is the historically grown
'hybrid' approach of items in Koha. We offer a MARC structure to edit them, but
we do not save them in MARC but just in item fields. When saving an edited
item, we call ModItemFromMarc with its default_values_for_mod_from_marc hash.
Jonathan rebuilds this hash, but imo we should more focus on this subroutine.
Let the subroutine only map the 952 fields to item fields, instead of doing
what it does now (new marc record, transformMarcToKoha, get_unlinked_fields,
etc.) This is too complicated. The second patch does not  simplify the
situation (not saying that it is theoretically correct btw).

The first patch removes a non-marc item field from the
default_values_for_mod_from_marc hash. This is a correct action; it should not
be there. Other non-marc item fields should not be there either. Like paidfor
(see follow-up).

The counterpatch is a more drastic approach. It is not simple as mentioned. It
does a lot more so it will add at least some to performance. And since it
touches a lot more, it should be tested a lot more. I would favor pushing the
first patch and refactor ModItemFromMarc with the counterpatch as a base.

I will add a followup for the first patch and unhide the first patch, leaving
the counterpatch as-is and set this report to In Discussion for one or two
weeks, allowing all involved to give their opinion too.

-- 
You are receiving this mail because:
You are watching all bug changes.