Bug 8688 - Add a recall button to the biblio record details page.
Summary: Add a recall button to the biblio record details page.
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Frédérick Capovilla
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-08-24 19:03 UTC by Frédérick Capovilla
Modified: 2022-06-07 20:31 UTC (History)
20 users (show)

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


Attachments
Add a recall button to the biblio record details page. (7.03 KB, patch)
2012-08-24 19:03 UTC, Frédérick Capovilla
Details | Diff | Splinter Review
Add a default RECALL notice to the database (5.16 KB, patch)
2012-08-24 19:04 UTC, Frédérick Capovilla
Details | Diff | Splinter Review
[SIGNED-OFF] Add a recall button to the biblio record details page. (7.26 KB, patch)
2012-08-31 13:44 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Add a default RECALL notice to the database (5.21 KB, patch)
2012-08-31 13:44 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 8688 - Followup - Fix permissions for send_recall_notice.pl (1.13 KB, patch)
2012-08-31 13:45 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 8688 - Followup - Append button to the "checked out" information. (3.45 KB, patch)
2012-08-31 14:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Add a Recall button to the biblio record details page. (Fused patch + syspref) (14.57 KB, patch)
2013-04-26 21:50 UTC, Frédérick Capovilla
Details | Diff | Splinter Review
Bug 8688 - Add a Recall button to the biblio record details page. (14.37 KB, patch)
2015-03-09 14:03 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 8688 - Add a Recall button to the biblio record details page. (14.73 KB, patch)
2015-04-18 03:15 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 8688: Cleaned up pointless parameter passing (1.94 KB, patch)
2015-04-18 19:38 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 8688: Update License, Perlcritic -3 friendly (2.98 KB, patch)
2015-04-18 19:53 UTC, Mark Tompsett
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8688: Add a Recall button to the biblio record details page. (16.10 KB, patch)
2015-05-14 00:36 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8688: Cleaned up pointless parameter passing (2.02 KB, patch)
2015-05-14 00:37 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8688: Update License, Perlcritic -3 friendly (3.05 KB, patch)
2015-05-14 00:37 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 8688: (follow-up) Fix quotes (1.23 KB, patch)
2015-05-14 00:38 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 8688: (follow-up 2) Add notice to all langs and fix permission (34.75 KB, patch)
2015-05-14 00:38 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[Signed-off] Bug 8688: (follow-up) Fix quotes (1.42 KB, patch)
2015-12-31 15:34 UTC, Marc Véron
Details | Diff | Splinter Review
(Signed-off) Bug 8688: (follow-up 2) Add notice to all langs and fix permission (36.93 KB, patch)
2015-12-31 15:44 UTC, Marc Véron
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Frédérick Capovilla 2012-08-24 19:03:14 UTC Comment hidden (obsolete)
Comment 1 Frédérick Capovilla 2012-08-24 19:04:55 UTC Comment hidden (obsolete)
Comment 2 Owen Leonard 2012-08-28 20:11:15 UTC
Instead of a new column with a button why not append a link to the "checked out" information?

 	Checked out to _Owen Leonard_ : due 08/06/2012 23:59 _Recall_
Comment 3 Kyle M Hall 2012-08-31 13:44:29 UTC Comment hidden (obsolete)
Comment 4 Kyle M Hall 2012-08-31 13:44:57 UTC Comment hidden (obsolete)
Comment 5 Kyle M Hall 2012-08-31 13:45:13 UTC Comment hidden (obsolete)
Comment 6 Katrin Fischer 2012-08-31 13:49:17 UTC
I think I like Owen's idea to show the recall option with the item status information - could this be implemented?
Comment 7 Kyle M Hall 2012-08-31 13:50:41 UTC
Works perfectly *if* you are the superlibrarian. There was an error in the permissions checking, I've attached a followup to fix it. I also had to add execution permissions to the file circ/send_recall_notice.pl.

These are extremely minor fixes, so I'll set it to 'signed off' for now and assume whoever passes qa on it will be testing it. If you feel this needs a separate signoff, please set the status back to 'Needs Signoff" and feel free to yell at me.
Comment 8 Kyle M Hall 2012-08-31 14:00:03 UTC Comment hidden (obsolete)
Comment 9 Mason James 2012-09-16 13:06:48 UTC
> Works perfectly *if* you are the superlibrarian. There was an error in the
> permissions checking, I've attached a followup to fix it. I also had to add
> execution permissions to the file circ/send_recall_notice.pl.
> 
> These are extremely minor fixes, so I'll set it to 'signed off' for now and
> assume whoever passes qa on it will be testing it. If you feel this needs a
> separate signoff, please set the status back to 'Needs Signoff" and feel
> free to yell at me.

