Description
Chris Cormack
2010-11-30 21:10:40 UTC
new/awaiting_qa/5462 has the fixes please test Branch bug_5462_signoff on git://github.com/colinsc/koha.git has been rebased against current master and tested for completeness Pushed, please test Created attachment 3085 [details] [review] Proposed correction Previous commit modified checks for the item-level_itypes preference to look for a different variable name but didn't update where that variable is set in Auth.pm. Other scripts perform a direct check of item-level_itypes and must continue to use the name of the variable in the database. Follow up patch pushed 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? Created attachment 3109 [details] [review] Correction for MARCdetail.pl A change was made to MARCdetail.tmpl without making a corresponding change to MARCdetail.pl. I've reworked the original change so that both can work together. 0XX --> tab0XX Apparently TMPL variables can't start with a number now? *** Bug 5744 has been marked as a duplicate of this bug. *** 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 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 Created attachment 3159 [details] [review] follow up patch Changing the variable names broke the messaging preferences. This patch brings back that functionality. Follow up patch pushed 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 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" --> Created attachment 3578 [details] [review] Fix for patron messaging attributes Doing a replace on non-word characters: $row->{'message_name'} =~ s/\W/\_/g; This seems to work with the existing message name values, but I don't know if that is robust enough to prevent future issues. 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 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. 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? Created attachment 4100 [details] [review] Update for IT sql definition 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. Link to koha-patches: http://lists.koha-community.org/pipermail/koha-patches/2011-May/015047.html 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> Signed off patch attached Easy patch to read, names consistent. Marking as Passed QA Pushed, please test |