Bug 12224

After talking with our staff, it's enough that the slip can be printed from the patron detail page - changed bug description accordingly. Patch to follow.

Patch tested with a sandbox, by Aleisha <aleishaamohia@hotmail.com>

I really like this new feature and it works pretty well already. I have some small things for a follow-up:

1) Entry in the print pull down in circ
After checking with a native speaker (I didn't want to suggest something wrong) I think "Print check-in slip" or "Print today's check-in slip" would be better than "Print checked-in today -slip".

2) The new notice template formats the title information quite differently to the "check-out" slip, here for comparison:

ISSUESLIP:
<checkedout>
<p>
<<biblio.title>> <br />
Barcode: <<items.barcode>><br />
Date due: <<issues.date_due>><br />
</p>
</checkedout>

New CHECKINSLIP:

<checkedin>
<li><<biblio.author>>: <<biblio.title>><br>
Barcode: <<items.barcode>><br>
</checkedin>

Not going to argue about <p> vs. <li>, although I personally prefer the first, but I think the <<biblio.author>>: is problematic. Lots of records don't have a first author in 100 and then the printed information is a bit ugly. If you want t include the author. Maybe just having the title and no : is a bit safer here.
But this is not a blocker, just noticing as my example record didn't have an author.

3) Please include the new sample template in the installers for the other languages. Without the template the print will fail and people won't know the code to use for the new notice. Better to have an English template that can be easily modified.

4) Misses unit tests for the new routines...

Would be great to see this moving.

(In reply to Katrin Fischer from comment #6)
> Would be great to see this moving.

It's starting to move. Follow-up coming soon! :)

With the follow-up version of the patch the unit test might fail if the test is run at 12 +- 0.0001s AM/PM. What shall we do with this? It would be great to keep those unit tests. Maybe we could change the function GetTodaysReturnsForBorrower to accept a day from which the returns should be retrieved, and also function CheckInSlip to accept a day from which to retrieve the slip. Then in the unit test we could specify the day.. And I think in this way the code would be more generalized. 

Btw, i'm thinking of integrating the function CheckInSlip with IssueSlip, so another follow-up maybe coming soon!

(In reply to Joonas Kylmälä from comment #10)
> With the follow-up version of the patch the unit test might fail if the test
> is run at 12 +- 0.0001s AM/PM. What shall we do with this? It would be great
> to keep those unit tests. Maybe we could change the function
> GetTodaysReturnsForBorrower to accept a day from which the returns should be
> retrieved, and also function CheckInSlip to accept a day from which to
> retrieve the slip. Then in the unit test we could specify the day.. And I
> think in this way the code would be more generalized. 

I have fixed the same kind of problem on bug 15029.

Actually it's the same script :)
You should provide a patch on top of it.

(In reply to Jonathan Druart from comment #12)
> Actually it's the same script :)
> You should provide a patch on top of it.

Or just rebase since it's pushed to master.

Created attachment 44582 [details] [review]
Bug 12224: Allow easy printing of patron check-ins for today

Let's try rebasing for the third time ! :)

This adds a printing option to the staff client patron details page
to print a list of items the patron returned today at this branch.

To test:

1) Apply patch.
2) Check in tools > Notices & Slips that you have CHECKINSLIP slip,
   and have print message there. If not, run updatedatabase.pl
3) Check-in items for a patron.
4) Go to that patron's detail page, and from the Print-menu in
   the toolbar, select "Print checked-in today -slip"
5) You should get a slip of the patron's checked-in items.
6) Optionally, test Check-ins for the same patron, but in different
   branches - should only print items for the current branch.
7) Optionally, test Check-ins for the same patron, but returned
   in different day - should only print items for today.

Signed-off-by: Aleisha <aleishaamohia@hotmail.com>

Created attachment 44584 [details] [review]
Bug 12224: (follow-up) Allow easy printing of patron check-ins for today

Rebased version

This adds a printing option to the staff client patron details page
to print a list of items the patron returned today at this branch.

