Bug 23292 - Use TransferSlip when transfering item which is on hold
Summary: Use TransferSlip when transfering item which is on hold
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Johanna Räisä
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-07-10 07:09 UTC by Johanna Räisä
Modified: 2020-10-06 06:52 UTC (History)
4 users (show)

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


Attachments
Bug 23292: Use TransferSlip when transfering item which is on hold (3.12 KB, patch)
2019-07-10 09:43 UTC, Johanna Räisä
Details | Diff | Splinter Review
Bug 23292: Use TransferSlip when transfering item which is on hold (3.37 KB, patch)
2019-09-26 10:37 UTC, Johanna Räisä
Details | Diff | Splinter Review
Bug 23292: Use TransferSlip when transfering item which is on hold (3.43 KB, patch)
2019-09-26 21:13 UTC, Lucas Gass
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Johanna Räisä 2019-07-10 07:09:40 UTC
Now when Koha asks to transfer an item which is on hold it will give HoldSlip to a librarian. We have TransferSlip which is used in other transfering situations, so we should use this also when transfering hold item. The HoldSlip should trigger when the item is checked in after transfer to the right branch.

Combining the hold and transfer slips might get confusing if the slip is used on self-service pickup as we are doing in Finnish public libraries. Also there comes the data protection issue if other patrons can see too much information about holds.
Comment 1 Johanna Räisä 2019-07-10 09:43:44 UTC
Created attachment 91438 [details] [review]
Bug 23292: Use TransferSlip when transfering item which is on hold

Combining hold and transfer slips might get confusing for patrons. Those represent quite different things.
Hold slip could be used on self-service pickup for patrons and transfer slip has information for librarians.
There can also be data protection issue if same slip is used in both cases.

Test plan:
- Apply the patch
- Create hold for a record, define pickup branch different than your current branch.
- Check in an item from your branch.
- Click 'Print slip, transfer, and confirm'
- See that the slip is TRANSFERSLIP
- Change your branch same as hold's pickup branch
- Check in the item again.
- Click 'Print slip and confirm'
- See that the slip is now HOLD_SLIP

Sponsored-by: Koha-Suomi Oy
Comment 2 Lucas Gass 2019-09-20 22:27:13 UTC
Johanna,

I cannot cleanly apply this patch to master.
Comment 3 Johanna Räisä 2019-09-26 10:37:30 UTC
Created attachment 93167 [details] [review]
Bug 23292: Use TransferSlip when transfering item which is on hold

Now the patch should apply to master.
Comment 4 Lucas Gass 2019-09-26 21:13:51 UTC
Created attachment 93177 [details] [review]
Bug 23292: Use TransferSlip when transfering item which is on hold

Combining hold and transfer slips might get confusing for patrons. Those represent quite different things.
Hold slip could be used on self-service pickup for patrons and transfer slip has information for librarians.
There can also be data protection issue if same slip is used in both cases.

Test plan:
- Apply the patch
- Create hold for a record, define pickup branch different than your current branch.
- Check in an item from your branch.
- Click 'Print slip, transfer, and confirm'
- See that the slip is TRANSFERSLIP
- Change your branch same as hold's pickup branch
- Check in the item again.
- Click 'Print slip and confirm'
- See that the slip is now HOLD_SLIP

Sponsored-by: Koha-Suomi Oy

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 5 Sally 2019-09-27 09:58:46 UTC
Changing from HoldSlip to TransferSlip would be a step back for us, as we differentiate between holds and ordinary transfers - which enables us to prioritise how we unpack our stock.

I think the problem described can be resolved with Template Toolkit - you can create an IF this THEN this ELSE this condition in the HoldSlip syntax.

Here's an example of the syntax (which you can then modify with HTML to lay it out correctly):  

