Bug 5462 - Fix variable names so we dont break template::toolkit
Summary: Fix variable names so we dont break template::toolkit
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: PATCH-Sent (DO NOT USE) enhancement (vote)
Assignee: Galen Charlton
QA Contact: Bugs List
URL:
Keywords:
: 5744 (view as bug list)
Depends on:
Blocks: 5917
  Show dependency treegraph
 
Reported: 2010-11-30 21:10 UTC by Chris Cormack
Modified: 2012-10-25 23:09 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Proposed correction (3.22 KB, patch)
2011-02-07 17:27 UTC, Owen Leonard
Details | Diff | Splinter Review
Correction for MARCdetail.pl (6.88 KB, patch)
2011-02-09 13:46 UTC, Owen Leonard
Details | Diff | Splinter Review
follow up patch (2.56 KB, patch)
2011-02-16 13:52 UTC, Nicole C. Engard
Details | Diff | Splinter Review
Fix for patron messaging attributes (2.36 KB, patch)
2011-03-31 16:21 UTC, Owen Leonard
Details | Diff | Splinter Review
Update for IT sql definition (1.37 KB, patch)
2011-05-13 08:13 UTC, Zeno Tajoli
Details | Diff | Splinter Review
Update of the italian sql files The bug is about strings that are used as variables by Template::Toolkit The strings need to be like valid perl variable names. To fix the bug we need to fix also .sql files. Others .sql files are OK. Italian .sql file need (1.41 KB, patch)
2011-05-30 08:26 UTC, Chris Cormack
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Cormack 2010-11-30 21:10:40 UTC
Variables for template toolkit need to be valid perl variable names.

Some of our template variables aren't. We should fix them even if we weren't changing to template::toolkit but we definitely need to before we can move.
Comment 1 Chris Cormack 2010-11-30 21:31:31 UTC
new/awaiting_qa/5462 has the fixes please test
Comment 2 Colin Campbell 2011-02-03 10:22:30 UTC
Branch bug_5462_signoff on git://github.com/colinsc/koha.git has been rebased against current master and tested for completeness
Comment 3 Chris Cormack 2011-02-03 19:58:39 UTC
Pushed, please test
Comment 4 Owen Leonard 2011-02-07 17:27:03 UTC Comment hidden (obsolete)
Comment 5 Chris Cormack 2011-02-08 08:09:15 UTC
Follow up patch pushed
Comment 6 Marcel de Rooy 2011-02-09 13:14:06 UTC
Current master does not show marc details for a record in staff client. All tabs have disappeared. (Click on MARC tab in normal view for one record.) 

Problem seems to be connected with one of the patches connected to this report.

Commit 8fd29f3d62d1aa5d7e728032ea00bd98f3aef52d for this report makes the marc details disappear..

Few commits before the patch, at e29aca325d199468673365ee7540760b38630833, I am still seeing marc details.

Apparently, some work on the templates should be adjusted?
Comment 7 Owen Leonard 2011-02-09 13:46:56 UTC Comment hidden (obsolete)
Comment 8 Owen Leonard 2011-02-11 19:32:12 UTC
*** Bug 5744 has been marked as a duplicate of this bug. ***
Comment 9 Chris Cormack 2011-02-13 21:36:36 UTC
Signed off patch does not apply

Applying: Follow-up fix for Bug 5462, Fix variable names so we dont break template::toolkit
Using index info to reconstruct a base tree...
error: patch failed: koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/MARCdetail.tmpl:45
error: koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/MARCdetail.tmpl: patch does not apply
Did you hand edit your patch?
It does not apply to blobs recorded in its index.
Cannot fall back to three-way merge.
Patch failed at 0001 Follow-up fix for Bug 5462, Fix variable names so we dont break template::toolkit
Comment 10 Marcel de Rooy 2011-02-14 08:12:58 UTC
I recreated the patch, but the referred blobs are still the same. And the patches still apply. I tested it again with latest commit mentioned below.

Are you sure that you tested this patch with a clean working dir? What could be the reason for not matching blobs?