To test:

1) Apply patch.
2) Check in tools > Notices & Slips that you have CHECKINSLIP slip,
   and have print message there. If not, run updatedatabase.pl
3) Check-in items for a patron.
4) Go to that patron's detail page, and from the Print-menu in
   the toolbar, select "Print check-in slip"
5) You should get a slip of the patron's checked-in items.
6) Optionally, test Check-ins for the same patron, but in different
   branches - should only print items for the current branch.
7) Optionally, test Check-ins for the same patron, but returned
   in different day - should only print items for today.
8) Run unit test t/db_dependent/Members/IssueSlip.t

These patches should be now ready to push to master. But maybe one more sign-off would be in place!

You add the new CHECKINSLIP to : installer/data/mysql/en/mandatory/sample_notices.sql

As suggested by Katrin in comment 5, you should add this also in other languages samples notices: a cut-and-paste of the English version is enough.

t/db_dependent/Members/IssueSlip.t raises warning about masking variables earlier declaration, and the test fails (for me).

CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl
Auto-merging installer/data/mysql/en/mandatory/sample_notices.sql
CONFLICT (content): Merge conflict in installer/data/mysql/en/mandatory/sample_notices.sql

Hello guys,

Any follow-up on this?
It seems it has not been sucessfully added to master branch.

Thanks,

Pedro Amorim

Hi Prdro,
this has the status Failed QA - it means that it needs more work to get into the mastr branch. Until someone invests some time here - it won't get there.

*** Bug 18404 has been marked as a duplicate of this bug. ***

Created attachment 91476 [details] [review]
Bug 12224: Allow easy printing of patron check-ins for today

Fixed patch to apply master version.

Created attachment 91477 [details] [review]
Bug 12224: (follow-up) Allow easy printing of patron check-ins for today

Fixed patch to apply master version.

Created attachment 91478 [details] [review]
Bug 12224: (follow-up) Allow easy printing of patron check-ins for today

This is adds sample notice for other languages.

Sponsored-by: Koha-Suomi Oy

Tested and works properly.

Created attachment 96970 [details] [review]
Bug 12224: (follow-up) Allow easy printing of patron check-ins for today

This is adds sample notice for other languages.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Guillaume Paquet <guillaume@inlibro.com>

Created attachment 96972 [details] [review]
Bug 12224: (follow-up) Allow easy printing of patron check-ins for today

This is adds sample notice for other languages.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Guillaume Paquet <guillaume@inlibro.com>

Hi Johanna,

can you please rebase?

Small thing:
<li><a id="printcheckinslip" href="#">Print checked-in today -slip</a></li>

I think I'd just label that as 'Print checkin slip'

Created attachment 110308 [details] [review]
Bug 12224: Print a slip of borrower's checkins for today

To test:

1) Apply patch and run installer
2) Check out two items (or more) to a borrower
3) Check in two items (or more) for this borrower (you should end up on
    circ/returns.pl)
4) Notice the checked-in items table has started building. There should
be a new column with the button 'Print checkin slip' in each row.
5) Click the button and confirm the checkin slip opens for printing in a
new window.
6) Go to the patron's account
7) Click the Print dropdown. Confirm there is a 'Print checkin slip'
option. Click this and confirm the checkin slip opens for printing in a
new window.

Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ)

Created attachment 110395 [details] [review]
Bug 12224: Print a slip of borrower's checkins for today

To test:

1) Apply patch and run installer
2) Check out two items (or more) to a borrower
3) Check in two items (or more) for this borrower (you should end up on
    circ/returns.pl)
4) Notice the checked-in items table has started building. There should
be a new column with the button 'Print checkin slip' in each row.
5) Click the button and confirm the checkin slip opens for printing in a
new window.
6) Go to the patron's account
7) Click the Print dropdown. Confirm there is a 'Print checkin slip'
option. Click this and confirm the checkin slip opens for printing in a
new window.

Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ)

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Created attachment 110396 [details] [review]
Bug 12224: (follow-up) Move button to patron column in checkins table

