Bug 13817 - Edit links on course items may point to the wrong course!
Summary: Edit links on course items may point to the wrong course!
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Course reserves (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Kyle M Hall
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-03-10 17:43 UTC by Kyle M Hall
Modified: 2015-12-03 22:02 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 13817 - Edit links on course items may point to the wrong course! (2.88 KB, patch)
2015-03-10 17:45 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13817 - Edit links on course items may point to the wrong course! (2.91 KB, patch)
2015-03-30 16:45 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13817 - Edit links on course items may point to the wrong course! (3.01 KB, patch)
2015-04-17 13:26 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2015-03-10 17:43:51 UTC
If items are assigned to multiple courses, the "edit" links for a course item may pass the wrong course id. This is really a cosmetic issue. 

It appears that in Template Toolkit, if you reuse an existing variable name in a foreach loop, it will not create a new temporary variable but will instead re-use the existing variable leaving assigned at the end to the last assignment, just as it would in Perl code. 

This is not a problem when all items are assigned to one and only one course, but can cause issues when we loop through the other courses an item may be on. 

This is solved easily by renaming the variable used in the foreach loop to another unused name.
Comment 1 Kyle M Hall 2015-03-10 17:45:06 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2015-03-30 16:45:30 UTC Comment hidden (obsolete)
Comment 3 Marcel de Rooy 2015-04-17 13:04:43 UTC
QA: Picking a trivial one now; spent hours in Search.pm :)
Comment 4 Marcel de Rooy 2015-04-17 13:26:42 UTC
Created attachment 38057 [details] [review]
Bug 13817 - Edit links on course items may point to the wrong course!

If items are assigned to multiple courses, the "edit" links for a course
item may pass the wrong course id. This is really a cosmetic issue.

It appears that in Template Toolkit, if you reuse an existing variable
name in a foreach loop, it will not create a new temporary variable but
will instead re-use the existing variable leaving assigned at the end to
the last assignment, just as it would in Perl code.

This is not a problem when all items are assigned to one and only one
course, but can cause issues when we loop through the other courses an
item may be on.

This is solved easily by renaming the variable used in the foreach loop
to another unused name.

Test Plan:
1) Apply this patch
2) Try the edit and delete links for an item on reserve for a course
3) No changes in behavior should be noted

Signed-off-by: Tom Misilo <misilot@fit.edu>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 5 Marcel de Rooy 2015-04-17 13:27:51 UTC
Not so trivial to reproduce. With three courses and two items, I couldn't somehow. But the code looks good.

Passed QA
Comment 6 Tomás Cohen Arazi 2015-04-22 18:25:29 UTC
Patch pushed to master.

Thanks Kyle!
Comment 7 Chris Cormack 2015-05-11 21:39:00 UTC
Pushed to 3.18.x will be in 3.18.7