ok, this follow-up patch still needs testing/sign-off
Comment 10 Owen Leonard 2012-10-29 16:31:21 UTC
This seems to be working properly--I tried both as superlibrarian and non-superlibrarian. However I really think this needs a system preference added to enable the feature if a library allows recalls. Mine doesn't, and we would have to add custom JS or CSS to hide the button. Better to be able to turn it on and off.
Comment 11 Frédérick Capovilla 2013-04-26 21:50:06 UTC Comment hidden (obsolete)
Comment 12 Galen Charlton 2013-05-20 20:12:45 UTC
I think a feature to add support for recalls needs more fleshing out before being committed to master, since most other implementations I know of have additional components.

The most important one is doing something to keep track of to whom the recalled item should be given.  Other ILSs do this by treating a recall as a special kind of hold request.  Note that if the feature were to allow patrons to place recall requests directly, that is something that is typically not offered to all patron categories -- for example, in an academic library, often it's only professors and maybe grad students who are allowed to recall books.

Secondarily, I feel that there should be a related permission on whether a staff member is allowed to place recalls in the first place.

Next, consideration should be given to whether recalls are allowed to shorten the loan period for the originally borrower.  In addition, I know that some libraries impose higher fines if a recalled items is not returned promptly enough.

Tying recalls into the holds system (or perhaps creating a separate class of recall request) should be considered a sine qua non, in my opinion.  I have doubts that a feature that simply sends recall notices at (staff) will is going to be sufficient.
Comment 13 Charles Farmer 2015-03-09 14:03:53 UTC Comment hidden (obsolete)
Comment 14 Charles Farmer 2015-03-09 14:06:29 UTC
I needed this patch for something else. Decided to update it to current master (DBRev 3.19.00.012)
Comment 15 Nicole C. Engard 2015-04-06 16:25:47 UTC
Why is this in discussion? I see a new patch here - are we ready for sign off again?

Nicole
Comment 16 Blou 2015-04-07 12:26:57 UTC
It seems Charles squashed the previous patches that were signed off to create this new one with the extra system preference to answer Galen's second point, as well as rebase I presume.

I understand Galen's points, but it seems to me that a simple Recall feature is a good step one.  It simnply allows for notices to be sent.  It doesn't claim to do more, but it seems certain libraries could already use this.

The discussion about holds, shortening loan period, higher fines, who, what... That is a full blown Discussion that should require its own taskId, wiki page, and time.  I do not think this should "stop" the current progress.
Comment 17 Nicole C. Engard 2015-04-17 13:22:07 UTC
08:16 ~/kohaclone (master % u=)$ git qa 8688
From git://git.koha-community.org/koha
 * branch            master     -> FETCH_HEAD
Branch bug8688-qa set up to track remote branch master from origin.
Switched to a new branch 'bug8688-qa'
Bug 8688 - Add a recall button to the biblio record details page.

36741 - Bug 8688 - Add a Recall button to the biblio record details page.

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 8688 - Add a Recall button to the biblio record details page.
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
Auto-merging installer/data/mysql/updatedatabase.pl
CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl
Auto-merging installer/data/mysql/sysprefs.sql
Auto-merging installer/data/mysql/en/mandatory/sample_notices.sql
Auto-merging catalogue/detail.pl
Failed to merge in the changes.
Patch failed at 0001 Bug 8688 - Add a Recall button to the biblio record details page.
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-8688---Add-a-Recall-button-to-the-biblio-recor-Glrmn_.patch
08:21 ~/kohaclone (bug8688-qa *+%|AM u=)$
Comment 18 Mark Tompsett 2015-04-18 03:15:02 UTC Comment hidden (obsolete)
Comment 19 Mark Tompsett 2015-04-18 03:16:07 UTC
Set to needs signoff, since I didn't know what status to dump it to.
Comment 20 Mark Tompsett 2015-04-18 19:38:18 UTC Comment hidden (obsolete)
Comment 21 Mark Tompsett 2015-04-18 19:53:07 UTC Comment hidden (obsolete)
Comment 22 Bernardo Gonzalez Kriegel 2015-05-14 00:36:56 UTC
Created attachment 39169 [details] [review]
[SIGNED-OFF] Bug 8688: Add a Recall button to the biblio record details page.

This patch is a fusion of all previous patches for this feature, in
addition to a system preference to control the appearance of the
"Recall" button.

NOTE: Rebased so updatedatabase.pl doesn't fail.
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Fixed conflicts
Moved update to atomic
Comment 23 Bernardo Gonzalez Kriegel 2015-05-14 00:37:08 UTC
Created attachment 39170 [details] [review]
[SIGNED-OFF] Bug 8688: Cleaned up pointless parameter passing

