Bug 35714 - Clicking Print slips when no letter template selected causes error
Summary: Clicking Print slips when no letter template selected causes error
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Preservation (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-01-05 15:53 UTC by Aude Charillon
Modified: 2024-05-28 16:55 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00,23.11.06
Circulation function:


Attachments
Bug 35714: Prevent items not linked with processing with a letter code a to be printed (4.32 KB, patch)
2024-01-09 15:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 35714: Prevent items not linked with processing with a letter code a to be printed (4.38 KB, patch)
2024-02-22 22:02 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 35714: Prevent items not linked with processing with a letter code a to be printed (4.43 KB, patch)
2024-04-02 18:22 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Aude Charillon 2024-01-05 15:53:55 UTC
In the preservation module, a "Letter template for printing slip" can be selected when creating a processing. This is not a required field; the field is blank by default.

When viewing a train that uses a processing where no letter template have been specified, I still see the Print slips button in the Items table actions. When I select items, the button becomes clickable. When I click on it, I get an error page (probably because there is no specified notice for Koha to use!)

On master, the error page starts with the lines:
Error trace
No letter_code at /kohadevbox/koha/preservation/print_slip.pl line 45.
 at /usr/lib/aarch64-linux-gnu/perl-base/Carp.pm line 289

On a system recently upgraded to 23.11, I am getting a 500 error at /cgi-bin/koha/preservation/print_slip.pl?train_item_id=1&train_item_id=2_blank

It would be better if either:
- the "Letter template for printing slip" field had the Train item slip selected by default;
- or, the Print slip button didn't appear at all when no letter templates are selected in the processing settings.
Comment 1 Jonathan Druart 2024-01-09 15:52:25 UTC
Created attachment 160707 [details] [review]
Bug 35714: Prevent items not linked with processing with a letter code a to be printed

Items linked with a processing which does not have a letter code are not
supposed to be printed.
Currently the code is allowing to "print slip" for items when using the
link on top of the table. It results in an error "No letter_code at
/kohadevbox/koha/preservation/print_slip.pl line 45."

Test plan:
Create a processing without a letter code
Add 2 items to the waiting list then add them to a train
Go to the "show train" view
=> notice that you cannot print slip for those items
Create another processing with a letter code
Edit one of the item and set it the new processing
=> A slip for this item can be printed
Edit the other item and the train to make them use the new processing
=> A slip for those items can be printed
Comment 2 Laura Escamilla 2024-01-12 15:30:17 UTC
Hi! 

Thank you for working on a patch. I've run into a few things when testing it out. 

1. I set up test process 1 that does not include a letter template. After applying the patch I am still seeing the option to "Print slip" at the top of the table. When I click on that option I still see the "Error trace
No letter_code at /kohadevbox/koha/preservation/print_slip.pl line 45.
 at /usr/lib/aarch64-linux-gnu/perl-base/Carp.pm line 289" error. 

2. Next I created a process with a letter code. I changed my item's process to the new one (test process 2). When I submit my change it breaks the item_list table. Essentially it removes the columns and rows and only shows a number, the pencil for editing, the trash can for deleting and the printer symbol for printing the slips. All symbols work and I can print the slip -- but the table needs to be adjusted.
Comment 3 Jonathan Druart 2024-01-12 15:42:47 UTC
(In reply to Laura Escamilla from comment #2)
> Hi! 
> 
> Thank you for working on a patch. I've run into a few things when testing it
> out. 
> 
> 1. I set up test process 1 that does not include a letter template. After
> applying the patch I am still seeing the option to "Print slip" at the top
> of the table. When I click on that option I still see the "Error trace
> No letter_code at /kohadevbox/koha/preservation/print_slip.pl line 45.
>  at /usr/lib/aarch64-linux-gnu/perl-base/Carp.pm line 289" error. 

Can you confirm that you have run `yarn js:build` after you applied the patch?

> 2. Next I created a process with a letter code. I changed my item's process
> to the new one (test process 2). When I submit my change it breaks the
> item_list table. Essentially it removes the columns and rows and only shows
> a number, the pencil for editing, the trash can for deleting and the printer
> symbol for printing the slips. All symbols work and I can print the slip --
> but the table needs to be adjusted.

Yes, if at least one item has a process different that the default (the one defined at the train level), then the items are not displayed in a table.
This is because we cannot display several items from different process (and so different attributes/columns) in the same table.
Comment 4 Laura Escamilla 2024-01-12 15:47:01 UTC Comment hidden (obsolete)
Comment 5 Laura Escamilla 2024-01-12 15:55:00 UTC
1. Can you confirm that you have run `yarn js:build` after you applied the
patch?
--Yes I ran the yarn build and I am still seeing errors when trying to print slips with processes that do not have a letter code. 
 

2. Yes, if at least one item has a process different that the default (the one
defined at the train level), then the items are not displayed in a table.
This is because we cannot display several items from different process (and
so different attributes/columns) in the same table.

--That makes sense! I missed the step of changing my default process on the train when I changed it for the item. Maybe we could add a warning for libraries that says something along the lines of "New item process does not match Train xx's process."? I can see how some staff members might miss this step.
Comment 6 Jonathan Druart 2024-01-12 15:59:22 UTC
(In reply to Laura Escamilla from comment #5)
> 1. Can you confirm that you have run `yarn js:build` after you applied the
> patch?
> --Yes I ran the yarn build and I am still seeing errors when trying to print
> slips with processes that do not have a letter code. 

Please make sure your browser's cache is correctly flushed. You are not supposed to be able to print slip an item if it has a process without a letter template.

If you still recreate can you screenshot and share it please?

> 2. Yes, if at least one item has a process different that the default (the
> one
> defined at the train level), then the items are not displayed in a table.
> This is because we cannot display several items from different process (and
> so different attributes/columns) in the same table.
> 
> --That makes sense! I missed the step of changing my default process on the
> train when I changed it for the item. Maybe we could add a warning for
> libraries that says something along the lines of "New item process does not
> match Train xx's process."? I can see how some staff members might miss this
> step.

I will be happy to implement your suggestion if you open a new bug report.
Comment 7 Jonathan Druart 2024-01-12 16:00:40 UTC
(In reply to Jonathan Druart from comment #6)
> Please make sure your browser's cache is correctly flushed. You are not
> supposed to be able to print slip an item if it has a process without a
> letter template.

ie. checkboxes should be disabled.
Comment 8 Victor Grousset/tuxayo 2024-02-22 22:02:05 UTC
Created attachment 162350 [details] [review]
Bug 35714: Prevent items not linked with processing with a letter code a to be printed

Items linked with a processing which does not have a letter code are not
supposed to be printed.
Currently the code is allowing to "print slip" for items when using the
link on top of the table. It results in an error "No letter_code at
/kohadevbox/koha/preservation/print_slip.pl line 45."

Test plan:
Create a processing without a letter code
Add 2 items to the waiting list then add them to a train
Go to the "show train" view
=> notice that you cannot print slip for those items
Create another processing with a letter code
Edit one of the item and set it the new processing
=> A slip for this item can be printed
Edit the other item and the train to make them use the new processing
=> A slip for those items can be printed

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 9 Victor Grousset/tuxayo 2024-02-22 22:10:06 UTC
It works :)

----

(hmm, when going back to waiting list to make the 2nd processing, I got "you need to configure this module first" I refreshed and it went again, weird, not related to this ticket but leaving that in case that happens to someone else and we can identify the issue)
Comment 10 Nick Clemens (kidclamp) 2024-04-02 18:22:08 UTC
Created attachment 164292 [details] [review]
Bug 35714: Prevent items not linked with processing with a letter code a to be printed

Items linked with a processing which does not have a letter code are not
supposed to be printed.
Currently the code is allowing to "print slip" for items when using the
link on top of the table. It results in an error "No letter_code at
/kohadevbox/koha/preservation/print_slip.pl line 45."

Test plan:
Create a processing without a letter code
Add 2 items to the waiting list then add them to a train
Go to the "show train" view
=> notice that you cannot print slip for those items
Create another processing with a letter code
Edit one of the item and set it the new processing
=> A slip for this item can be printed
Edit the other item and the train to make them use the new processing
=> A slip for those items can be printed

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 11 Katrin Fischer 2024-04-05 09:52:28 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 12 Fridolin Somers 2024-05-23 09:31:08 UTC
Pushed to 23.11.x for 23.11.06
Comment 13 Lucas Gass (lukeg) 2024-05-28 16:55:50 UTC
Not needed in 23.05.x, no backport.