Summary: | GetPreparedLetter should not allow multiple FK defined in arrayref | ||
---|---|---|---|
Product: | Koha | Reporter: | Jonathan Druart <jonathan.druart> |
Component: | Architecture, internals, and plumbing | Assignee: | Jonathan Druart <jonathan.druart> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | major | ||
Priority: | P5 - low | CC: | chris, kyle |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16942 | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Bug Depends on: | 14757 | ||
Bug Blocks: | |||
Attachments: |
Bug 17246: Do no support arrayref to define multiple FK
[SIGNED-OFF] Bug 17246: Do no support arrayref to define multiple FK Bug 17246: Do no support arrayref to define multiple FK |
Description
Jonathan Druart
2016-09-05 09:20:08 UTC
Created attachment 55166 [details] [review] 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 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 Upgrading severity to get attention Created attachment 58091 [details] [review] [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> Created attachment 58415 [details] [review] 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> Pushed to master for 17.05, thanks Jonathan! This patch has been pushed to 16.11.x, will be in 16.11.02. |