It does not make sense to $template->param( 'foo' => 'bar' );
when in the template file we can Koha.Preference('foo') in order
to get 'bar'.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Comment 24 Bernardo Gonzalez Kriegel 2015-05-14 00:37:18 UTC
Created attachment 39171 [details] [review]
[SIGNED-OFF] Bug 8688: Update License, Perlcritic -3 friendly

This patch tweaks the license information to v3, as per the
license found in the Coding Guidelines on the wiki:
http://wiki.koha-community.org/wiki/Coding_Guidelines#Licence

This also changes:
    use strict;
    use warnings;
into:
    use Modern::Perl;
As per the PERL2: Modern::Perl requirement in the Guidelines:
http://wiki.koha-community.org/wiki/Coding_Guidelines#PERL2:_Modern::Perl

And though the guidelines only require perlcritic -5, a single
line tweak gets a bump to perlcritic -3.

TEST PLAN
---------
1) perlcritic -5 circ/send_recall_notice.pl
   -- success, so technically no change was required.
2) perlcritic -3 circ/send_recall_notice.pl
   -- fails
3) apply patch
4) perlcritic -3 circ/send_recall_notice.pl
   -- success
5) run koha qa test tools

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Comment 25 Bernardo Gonzalez Kriegel 2015-05-14 00:38:17 UTC Comment hidden (obsolete)
Comment 26 Bernardo Gonzalez Kriegel 2015-05-14 00:38:41 UTC Comment hidden (obsolete)
Comment 27 Bernardo Gonzalez Kriegel 2015-05-14 00:40:52 UTC
Re-signed first three patches, after fixing conflicts
Added two new ones, first for koha-qa, second to add notices to all langs

They need a check and sign
Comment 28 Marc Véron 2015-12-31 15:34:25 UTC
Created attachment 46155 [details] [review]
[Signed-off] Bug 8688: (follow-up) Fix quotes

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Sign-off on last two patches, see comment 27
Signed-off-by: Marc Véron <veron@veron.ch>
Comment 29 Marc Véron 2015-12-31 15:44:03 UTC
Created attachment 46158 [details] [review]
(Signed-off) Bug 8688: (follow-up 2) Add notice to all langs and fix permission

Set eXecute bit for script

Added notice on all langs

Tested by me but if you must, Run

SET foreign_key_checks = 0;
truncate letter;
source installer/data/mysql/de-DE/mandatory/sample_notices.sql
SELECT COUNT(*) FROM letter where code='RECALL';
truncate letter;
source installer/data/mysql/es-ES/mandatory/sample_notices.sql
SELECT COUNT(*) FROM letter where code='RECALL';
truncate letter;
source installer/data/mysql/fr-FR/1-Obligatoire/sample_notices.sql
SELECT COUNT(*) FROM letter where code='RECALL';
truncate letter;
source installer/data/mysql/it-IT/necessari/notices.sql
SELECT COUNT(*) FROM letter where code='RECALL';
truncate letter;
source installer/data/mysql/nb-NO/1-Obligatorisk/sample_notices.sql
SELECT COUNT(*) FROM letter where code='RECALL';
truncate letter;
source installer/data/mysql/pl-PL/mandatory/sample_notices.sql
SELECT COUNT(*) FROM letter where code='RECALL';
truncate letter;
source installer/data/mysql/ru-RU/mandatory/sample_notices.sql
SELECT COUNT(*) FROM letter where code='RECALL';
truncate letter;
source installer/data/mysql/uk-UA/mandatory/sample_notices.sql
SELECT COUNT(*) FROM letter where code='RECALL';
truncate letter;
source installer/data/mysql/en/mandatory/sample_notices.sql
SELECT COUNT(*) FROM letter where code='RECALL';

All counts must be 1

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Marc Véron <veron@veron.ch>
Comment 30 Marc Véron 2015-12-31 15:46:09 UTC
Tested overall functionality and sign-off on 2 latest patches (see comment #27)
Comment 31 Jonathan Druart 2016-01-04 12:22:04 UTC
QA comments:
This is a very simple implementation, but could be considered as a first step.
I would like to get feedback from someone else (ccing some QA people).
For instance 1. there is no way to sent an sms instead of an email, 2. the letter is generated even if the patron does not have an email address defined, 3. there is no way to know if a recall has already been generated

I don't understand the use case, why don't you use the overdue_notices script instead?

Technical review:
1/ A variable items_checked_out is passed to the template but never used.
2/ ajax script should be moved to the svc/ directory
3/ The Recall button is display if the pref is set, then the user get an unauthorised message if does not have the permission.
It would be easier not to display the button in this case... (and sent a 403 if the script is called).
Comment 32 Jonathan Druart 2016-01-04 12:22:42 UTC
I have set the status to "Failed QA", but it's more a "In discussion".
Comment 33 David Nind 2022-06-07 20:31:42 UTC
Recalls implemented in bug 19532 - changed status to resolved.