This patch moves the "Print checkin slip" out of its own column and into
the colum containing patron information. I think this makes more sense
because the slip is specific to a patron, not the individual item which
has been checked in.

Also changed: printcheckinslip function should be triggered via a class,
not a non-unique ID. The function should handle the click using
"preventDefault" rather than "return false."

To test, apply the patch and check in some items checked out to various
patrons.

- In the table of checked-in items, confirm that the "Print checkin
  slip" appears correctly in the column of patron information.
- Confirm that the button works correctly to print the correct slip for
  the associated patron.

Created attachment 110403 [details] [review]
Bug 12224: Print a slip of borrower's checkins for today

To test:

1) Apply patch and run installer
2) Check out two items (or more) to a borrower
3) Check in two items (or more) for this borrower (you should end up on
    circ/returns.pl)
4) Notice the checked-in items table has started building. There should
be a new column with the button 'Print checkin slip' in each row.
5) Click the button and confirm the checkin slip opens for printing in a
new window.
6) Go to the patron's account
7) Click the Print dropdown. Confirm there is a 'Print checkin slip'
option. Click this and confirm the checkin slip opens for printing in a
new window.

Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ)

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 110404 [details] [review]
Bug 12224: (follow-up) Move button to patron column in checkins table

This patch moves the "Print checkin slip" out of its own column and into
the colum containing patron information. I think this makes more sense
because the slip is specific to a patron, not the individual item which
has been checked in.

Also changed: printcheckinslip function should be triggered via a class,
not a non-unique ID. The function should handle the click using
"preventDefault" rather than "return false."

To test, apply the patch and check in some items checked out to various
patrons.

- In the table of checked-in items, confirm that the "Print checkin
  slip" appears correctly in the column of patron information.
- Confirm that the button works correctly to print the correct slip for
  the associated patron.

Signed-off-by: David Nind <david@davidnind.com>

Hi Aleisha,

this already works great - thx for thinking about all the installers too. Only one small glitch: When editing the notice in tools, you can add fields from issues, but we need old_issues here as the line has alredy been removed from the issues table (this already works for CHECKOUT and CHECKIN notices). 

Can you pleas provide a small follow-up?

Created attachment 110516 [details] [review]
Bug 12224: (follow-up) Access old_issues table when editing notice

This QA follow-up allows access to the old_issues table when editing the
notice in Tools.

Created attachment 110529 [details] [review]
Bug 12224: Print a slip of borrower's checkins for today

To test:

1) Apply patch and run installer
2) Check out two items (or more) to a borrower
3) Check in two items (or more) for this borrower (you should end up on
    circ/returns.pl)
4) Notice the checked-in items table has started building. There should
be a new column with the button 'Print checkin slip' in each row.
5) Click the button and confirm the checkin slip opens for printing in a
new window.
6) Go to the patron's account
7) Click the Print dropdown. Confirm there is a 'Print checkin slip'
option. Click this and confirm the checkin slip opens for printing in a
new window.

Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ)

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Alexandra Speer <alexandra.speer@bsz-bw.de>

Created attachment 110530 [details] [review]
Bug 12224: (follow-up) Move button to patron column in checkins table

This patch moves the "Print checkin slip" out of its own column and into
the colum containing patron information. I think this makes more sense
because the slip is specific to a patron, not the individual item which
has been checked in.

Also changed: printcheckinslip function should be triggered via a class,
not a non-unique ID. The function should handle the click using
"preventDefault" rather than "return false."

To test, apply the patch and check in some items checked out to various
patrons.

- In the table of checked-in items, confirm that the "Print checkin
  slip" appears correctly in the column of patron information.
- Confirm that the button works correctly to print the correct slip for
  the associated patron.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Alexandra Speer <alexandra.speer@bsz-bw.de>

Created attachment 110531 [details] [review]
Bug 12224: (follow-up) Access old_issues table when editing notice

