It currently uses a global variable: our %default_values_for_mod_from_marc;
Created attachment 51126 [details] [review] Bug 13074: Use Koha::Cache to cache the defaults values of a MARC record With the global %default_values_for_mod_from_marc variable, the changes made to the marc_subfield_structure table and especially the links between MARC and DB fields are not safe and might be outdated (if a field is linked/unlinked) Test plan: Under Plack: - Link the barcode field, edit a record and set a barcode. - Remove the mapping for the barcode field and then update again the barcode of the record. The items.barcode DB field must not have been updated. Without this patch, the field should have been updated.
Created attachment 51138 [details] [review] Bug 13074: Use Koha::Cache to cache the defaults values of a MARC record With the global %default_values_for_mod_from_marc variable, the changes made to the marc_subfield_structure table and especially the links between MARC and DB fields are not safe and might be outdated (if a field is linked/unlinked) Test plan: Under Plack: - Link the barcode field, edit a record and set a barcode. - Remove the mapping for the barcode field and then update again the barcode of the record. The items.barcode DB field must not have been updated. Without this patch, the field should have been updated.
Hi, found a fail in t/db_dependent/Items.t, subtest 4 of test 10 not ok 4 - items.barcode is not mapped anymore, so the DB column has not been updated # Failed test 'items.barcode is not mapped anymore, so the DB column has not been updated' # at t/db_dependent/Items.t line 661. # got: undef # expected: 'a barcode' # Looks like you failed 1 test of 4. not ok 10 - C4::Items::_build_default_values_for_mod_marc Something in my data? Does not fail in master.
(In reply to Bernardo Gonzalez Kriegel from comment #3) > Hi, > found a fail in t/db_dependent/Items.t, subtest 4 of test 10 What I found is that I need more sleep, forget dependency again. Sorry
Created attachment 51329 [details] [review] Bug 13074: Use Koha::Cache to cache the defaults values of a MARC record With the global %default_values_for_mod_from_marc variable, the changes made to the marc_subfield_structure table and especially the links between MARC and DB fields are not safe and might be outdated (if a field is linked/unlinked) Test plan: Under Plack: - Link the barcode field, edit a record and set a barcode. - Remove the mapping for the barcode field and then update again the barcode of the record. The items.barcode DB field must not have been updated. Without this patch, the field should have been updated. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 51330 [details] [review] Bug 13074: (followup) remove useless diag On running all Items-related tests some useless diag showed up. Removing it. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Small conflict in Items.pm Also, did you mean to use $another_barcode here: 660 $item = GetItem($item_itemnumber); 661 is ( $item->{barcode}, $a_barcode, 'items.barcode is not mapped anymore, so the DB column has not been updated' );
Created attachment 52249 [details] [review] Bug 13074: Use Koha::Cache to cache the defaults values of a MARC record With the global %default_values_for_mod_from_marc variable, the changes made to the marc_subfield_structure table and especially the links between MARC and DB fields are not safe and might be outdated (if a field is linked/unlinked) Test plan: Under Plack: - Link the barcode field, edit a record and set a barcode. - Remove the mapping for the barcode field and then update again the barcode of the record. The items.barcode DB field must not have been updated. Without this patch, the field should have been updated. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 52250 [details] [review] Bug 13074: (followup) remove useless diag On running all Items-related tests some useless diag showed up. Removing it. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
(In reply to Nick Clemens from comment #7) > Small conflict in Items.pm > > Also, did you mean to use $another_barcode here: > 660 $item = GetItem($item_itemnumber); > 661 is ( $item->{barcode}, $a_barcode, 'items.barcode is not mapped > anymore, so the DB column has not been updated' ); No I don't think so. I don't remember exactly what I did, just remember that they were crazy tests to write...
(In reply to Jonathan Druart from comment #10) > (In reply to Nick Clemens from comment #7) > > Small conflict in Items.pm > > > > Also, did you mean to use $another_barcode here: > > 660 $item = GetItem($item_itemnumber); > > 661 is ( $item->{barcode}, $a_barcode, 'items.barcode is not mapped > > anymore, so the DB column has not been updated' ); > > No I don't think so. > I don't remember exactly what I did, just remember that they were crazy > tests to write... Ok so actually we do not want to update the DB column because the marc field is not mapped. Just what the test description says :)
Yes, entirely, just read that backwards in my head. Tests pass, but I have to reset plack to see the change happen when testing in client. Will test again next week to see if it is me (In reply to Jonathan Druart from comment #11) > Ok so actually we do not want to update the DB column because the marc field > is not mapped. Just what the test description says :)
(In reply to Nick Clemens from comment #12) > Tests pass, but I have to reset plack to see the change happen when testing > in client. Will test again next week to see if it is me Yes, confirmed. This should be fixed by bug 16431.
Created attachment 52254 [details] [review] Bug 13074: Use Koha::Cache to cache the defaults values of a MARC record With the global %default_values_for_mod_from_marc variable, the changes made to the marc_subfield_structure table and especially the links between MARC and DB fields are not safe and might be outdated (if a field is linked/unlinked) Test plan: Under Plack: - Link the barcode field, edit a record and set a barcode. - Remove the mapping for the barcode field and then update again the barcode of the record. The items.barcode DB field must not have been updated. Without this patch, the field should have been updated. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 52255 [details] [review] Bug 13074: (followup) remove useless diag On running all Items-related tests some useless diag showed up. Removing it. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Pushed for 16.11, thanks Jonathan, Tomas!
Pushed in 16.05. Will be in 16.05.01.
Patches pushed to 3.22.x, will be in 3.22.9