[Bug 22905] New: Edit suggestions - wrong branchcode behavior
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22905 Bug ID: 22905 Summary: Edit suggestions - wrong branchcode behavior Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Acquisitions Assignee: koha-bugs@lists.koha-community.org Reporter: jonathan.druart@bugs.koha-community.org QA Contact: testopia@bugs.koha-community.org Depends on: 22368 Target Milestone: --- Because of the "Library" filter on the left of the "Suggestions management" screen, there is something wrong happening: 1. Create a suggestion from library A, login from library B 2. Go to Home › Acquisitions › Suggestions management => The suggestion does not appear - OK 3. In the filter on the left, select "all library" => The suggestion appears on the pending tab - KO 4. Select the suggestion and mark is as 'Accepted' => => The suggestion still appears on the pending tab - Failure The log says: DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`suggestions`, CONSTRAINT `suggestions_ibfk_branchcode` FOREIGN KEY (`branchcode`) REFERENCES `branches` (`branchcode`) ON DELETE SET NULL ON UPDATE CASCADE) [for Statement "UPDATE `suggestions` SET `accepteddate` = ?, `branchcode` = ?, `currency` = ?, `manageddate` = ?, `price` = ?, `reason` = ?, `suggesteddate` = ?, `total` = ? WHERE ( `suggestionid` = ? )" with ParamValues: 0='2019-05-14T15:48:18', 1="", 2="CAD", 3='2019-05-14T15:48:18', 4="0.00", 5="", 6='2019-05-14T00:00:00', 7="0.00", 8=3] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1836. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22368 [Bug 22368] Table suggestions lacks foreign key constraints -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22905 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com, | |nick@bywatersolutions.com Severity|major |critical --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- All this code is awful, I have no idea how to fix it simply and without introduce regressions. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22905 --- Comment #2 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- All of this is very confusing, it's full of bugs. Play with 'Organize by'=Library and you will get a strong headache quickly. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22905 --- Comment #3 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 89828 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=89828&action=edit Bug 22905: A quick, but horrible, fix. 'branchcode' appears to be used as a uri param if displayby is set to branchcode. However, this also matches a field name in the suggestions table so the reverse mapping of params to suggestions fields catches it incorrectly. I believe this is the correct method for removing the incorrect assignment but the better fix would be to clean up the whole script and use unique params for filters. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22905 --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Mind well and truly melted.. I'm not entirely confident in this fix.. it'll need a thorough testing for side effects :S -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22905 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22905 Liz Rea <wizzyrea@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA CC| |wizzyrea@gmail.com --- Comment #5 from Liz Rea <wizzyrea@gmail.com> --- Interesting, this works if you edit the individual suggestion (use the edit button - I did it beccause I was curious) and accept it there, but not if you check it and use the "mark selected as" from the bottom of the suggestions page. The behaviour is exactly the same with or without this patch? Liz -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22905 --- Comment #6 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I am confused. If I apply the patches from bug 22907, I cannot recreate the original issue. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22905 --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Jonathan Druart from comment #6)
I am confused. If I apply the patches from bug 22907, I cannot recreate the original issue.
Scratch that, it's wrong! -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22905 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |jonathan.druart@bugs.koha-c |ity.org |ommunity.org -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22905 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Edit suggestions - wrong |Cannot upate the status of |branchcode behavior |suggestions if the | |branchcode filter is set to | |all -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22905 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22905 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #89828|0 |1 is obsolete| | --- Comment #8 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 89868 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=89868&action=edit Bug 22905: Fix update of suggestion's status if branchcode filter is 'all' Because of the "Library" filter on the left of the "Suggestions management" screen, there is something wrong happening: 1. Create a suggestion from library A, login from library B 2. Go to Home › Acquisitions › Suggestions management => The suggestion does not appear - OK 3. In the filter on the left, select "all library" => The suggestion appears on the pending tab - KO 4. Select the suggestion and mark is as 'Accepted' => The suggestion still appears on the pending tab - Failure The log says: DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`suggestions`, CONSTRAINT `suggestions_ibfk_branchcode` FOREIGN KEY (`branchcode`) REFERENCES `branches` (`branchcode`) ON DELETE SET NULL ON UPDATE CASCADE) [for Statement "UPDATE `suggestions` SET `accepteddate` = ?, `branchcode` = ?, `currency` = ?, `manageddate` = ?, `price` = ?, `reason` = ?, `suggesteddate` = ?, `total` = ? WHERE ( `suggestionid` = ? )" with ParamValues: 0='2019-05-14T15:48:18', 1="", 2="CAD", 3='2019-05-14T15:48:18', 4="0.00", 5="", 6='2019-05-14T00:00:00', 7="0.00", 8=3] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1836. Let forget what could have happened earlier in the script and do it the regular way, from $input. Then call ModSuggestion with only what we need. Test plan: Confirm that the steps described before work as expected once this patch is applied -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22905 Liz Rea <wizzyrea@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22905 Liz Rea <wizzyrea@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #89868|0 |1 is obsolete| | --- Comment #9 from Liz Rea <wizzyrea@gmail.com> --- Created attachment 89870 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=89870&action=edit Bug 22905: Fix update of suggestion's status if branchcode filter is 'all' Because of the "Library" filter on the left of the "Suggestions management" screen, there is something wrong happening: 1. Create a suggestion from library A, login from library B 2. Go to Home › Acquisitions › Suggestions management => The suggestion does not appear - OK 3. In the filter on the left, select "all library" => The suggestion appears on the pending tab - KO 4. Select the suggestion and mark is as 'Accepted' => The suggestion still appears on the pending tab - Failure The log says: DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`suggestions`, CONSTRAINT `suggestions_ibfk_branchcode` FOREIGN KEY (`branchcode`) REFERENCES `branches` (`branchcode`) ON DELETE SET NULL ON UPDATE CASCADE) [for Statement "UPDATE `suggestions` SET `accepteddate` = ?, `branchcode` = ?, `currency` = ?, `manageddate` = ?, `price` = ?, `reason` = ?, `suggesteddate` = ?, `total` = ? WHERE ( `suggestionid` = ? )" with ParamValues: 0='2019-05-14T15:48:18', 1="", 2="CAD", 3='2019-05-14T15:48:18', 4="0.00", 5="", 6='2019-05-14T00:00:00', 7="0.00", 8=3] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1836. Let forget what could have happened earlier in the script and do it the regular way, from $input. Then call ModSuggestion with only what we need. Test plan: Confirm that the steps described before work as expected once this patch is applied Signed-off-by: Liz Rea <wizzyrea@gmail.com> https://bugs.koha-community.org/show_bug.cgi?id=22907 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22905 --- Comment #10 from Liz Rea <wizzyrea@gmail.com> --- Created attachment 89871 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=89871&action=edit Bug 22905: (QA follow-up) remove bad warn Signed-off-by: Liz Rea <wizzyrea@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22905 --- Comment #11 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 89884 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=89884&action=edit Bug 22905: Counterpatch removing just branchcode The suggested patches replace using $suggestion_only, making much more changes than needed. I would opt for removing the offending branchcode. In this case __ANY__. Apart from strict SQL mode, it is just wrong. This patch touches just one line. Same test plan. 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=22905 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #12 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #1)
All this code is awful, I have no idea how to fix it simply and without introduce regressions.
Yes, this definitely is horrible. I suggest a simpler fix. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22905 --- Comment #13 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I think my patch is much more regression proof. It makes the code does what it must do. There are much more variables in the filters that have the same name: budgetid, title, etc. I wanted to provide a patch in which we can trust. I do not trust yours, because I did not spend 2 hours to make sure there are no more bugs (interference with variable's name). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22905 --- Comment #14 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #13)
I think my patch is much more regression proof. Sure. Any dev should be convinced of that ;)
I do not trust yours. Good to hear.
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22905 --- Comment #15 from Liz Rea <wizzyrea@gmail.com> --- Well, I'm going to say I've signed off Jonathan's, it fixes the problem and I prefer the approach there. I'd like to get this fixed and we have a patch, let's have another QA person look at it and decide. Liz -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22905 --- Comment #16 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Liz Rea from comment #15)
Well, I'm going to say I've signed off Jonathan's, it fixes the problem and I prefer the approach there.
I'd like to get this fixed and we have a patch, let's have another QA person look at it and decide.
Liz
Dont worry -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22905 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #89884|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22905 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Cannot upate the status of |Cannot update the status of |suggestions if the |suggestions if the |branchcode filter is set to |branchcode filter is set to |all |all -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22905 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> 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.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22905 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #89870|0 |1 is obsolete| | --- Comment #17 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 89930 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=89930&action=edit Bug 22905: Fix update of suggestion's status if branchcode filter is 'all' Because of the "Library" filter on the left of the "Suggestions management" screen, there is something wrong happening: 1. Create a suggestion from library A, login from library B 2. Go to Home › Acquisitions › Suggestions management => The suggestion does not appear - OK 3. In the filter on the left, select "all library" => The suggestion appears on the pending tab - KO 4. Select the suggestion and mark is as 'Accepted' => The suggestion still appears on the pending tab - Failure The log says: DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`suggestions`, CONSTRAINT `suggestions_ibfk_branchcode` FOREIGN KEY (`branchcode`) REFERENCES `branches` (`branchcode`) ON DELETE SET NULL ON UPDATE CASCADE) [for Statement "UPDATE `suggestions` SET `accepteddate` = ?, `branchcode` = ?, `currency` = ?, `manageddate` = ?, `price` = ?, `reason` = ?, `suggesteddate` = ?, `total` = ? WHERE ( `suggestionid` = ? )" with ParamValues: 0='2019-05-14T15:48:18', 1="", 2="CAD", 3='2019-05-14T15:48:18', 4="0.00", 5="", 6='2019-05-14T00:00:00', 7="0.00", 8=3] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1836. Let forget what could have happened earlier in the script and do it the regular way, from $input. Then call ModSuggestion with only what we need. Test plan: Confirm that the steps described before work as expected once this patch is applied Signed-off-by: Liz Rea <wizzyrea@gmail.com> https://bugs.koha-community.org/show_bug.cgi?id=22907 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=22905 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #89871|0 |1 is obsolete| | --- Comment #18 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 89931 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=89931&action=edit Bug 22905: (QA follow-up) remove bad warn Signed-off-by: Liz Rea <wizzyrea@gmail.com> 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=22905 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | --- Comment #19 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- KohaCon19 PQA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22905 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #20 from Nick Clemens <nick@bywatersolutions.com> --- Awesome work all! Pushed to master for 19.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22905 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |19.05.00 released in| | Resolution|--- |FIXED Status|Pushed to Master |RESOLVED --- Comment #21 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Bug with enhancement not in 18.11.x series. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22905 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |23855 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23855 [Bug 23855] Cannot mark the selected suggestion as "pending" -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org