This QA follow-up allows access to the old_issues table when editing the
notice in Tools.

Signed-off-by: Alexandra Speer <alexandra.speer@bsz-bw.de>

Created attachment 110576 [details] [review]
Bug 12224: Print a slip of borrower's checkins for today

To test:

1) Apply patch and run installer
2) Check out two items (or more) to a borrower
3) Check in two items (or more) for this borrower (you should end up on
    circ/returns.pl)
4) Notice the checked-in items table has started building. There should
be a new column with the button 'Print checkin slip' in each row.
5) Click the button and confirm the checkin slip opens for printing in a
new window.
6) Go to the patron's account
7) Click the Print dropdown. Confirm there is a 'Print checkin slip'
option. Click this and confirm the checkin slip opens for printing in a
new window.

Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ)

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Alexandra Speer <alexandra.speer@bsz-bw.de>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Created attachment 110577 [details] [review]
Bug 12224: (follow-up) Move button to patron column in checkins table

This patch moves the "Print checkin slip" out of its own column and into
the colum containing patron information. I think this makes more sense
because the slip is specific to a patron, not the individual item which
has been checked in.

Also changed: printcheckinslip function should be triggered via a class,
not a non-unique ID. The function should handle the click using
"preventDefault" rather than "return false."

To test, apply the patch and check in some items checked out to various
patrons.

- In the table of checked-in items, confirm that the "Print checkin
  slip" appears correctly in the column of patron information.
- Confirm that the button works correctly to print the correct slip for
  the associated patron.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Alexandra Speer <alexandra.speer@bsz-bw.de>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Created attachment 110578 [details] [review]
Bug 12224: (follow-up) Access old_issues table when editing notice

This QA follow-up allows access to the old_issues table when editing the
notice in Tools.

Signed-off-by: Alexandra Speer <alexandra.speer@bsz-bw.de>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Should not we use TT syntax for new notice templates?

IMO we should mimic (completely) what is done for ISSUEQSLIP in C4::Members::IssueSlip but remove the historical syntax part to only keep the TT syntax.

Created attachment 114121 [details] [review]
Bug 12224: Print a slip of borrower's checkins for today

To test:

1) Apply patch and run installer
2) Check out two items (or more) to a borrower
3) Check in two items (or more) for this borrower (you should end up on
    circ/returns.pl)
4) Notice the checked-in items table has started building. There should
be a new column with the button 'Print checkin slip' in each row.
5) Click the button and confirm the checkin slip opens for printing in a
new window.
6) Go to the patron's account
7) Click the Print dropdown. Confirm there is a 'Print checkin slip'
option. Click this and confirm the checkin slip opens for printing in a
new window.

Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ)

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Alexandra Speer <alexandra.speer@bsz-bw.de>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Created attachment 114122 [details] [review]
Bug 12224: (follow-up) Move button to patron column in checkins table

This patch moves the "Print checkin slip" out of its own column and into
the colum containing patron information. I think this makes more sense
because the slip is specific to a patron, not the individual item which
has been checked in.

Also changed: printcheckinslip function should be triggered via a class,
not a non-unique ID. The function should handle the click using
"preventDefault" rather than "return false."

To test, apply the patch and check in some items checked out to various
patrons.

- In the table of checked-in items, confirm that the "Print checkin
  slip" appears correctly in the column of patron information.
- Confirm that the button works correctly to print the correct slip for
  the associated patron.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Alexandra Speer <alexandra.speer@bsz-bw.de>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Created attachment 114123 [details] [review]
Bug 12224: (follow-up) Access old_issues table when editing notice

This QA follow-up allows access to the old_issues table when editing the
notice in Tools.

Signed-off-by: Alexandra Speer <alexandra.speer@bsz-bw.de>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Created attachment 114124 [details] [review]
Bug 12224: (follow-up) Switching to TT notice

Followed test plan(s), "Print checkin slip" buttons work and new window is opened for printing. However list of checked in items isn't printed.