[% USE Branches %]
[% loggedinbranchname = Branches.GetName( Branches.GetLoggedInBranchcode() ) %]
[% IF loggedinbranchname == branch.branchname%]
[% hold.expirationdate | $KohaDates %]
[% borrower.surname %] 
[% USE String %]
[% patronsname = borrower.firstname %] 
[% patronsname.substr (0, 1) %]
[% USE String %]
[% patronscard = borrower.cardnumber %] 
*******[% patronscard.substr (7, 3) %]
[% branch.branchname %] 
[% biblio.title %]
[% biblio.author %]
[% item.barcode %]
[% branch.branchname %]
[% today | $KohaDates %]

[% ELSE %]

[% branch.branchcode %]*
[% branch.branchname %]
[% biblio.title %]
[% biblio.author %]
[% item.barcode %]
[% today | $KohaDates %]
[% END %]
Comment 6 Katrin Fischer 2019-10-14 06:16:12 UTC
Johanna, 

seeing Sally's comment: would it be possible to make this behavior optional? I agree that using the transfer slip seems logical to me as well, but as Koha never worked that way, libraries might not be happy about the change and there might be use cases we are missing.

There is also some things missing:
- You change the en installer, but you also need to add to the other installers (there is a unit test helping with that in xt I think)
- Database update statement

Currently your patch would only work for new English installations, need to take care of the other options as well.
Comment 7 Andreas Hedström Mace 2019-10-14 08:14:14 UTC
I still see Johannas approach as the more valid one, especially concerning issues of data privacy. For Sallys use case, I would suggest changing the approach: adding holds information to the transfer slips using the IF statement in the TT syntax, rather than keeping todays behaviour.
Comment 8 Katrin Fischer 2019-10-14 10:06:38 UTC
(In reply to Andreas Hedström Mace from comment #7)
> I still see Johannas approach as the more valid one, especially concerning
> issues of data privacy. For Sallys use case, I would suggest changing the
> approach: adding holds information to the transfer slips using the IF
> statement in the TT syntax, rather than keeping todays behaviour.

I agree with you, but while we think this is better, others might not.

It will also create problems with updates and conflict with our principle of not interrupting workflows on update.

- notices need to be changed and possibly translated
- we can't automatically do that on update, so it would need a big warning sign

That's why I have been asking for making this optional. We could keep the existing behaviour and switch it for new installations. Also - everyone who wants to, can switch as soon as things are configured.

For you suggestion we'd also need to make sure that TRANSFERSLIP has access to the same tables/objects as the current HOLDSLIP.
Comment 9 Andreas Hedström Mace 2019-10-14 10:21:36 UTC
(In reply to Katrin Fischer from comment #8):

Yes, it might make most sense to make it optional. I see your point. I'm just thinking that there are so many system preferences in Koha, that new ones should be added carefully. But here it makes sense. =)
Comment 10 Sally 2019-10-14 10:25:49 UTC
(In reply to Andreas Hedström Mace from comment #7)
> I still see Johannas approach as the more valid one, especially concerning
> issues of data privacy. For Sallys use case, I would suggest changing the
> approach: adding holds information to the transfer slips using the IF
> statement in the TT syntax, rather than keeping todays behaviour.

Thanks Katrin - making it optional would be a good solution.  

Andreas - as long as our workflow can remain the same (it is vital that staff packing and unpacking items in transit can differentiate on the printed slip between low and high priority transfers - i.e. ordinary returns vs items on hold), I don't mind how we approach this.

...but I can't see how to implement your suggestion of using TransferSlip instead of HoldSlip?  The two notices don't share the same object variables - hold and borrower aren't available in TransferSlip, so how would we identify something as being on hold?
Comment 11 Andreas Hedström Mace 2019-10-14 10:30:20 UTC
(In reply to Sally from comment #10)
> ...but I can't see how to implement your suggestion of using TransferSlip
> instead of HoldSlip?  The two notices don't share the same object variables
> - hold and borrower aren't available in TransferSlip, so how would we
> identify something as being on hold?

I had missed that those objects aren't available for the transfer-slip. So making the new bahviour optional as Katrin suggest makes more sense than adding more objects to the transfer slips.
Comment 12 Johanna Räisä 2019-10-22 12:59:58 UTC
Ok, I will make this optional and looking those other things mentioned.