Bug 20084

Summary: Patron card creator: layouts Industrial2of5 and COOP2of5 broken with error "Invalid Characters"
Product: Koha Reporter: Victor Grousset/tuxayo <victor>
Component: ToolsAssignee: Victor Grousset/tuxayo <victor>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: major    
Priority: P5 - low CC: fridolin.somers, katrin.fischer, martin.renvoize, nick
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21123
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 20084: fix Industrial2of5 and COOP2of5 patron card layouts
Bug 20084: fix Industrial2of5 and COOP2of5 patron card layouts
Bug 20084: Fix Industrial2of5 and COOP2of5 patron card layouts
Bug 20084: Fix Industrial2of5 and COOP2of5 patron card layouts

Description Victor Grousset/tuxayo 2018-01-24 08:55:40 UTC Comment hidden (obsolete)
Comment 1 Victor Grousset/tuxayo 2018-01-24 13:30:27 UTC Comment hidden (obsolete)
Comment 2 Fridolin Somers 2018-01-24 14:25:00 UTC
I looked at http://manpages.org/pdfreusebarcode/3
Comment 3 Victor Grousset/tuxayo 2018-01-25 13:40:21 UTC
Thanks Fridolin :)
Comment 4 Victor Grousset/tuxayo 2018-01-25 13:46:06 UTC Comment hidden (obsolete)
Comment 5 Victor Grousset/tuxayo 2018-01-25 14:19:54 UTC
== Going further, not confirmed or not fixed here ==
Someone interested might want to check issues with 2 other types.

1. "Code 39 + Modulo10" doesn't seem to work
Although "Code 39" and "Code 39 + Modulo43" work.
Might be my test setup.

2. EAN13 barcodes doesn't seem to work.
C4::Patroncards::Patroncard->draw_barcode() has an if-elsif for all types but EAN13.
C4/Labels/Label.pm has an elsif for EAN13
This is suspicious!
I tried to the copy it to Patroncard but got an Invalid Length error.
With the quick solution falling and having no idea what was I doing, Ctrl-Z was spammed and these regretful words were written.
Comment 6 Victor Grousset/tuxayo 2018-01-25 15:05:34 UTC
Created attachment 70914 [details] [review]
Bug 20084: fix Industrial2of5 and COOP2of5 patron card layouts

== Test plan ==
This is an oversimplification of a full patron card setup used in production.

1. Create a batch with 1 patron
2. Create a layout and set
     the name
     "Print card number as barcode"
     "Barcode type:" to "Code 39"
3. Create a card template without filling anything
4. Export the batch using the layout and template
5. You should have a white page with a barcode
     And no errors in the relevant log file
     This show that this setup isn't completely bogus
     (although Code 39 is the only type working...)
6. Layout: set "Barcode type:" to Industrial2of5
7. Export the batch
8. You should have a white page with no barcode
     And errors in the relevant log file
     "Invalid Characters"
     This is the bug.
9. Layout: set "Barcode type:" to COOP2of5
10. Export the batch
11. You should have a white page with no barcode
      And errors in the relevant log file
      "Invalid Characters"
      This is the bug.
12. Apply this patch
13. Retry with both non-working patches
    1. You should have a white page with a barcode
    2. And no errors in the relevant log file
Comment 7 Victor Grousset/tuxayo 2018-01-25 15:09:16 UTC
Fixed quotes not needed anymore
"*$params{barcode_data}" → $params{barcode_data}
Comment 8 Katrin Fischer 2018-07-08 14:56:30 UTC
Created attachment 76760 [details] [review]
Bug 20084: Fix Industrial2of5 and COOP2of5 patron card layouts

== Test plan ==
This is an oversimplification of a full patron card setup used in production.

1. Create a batch with 1 patron
2. Create a layout and set
     the name
     "Print card number as barcode"
     "Barcode type:" to "Code 39"
3. Create a card template without filling anything
4. Export the batch using the layout and template
5. You should have a white page with a barcode
     And no errors in the relevant log file
     This show that this setup isn't completely bogus
     (although Code 39 is the only type working...)
6. Layout: set "Barcode type:" to Industrial2of5
7. Export the batch
8. You should have a white page with no barcode
     And errors in the relevant log file
     "Invalid Characters"
     This is the bug.
9. Layout: set "Barcode type:" to COOP2of5
10. Export the batch
11. You should have a white page with no barcode
      And errors in the relevant log file
      "Invalid Characters"
      This is the bug.
12. Apply this patch
13. Retry with both non-working patches
    1. You should have a white page with a barcode
    2. And no errors in the relevant log file

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 9 Katrin Fischer 2018-07-08 14:58:08 UTC
Updating severity too - not being able to print is not good.
Comment 10 Martin Renvoize 2018-07-12 10:32:44 UTC
Created attachment 76883 [details] [review]
Bug 20084: Fix Industrial2of5 and COOP2of5 patron card layouts

== Test plan ==
This is an oversimplification of a full patron card setup used in production.

1. Create a batch with 1 patron
2. Create a layout and set
     the name
     "Print card number as barcode"
     "Barcode type:" to "Code 39"
3. Create a card template without filling anything
4. Export the batch using the layout and template
5. You should have a white page with a barcode
     And no errors in the relevant log file
     This show that this setup isn't completely bogus
     (although Code 39 is the only type working...)
6. Layout: set "Barcode type:" to Industrial2of5
7. Export the batch
8. You should have a white page with no barcode
     And errors in the relevant log file
     "Invalid Characters"
     This is the bug.
9. Layout: set "Barcode type:" to COOP2of5
10. Export the batch
11. You should have a white page with no barcode
      And errors in the relevant log file
      "Invalid Characters"
      This is the bug.
12. Apply this patch
13. Retry with both non-working patches
    1. You should have a white page with a barcode
    2. And no errors in the relevant log file

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Martin Renvoize 2018-07-12 10:34:20 UTC
Fairly trivial fix, I can't find any regressions and the QA scripts pass..  i'd love to see accompanying tests, but that can wait as we're in C4.

Passing QA.
Comment 12 Nick Clemens 2018-07-13 11:24:58 UTC
Awesome work all!

Pushed to master for 18.11
Comment 13 Martin Renvoize 2018-07-15 18:42:46 UTC
Pushed to 18.05.x for 18.05.02
Comment 14 Fridolin Somers 2018-07-26 08:05:23 UTC
Pushed to 17.11.x, will be in 17.11.08
Comment 15 Katrin Fischer 2018-07-26 09:30:34 UTC
Code 39 + Modulo43 is still not working for me in 16.11 and 17.11. Could someone confirm?
Comment 16 Katrin Fischer 2018-07-26 17:28:26 UTC
Filed a new bug 21123.
Comment 17 Victor Grousset/tuxayo 2018-07-26 18:03:34 UTC
(In reply to Katrin Fischer from comment #15)
> Code 39 + Modulo43 is still not working for me in 16.11 and 17.11. Could
> someone confirm?

Tried the test plan with "Code 39 + Modulo43" and it doesn't work :(

Nor Modulo 10 but that was already the case before.

Either I got confused when trying "Code 39 + Modulo43" or something else happened.
Comment 18 Fridolin Somers 2018-09-24 13:27:42 UTC
I choose no to push to 17.05.x

Continues on Bug 21123.