(In reply to Emmi Takkinen from comment #50)
> Followed test plan(s), "Print checkin slip" buttons work and new window is
> opened for printing. However list of checked in items isn't printed.

Hi Emmi, the list of checked in items does print for me, can you please let me know what steps you're taking?

(In reply to Aleisha Amohia from comment #51)
> (In reply to Emmi Takkinen from comment #50)
> > Followed test plan(s), "Print checkin slip" buttons work and new window is
> > opened for printing. However list of checked in items isn't printed.
> 
> Hi Emmi, the list of checked in items does print for me, can you please let
> me know what steps you're taking?

Tested again, first three patches work fine. After applying "Bug 12224: (follow-up) Switching to TT notice" and repeating test plan from first patch, just this is printed:

Library
Checked in items for Patron(123)
12/16/2020 14:20
Checked in today
 
Did some checking and it seems problem comes from this:

my %loops = (
 old_issues => [ map { $_->issue_id } @todays_checkins ],
);

This ends up to _get_tt_params where old_issues uses fk => itemnumber to search and fill TT params not issue_id which it receives.

Created attachment 116135 [details] [review]
Bug 12224: (follow-up) Loop old_issues with itemnumber

Thank you for the helpful comment Emmi, hopefully the latest follow-up solves the problem.

Aleisha

Created attachment 116482 [details] [review]
Bug 12224: (follow-up) Loop old_issues with itemnumber

Signed-off-by: Christian <christian.stelzenmueller@bsz-bw.de>

Created attachment 116485 [details] [review]
Bug 12224: Print a slip of borrower's checkins for today

To test:

1) Apply patch and run installer
2) Check out two items (or more) to a borrower
3) Check in two items (or more) for this borrower (you should end up on
    circ/returns.pl)
4) Notice the checked-in items table has started building. There should
be a new column with the button 'Print checkin slip' in each row.
5) Click the button and confirm the checkin slip opens for printing in a
new window.
6) Go to the patron's account
7) Click the Print dropdown. Confirm there is a 'Print checkin slip'
option. Click this and confirm the checkin slip opens for printing in a
new window.

Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ)

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Alexandra Speer <alexandra.speer@bsz-bw.de>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>

Created attachment 116486 [details] [review]
Bug 12224: (follow-up) Move button to patron column in checkins table

This patch moves the "Print checkin slip" out of its own column and into
the colum containing patron information. I think this makes more sense
because the slip is specific to a patron, not the individual item which
has been checked in.

Also changed: printcheckinslip function should be triggered via a class,
not a non-unique ID. The function should handle the click using
"preventDefault" rather than "return false."

To test, apply the patch and check in some items checked out to various
patrons.

- In the table of checked-in items, confirm that the "Print checkin
  slip" appears correctly in the column of patron information.
- Confirm that the button works correctly to print the correct slip for
  the associated patron.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Alexandra Speer <alexandra.speer@bsz-bw.de>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>

Created attachment 116487 [details] [review]
Bug 12224: (follow-up) Access old_issues table when editing notice

This QA follow-up allows access to the old_issues table when editing the
notice in Tools.

Signed-off-by: Alexandra Speer <alexandra.speer@bsz-bw.de>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>

Created attachment 116488 [details] [review]
Bug 12224: (follow-up) Switching to TT notice

Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>

Created attachment 116489 [details] [review]
Bug 12224: (follow-up) Loop old_issues with itemnumber

Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>

Fixed the sign-off lines from the sandboxes.

Created attachment 117245 [details] [review]
Bug 12224: Print a slip of borrower's checkins for today

To test:

1) Apply patch and run installer
2) Check out two items (or more) to a borrower
3) Check in two items (or more) for this borrower (you should end up on
    circ/returns.pl)
