[Koha-bugs] [Bug 30904] (bug 24387 follow-up) Modifying library in news (additional contents) causes inconsistencies
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30904 Janusz Kaczmarek changed: What|Removed |Added Status|NEW |Needs Signoff Assignee|koha-b...@lists.koha-commun |janus...@gmail.com |ity.org | --- Comment #1 from Janusz Kaczmarek --- Created attachment 135701 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135701&action=edit Bug 30904 - (bug 24387 follow-up) Modifying library in news (additional contents) causes inconsistencies Modifying an existing element of additional content (News) by changing the library causes a duplication of the entry on the Additional contents (News) list. A subsequent attempt of deleting one of the entries results in deleting both of them. This is due to the fact that after the unwanted duplication all the entries share the same additional_contents.code. Test plan = 1. Create a piece of news under Tools > News. 2. After having saved it make a modification by changing the library. It should result in two rows in the table. 3. Delete one of the lines with Delete button. Both lines will disappear. 4. Apply the patch. 5. Repeat the steps 1 and 2. Only the one (the modified) entry should be present in the table. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org 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/
[Koha-bugs] [Bug 30904] (bug 24387 follow-up) Modifying library in news (additional contents) causes inconsistencies
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30904 Janusz Kaczmarek changed: What|Removed |Added CC||jonathan.druart+koha@gmail. ||com -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org 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/
[Koha-bugs] [Bug 30904] (bug 24387 follow-up) Modifying library in news (additional contents) causes inconsistencies
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30904 --- Comment #2 from Jonathan Druart --- 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`), -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org 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/
[Koha-bugs] [Bug 30904] (bug 24387 follow-up) Modifying library in news (additional contents) causes inconsistencies
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30904 --- Comment #3 from Janusz Kaczmarek --- (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 Koha-bugs@lists.koha-community.org 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/
[Koha-bugs] [Bug 30904] (bug 24387 follow-up) Modifying library in news (additional contents) causes inconsistencies
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30904 --- Comment #4 from Janusz Kaczmarek --- Another possibility would be issuing search with three criteria instead of find. Maybe this would be simpler and still correct? -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org 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/
[Koha-bugs] [Bug 30904] (bug 24387 follow-up) Modifying library in news (additional contents) causes inconsistencies
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30904 --- Comment #5 from Jonathan Druart --- Created attachment 135792 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135792&action=edit [ALTERNATIVE-PATCH] Bug 30904: (bug 24387 follow-up) Fix library modification of additional contents Modifying an existing element of additional content (News) by changing the library causes a duplication of the entry on the Additional contents (News) list. A subsequent attempt of deleting one of the entries results in deleting both of them. This is due to the fact that after the unwanted duplication all the entries share the same additional_contents.code. Test plan = 1. Create a piece of news under Tools > News. 2. After having saved it make a modification by changing the library. It should result in two rows in the table. 3. Delete one of the lines with Delete button. Both lines will disappear. 4. Apply the patch. 5. Repeat the steps 1 and 2. Only the one (the modified) entry should be present in the table. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org 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/
[Koha-bugs] [Bug 30904] (bug 24387 follow-up) Modifying library in news (additional contents) causes inconsistencies
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30904 --- Comment #6 from Jonathan Druart --- What about this patch? -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org 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/
[Koha-bugs] [Bug 30904] (bug 24387 follow-up) Modifying library in news (additional contents) causes inconsistencies
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30904 --- Comment #7 from Jonathan Druart --- (In reply to Janusz Kaczmarek from comment #3) > 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. Which means you couldn't have 2 identical contents for different libraries? Sounds wrong to me. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org 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/
[Koha-bugs] [Bug 30904] (bug 24387 follow-up) Modifying library in news (additional contents) causes inconsistencies
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30904 --- Comment #8 from Janusz Kaczmarek --- (In reply to Jonathan Druart from comment #7) > (In reply to Janusz Kaczmarek from comment #3) > > 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. > > Which means you couldn't have 2 identical contents for different libraries? > Sounds wrong to me. Why not? In every case you have to create them separately, which means they would get a different additional_contents.code. The presence or absence of branchcode in unique key does not change anything here, IMHO. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org 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/
[Koha-bugs] [Bug 30904] (bug 24387 follow-up) Modifying library in news (additional contents) causes inconsistencies
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30904 Lucas Gass changed: What|Removed |Added Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org 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/
[Koha-bugs] [Bug 30904] (bug 24387 follow-up) Modifying library in news (additional contents) causes inconsistencies
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30904 --- Comment #9 from Lucas Gass --- Created attachment 135849 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135849&action=edit Bug 30904: (bug 24387 follow-up) Fix library modification of additional contents Modifying an existing element of additional content (News) by changing the library causes a duplication of the entry on the Additional contents (News) list. A subsequent attempt of deleting one of the entries results in deleting both of them. This is due to the fact that after the unwanted duplication all the entries share the same additional_contents.code. Test plan = 1. Create a piece of news under Tools > News. 2. After having saved it make a modification by changing the library. It should result in two rows in the table. 3. Delete one of the lines with Delete button. Both lines will disappear. 4. Apply the patch. 5. Repeat the steps 1 and 2. Only the one (the modified) entry should be present in the table. Signed-off-by: Lucas Gass -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org 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/
[Koha-bugs] [Bug 30904] (bug 24387 follow-up) Modifying library in news (additional contents) causes inconsistencies
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30904 Lucas Gass changed: What|Removed |Added CC||lu...@bywatersolutions.com Attachment #135701|0 |1 is obsolete|| -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org 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/
[Koha-bugs] [Bug 30904] (bug 24387 follow-up) Modifying library in news (additional contents) causes inconsistencies
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30904 Lucas Gass changed: What|Removed |Added Attachment #135792|0 |1 is obsolete|| -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org 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/
[Koha-bugs] [Bug 30904] (bug 24387 follow-up) Modifying library in news (additional contents) causes inconsistencies
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30904 --- Comment #10 from Jonathan Druart --- (In reply to Janusz Kaczmarek from comment #8) > (In reply to Jonathan Druart from comment #7) > > (In reply to Janusz Kaczmarek from comment #3) > > > 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. > > > > Which means you couldn't have 2 identical contents for different libraries? > > Sounds wrong to me. > > Why not? In every case you have to create them separately, which means they > would get a different additional_contents.code. The presence or absence of > branchcode in unique key does not change anything here, IMHO. You are right. I would like to not modify the DB structure and keep the patch as simple as possible, for backporting purposes. When I rewrote this module I wanted to stick to the same structure as the 'letter' one. I should not have done that, and preferred a better architecture. This table should have been split into two. Unfortunately it's now too much work and I won't have time for that soon. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org 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/
[Koha-bugs] [Bug 30904] (bug 24387 follow-up) Modifying library in news (additional contents) causes inconsistencies
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30904 Katrin Fischer changed: What|Removed |Added Status|Signed Off |Passed QA Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org 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/
[Koha-bugs] [Bug 30904] (bug 24387 follow-up) Modifying library in news (additional contents) causes inconsistencies
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30904 Katrin Fischer changed: What|Removed |Added Attachment #135849|0 |1 is obsolete|| --- Comment #11 from Katrin Fischer --- Created attachment 135965 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135965&action=edit Bug 30904: (bug 24387 follow-up) Fix library modification of additional contents Modifying an existing element of additional content (News) by changing the library causes a duplication of the entry on the Additional contents (News) list. A subsequent attempt of deleting one of the entries results in deleting both of them. This is due to the fact that after the unwanted duplication all the entries share the same additional_contents.code. Test plan = 1. Create a piece of news under Tools > News. 2. After having saved it make a modification by changing the library. It should result in two rows in the table. 3. Delete one of the lines with Delete button. Both lines will disappear. 4. Apply the patch. 5. Repeat the steps 1 and 2. Only the one (the modified) entry should be present in the table. Signed-off-by: Lucas Gass Signed-off-by: Katrin Fischer -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org 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/
[Koha-bugs] [Bug 30904] (bug 24387 follow-up) Modifying library in news (additional contents) causes inconsistencies
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30904 Katrin Fischer changed: What|Removed |Added Severity|minor |normal -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org 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/
[Koha-bugs] [Bug 30904] (bug 24387 follow-up) Modifying library in news (additional contents) causes inconsistencies
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30904 Katrin Fischer changed: What|Removed |Added QA Contact|testo...@bugs.koha-communit |katrin.fisc...@bsz-bw.de |y.org | -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org 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/
[Koha-bugs] [Bug 30904] (bug 24387 follow-up) Modifying library in news (additional contents) causes inconsistencies
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30904 Tomás Cohen Arazi changed: What|Removed |Added Version(s)||22.11.00 released in|| Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org 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/
[Koha-bugs] [Bug 30904] (bug 24387 follow-up) Modifying library in news (additional contents) causes inconsistencies
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30904 --- Comment #12 from Tomás Cohen Arazi --- Pushed to master for 22.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org 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/
[Koha-bugs] [Bug 30904] (bug 24387 follow-up) Modifying library in news (additional contents) causes inconsistencies
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30904 Tomás Cohen Arazi changed: What|Removed |Added Assignee|janus...@gmail.com |jonathan.druart+koha@gmail. ||com CC||tomasco...@gmail.com Keywords||rel_22_05_candidate -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org 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/
[Koha-bugs] [Bug 30904] (bug 24387 follow-up) Modifying library in news (additional contents) causes inconsistencies
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30904 Lucas Gass changed: What|Removed |Added Status|Pushed to master|Pushed to stable Version(s)|22.11.00|22.11.00, 22.05.03 released in|| --- Comment #13 from Lucas Gass --- Backported to 22.05.x for 22.05.03 -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org 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/
[Koha-bugs] [Bug 30904] (bug 24387 follow-up) Modifying library in news (additional contents) causes inconsistencies
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30904 Lucas Gass changed: What|Removed |Added Keywords|rel_22_05_candidate | -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org 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/
