[Bug 17246] New: GetPreparedLetter should not allow multiple FK defined in arrayref
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17246 Bug ID: 17246 Summary: GetPreparedLetter should not allow multiple FK defined in arrayref Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: ASSIGNED Severity: normal Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: jonathan.druart@bugs.koha-community.org Reporter: jonathan.druart@bugs.koha-community.org QA Contact: testopia@bugs.koha-community.org Depends on: 14757 Currently you can call GetPreparedLetter like: $prepared_letter = GetPreparedLetter( ( module => 'test', letter_code => 'TEST_HOLD', tables => { reserves => [ $fk1, $fk2 ], }, ) ); It assumes that $fk1 is a borrowernumber and $fk2 a biblionumber. It seems hazardous to do this guess. I suggest to remove this feature and only allow hashref indeed. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14757 [Bug 14757] Allow the use of Template Toolkit syntax for slips and notices -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17246 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=16942 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17246 --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 55166 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=55166&action=edit Bug 17246: Do no support arrayref to define multiple FK Currently you can call GetPreparedLetter like: $prepared_letter = GetPreparedLetter( ( module => 'test', letter_code => 'TEST_HOLD', tables => { reserves => [ $fk1, $fk2 ], }, ) ); It assumes that $fk1 is a borrowernumber and $fk2 a biblionumber. It seems hazardous to do this guess. I suggest to remove this feature and only allow hashref indeed. Test plan: Use different way to generate letters and make sure you do not reach the croak -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17246 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17246 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |chris@bigballofwax.co.nz --- Comment #2 from Chris Cormack <chris@bigballofwax.co.nz> --- We aren't using this feature anywhere in Koha, except in t/db_dependent/Letters/TemplateToolkit.t So yes, I agree we should remove it, because it is dangerous and someone may start using it. Testing now -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17246 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |major --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Upgrading severity to get attention -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17246 Josef Moravec <josef.moravec@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=17246 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #55166|0 |1 is obsolete| | --- Comment #4 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 58091 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58091&action=edit [SIGNED-OFF] Bug 17246: Do no support arrayref to define multiple FK Currently you can call GetPreparedLetter like: $prepared_letter = GetPreparedLetter( ( module => 'test', letter_code => 'TEST_HOLD', tables => { reserves => [ $fk1, $fk2 ], }, ) ); It assumes that $fk1 is a borrowernumber and $fk2 a biblionumber. It seems hazardous to do this guess. I suggest to remove this feature and only allow hashref indeed. Test plan: Use different way to generate letters and make sure you do not reach the croak Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17246 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17246 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #58091|0 |1 is obsolete| | --- Comment #5 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 58415 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58415&action=edit Bug 17246: Do no support arrayref to define multiple FK Currently you can call GetPreparedLetter like: $prepared_letter = GetPreparedLetter( ( module => 'test', letter_code => 'TEST_HOLD', tables => { reserves => [ $fk1, $fk2 ], }, ) ); It assumes that $fk1 is a borrowernumber and $fk2 a biblionumber. It seems hazardous to do this guess. I suggest to remove this feature and only allow hashref indeed. Test plan: Use different way to generate letters and make sure you do not reach the croak Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17246 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |kyle@bywatersolutions.com --- Comment #6 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to master for 17.05, thanks Jonathan! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17246 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de Status|Pushed to Master |Pushed to Stable --- Comment #7 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- This patch has been pushed to 16.11.x, will be in 16.11.02. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org