4) Notice the checked-in items table has started building. There should
be a new column with the button 'Print checkin slip' in each row.
5) Click the button and confirm the checkin slip opens for printing in a
new window.
6) Go to the patron's account
7) Click the Print dropdown. Confirm there is a 'Print checkin slip'
option. Click this and confirm the checkin slip opens for printing in a
new window.

Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ)
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Alexandra Speer <alexandra.speer@bsz-bw.de>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 117246 [details] [review]
Bug 12224: (follow-up) Move button to patron column in checkins table

This patch moves the "Print checkin slip" out of its own column and into
the colum containing patron information. I think this makes more sense
because the slip is specific to a patron, not the individual item which
has been checked in.

Also changed: printcheckinslip function should be triggered via a class,
not a non-unique ID. The function should handle the click using
"preventDefault" rather than "return false."

To test, apply the patch and check in some items checked out to various
patrons.

- In the table of checked-in items, confirm that the "Print checkin
  slip" appears correctly in the column of patron information.
- Confirm that the button works correctly to print the correct slip for
  the associated patron.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Alexandra Speer <alexandra.speer@bsz-bw.de>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 117247 [details] [review]
Bug 12224: (follow-up) Access old_issues table when editing notice

This QA follow-up allows access to the old_issues table when editing the
notice in Tools.

Signed-off-by: Alexandra Speer <alexandra.speer@bsz-bw.de>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 117248 [details] [review]
Bug 12224: (follow-up) Switching to TT notice

Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 117249 [details] [review]
Bug 12224: (follow-up) Loop old_issues with itemnumber

Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 117250 [details] [review]
Bug 12224: (QA follow-up) Ensure patron language is respected

Things have moved on a bit since this code was first written, we now
expect a 'lang' parameter passed into GetPrepparedLetter to allow for
translated notices.

Simple fix, so a quick QA follow-up will suffice :)

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

I've added a minor QA follow-up to pass patron language into GetPreparedLetter as expected.

This is a long overdue enhancement and is great to see making it's way into Koha.

I would love to see the database call moved into Koha::Patron as a method (->todays_checkins perhaps) and as such have a unit test associated with it.. but we can handle that in another bug.

QA script is happy, no regressions found.

Passing QA :)

(In reply to Martin Renvoize from comment #68)
> I would love to see the database call moved into Koha::Patron as a method
> (->todays_checkins perhaps) and as such have a unit test associated with
> it.. but we can handle that in another bug.

That's (almost) exactly what I was coming to write.

There is already kind of the same code in members/readingrec.pl (op=export_barcodes).

We should provide a Koha::Old::Checkouts->filter_by_todays_checkins method and use it in both places.

I opened bug 27851. Please help testing it when patches will be attached!

Created attachment 117620 [details] [review]
Bug 12224: Some code improvements

* Use Koha::Patrons->old_checkouts
* perltidy
* Make sure $today_start and $today_end will always be on the same day
(we never know!)

Should not we format the date (today) using the $KohaDates TT filter?

Created attachment 117812 [details] [review]
Bug 12224: Print a slip of borrower's checkins for today

To test:

1) Apply patch and run installer
2) Check out two items (or more) to a borrower
3) Check in two items (or more) for this borrower (you should end up on
    circ/returns.pl)
4) Notice the checked-in items table has started building. There should
be a new column with the button 'Print checkin slip' in each row.
5) Click the button and confirm the checkin slip opens for printing in a
new window.
6) Go to the patron's account
7) Click the Print dropdown. Confirm there is a 'Print checkin slip'
option. Click this and confirm the checkin slip opens for printing in a
new window.

Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ)
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Alexandra Speer <alexandra.speer@bsz-bw.de>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Created attachment 117813 [details] [review]
Bug 12224: (follow-up) Move button to patron column in checkins table

This patch moves the "Print checkin slip" out of its own column and into
the colum containing patron information. I think this makes more sense
because the slip is specific to a patron, not the individual item which
has been checked in.

Also changed: printcheckinslip function should be triggered via a class,
not a non-unique ID. The function should handle the click using
"preventDefault" rather than "return false."

