A patron with an expired card can still renew books and place holds via the opac. Since the staff can't do these thing without renewing the patron's card shouldn't the patron be blocked in the OPAC as well? Also, there should be some kind of note to the patron that the account has expired and they need to see a librarian.
Working on this
Work to be done * Add note on opac-user if users card has expired * Block renewals and holds
The note already shows I just marked a patron as restricted, and got Please note: Your account has been frozen. Usually the reason for freezing an account is old overdues or damage fees.If your account page shows your account to be clear, please contact the library.
Will work on a patch to make it block holds and renewals too
Using the text of the note for expired also
You can place a hold in the staff client without renewing the expiry, ditto renewals. You just cant issue a new item to them if their card has expired. Is there a system preference I need to set to make them be blocked Nicole?
Chris, There is an override renewals limits preference (AllowRenewalLimitOverride), but it's specifically associated with the limit on number of renewals - not the fact that the patron is expired. Basically, what I think is being asked for here is if a patron is expired their account acts as if it's restricted and renewals, holds, checkouts are all blocked until the card is renewed by a librarian. This block should happen in both the OPAC and the staff client - otherwise, why have an expiration date? Make sense? Nicole
Makes sense But I have an expired patron, and their renewals and holds are NOT blocked in the staff client. So I was wondering how/what I had to set to make that blocking happen in the staff.
Chris, Are you asking me a question still? Nicole
Yes, im trying to recreate the problem "But I have an expired patron, and their renewals and holds are NOT blocked in the staff client. So I was wondering how/what I had to set to make that blocking happen in the staff.?"
OH! I see - it sounded like you were repeating my question. But i see what you're saying. I think that if they're expired they should be blocked all around - in the staff and the opac until the librarian addresses the expiration message. Right now only checkouts appear to be blocked, which is silly if you're not blocking the other actions. I'm up for others to chime in with their opinions. Nicole
*** Bug 6785 has been marked as a duplicate of this bug. ***
I think renewals should not be possible for expired patrons. But perhaps allow holds so that they have a reason to go into the library and have their patron cards renewed? Might be more consistent though to block everything.
This is going to need to be one of those system-preferenced kind of things. There is no way every library is going to agree whether expired patrons should be any to do X but not Y and Z. What we really need is patron permissions (CanBorrow, CanOPACRenew, CanOPACPlaceHold etc) on an individual basis, with defaults set by patron type like we do for message preferences, but with hooks/triggers that change this permissions under certain conditions (expired account, fines too high, too many overdues, etc).
Created attachment 7814 [details] [review] Ability to block expired patrons placing OPAC reserves or holds on a system and category levels Patch attached Patch has a syspref and a setting in patron categories that allow you to configure blocking of OPAC actions (currently renew and reserve). Syspref is either block or don't block. Patron category settings are 'use syspref', 'block' and 'dont block' and will take priority over the syspref unless set to 'use syspref'. ~Chris H
Hi Chris H., I was looking at your patch and I am a bit worried about the logic of the configuration options. The patron category now has a pull down with 3 settings: block, don't block, use syspref. And the system preference is block or don't block. When I am in the system preference editor, I have no way to find out, if my patron categories are affected by my changes. And when I am looking at patron categories I don't know what 'use syspref' will do. This seems confusing to me. Perhaps we could do it the other way around matching the behaviour of enhancedmessaging a bit? System preference: Block, Don't block, Configure on category type level Settings on patron category only show up when system preference is set to allow configuraton on category tyle level. For testing I fixed 2 small conflicts in your patch, be careful to keep the check on existence of dateexpiry when you rebase your patch. This was a recent change. Also the version number in updatedatabase should be XXX - this will make it easier for RM and is easier to test. With XXX you only have to update the kohaversion to the next higher number and the database update will happen.
I can't figure out how to resolve the conflict in opac/opac-user.pl, so I am marking this "Patch doesn't apply."
Created attachment 9108 [details] [review] Ability to block expired patrons placing OPAC reserves or holds on a system and category levels Corrected patch to apply cleanly against master. Thanks for mentioning both the XXX and dateexpiry check Katrin. Thank you for your explanation of the alternative behavior, I am looking into this now.
I tried setting this BlockExpiredPatronOpacActions to "block," but it doesn't seem to have any effect. I'm still able to place holds in the OPAC.
Created attachment 16061 [details] [review] Bug 6739 - added the ability to configure whether an expired patron can perform opac actions such as renew and reserve on both a syspref and patron-category level
Created attachment 16063 [details] [review] Bug 6739 - expired patrons not blocked from opac actions Test Plan: 1) Apply patch 2) Run updatedatabase.pl 3) Pick a patron, note the patron's category 5) Issue an item to this patron 4) Edit that category, set "Block expired patrons" to "Block" 5) Verify the patron cannot renew or place holds in the opac 6) Edit the category again, set "Block expired patrons" to "Don't block" 7) Verify the patron *can* renew and place holds in the opac 8) Edit the category again, set "Block expired patrons" to "Use syspref" 9) Set the system preference BlockExpiredPatronOpacActions to "Block" 10) Verify the patron cannot renew or place holds in the opac 11) Set the system preference BlockExpiredPatronOpacActions to "Don't block" 12) Verify the patron *can* renew and place holds in the opac Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 16064 [details] [review] Bug 6739 - expired patrons not blocked from opac actions Test Plan: 1) Apply patch 2) Run updatedatabase.pl 3) Pick a patron, note the patron's category 5) Issue an item to this patron 4) Edit that category, set "Block expired patrons" to "Block" 5) Verify the patron cannot renew or place holds in the opac 6) Edit the category again, set "Block expired patrons" to "Don't block" 7) Verify the patron *can* renew and place holds in the opac 8) Edit the category again, set "Block expired patrons" to "Follow system preference BlockExpiredPatronOpacActions" 9) Set the system preference BlockExpiredPatronOpacActions to "Block" 10) Verify the patron cannot renew or place holds in the opac 11) Set the system preference BlockExpiredPatronOpacActions to "Don't block" 12) Verify the patron *can* renew and place holds in the opac Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 16065 [details] [review] Bug 6739 - expired patrons not blocked from opac actions Test Plan: 1) Apply patch 2) Run updatedatabase.pl 3) Pick a patron, note the patron's category 5) Issue an item to this patron 4) Edit that category, set "Block expired patrons" to "Block" 5) Verify the patron cannot renew or place holds in the opac 6) Edit the category again, set "Block expired patrons" to "Don't block" 7) Verify the patron *can* renew and place holds in the opac 8) Edit the category again, set "Block expired patrons" to "Follow system preference BlockExpiredPatronOpacActions" 9) Set the system preference BlockExpiredPatronOpacActions to "Block" 10) Verify the patron cannot renew or place holds in the opac 11) Set the system preference BlockExpiredPatronOpacActions to "Don't block" 12) Verify the patron *can* renew and place holds in the opac Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This should probably have another signer before getting a signed-off status.
After applying the patch I no longer see the logged-in user's name at the top of the screen ("Welcome, "). This kind of construction is a violation of coding guidelines because it breaks the translator: <option value="1" [% IF ( BlockExpiredPatronOpacActions == 1 ) %] selected="selected" [% END %]> Block </option> http://wiki.koha-community.org/wiki/Coding_Guidelines#HTML1:_Template_Toolkit_markup_inside_HTML In opac-user.tt: "Please contact the library if you wish to renew your subscription." I don't think we use the term "subscription" anywhere in Koha to refer to the patron's account. I see two variations in the template right now: "card has expired" and "account has expired." We should pick one of these for consistency's sake: "renew your card" or "renew your account." There are a couple of instances of this HTML typo in opac-user.tt: <string>Please note:</string> I assume that should be "strong." Also in opac-user.tt: You're renew failed That should be "Your." In opac-renew.pl: print $query->redirect("/cgi-bin/koha/opac-user.pl?renew_error=$errorstring"); You can't build an error string (in English) in the script and pass it to the template for display. This is untranslatable. It's also bad practice to pass unsanitized data directly to the template . Also, this patch has a huge amount of whitespace changes that appear to be unrelated to functional changes. I don't know if that is another cause to fail the patch, but it sure makes it harder to understand.
> Also, this patch has a huge amount of whitespace changes that appear to be > unrelated to functional changes. I don't know if that is another cause to > fail the patch, but it sure makes it harder to understand. I'll take care of those changes. I had to make a lot of whitespace changes due to the fact that the code layout was a bit on the side of madness. "git diff -w" will hide changes that are just changes in indentation. I hope that is not cause alone for failing the patch. It's quite onerous to have to fix whitespace to understand the functioning of a script, undo those fixes, then make edits, then redo the fixes in a second patch.
Created attachment 16069 [details] [review] Bug 6739 - expired patrons not blocked from opac actions - Followup
Created attachment 16071 [details] [review] Bug 6739 - expired patrons not blocked from opac actions Test Plan: 1) Apply patch 2) Run updatedatabase.pl 3) Pick a patron, note the patron's category 5) Issue an item to this patron 4) Edit that category, set "Block expired patrons" to "Block" 5) Verify the patron cannot renew or place holds in the opac 6) Edit the category again, set "Block expired patrons" to "Don't block" 7) Verify the patron *can* renew and place holds in the opac 8) Edit the category again, set "Block expired patrons" to "Follow system preference BlockExpiredPatronOpacActions" 9) Set the system preference BlockExpiredPatronOpacActions to "Block" 10) Verify the patron cannot renew or place holds in the opac 11) Set the system preference BlockExpiredPatronOpacActions to "Don't block" 12) Verify the patron *can* renew and place holds in the opac Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 16072 [details] [review] Bug 6739 - expired patrons not blocked from opac actions - Followup
Something went wrong ! CONFLICT (content)
Created attachment 17356 [details] [review] Bug 6739 - expired patrons not blocked from opac actions (revised) Revised main patch to rebase master and also remove all changes only related to code format (white-spaces, new lines, folding ...). There where a lot ; such changes must not be in main patch because it makes the rebase much more difficult. Can you re-signoff Kyle ?
Created attachment 17357 [details] [review] Bug 6739 - expired patrons not blocked from opac actions - Followup Rebased patch. Also made a change : Calling $query->param() into $template->param() is a bad idea, because $query->param() result type depends on context (normal or array contexts) and $template->param() is an array context.
I noticed that some changes in opac-user.pl seems to be not directly related to this bug purpose. Some lines concerns OPACDisplayRequestPriority and NotifyBorrowerDeparture syspref behaviors. Could this be explained is patch comment ?
Hello, I applied the revised and followup patches together, but even with "Block expired patrons" set to "Block," I was still able to renew from the OPAC as an expired patron. I also discovered some strange behavior with renewing patrons while testing, but I'm not sure if it's related to this patch. Here's what happened: first I created a test patron and gave him an expiry date in the past. Then after going to the OPAC to test placing a hold, I realized that I would need to check something out to my patron so that I could test renewing an item from the OPAC. I couldn't check out to the expired test patron, so I tried to renew him. When I tried to renew the patron, things got weird. I got a message saying "Patron renewed until" with no date, but the warning "Patron expired" didn't go away. I then tried actually changing the expiry date from the patron record, but I still couldn't renew him. I had to switch back to master to renew my patron. Not sure if that is a consequence of this patch or not, but renewing the patron did seem to work fine in master. Thanks, Melia
*** Bug 6348 has been marked as a duplicate of this bug. ***
Created attachment 17463 [details] [review] Bug 6739 - expired patrons not blocked from opac actions - Followup 2
Created attachment 17464 [details] [review] Bug 6739 - expired patrons not blocked from opac actions - Followup
Created attachment 17466 [details] [review] Bug 6739 - expired patrons not blocked from opac actions - Followup
(In reply to comment #14) > This is going to need to be one of those system-preferenced kind of things. > There is no way every library is going to agree whether expired patrons > should be any to do X but not Y and Z. > > What we really need is patron permissions (CanBorrow, CanOPACRenew, > CanOPACPlaceHold etc) on an individual basis, with defaults set by patron > type like we do for message preferences, but with hooks/triggers that change > this permissions under certain conditions (expired account, fines too high, > too many overdues, etc). Maybe I'm overlooking something obvious, but why are we letting an expired patron do "anything". This seems like a huge patch for something that should be straightforward, no? When logging into the OPAC, shouldn't we just check to see if the patron is expired, and if so...throw up a page that says "You're expired. Consult the staff at your library to be renewed" or something along those lines.
As an example: I think in order to make them renew their account it can be good to allow holds. You see a book you want to get, place a hold on it, go to the library... renew your account. But that's something not every library might want to do this way, so it needs options. I would argue that if a patron is restricted they should not be able to do anything, at least for something like lost cards. But that's another discussion probably. :)
(In reply to comment #40) > As an example: I think in order to make them renew their account it can be > good to allow holds. You see a book you want to get, place a hold on it, go > to the library... renew your account. But that's something not every library > might want to do this way, so it needs options. > > I would argue that if a patron is restricted they should not be able to do > anything, at least for something like lost cards. But that's another > discussion probably. :) Mmm, I see what you're saying now. That is rather obvious now that I think about it ;).
Created attachment 17517 [details] [review] [Signed off] Bug 6739 - expired patrons not blocked from opac actions Test Plan: 1) Apply patch 2) Run updatedatabase.pl 3) Pick a patron, note the patron's category 5) Issue an item to this patron 4) Edit that category, set "Block expired patrons" to "Block" 5) Verify the patron cannot renew or place holds in the opac 6) Edit the category again, set "Block expired patrons" to "Don't block" 7) Verify the patron *can* renew and place holds in the opac 8) Edit the category again, set "Block expired patrons" to "Follow system preference BlockExpiredPatronOpacActions" 9) Set the system preference BlockExpiredPatronOpacActions to "Block" 10) Verify the patron cannot renew or place holds in the opac 11) Set the system preference BlockExpiredPatronOpacActions to "Don't block" 12) Verify the patron *can* renew and place holds in the opac Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Melia Meggs <melia@bywatersolutions.com>
Created attachment 17518 [details] [review] [Signed off] Bug 6739 - expired patrons not blocked from opac actions - Followup Signed-off-by: Melia Meggs <melia@bywatersolutions.com>
Ok, I followed the test plan again with the new follow up patch, and it all seems to be working perfectly now. Thanks, Kyle! Could we please get one more signoff on this patch? Thanks, Melia
(In reply to comment #42) > 1) Apply patch > 2) Run updatedatabase.pl > 3) Pick a patron, note the patron's category > 5) Issue an item to this patron > 4) Edit that category, set "Block expired patrons" to "Block" > 5) Verify the patron cannot renew or place holds in the opac At this step in the test plan I would expect that the summary of checkouts on opac-user.pl not show me checkboxes for performing renewals, or any of the other controls related to renewals. If I check a box and try to renew, I get a message that says " Please note: Your account renewal failed because of the following: "
Created attachment 17907 [details] [review] Bug 6739 - expired patrons not blocked from opac actions Test Plan: 1) Apply patch 2) Run updatedatabase.pl 3) Pick a patron, note the patron's category 5) Issue an item to this patron 4) Edit that category, set "Block expired patrons" to "Block" 5) Verify the patron cannot renew or place holds in the opac 6) Edit the category again, set "Block expired patrons" to "Don't block" 7) Verify the patron *can* renew and place holds in the opac 8) Edit the category again, set "Block expired patrons" to "Follow system preference BlockExpiredPatronOpacActions" 9) Set the system preference BlockExpiredPatronOpacActions to "Block" 10) Verify the patron cannot renew or place holds in the opac 11) Set the system preference BlockExpiredPatronOpacActions to "Don't block" 12) Verify the patron *can* renew and place holds in the opac Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Melia Meggs <melia@bywatersolutions.com>
Created attachment 17908 [details] [review] Bug 6739 - expired patrons not blocked from opac actions - Followup Signed-off-by: Melia Meggs <melia@bywatersolutions.com>
Created attachment 17909 [details] [review] Bug 6739 - expired patrons not blocked from opac actions - Followup 2 * Hide renewal column, buttons and links * Fixes minor bug where upper-right table links would show even if renewal impossible * Cleans up some of the template formatting ( whitespace changes only )
Comment on attachment 17909 [details] [review] Bug 6739 - expired patrons not blocked from opac actions - Followup 2 Review of attachment 17909 [details] [review]: ----------------------------------------------------------------- Most of the changes in opac-user.tt are whitespace cleanup. I've marked out the actual changes. ::: koha-tmpl/opac-tmpl/prog/en/modules/opac-user.tt @@ +70,3 @@ > }); > + > + [% IF ( canrenew && !userdebarred && OpacRenewalAllowed && !( borrower.is_expired && borrower.BlockExpiredPatronOpacActions ) ) %] This line fixes the renewal links bug. @@ +132,4 @@ > <string>Please note:</string> > <span> > Your account renewal failed because of the following: > + [% FOREACH error IN RENEW_ERROR.split('\|') %] The pipe character needed escaping, otherwise it was split into an array of single characters. @@ +205,5 @@ > + [% END %] > + > + <th>Call No.</th> > + > + [% IF OpacRenewalAllowed && !( borrower.is_expired && borrower.BlockExpiredPatronOpacActions ) %] Here is another actual change.
Created attachment 18041 [details] [review] [SIGNED-OFF] Bug 6739 - expired patrons not blocked from opac actions Test Plan: 1) Apply patch 2) Run updatedatabase.pl 3) Pick a patron, note the patron's category 5) Issue an item to this patron 4) Edit that category, set "Block expired patrons" to "Block" 5) Verify the patron cannot renew or place holds in the opac 6) Edit the category again, set "Block expired patrons" to "Don't block" 7) Verify the patron *can* renew and place holds in the opac 8) Edit the category again, set "Block expired patrons" to "Follow system preference BlockExpiredPatronOpacActions" 9) Set the system preference BlockExpiredPatronOpacActions to "Block" 10) Verify the patron cannot renew or place holds in the opac 11) Set the system preference BlockExpiredPatronOpacActions to "Don't block" 12) Verify the patron *can* renew and place holds in the opac Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Melia Meggs <melia@bywatersolutions.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 18042 [details] [review] [SIGNED-OFF] Bug 6739 - expired patrons not blocked from opac actions - Followup Signed-off-by: Melia Meggs <melia@bywatersolutions.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 18043 [details] [review] [SIGNED-OFF] Bug 6739 - expired patrons not blocked from opac actions - Followup 2 * Hide renewal column, buttons and links * Fixes minor bug where upper-right table links would show even if renewal impossible * Cleans up some of the template formatting ( whitespace changes only ) Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment on attachment 18041 [details] [review] [SIGNED-OFF] Bug 6739 - expired patrons not blocked from opac actions Review of attachment 18041 [details] [review]: ----------------------------------------------------------------- Hi Chris H. and Kyle, I have taken a look at those patches and found some problems, the most severe is in the database update (see splinter review comments below). Please fix and resubmit. And a little note for a possible enhancement: The new option doesn't show up on the patron categories summary table in administration. ::: installer/data/mysql/kohastructure.sql @@ +441,4 @@ > `reservefee` decimal(28,6) default NULL, -- cost to place holds > `hidelostitems` tinyint(1) NOT NULL default '0', -- are lost items shown to this category (1 for yes, 0 for no) > `category_type` varchar(1) NOT NULL default 'A', -- type of Koha patron (Adult, Child, Professional, Organizational, Statistical, Staff) > + `BlockExpiredPatronOpacActions` tinyint(1) NOT NULL default '-1', -- wheither or not a patron of this categori can renew books or place holds once their card has expired. 0 means they can, 1 means they cannot, -1 means use syspref BLockExpiredPAtronOpacACtions Tiny typo: categori ::: installer/data/mysql/updatedatabase.pl @@ +6845,5 @@ > + NULL, > + 'YesNo' > + ) > + "); > + $dbh->do("ALTER TABLE `categories` ADD COLUMN `BlockExpiredPatronOpacActions` TINYINT(1) DEFAULT 0 NOT NULL"); The default value for new patron categories is -1 (use system preference), but for existing installations we create the column with default 0. I checked that the follow ups are not fixing this. Failing QA. Also: It would be a bit safer to make sure the column is added after category_type like in kohastructure.sql.
Created attachment 19553 [details] [review] [SIGNED-OFF] Bug 6739 - expired patrons not blocked from opac actions Rebased master
Created attachment 19554 [details] [review] [SIGNED-OFF] Bug 6739 - expired patrons not blocked from opac actions - Followup Rebased master
Created attachment 19555 [details] [review] [SIGNED-OFF] Bug 6739 - expired patrons not blocked from opac actions - Followup 2 Rebased master
Created attachment 19558 [details] [review] Bug 6739 - expired patrons not blocked from opac actions - Followup 3 Follow-up to correct files in installer. See commit message.
Followup 3 needs signoff
Created attachment 19559 [details] [review] Bug 6739 - expired patrons not blocked from opac actions - Followup 4 Some small corrections, see commit message. Needs sign off.
Created attachment 19560 [details] [review] Bug 6739 - expired patrons not blocked from opac actions - Followup 4 More infos in commit comment
Created attachment 19678 [details] [review] Bug 6739 - expired patrons not blocked from opac actions - Followup 3 Installer fixes : - in kohastructure.sql, category.BlockExpiredPatronOpacActions default value is -1, sets the same in updatedatabase.pl - in syspref comment, replaces "opac actions such as placing a hold or reserve" by "opac actions such as placing holds or renrw books" - A 'YesNo' does not have 'yes' as value in database, it is '1'. - corrects small typo "categori" and syspref name case Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 19679 [details] [review] Bug 6739 - expired patrons not blocked from opac actions - Followup 4 Some corrections : - opac-reserve.tt : opening <p> instead of closing - opac-user.tt : warnexpired was in database format, adds the use of KohaDates template plugin - opac-user.tt : duplicated TT test : [% IF ( BORROWER_INF.warnexpired ) %] and [% ELSIF ( BORROWER_INF.warnexpired ) %], maybe a merge error - opac-user.tt : <string> instead of <strong>, maybe for HTML 6 :-) - opac-user.pl : adding dateformat var to template is already done by Auth.pm Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
I am sorry, I know this has gone rounds for a while, but I can't get follow up 3 to apply after fixing the merge conflict in the first patch. Can someone please provide rebased patches? Thx!
Apply? [y]es/[n]o/[e]dit/[v]iew patch/[a]ccept all y Applying: Bug 6739 - expired patrons not blocked from opac actions - Followup 3 fatal: sha1 information is lacking or useless (installer/data/mysql/kohastructure.sql). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge.
Created attachment 19993 [details] [review] [SIGNED-OFF] Bug 6739 - expired patrons not blocked from opac actions Rebased master
Created attachment 19994 [details] [review] [SIGNED-OFF] Bug 6739 - expired patrons not blocked from opac actions - Followup 1 Rebased master
Created attachment 19995 [details] [review] [SIGNED-OFF] Bug 6739 - expired patrons not blocked from opac actions - Followup 2 Rebased master
Created attachment 19996 [details] [review] [SIGNED-OFF] Bug 6739 - expired patrons not blocked from opac actions - Followup 3 Rebased master. I recreated this patch manually.
Created attachment 19997 [details] [review] [SIGNED-OFF] Bug 6739 - expired patrons not blocked from opac actions - Followup 4 Rebased master
Patches no longer apply, I fixed the conflict in sysprefs.sql in the first patch, but got stuck on opac.tt in the third. Can you please rebase?
Created attachment 20641 [details] [review] [SIGNED-OFF] Bug 6739 - expired patrons not blocked from opac actions Rebased master. syspref.sql is now a single insert with alphabetical ordered sysprefs.
Created attachment 20642 [details] [review] [SIGNED-OFF] Bug 6739 - expired patrons not blocked from opac actions - Followup 1 Rebased master
Created attachment 20643 [details] [review] [SIGNED-OFF] Bug 6739 - expired patrons not blocked from opac actions - Followup 2 Rebased master. I minimized the changes not directly concerning this new feature (new lines, spaces ...). They make rebase more difficult.
Created attachment 20644 [details] [review] [SIGNED-OFF] Bug 6739 - expired patrons not blocked from opac actions - Followup 3 Rebased master
Created attachment 20645 [details] [review] [SIGNED-OFF] Bug 6739 - expired patrons not blocked from opac actions - Followup 4 Rebased master
Working on this now...
Happy to say that it applied cleanly and passed the automatic tests :) Continueing testing...
Created attachment 21312 [details] [review] [PASSED QA] Bug 6739 - expired patrons not blocked from opac actions Test Plan: 1) Apply patch 2) Run updatedatabase.pl 3) Pick a patron, note the patron's category 5) Issue an item to this patron 4) Edit that category, set "Block expired patrons" to "Block" 5) Verify the patron cannot renew or place holds in the opac 6) Edit the category again, set "Block expired patrons" to "Don't block" 7) Verify the patron *can* renew and place holds in the opac 8) Edit the category again, set "Block expired patrons" to "Follow system preference BlockExpiredPatronOpacActions" 9) Set the system preference BlockExpiredPatronOpacActions to "Block" 10) Verify the patron cannot renew or place holds in the opac 11) Set the system preference BlockExpiredPatronOpacActions to "Don't block" 12) Verify the patron *can* renew and place holds in the opac Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Melia Meggs <melia@bywatersolutions.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Testing notes on last patch in series.
Created attachment 21313 [details] [review] [PASSED QA] Bug 6739 - expired patrons not blocked from opac actions - Followup Signed-off-by: Melia Meggs <melia@bywatersolutions.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 21314 [details] [review] [PASSED QA] Bug 6739 - expired patrons not blocked from opac actions - Followup 2 * Hide renewal column, buttons and links * Fixes minor bug where upper-right table links would show even if renewal impossible * Cleans up some of the template formatting ( whitespace changes only ) Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 21315 [details] [review] [PASSED QA] Bug 6739 - expired patrons not blocked from opac actions - Followup 3 Installer fixes : - in kohastructure.sql, category.BlockExpiredPatronOpacActions default value is -1, sets the same in updatedatabase.pl - in syspref comment, replaces "opac actions such as placing a hold or reserve" by "opac actions such as placing holds or renrw books" - A 'YesNo' does not have 'yes' as value in database, it is '1'. - corrects small typo "categori" and syspref name case Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 21316 [details] [review] [PASSED QA] Bug 6739 - expired patrons not blocked from opac actions - Followup 4 Some corrections : - opac-reserve.tt : opening <p> instead of closing - opac-user.tt : warnexpired was in database format, adds the use of KohaDates template plugin - opac-user.tt : duplicated TT test : [% IF ( BORROWER_INF.warnexpired ) %] and [% ELSIF ( BORROWER_INF.warnexpired ) %], maybe a merge error - opac-user.tt : <string> instead of <strong>, maybe for HTML 6 :-) - opac-user.pl : adding dateformat var to template is already done by Auth.pm Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Passes all tests and QA script. Testing notes: - Database update * Changes to kohastructure match changes done by the updatedatabase statement. Feature is activated by default. Fixing 'yes' to be '1' in a follow up. * Ran database update succesfully. * Note: Patrons are now blocked by default in new installations AND in updated installations. - System preference * Verified system preference shows up correctly. - Category configuration * Add new patron category * Edit existing patron category * Delete patron category * Check patron category summary table. => Verified all actions work as expected. => Verified chosen value for BlockExpiredPatronOpacActions' is always displayed and saved correctly. * Note: The new value is missing from the summary table. * Note: The new value is also not shown when deleting a patron category. - Check functionality * Renew and place a hold for an NOT EXPIRED patron with a) category: use syspref (default) syspref: block (default) b) category: use syspref (default) syspref: don't block c) category: block syspref: don't block d) category: block syspref: block e) category: don't block sypref: block * Verified renewals and placing holds were never blocked. * Also verified that the warning from NotifyBorrowerDeparture still shows up correctly. * Renew and place a hold for an EXPIRED patron with a) category: use syspref (default) syspref: block (default) => OK, both actions are blocked. b) category: use syspref (default) syspref: don't block => OK, both actions possible. c) category: block syspref: don't block => OK, both actions are blocked. d) category: block syspref: block => OK, both actions are blocked. e) category: don't block sypref: block => OK, both actions possible.
Created attachment 21317 [details] [review] Bug 6739 - expired patrons not blocked from opac actions - Followup 5 - Fixes a typo in one of the OPAC messages: "you account" - Fixes the updatedatabase script to use 1 instead of yes for the system preference default matching the sysprefs.sql file. - Fixes capitalization "Don't Block" in system preference description. - Adding . to the end of the warning, when renewals are blocked.
This is something that is quite annoying for some libraries and I feel like I have tested thoroughly (which probably means I missed something). So some automated regression tests would be very much appreciated.
Galen, Are you waiting for something to push these patches ?
Comment on attachment 21312 [details] [review] [PASSED QA] Bug 6739 - expired patrons not blocked from opac actions Review of attachment 21312 [details] [review]: ----------------------------------------------------------------- ::: opac/opac-user.pl @@ +65,4 @@ > } > ); > > +my $OPACDisplayRequestPriority = (C4::Context->preference("OPACDisplayRequestPriority")) ? 1 : 0; This is a regression on bug 7398; OPACDisplayRequestPriority no longer exists. @@ -110,4 @@ > $canrenew = 0; > $template->param( > renewal_blocked_fines => sprintf( '%.02f', $no_renewal_amt ), > - renewal_blocked_fines_amountoutstanding => sprintf( '%.02f', $borr->{amountoutstanding} ), Removing this would cause a regression of bug 8408. @@ -170,5 @@ > $charges += $ac->{'amountoutstanding'} > if $ac->{'accounttype'} eq 'F'; > $charges += $ac->{'amountoutstanding'} > - if $ac->{'accounttype'} eq 'FU'; > - $charges += $ac->{'amountoutstanding'} Removing this would cause a regression on bug 8079.
Marked as failed QA on account of the three regressions that this patch series would cause. Overall, looks reasonable, and I expect it to pass into master once the regressions have been fixed.
Comment on attachment 21312 [details] [review] [PASSED QA] Bug 6739 - expired patrons not blocked from opac actions Review of attachment 21312 [details] [review]: ----------------------------------------------------------------- ::: opac/opac-user.pl @@ +157,4 @@ > if ($issues){ > foreach my $issue ( sort { $b->{date_due}->datetime() cmp $a->{date_due}->datetime() } @{$issues} ) { > # check for reserves > + my ( $restype, $res, undef ) = CheckReserves( $issue->{'itemnumber'} ); This is a regression of bug 9367.
Created attachment 24385 [details] [review] Bug 6739 - expired patrons not blocked from opac actions Test Plan: 1) Apply patch 2) Run updatedatabase.pl 3) Pick a patron, note the patron's category 5) Issue an item to this patron 4) Edit that category, set "Block expired patrons" to "Block" 5) Verify the patron cannot renew or place holds in the opac 6) Edit the category again, set "Block expired patrons" to "Don't block" 7) Verify the patron *can* renew and place holds in the opac 8) Edit the category again, set "Block expired patrons" to "Follow system preference BlockExpiredPatronOpacActions" 9) Set the system preference BlockExpiredPatronOpacActions to "Block" 10) Verify the patron cannot renew or place holds in the opac 11) Set the system preference BlockExpiredPatronOpacActions to "Don't block" 12) Verify the patron *can* renew and place holds in the opac Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Melia Meggs <melia@bywatersolutions.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Testing notes on last patch in series.
Created attachment 24386 [details] [review] Bug 6739 - expired patrons not blocked from opac actions - Followup Signed-off-by: Melia Meggs <melia@bywatersolutions.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 24387 [details] [review] Bug 6739 - expired patrons not blocked from opac actions - Followup 2 * Hide renewal column, buttons and links * Fixes minor bug where upper-right table links would show even if renewal impossible * Cleans up some of the template formatting ( whitespace changes only ) Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 24388 [details] [review] Bug 6739 - expired patrons not blocked from opac actions - Followup 3 Installer fixes : - in kohastructure.sql, category.BlockExpiredPatronOpacActions default value is -1, sets the same in updatedatabase.pl - in syspref comment, replaces "opac actions such as placing a hold or reserve" by "opac actions such as placing holds or renrw books" - A 'YesNo' does not have 'yes' as value in database, it is '1'. - corrects small typo "categori" and syspref name case Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 24389 [details] [review] Bug 6739 - expired patrons not blocked from opac actions - Followup 4 Some corrections : - opac-reserve.tt : opening <p> instead of closing - opac-user.tt : warnexpired was in database format, adds the use of KohaDates template plugin - opac-user.tt : duplicated TT test : [% IF ( BORROWER_INF.warnexpired ) %] and [% ELSIF ( BORROWER_INF.warnexpired ) %], maybe a merge error - opac-user.tt : <string> instead of <strong>, maybe for HTML 6 :-) - opac-user.pl : adding dateformat var to template is already done by Auth.pm Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Passes all tests and QA script. Testing notes: - Database update * Changes to kohastructure match changes done by the updatedatabase statement. Feature is activated by default. Fixing 'yes' to be '1' in a follow up. * Ran database update succesfully. * Note: Patrons are now blocked by default in new installations AND in updated installations. - System preference * Verified system preference shows up correctly. - Category configuration * Add new patron category * Edit existing patron category * Delete patron category * Check patron category summary table. => Verified all actions work as expected. => Verified chosen value for BlockExpiredPatronOpacActions' is always displayed and saved correctly. * Note: The new value is missing from the summary table. * Note: The new value is also not shown when deleting a patron category. - Check functionality * Renew and place a hold for an NOT EXPIRED patron with a) category: use syspref (default) syspref: block (default) b) category: use syspref (default) syspref: don't block c) category: block syspref: don't block d) category: block syspref: block e) category: don't block sypref: block * Verified renewals and placing holds were never blocked. * Also verified that the warning from NotifyBorrowerDeparture still shows up correctly. * Renew and place a hold for an EXPIRED patron with a) category: use syspref (default) syspref: block (default) => OK, both actions are blocked. b) category: use syspref (default) syspref: don't block => OK, both actions possible. c) category: block syspref: don't block => OK, both actions are blocked. d) category: block syspref: block => OK, both actions are blocked. e) category: don't block sypref: block => OK, both actions possible.
Created attachment 24390 [details] [review] Bug 6739 - expired patrons not blocked from opac actions - Followup 5 - Fixes a typo in one of the OPAC messages: "you account" - Fixes the updatedatabase script to use 1 instead of yes for the system preference default matching the sysprefs.sql file. - Fixes capitalization "Don't Block" in system preference description. - Adding . to the end of the warning, when renewals are blocked.
Created attachment 24391 [details] [review] Bug 6739: FIX conflicts with some others bugs. see comments 86 and 88. This patch fixes a bad conflict resolution in a previous patch.
Created attachment 24392 [details] [review] Bug 6739: template changes for the bootstrap theme
Created attachment 25158 [details] [review] Bug 6739 - expired patrons not blocked from opac actions Test Plan: 1) Apply patch 2) Run updatedatabase.pl 3) Pick a patron, note the patron's category 5) Issue an item to this patron 4) Edit that category, set "Block expired patrons" to "Block" 5) Verify the patron cannot renew or place holds in the opac 6) Edit the category again, set "Block expired patrons" to "Don't block" 7) Verify the patron *can* renew and place holds in the opac 8) Edit the category again, set "Block expired patrons" to "Follow system preference BlockExpiredPatronOpacActions" 9) Set the system preference BlockExpiredPatronOpacActions to "Block" 10) Verify the patron cannot renew or place holds in the opac 11) Set the system preference BlockExpiredPatronOpacActions to "Don't block" 12) Verify the patron *can* renew and place holds in the opac Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Melia Meggs <melia@bywatersolutions.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Testing notes on last patch in series.
Created attachment 25159 [details] [review] Bug 6739 - expired patrons not blocked from opac actions - Followup Signed-off-by: Melia Meggs <melia@bywatersolutions.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 25160 [details] [review] Bug 6739 - expired patrons not blocked from opac actions - Followup 2 * Hide renewal column, buttons and links * Fixes minor bug where upper-right table links would show even if renewal impossible * Cleans up some of the template formatting ( whitespace changes only ) Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 25161 [details] [review] Bug 6739 - expired patrons not blocked from opac actions - Followup 3 Installer fixes : - in kohastructure.sql, category.BlockExpiredPatronOpacActions default value is -1, sets the same in updatedatabase.pl - in syspref comment, replaces "opac actions such as placing a hold or reserve" by "opac actions such as placing holds or renrw books" - A 'YesNo' does not have 'yes' as value in database, it is '1'. - corrects small typo "categori" and syspref name case Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 25162 [details] [review] Bug 6739 - expired patrons not blocked from opac actions - Followup 4 Some corrections : - opac-reserve.tt : opening <p> instead of closing - opac-user.tt : warnexpired was in database format, adds the use of KohaDates template plugin - opac-user.tt : duplicated TT test : [% IF ( BORROWER_INF.warnexpired ) %] and [% ELSIF ( BORROWER_INF.warnexpired ) %], maybe a merge error - opac-user.tt : <string> instead of <strong>, maybe for HTML 6 :-) - opac-user.pl : adding dateformat var to template is already done by Auth.pm Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Passes all tests and QA script. Testing notes: - Database update * Changes to kohastructure match changes done by the updatedatabase statement. Feature is activated by default. Fixing 'yes' to be '1' in a follow up. * Ran database update succesfully. * Note: Patrons are now blocked by default in new installations AND in updated installations. - System preference * Verified system preference shows up correctly. - Category configuration * Add new patron category * Edit existing patron category * Delete patron category * Check patron category summary table. => Verified all actions work as expected. => Verified chosen value for BlockExpiredPatronOpacActions' is always displayed and saved correctly. * Note: The new value is missing from the summary table. * Note: The new value is also not shown when deleting a patron category. - Check functionality * Renew and place a hold for an NOT EXPIRED patron with a) category: use syspref (default) syspref: block (default) b) category: use syspref (default) syspref: don't block c) category: block syspref: don't block d) category: block syspref: block e) category: don't block sypref: block * Verified renewals and placing holds were never blocked. * Also verified that the warning from NotifyBorrowerDeparture still shows up correctly. * Renew and place a hold for an EXPIRED patron with a) category: use syspref (default) syspref: block (default) => OK, both actions are blocked. b) category: use syspref (default) syspref: don't block => OK, both actions possible. c) category: block syspref: don't block => OK, both actions are blocked. d) category: block syspref: block => OK, both actions are blocked. e) category: don't block sypref: block => OK, both actions possible.
Created attachment 25163 [details] [review] Bug 6739 - expired patrons not blocked from opac actions - Followup 5 - Fixes a typo in one of the OPAC messages: "you account" - Fixes the updatedatabase script to use 1 instead of yes for the system preference default matching the sysprefs.sql file. - Fixes capitalization "Don't Block" in system preference description. - Adding . to the end of the warning, when renewals are blocked.
Created attachment 25164 [details] [review] Bug 6739: FIX conflicts with some others bugs. see comments 86 and 88. This patch fixes a bad conflict resolution in a previous patch.
Created attachment 25165 [details] [review] Bug 6739: template changes for the bootstrap theme
Created attachment 25231 [details] [review] Bug 6739 - expired patrons not blocked from opac actions - Followup 5 - Fixes a typo in one of the OPAC messages: "you account" - Fixes the updatedatabase script to use 1 instead of yes for the system preference default matching the sysprefs.sql file. - Fixes capitalization "Don't Block" in system preference description. - Adding . to the end of the warning, when renewals are blocked. Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Created attachment 25232 [details] [review] Bug 6739: FIX conflicts with some others bugs. see comments 86 and 88. This patch fixes a bad conflict resolution in a previous patch. Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Created attachment 25233 [details] [review] Bug 6739: template changes for the bootstrap theme Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
I am sorry, there is a conflict I can't resolve in this patch series: Apply? [y]es/[n]o/[e]dit/[v]iew patch/[a]ccept all y Applying: Bug 6739 - expired patrons not blocked from opac actions - Followup 2 fatal: sha1 information is lacking or useless (koha-tmpl/opac-tmpl/prog/en/modules/opac-user.tt). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 Bug 6739 - expired patrons not blocked from opac actions - Followup 2
Created attachment 26546 [details] [review] Bug 6739 - expired patrons not blocked from opac actions Test Plan: 1) Apply patch 2) Run updatedatabase.pl 3) Pick a patron, note the patron's category 5) Issue an item to this patron 4) Edit that category, set "Block expired patrons" to "Block" 5) Verify the patron cannot renew or place holds in the opac 6) Edit the category again, set "Block expired patrons" to "Don't block" 7) Verify the patron *can* renew and place holds in the opac 8) Edit the category again, set "Block expired patrons" to "Follow system preference BlockExpiredPatronOpacActions" 9) Set the system preference BlockExpiredPatronOpacActions to "Block" 10) Verify the patron cannot renew or place holds in the opac 11) Set the system preference BlockExpiredPatronOpacActions to "Don't block" 12) Verify the patron *can* renew and place holds in the opac Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Melia Meggs <melia@bywatersolutions.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Testing notes on last patch in series.
Created attachment 26547 [details] [review] Bug 6739 - expired patrons not blocked from opac actions - Followup Signed-off-by: Melia Meggs <melia@bywatersolutions.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 26548 [details] [review] Bug 6739 - expired patrons not blocked from opac actions - Followup 2 * Hide renewal column, buttons and links * Fixes minor bug where upper-right table links would show even if renewal impossible * Cleans up some of the template formatting ( whitespace changes only ) Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 26549 [details] [review] Bug 6739 - expired patrons not blocked from opac actions - Followup 3 Installer fixes : - in kohastructure.sql, category.BlockExpiredPatronOpacActions default value is -1, sets the same in updatedatabase.pl - in syspref comment, replaces "opac actions such as placing a hold or reserve" by "opac actions such as placing holds or renrw books" - A 'YesNo' does not have 'yes' as value in database, it is '1'. - corrects small typo "categori" and syspref name case Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 26550 [details] [review] Bug 6739 - expired patrons not blocked from opac actions - Followup 4 Some corrections : - opac-reserve.tt : opening <p> instead of closing - opac-user.tt : warnexpired was in database format, adds the use of KohaDates template plugin - opac-user.tt : duplicated TT test : [% IF ( BORROWER_INF.warnexpired ) %] and [% ELSIF ( BORROWER_INF.warnexpired ) %], maybe a merge error - opac-user.tt : <string> instead of <strong>, maybe for HTML 6 :-) - opac-user.pl : adding dateformat var to template is already done by Auth.pm Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Passes all tests and QA script. Testing notes: - Database update * Changes to kohastructure match changes done by the updatedatabase statement. Feature is activated by default. Fixing 'yes' to be '1' in a follow up. * Ran database update succesfully. * Note: Patrons are now blocked by default in new installations AND in updated installations. - System preference * Verified system preference shows up correctly. - Category configuration * Add new patron category * Edit existing patron category * Delete patron category * Check patron category summary table. => Verified all actions work as expected. => Verified chosen value for BlockExpiredPatronOpacActions' is always displayed and saved correctly. * Note: The new value is missing from the summary table. * Note: The new value is also not shown when deleting a patron category. - Check functionality * Renew and place a hold for an NOT EXPIRED patron with a) category: use syspref (default) syspref: block (default) b) category: use syspref (default) syspref: don't block c) category: block syspref: don't block d) category: block syspref: block e) category: don't block sypref: block * Verified renewals and placing holds were never blocked. * Also verified that the warning from NotifyBorrowerDeparture still shows up correctly. * Renew and place a hold for an EXPIRED patron with a) category: use syspref (default) syspref: block (default) => OK, both actions are blocked. b) category: use syspref (default) syspref: don't block => OK, both actions possible. c) category: block syspref: don't block => OK, both actions are blocked. d) category: block syspref: block => OK, both actions are blocked. e) category: don't block sypref: block => OK, both actions possible.
Created attachment 26551 [details] [review] Bug 6739 - expired patrons not blocked from opac actions - Followup 5 - Fixes a typo in one of the OPAC messages: "you account" - Fixes the updatedatabase script to use 1 instead of yes for the system preference default matching the sysprefs.sql file. - Fixes capitalization "Don't Block" in system preference description. - Adding . to the end of the warning, when renewals are blocked.
Created attachment 26552 [details] [review] Bug 6739: FIX conflicts with some others bugs. see comments 86 and 88. This patch fixes a bad conflict resolution in a previous patch.
Created attachment 26553 [details] [review] Bug 6739: template changes for the bootstrap theme
(In reply to Katrin Fischer from comment #108) > I am sorry, there is a conflict I can't resolve in this patch series: Patches rebased against master.
Applies now and passes QA script, more testing later.
Testing this again, please have a little patience.
Back to this...
Created attachment 26836 [details] [review] [PASSED QA] Bug 6739 - expired patrons not blocked from opac actions Test Plan: 1) Apply patch 2) Run updatedatabase.pl 3) Pick a patron, note the patron's category 5) Issue an item to this patron 4) Edit that category, set "Block expired patrons" to "Block" 5) Verify the patron cannot renew or place holds in the opac 6) Edit the category again, set "Block expired patrons" to "Don't block" 7) Verify the patron *can* renew and place holds in the opac 8) Edit the category again, set "Block expired patrons" to "Follow system preference BlockExpiredPatronOpacActions" 9) Set the system preference BlockExpiredPatronOpacActions to "Block" 10) Verify the patron cannot renew or place holds in the opac 11) Set the system preference BlockExpiredPatronOpacActions to "Don't block" 12) Verify the patron *can* renew and place holds in the opac Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Melia Meggs <melia@bywatersolutions.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Testing notes on last patch in series.
Created attachment 26837 [details] [review] [PASSED QA] Bug 6739 - expired patrons not blocked from opac actions - Followup Signed-off-by: Melia Meggs <melia@bywatersolutions.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 26838 [details] [review] [PASSED QA] Bug 6739 - expired patrons not blocked from opac actions - Followup 2 * Hide renewal column, buttons and links * Fixes minor bug where upper-right table links would show even if renewal impossible * Cleans up some of the template formatting ( whitespace changes only ) Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 26839 [details] [review] [PASSED QA] Bug 6739 - expired patrons not blocked from opac actions - Followup 3 Installer fixes : - in kohastructure.sql, category.BlockExpiredPatronOpacActions default value is -1, sets the same in updatedatabase.pl - in syspref comment, replaces "opac actions such as placing a hold or reserve" by "opac actions such as placing holds or renrw books" - A 'YesNo' does not have 'yes' as value in database, it is '1'. - corrects small typo "categori" and syspref name case Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 26840 [details] [review] [PASSED QA] Bug 6739 - expired patrons not blocked from opac actions - Followup 4 Some corrections : - opac-reserve.tt : opening <p> instead of closing - opac-user.tt : warnexpired was in database format, adds the use of KohaDates template plugin - opac-user.tt : duplicated TT test : [% IF ( BORROWER_INF.warnexpired ) %] and [% ELSIF ( BORROWER_INF.warnexpired ) %], maybe a merge error - opac-user.tt : <string> instead of <strong>, maybe for HTML 6 :-) - opac-user.pl : adding dateformat var to template is already done by Auth.pm Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Passes all tests and QA script. Testing notes: - Database update * Changes to kohastructure match changes done by the updatedatabase statement. Feature is activated by default. Fixing 'yes' to be '1' in a follow up. * Ran database update succesfully. * Note: Patrons are now blocked by default in new installations AND in updated installations. - System preference * Verified system preference shows up correctly. - Category configuration * Add new patron category * Edit existing patron category * Delete patron category * Check patron category summary table. => Verified all actions work as expected. => Verified chosen value for BlockExpiredPatronOpacActions' is always displayed and saved correctly. * Note: The new value is missing from the summary table. * Note: The new value is also not shown when deleting a patron category. - Check functionality * Renew and place a hold for an NOT EXPIRED patron with a) category: use syspref (default) syspref: block (default) b) category: use syspref (default) syspref: don't block c) category: block syspref: don't block d) category: block syspref: block e) category: don't block sypref: block * Verified renewals and placing holds were never blocked. * Also verified that the warning from NotifyBorrowerDeparture still shows up correctly. * Renew and place a hold for an EXPIRED patron with a) category: use syspref (default) syspref: block (default) => OK, both actions are blocked. b) category: use syspref (default) syspref: don't block => OK, both actions possible. c) category: block syspref: don't block => OK, both actions are blocked. d) category: block syspref: block => OK, both actions are blocked. e) category: don't block sypref: block => OK, both actions possible. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> 2014-04-06 Update: Will repeat and amend above test plan on last patch in this series.
Created attachment 26841 [details] [review] [PASSED QA] Bug 6739 - expired patrons not blocked from opac actions - Followup 5 - Fixes a typo in one of the OPAC messages: "you account" - Fixes the updatedatabase script to use 1 instead of yes for the system preference default matching the sysprefs.sql file. - Fixes capitalization "Don't Block" in system preference description. - Adding . to the end of the warning, when renewals are blocked. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 26842 [details] [review] [PASSED QA] Bug 6739: FIX conflicts with some others bugs. see comments 86 and 88. This patch fixes a bad conflict resolution in a previous patch. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 26843 [details] [review] [PASSED QA] Bug 6739: Template changes for the bootstrap theme Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Passes QA script, t, xt and t/db_dependent/Members*. Testing notes: - Database update * Changes to kohastructure match changes done by the updatedatabase statement. Feature is activated by default! * Ran database update succesfully. * Note: Expired patrons are now blocked by default in new AND in updated installations. - System preference * Verified system preference shows up correctly. - Category configuration * Add new patron category * Edit existing patron category * Delete patron category * Check patron category summary table. => Verified all actions work as expected. => Verified chosen value for BlockExpiredPatronOpacActions is always displayed and saved correctly. * Note: The new value is missing from the summary table. * Note: The new value is also not shown when deleting a patron category. - Check functionality * Renew and place a hold for an NOT EXPIRED patron with a) category: use syspref (default) syspref: block (default) b) category: use syspref (default) syspref: don't block c) category: block syspref: don't block d) category: block syspref: block e) category: don't block sypref: block f) category: don't block syspref: don't block Holds and renewals were always possible. * Also verified that the warning from NotifyBorrowerDeparture still shows up correctly. * Verified holds and renewals are still blocked, when the user has a restriction with a date in the future. * Renew and place a hold for an EXPIRED patron with a) category: use syspref (default) syspref: block (default) Holds and renewals blocked. b) category: use syspref (default) syspref: don't block Holds an renewals possible. c) category: block syspref: don't block Holds and renewals blocked. d) category: block syspref: block Holds and renewals blocked. e) category: don't block sypref: block Holds and renewals possible. f) category: don't block syspref: don't block Holds and renewals possible.
There are a few more things to note: - Changing the existing behaviour by activating the feature by default might be debatable. I am going to leave this as it is and it could be changed easily if we get some opionions on this before release. - The changes to GetMemberDetails are currently not covered by unit tests. All existing tests in for Members* pass, so I am going to pass this on the condition, that we get some more to cover the changes. Enhancements: - Show the new value on the patron category summary table - Show the new field when deleting a patron category
Pushed to master. Thanks, Chris, Kyle, Fridolin, Katrin, and Jonathan! Note that I have elected to have the feature be OFF by default for upgrades. It will be on by default for new installations.
Patch too complex and big to backport to 3.14.x