[Bug 16428] New: The framework is not checked to know if a field is mapped
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16428 Bug ID: 16428 Summary: The framework is not checked to know if a field is mapped Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: ASSIGNED Severity: normal Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: jonathan.druart@bugs.koha-community.org Reporter: jonathan.druart@bugs.koha-community.org QA Contact: testopia@bugs.koha-community.org Depends on: 12874 Blocks: 13074 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12874 [Bug 12874] On editing an item, some fields can be set to NULL https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13074 [Bug 13074] C4::Items::_build_default_values_for_mod_marc should use Koha::Cache -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16428 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16428 --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 51125 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51125&action=edit Bug 16428: Use the biblio framework to know if a field is mapped The subroutine _build_default_values_for_mod_marc takes the frameworkcode in parameter, but ModItemFromMarc did not pass it. It uses it to know if a field is mapped or not to a Koha field (C4::Koha::IsKohaFieldLinked). Consequently the default framework ("") was always used. This bug has been found working on bug 13074 and has been put on a separate bug report to ease the backport. Test plan: Without this change, the tests added by bug 16428 won't pass -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16428 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 51230 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51230&action=edit Bug 16428: (regression tests) This patch adds tests to t/db_dependent/Items.t to verify the kohafield mapping is not always taken from the default framework, but from the correct one. To test: - Apply the patch - Run: $ prove t/db_dependent/Items.t => FAIL: Mapping is not done on the right framework Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16428 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com Attachment #51230|0 |1 is obsolete| | --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Comment on attachment 51230 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51230 Bug 16428: (regression tests) The same tests are introduced on 13074 by Jonathan. I'm picking his tests. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16428 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16428 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #51125|0 |1 is obsolete| | --- Comment #4 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 51327 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51327&action=edit Bug 16428: Use the biblio framework to know if a field is mapped The subroutine _build_default_values_for_mod_marc takes the frameworkcode in parameter, but ModItemFromMarc did not pass it. It uses it to know if a field is mapped or not to a Koha field (C4::Koha::IsKohaFieldLinked). Consequently the default framework ("") was always used. This bug has been found working on bug 13074 and has been put on a separate bug report to ease the backport. Test plan: Without this change, the tests added by bug 16428 won't pass Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16428 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16428 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #51327|0 |1 is obsolete| | --- Comment #5 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 51430 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51430&action=edit Bug 16428: Use the biblio framework to know if a field is mapped The subroutine _build_default_values_for_mod_marc takes the frameworkcode in parameter, but ModItemFromMarc did not pass it. It uses it to know if a field is mapped or not to a Koha field (C4::Koha::IsKohaFieldLinked). Consequently the default framework ("") was always used. This bug has been found working on bug 13074 and has been put on a separate bug report to ease the backport. Test plan: Without this change, the tests added by bug 16428 won't pass Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16428 --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 51431 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51431&action=edit Bug 16428: [QA Follow-up] Useless call to GetMarcStructure in _build_default_values_for_mod_marc The routine calls GetMarcStructure and does not use its return value after all. Test plan: Run t/db_dependent/Items.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16428 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16428 Jesse Weaver <jweaver@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |In Discussion CC| |jweaver@bywatersolutions.co | |m --- Comment #7 from Jesse Weaver <jweaver@bywatersolutions.com> --- I have some qualms about pushing this... there are a very significant number of places in Koha that use the default framework's mappings (enough to make it a de-facto standard). See the results of a $ git grep TransformMarcToKoha -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16428 --- Comment #8 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Jesse Weaver from comment #7)
I have some qualms about pushing this... there are a very significant number of places in Koha that use the default framework's mappings (enough to make it a de-facto standard). See the results of a $ git grep TransformMarcToKoha
It seems that we are using it when adding/modifying a record/item. git grep TransformMarcToKoha|grep -v frameworkcode does not return a lot of relevant results :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16428 Jacek Ablewicz <abl@biblos.pk.edu.pl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |abl@biblos.pk.edu.pl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16428 --- Comment #9 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Just adding to the discussion, this is a specific context. We are looking at items. And only looking at default values to use when an item field would be empty. And maybe adding somewhat more weight, the default framework by standard does not even contain any other default value than NULL.. Since this patch theoretically does the right thing and its change of behavior may probably be quite limited (only impacting local defaults for items), I still would support to push it to master. We c/should add a dbrev with a warning about this change. Or even copy (in this dbrev) all item defaults from the current default framework to other frameworks (not overwriting not-null values), more or less ensuring unchanged behavior? Note that not-null items defaults currently are not used in this context, but it seems better not to overwrite them without consent. What do you think? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16428 --- Comment #10 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Sent back to the RM queue -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16428 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16428 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com Status|Passed QA |Pushed to Master --- Comment #11 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to master for 16.11, thanks Jonathan, Marcel! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16428 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |frederic@tamil.fr --- Comment #12 from Frédéric Demians <frederic@tamil.fr> --- Pushed in 16.05. Will be in 16.05.01. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16428 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |julian.maurice@biblibre.com Status|Pushed to Master |Pushed to Stable --- Comment #13 from Julian Maurice <julian.maurice@biblibre.com> --- Patches pushed to 3.22.x, will be in 3.22.8 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org