Created attachment 7756 [details] hardcoded note on routing list Remove the hardcoded note from the routing list in favor of a new system preference for displaying a note on all routing lists. Notes: Please return this item promptly as others are waiting for it.
Created attachment 7757 [details] [review] Bug 7557: Change hardcoded routing list note into a system preference Adds a new system preference RoutingListNote under the Serials tab. The note will display above the note from the subscription and replace the current hardcoded note: "Notes: Please return this item promptly as others are waiting for it." The patch adds unique ids to all notes and the note in general, so it can be styled using CSS. Also corrects the routing slip template to follow the HTML4 rule.
Passes valid template tests rorohiko:[git/7557-]:~/git/koha% perl xt/tt_valid.t 1..1 ok 1 - TT syntax: not using TT directive within HTML tag Passes translation tests rorohiko:[git/7557-]:~/git/koha% perl xt/author/translatable-templates.t 1..2 ok 1 - opac templates are translatable ok 2 - intranet templates are translatable Functionality works as described and the note is now editable, signed off patch to follow
Created attachment 7780 [details] [review] Bug 7557: Change hardcoded routing list note into a system preference Adds a new system preference RoutingListNote under the Serials tab. The note will display above the note from the subscription and replace the current hardcoded note: "Notes: Please return this item promptly as others are waiting for it." The patch adds unique ids to all notes and the note in general, so it can be styled using CSS. Also corrects the routing slip template to follow the HTML4 rule. Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
HELP: Patch fails to apply!
This is a run of the mill updatedatabase.pl conflict. It should be easy to resolve. I don't think it's necessary to ask the patch submitter to resubmit.
Forget my comment: vim created it.
Asking a dumb question here: The hardcoded note just comes from a template file. So it should be translatable. Why put in a pref? Note that when you switch language, the pref does not change with you. So I do not really understand the advantage of this solution. Please clarify.
The advantage is, that it's configurable. At the moment the note is hardcoded and it does assume that this is the way routing lists in libraries work. You could always add a note for each routing slip below the hardcoded note - but you could not get rid of the harcoded note or hide it. I thought about removing it, but then on second thought I was thinking that having a note that will appear on all routing lists + an individual note wouuld be nice. As you have noted, there is the problem of translatability. like there is for all things being stored in the database. In my opinion this was that the routing slip would only appear in the "main" language. Here practice is that you print the slip and attach it to the issue so people can note when they got the issue or when they gave it to the next person on list. So I don't see a loss of functionality here.
As Katrin clarified, there is a need for individual libraries to change this default note. Editing the template is not an option then. Patch looks good. Marked as Passed QA.
yikes, big bug, the patch can't be included: => Operator or semicolon missing before &searchfield at installer/data/mysql/updatedatabase.pl line 4731. Ambiguous use of & resolved as operator & at installer/data/mysql/updatedatabase.pl line 4731. Can't modify bitwise and (&) in scalar assignment at installer/data/mysql/updatedatabase.pl line 4732, near "RoutingListNote#jumped">RoutlingListNote</a> system preference.','Define a note to be shown on all routing lists','70|10','Textarea');"); (thx my pre-apply that detected this) => the updatedatabase containt a do("INSERT ... <a href="...">") " of href must be escaped
Good catch, Paul! Looks good was not is good :-) Hard to see when browsing through the code, but this error should have been seen in the output of the webinstaller when running the update?
marcel: yes, it should. It probably means the tester installed a new database from scratch, and not did an update (is it me or the "reply" link doesn't copy the original comment anymore ?
And before you ask... I didn't notice because the link was something I put in last minute. I will fix the patch :) QA++
(In reply to comment #12) > (is it me or the "reply" link doesn't copy the original comment anymore ? I think you have to make sure you're not using a cached copy of a javascript file. I noticed the same thing and a shift-reload fixed it.
(In reply to comment #14) > (In reply to comment #12) > > (is it me or the "reply" link doesn't copy the original comment anymore ? > I think you have to make sure you're not using a cached copy of a javascript > file. I noticed the same thing and a shift-reload fixed it. good catch, that's it, as you can see
Created attachment 8170 [details] [review] Bug 7557: Change hardcoded routing list note into a system preference Adds a new system preference RoutingListNote under the Serials tab. The note will display above the note from the subscription and replace the current hardcoded note: "Notes: Please return this item promptly as others are waiting for it." The patch adds unique ids to all notes and the note in general, so it can be styled using CSS. Also corrects the routing slip template to follow the HTML4 rule. Update 2012-03-12: Fixed problem in updatedatebase.
(considering the signed-off is still valid, as this bug already made a full loop and failed QA because of a small typo) QA comment: one of our rule is that a new feature should be introduced without trouble for existing setups. The idea of having To change this note edit RoutlingListNote system preference. into the syspref by default may seem good, but if the user don't spot this, or don't care, it will be written suddenly on printed routing slips. So i've removed this default RoutingListNote Other than this, passed QA, will push
I was torn here. Now the old sentence will no longer be printed out - but putting the english sentence in the system prefernece would not have worked either. I think removing it is ok, but will not advertise the new feature. There is a disadvantage to every solution I could think of.
> I think removing it is ok, but will not advertise the new feature. => that's what Release Notes are made for (even if I know many ppl don't read them)
This says pushed to master but I don't see it in my admin area - was there a problem with my upgrade?
Hi Nicole, I think it was not pushed - i can't see it on git. I reset the status to "Passed QA" so it shows up in Paul's list :)
sorry, I pushed the branch, but forgot to push master before closing my computer yesterday !
Created attachment 8498 [details] [review] Bug 7557 - corrects missing semi-colon ';'
Created attachment 8504 [details] [review] Bug 7557 - corrects missing semi-colon ';', removes extra '`'
Created attachment 8505 [details] [review] Bug 7557 - corrects missing semi-colon ';', removes extra '`'
Hi Mason, updatedatabase got fixed - I am closing this bug. Please reopen if I missed something.