Created attachment 11827 [details] [review] Add a recall button to the biblio record details page. This feature adds a "Recall" button next to each item that are "on loan" on the "catalogue/detail.pl" page. This button sends an email to the borrower who currently has the item in his possession, telling him to return the item as soon as possible. This feature uses the "RECALL" notice. This notice will be included in the next patch. Patch sponsored by the CCSR ( http://www.ccsr.qc.ca )
Created attachment 11828 [details] [review] Add a default RECALL notice to the database This patch adds a default the "RECALL" notice to the database.
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_
Created attachment 11913 [details] [review] [SIGNED-OFF] Add a recall button to the biblio record details page. The recall button sends an email to the user who currently has the item in his possession, telling him to return the item as soon as possible. Patch sponsored by the CCSR ( http://www.ccsr.qc.ca ) Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 11914 [details] [review] [SIGNED-OFF] Add a default RECALL notice to the database For use with the "Recall button" feature Patch sponsored by the CCSR ( http://www.ccsr.qc.ca ) Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 11915 [details] [review] Bug 8688 - Followup - Fix permissions for send_recall_notice.pl
I think I like Owen's idea to show the recall option with the item status information - could this be implemented?
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.
Created attachment 11916 [details] [review] Bug 8688 - Followup - Append button to the "checked out" information.
> 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
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.
Created attachment 17740 [details] [review] Add a Recall button to the biblio record details page. (Fused patch + syspref) Here is a new version of the "Recall button" feature, now including the "ShowRecallButton" system preference so the button can now be hidden if not needed. This new patch is a fusion of all previous patches, and should be based on the current master.
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.
Created attachment 36741 [details] [review] 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.
I needed this patch for something else. Decided to update it to current master (DBRev 3.19.00.012)
Why is this in discussion? I see a new patch here - are we ready for sign off again? Nicole
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.
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=)$
Created attachment 38111 [details] [review] 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.
Set to needs signoff, since I didn't know what status to dump it to.
Created attachment 38118 [details] [review] 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'.
Created attachment 38119 [details] [review] 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
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
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>
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>
Created attachment 39172 [details] [review] Bug 8688: (follow-up) Fix quotes Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Created attachment 39173 [details] [review] 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>
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
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>
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>
Tested overall functionality and sign-off on 2 latest patches (see comment #27)
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).
I have set the status to "Failed QA", but it's more a "In discussion".
Recalls implemented in bug 19532 - changed status to resolved.