| 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 | ||
| GIT URL: | Initiative type: | --- | |
| Sponsorship status: | --- | Comma delimited list of Sponsors: | |
| Crowdfunding goal: | 0 | 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 |
||
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. |
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.