|
Description
Rafal Kopaczka
2014-09-05 09:28:28 UTC
Damm, wrong number... Moved to security since it depends on other bug from security. Created attachment 31393 [details] [review] [PATCH] Bug 12876 - Reserve in waiting/transfer status may be cancelled by user User may cancel his own reservation at waiting or in transit status through calling opac-modrequest.pl. Cancel button is disabled in interface but possibility to cancel should Rafal, Thank to provide a patch. I have 3 remarks: 1/ Please have a look at the wiki patch for the commit message formatting: http://wiki.koha-community.org/wiki/Commit_messages 2/ I think the routine name is a little bit ambiguous. We could believe that your are checking the permission for a librarian. 3/ If I am not wrong (which is possible...), a reserve in transit have the found value set to 'T'. Easier to check than calling GetTransfers. (In reply to Jonathan Druart from comment #4) > Rafal, > Thank to provide a patch. > I have 3 remarks: > 1/ Please have a look at the wiki patch for the commit message formatting: > http://wiki.koha-community.org/wiki/Commit_messages > > 2/ I think the routine name is a little bit ambiguous. We could believe that > your are checking the permission for a librarian. What name do you suggest then? CanReserveBeCanceledFromOpac ? > > 3/ If I am not wrong (which is possible...), a reserve in transit have the > found value set to 'T'. Easier to check than calling GetTransfers. I think you are true with this. I was suggested by opac-user.pl with checking for transfers. My library don't use transfers, so I'm not familiar with it at all. :) (In reply to Rafal Kopaczka from comment #5) > > 2/ I think the routine name is a little bit ambiguous. We could believe that > > your are checking the permission for a librarian. > What name do you suggest then? CanReserveBeCanceledFromOpac ? Yep, certainly better :) Created attachment 31404 [details] [review] Bug 12876 - Reserve in waiting/transfer status may be cancelled by user User may cancel his own reservation at waiting or in transit status through calling opac-modrequest.pl. Cancel button is disabled in interface but possibility to cancel should be checked also in opac-moderequest.pl, before calling CancelReserve(). Similar situation is with opac-modrequest-suspend.pl This patch provides new soubroutine to chceck if user can cancel given reserve. It's possible only when he's owner of hold and hold isn't in transfer or waiting status. Additionaly there are new test for this function in Reserves.t Created attachment 31406 [details] [review] 0001-Bug-12876-Reserve-in-waiting-transfer-status-may-be-.patch Slightly better this time I hope. Created attachment 31413 [details] [review] Bug 12876 - Reserve in waiting/transfer status may be cancelled by user User may cancel his own reservation at waiting or in transit status through calling opac-modrequest.pl. Cancel button is disabled in interface but possibility to cancel should be checked also in opac-moderequest.pl, before calling CancelReserve(). Similar situation is with opac-modrequest-suspend.pl This patch provides new soubroutine to chceck if user can cancel given reserve. It's possible only when he's owner of hold and hold isn't in transfer or waiting status. Additionaly there are new test for this function in Reserves.t Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Created attachment 31414 [details] [review] Bug 12876: Improve unit tests for CanReserveBeCanceledFromOpac This patch fix the subroutine name and add a restriction on the arguments: both argument are mandatory! Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Sorry, I can't get this to apply: Apply? [y]es/[n]o/[e]dit/[v]iew patch/[a]ccept all y Applying: Bug 12876 - Reserve in waiting/transfer status may be cancelled by user fatal: sha1 information is lacking or useless (opac/opac-modrequest.pl). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 Bug 12876 - Reserve in waiting/transfer status may be cancelled by user The copy of the patch that failed is found in: /home/katrin/kohaclone/.git/rebase-apply/patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". Missed the dependency :) Created attachment 31440 [details] [review] [PASSED QA] Bug 12876 - Reserve in waiting/transfer status may be cancelled by user User may cancel his own reservation at waiting or in transit status through calling opac-modrequest.pl. Cancel button is disabled in interface but possibility to cancel should be checked also in opac-moderequest.pl, before calling CancelReserve(). Similar situation is with opac-modrequest-suspend.pl This patch provides new soubroutine to chceck if user can cancel given reserve. It's possible only when he's owner of hold and hold isn't in transfer or waiting status. Additionaly there are new test for this function in Reserves.t Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Passes all tests, QA script and new tests. Works as described, tested with: .../cgi-bin/koha/opac-modrequest.pl?reserve_id=XXX Created attachment 31441 [details] [review] [PASSED QA] Bug 12876: Improve unit tests for CanReserveBeCanceledFromOpac This patch fix the subroutine name and add a restriction on the arguments: both argument are mandatory! Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Created attachment 31846 [details] [review] Bug 12876: (followup) remove useless diags Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com> Patches pushed to master. Thanks Rafal! Pushed to 3.16.x for inclusion in 3.16.4. Pushed to 3.16.x for inclusion in 3.16.4. Pushed to 3.14.x, will be in 3.14.11 |