[Koha-bugs] [Bug 30904] (bug 24387 follow-up) Modifying library in news (additional contents) causes inconsistencies

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

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30904

--- Comment #3 from Janusz Kaczmarek <januszop at gmail.com> ---
(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.