To test, apply the patch and check in some items checked out to various
patrons.

- In the table of checked-in items, confirm that the "Print checkin
  slip" appears correctly in the column of patron information.
- Confirm that the button works correctly to print the correct slip for
  the associated patron.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Alexandra Speer <alexandra.speer@bsz-bw.de>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Created attachment 117814 [details] [review]
Bug 12224: (follow-up) Access old_issues table when editing notice

This QA follow-up allows access to the old_issues table when editing the
notice in Tools.

Signed-off-by: Alexandra Speer <alexandra.speer@bsz-bw.de>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Created attachment 117815 [details] [review]
Bug 12224: (follow-up) Switching to TT notice

Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Created attachment 117816 [details] [review]
Bug 12224: (follow-up) Loop old_issues with itemnumber

Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Created attachment 117817 [details] [review]
Bug 12224: (QA follow-up) Ensure patron language is respected

Things have moved on a bit since this code was first written, we now
expect a 'lang' parameter passed into GetPrepparedLetter to allow for
translated notices.

Simple fix, so a quick QA follow-up will suffice :)

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Created attachment 117818 [details] [review]
Bug 12224: Some code improvements

* Use Koha::Patrons->old_checkouts
* perltidy
* Make sure $today_start and $today_end will always be on the same day
(we never know!)

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Created attachment 117819 [details] [review]
Bug 12224: (follow-up) Format today with KohaDates

Updates the slip templates to use [% today | $KohaDates %].
Adding an explicit USE statement is not necessary in this case.

To test:
- When testing the feature according to the test plan, make sure
  that the date for today shows up formatted to the DateFormat
  system preference.

Note: As we are printing today's checkins, I have intentionally left
oft the time. But it could be easily added if a library wanted it.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Pushed to master for 21.05, thanks to everybody involved!

> Signed-off-by: Alexandra Speer <alexandra.speer@bsz-bw.de>

Congrats Alexandra for your first signoff!

I think this could be backported to 20.11.x.
What do you think ?

(In reply to Fridolin Somers from comment #83)
> I think this could be backported to 20.11.x.
> What do you think ?

I think it would probably be okay but usually want to avoid backporting enhancements in case they introduce new bugs to a stable version.

Ok for me, no backport.

Not pushed to 20.11.x

This leaves the existing updates after upgrade with broken check-in receipt printing because the existing template is not updated automatically to the new syntax. I think we need to add a big warning here during upgrade similarly to the patch attached in this patch of bug 26734 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26734#c97 The release note text here should be also updated with a warning about this. Also maybe we should get the action log backup mechanism of the old template from bug 26734 here as well, now the user doesn't know what the new syntax is without delving into the Koha code.

(In reply to Joonas Kylmälä from comment #86)
> This leaves the existing updates after upgrade with broken check-in receipt
> printing because the existing template is not updated automatically to the
> new syntax. I think we need to add a big warning here during upgrade
> similarly to the patch attached in this patch of bug 26734
> https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26734#c97 The
> release note text here should be also updated with a warning about this.
> Also maybe we should get the action log backup mechanism of the old template
> from bug 26734 here as well, now the user doesn't know what the new syntax
> is without delving into the Koha code.

Hi Joonas,

there is no existing template for slip printing? Could it be a customization?

The CHECKINSLIP slip is brand new. Can you verify which notice you are talking about? Do you mean the CHECKIN email notice?

(In reply to Katrin Fischer from comment #87)
> there is no existing template for slip printing? Could it be a customization?
> 
> The CHECKINSLIP slip is brand new. Can you verify which notice you are
> talking about? Do you mean the CHECKIN email notice?

Your right so sorry about that, this patch was already applied in my testing and the follow-ups here changed the syntax, sorry about the false alarm!

> Your right so sorry about that, this patch was already applied in my testing
> and the follow-ups here changed the syntax, sorry about the false alarm!

If just all bugs turned out to be false alarms that easily... :)