Bug 9886 - ParseLetter removes too often punctuation characters from fields
Summary: ParseLetter removes too often punctuation characters from fields
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Marcel de Rooy
QA Contact: Katrin Fischer
URL:
Keywords:
: 9079 (view as bug list)
Depends on:
Blocks:
 
Reported: 2013-03-21 12:53 UTC by Marcel de Rooy
Modified: 2024-03-28 08:45 UTC (History)
1 user (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 9886: ParseLetter removes too often punctuation characters from fields (1.86 KB, patch)
2013-03-28 11:03 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 9886: ParseLetter removes too often punctuation characters from fields (1.88 KB, patch)
2013-04-12 16:47 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 9886: ParseLetter removes too often punctuation characters from fields (2.18 KB, patch)
2013-04-22 06:22 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2013-03-21 12:53:08 UTC
This line in C4::Letters, sub parseletter:
    $val =~ s/\p{P}(?=$)//g if $val;
makes a callnumber like 535 G 48 (18) appear as 535 G 48 (18 on notices.

The right parenthesis at the end is being recogized as general punctuation.

Does anyone know why we should remove "punctuation" from fields such as the above? I would like to remove this line. Is there a valid reason not to do so?
Comment 1 Katrin Fischer 2013-03-21 22:46:25 UTC
*** Bug 9079 has been marked as a duplicate of this bug. ***
Comment 2 Katrin Fischer 2013-03-21 22:50:43 UTC
I have run into this in the past and reported it asbug 9079. I am really not sure what that line is supposed to do - I can only guess it might have something to do with stripping ISBD punctuation off fields?
Comment 3 Marcel de Rooy 2013-03-28 11:03:37 UTC Comment hidden (obsolete)
Comment 4 Kyle M Hall 2013-04-12 16:47:38 UTC Comment hidden (obsolete)
Comment 5 Katrin Fischer 2013-04-16 06:46:26 UTC
Started testing this - hopefully finishing later today. :)
Comment 6 Katrin Fischer 2013-04-22 06:22:44 UTC
Created attachment 17572 [details] [review]
[PASSED QA] Bug 9886: ParseLetter removes too often punctuation characters from fields

This patch makes ParseLetter somewhat more restrictive in removing
punctuation characters from the end of a table field.
Based on the assumption that we want to remove punctuation from fields in
biblio and biblioitems (like ISBD).
ParseLetter should not remove e.g. a parenthesis in itemcallnumber, but still
removes e.g. a colon (:) at the end of a title.
Removed an unneeded global and lookahead from the regex.

Test plan:
1) Add a colon (:) to the end of a title.
2) Add a colon to the end of item copynumber.
3) Place a hold on that item. Check it in. Confirm hold.
4) Check the email or print notice generated. There should be no colon at the
end of the title, but the colon in the copynumber should still be there.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
I compared checkout notices with lots of different fields before
and after applying the patch. For example the ) at the end of a
field in branches is now longer removed. Other fields looked ok
before and after.
Passes all tests and QA script.
Comment 7 Jared Camins-Esakov 2013-04-26 22:38:00 UTC
This patch has been pushed to master and 3.12.x.