[marcel@RKM004 testclone]$ git apply ../0001-5462.patch
[marcel@RKM004 testclone]$ git reset --hard
HEAD is now at f6be5ef Bug 5751 : Follow up patch, removing unused variable and fixing copyright
[marcel@RKM004 testclone]$ git am -3 ../0001-5462.patch
Applying: Follow-up fix for Bug 5462, Fix variable names so we dont break template::toolkit
Comment 11 Nicole C. Engard 2011-02-16 13:52:52 UTC Comment hidden (obsolete)
Comment 12 Chris Cormack 2011-02-16 20:29:01 UTC
Follow up patch pushed
Comment 13 MJ Ray (software.coop) 2011-03-04 10:26:52 UTC
This bug is mentioned in:
Follow-up correction for Bug 5462 - Fix	variable names for template::toolkit http://lists.koha-community.org/pipermail/koha-patches/2011-February/013775.html
Follow-up fix for Bug 5462,	Fix variable names so we dont break template::toolkit http://lists.koha-community.org/pipermail/koha-patches/2011-February/013796.html
Follow-up fix for Bug 5462, Fix variable names so we dont break template::toolkit http://lists.koha-community.org/pipermail/koha-patches/2011-February/013797.html
Follow up on Bug 5462: fixing	variable names breaks messaging preference form http://lists.koha-community.org/pipermail/koha-patches/2011-February/013873.html
Follow-up fix for Bug 5462, Fix variable names so we dont break template::toolkit http://lists.koha-community.org/pipermail/koha-patches/2011-February/013848.html
Comment 14 Owen Leonard 2011-03-30 20:16:12 UTC
I discovered a new instance of this bug. In Messaging.pm sub GetMessagingOptions queries the message_attributes table for message_attributes.message_name and assigns a series of variables based on that name. This means we end up with variables named things like "Item due," and thus the template has checks for things like <!-- TMPL_IF NAME="Item Due" --> and <!-- TMPL_ELSIF NAME="Item Check-in" -->
Comment 15 Owen Leonard 2011-03-31 16:21:01 UTC Comment hidden (obsolete)
Comment 16 MJ Ray (software.coop) 2011-04-11 08:32:25 UTC
This bug is mentioned in:
Incremental fix for Bug 5462,	Fix variable names so we dont break template::toolkit http://lists.koha-community.org/pipermail/koha-patches/2011-March/014402.html
Comment 17 Zeno Tajoli 2011-05-12 19:55:34 UTC
This bug is partial done in 3.6.x tree.
To do:
Insert this line at C4/Members/Messaging.pm,line 212:

$row->{'message_name'} =~ s/\W/\_/g;

With this last commit the bug is done.
Or, more exactly, all patch here present will be commit into the master.
Comment 18 Chris Cormack 2011-05-12 22:19:30 UTC
Zeno, the message names have now been fixed in the database.

So that line should not be needed, ie

insert into `message_attributes`
(`message_attribute_id`, message_name, `takes_days`)
values
(1, 'Item_Due', 0),
(2, 'Advance_Notice', 1),
(4, 'Hold_Filled', 0),
(5, 'Item_Check_in', 0),
(6, 'Item_Checkout', 0);

$DBversion = "3.03.00.051";
if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
    print "Upgrade to $DBversion done (Remove spaces and dashes from message_attribute names)\n";
    $dbh->do("UPDATE message_attributes SET message_name = 'Item_Due' WHERE message_name='Item Due'");
    $dbh->do("UPDATE message_attributes SET message_name = 'Advance_Notice' WHERE message_name='Advance Notice'");
    $dbh->do("UPDATE message_attributes SET message_name = 'Hold_Filled' WHERE message_name='Hold Filled'");
    $dbh->do("UPDATE message_attributes SET message_name = 'Item_Check_in' WHERE message_name='Item Check-in'");
    $dbh->do("UPDATE message_attributes SET message_name = 'Item_Checkout' WHERE message_name='Item Checkout'");
    SetVersion ($DBversion);
}

So i think we can mark this resolved fixed ... do you agree?
Comment 19 Zeno Tajoli 2011-05-13 08:13:59 UTC Comment hidden (obsolete)
Comment 20 Zeno Tajoli 2011-05-13 08:16:22 UTC
To cloese the patch, we need to to update also italian .sql file
installer/data/mysql/it-IT/necessari/sample_notices_message_attributes.sql

I have insert a patch for it here (as attach) and I send it to koha-patches.
The patch is for 3.6 and 3.4 also.
Comment 21 Zeno Tajoli 2011-05-13 08:17:25 UTC
Link to koha-patches:
http://lists.koha-community.org/pipermail/koha-patches/2011-May/015047.html
Comment 22 Chris Cormack 2011-05-30 08:26:12 UTC
Created attachment 4299 [details] [review]
Update of the italian sql files The bug is about strings that are used as variables by Template::Toolkit The strings need to be like valid perl variable names. To fix the bug we need to fix also .sql files. Others .sql files are OK. Italian .sql file need

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Comment 23 Chris Cormack 2011-05-30 08:26:54 UTC
Signed off patch attached
Comment 24 Ian Walls 2011-07-14 22:32:28 UTC
Easy patch to read, names consistent.  Marking as Passed QA
Comment 25 Chris Cormack 2011-07-14 23:11:37 UTC
Pushed, please test