https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30904
--- Comment #3 from Janusz Kaczmarek <[email protected]> --- (In reply to Jonathan Druart from comment #2) > I am not expecting this patch to work correctly. > You are removing branchcode from ->find, in my understanding ->find is > expecting only 1 result to be returned and for that you have to pass either > the PK or the unique key. branchcode is part of the unique key > > UNIQUE KEY `additional_contents_uniq` > (`category`,`code`,`branchcode`,`lang`), Thank you, Jonathan, for your comment. IMVHO, from the logical point of view, if the change of the additional_contents.branchcode is possible by the design (exactly equal as it is in the case of Display location = additional_contents.location), additional_contents.branchcode should not be part of the unique key. For unambiguous identification of the row in question, category + code + lang is enough (in fact, probably even code + lang would be enough, keeping in mind how the data is generated). If I understand well, DBIx::Class::ResultSet->find would currently issue a warning in case when the find query based on supplied criteria produced more than one row. This will never happen since—I think, you would agree–category + code + lang would identify exactly one row (or no row) under each condition. Supplying four criteria to this find, i.e. category + code + lang + branchcode, will result with empty result in case of a change made to the library. This will make $additional_content undef (line 116) and so we jump to the line 147, with $code defined. So we create a new set of news rows, sharing the old code. This results in severe data inconsistency. Hence, the four columns as unique key should be considered superfluous and incorrect. To sum up—I would suggest considering the patch OK, but probably data definition introduced by you with the commit 8df3116760bdf1b889dc6f78e4605e231c4d7d6d requires correction as well. IMO, UNIQUE KEY `additional_contents_uniq` (`category`,`code`,`lang`) would be perfect. What do you think? -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
