I thought there was a report for this, but I don't see it (feel free to mark as a duplicate). It would be nice if the patron could register for a card via the OPAC.
Moving this discussion from Bug 1153: (In reply to comment #3) > That sounds like an excellent idea. We could even have printable temporary > library cards! One issue: my library system requires a piece of mail to verify > a home address. What do other libraries require? Should it include the ability > to submit scanned documents, or should just let you fill out your data, bring > your documents in and let a librarian verify and complete the process. > > (In reply to comment #2) > > I think we have a great opportunity to integrate and update to this system with > > a system for online patron registration: Both could potentially use the same > > table to store information which would require the librarian to validate and > > approve. I think the idea merits a wider discussion but I would propose: - Allow, or not, the user to submit a new registration online. - Allow, or not, the online registration to become fully active automatically. If not: - Allow, or not, that online registration to give the user "probationary" privileges in the OPAC and for the purposes of SIP authentication. In other words, the patron can place holds, but the library might require documentation upon pickup. Or the patron might be able to access electronic resources which are authenticated against Koha's patron table. In the latter two cases Koha would need to email a response to the user so that they could activate their account (and so that we could validate the email). A cron job could be set up to delete accounts which hadn't been activated after a certain number of days.
(In reply to comment #1) > Moving this discussion from Bug 1153: > > (In reply to comment #3) > > That sounds like an excellent idea. We could even have printable temporary > > library cards! One issue: my library system requires a piece of mail to verify > > a home address. What do other libraries require? Should it include the ability > > to submit scanned documents, or should just let you fill out your data, bring > > your documents in and let a librarian verify and complete the process. > > > > (In reply to comment #2) > > > I think we have a great opportunity to integrate and update to this system with > > > a system for online patron registration: Both could potentially use the same > > > table to store information which would require the librarian to validate and > > > approve. > > I think the idea merits a wider discussion but I would propose: > > - Allow, or not, the user to submit a new registration online. > > - Allow, or not, the online registration to become fully active > automatically. If not: > > - Allow, or not, that online registration to give the user "probationary" > privileges in the OPAC and for the purposes of SIP authentication. In > other words, the patron can place holds, but the library might require > documentation upon pickup. Or the patron might be able to access > electronic resources which are authenticated against Koha's patron table. Maybe a new hard-coded patron category type of "provisional" that you can then assign any patron category code? Category codes in "provisional" have special rules that can be granularly defined: * can place holds (x number of holds, or not at all) * can authenticate via SIP (for x period, or not at all) * can circulate (x times, or not at all) * can only circulate items (not) of type x (say, you can't check out new dvd's as a provisional patron - how rude, but I can so see libraries doing it). * Alert staff every visit to any patron info page *this is a provisional patron - pursue creating permanent patron!* Additional functionality: * Crons to expire and/or remove provisional patrons, or at least report them out so they can be manually pursued. > > In the latter two cases Koha would need to email a response to the user so that > they could activate their account (and so that we could validate the email). A > cron job could be set up to delete accounts which hadn't been activated after a > certain number of days. I like the idea of at least requiring a valid email address and forcing activation of the account by clicking a link from the email. It would have to be optional, of course. Practically speaking, I think this fits under Patron Categories administration.
Progilone did something about that for BULAC = http://catalogue.bulac.fr/cgi-bin/koha/opac-memberentry.pl We've sent a mail to them asking for opening the source. Let's wait for their feedback. If something is proposed, i'm for including it and improving the behaviour with options like "pre-registering" or "full registering". ( I agree the minimum is a syspref to say "enable" or "do not enable" self-registration ) I remove the "in discussion status" and set to ASSIGNED until we've some feedback from progilone (i've added fridolyn to this entry)
Hie, Sure it will be done soon. Note that for this self-registration, a special branch code has been choosen to block the user's circulation untill he is really regritrated (by paying and showing his cards). So the functionality is more a "pre"-registration online.
(In reply to comment #4) > Hie, > Sure it will be done soon. > Note that for this self-registration, a special branch code has been choosen to > block the user's circulation untill he is really regritrated (by paying and > showing his cards). > So the functionality is more a "pre"-registration online. Hello sorry to bother you folks, but how did you manage to create the "pre"-registration form. did you edit the staff client page or did you create a separate perl page with hooks into the database/koha? Thanks so much, James Ghiorzi Library Assistant II San Benito County Free Library 470 Fifth St. Hollister, CA 95023 (831) 636-4107
Hie, We created a new Perl page in OPAC based on Intranet patron creation page. It allows account creation for all OPAC signed-in functions (lists, history) but not circulation (checkouts, holds). For security, we added a Captcha system and server-side controls. Whe also added a page for lost password but it is doesn't reset the password.
Is anyone thinking/working on getting this into Koha?
(In reply to comment #7) > Is anyone thinking/working on getting this into Koha? I know for sure that BULAC has this feature. They have progilone as support company, and Corinne Hayet (that I just added in cc: of this bug) was at the hackfest, and told us she was responsible of managing contributing all what has been made for official Koha. so, corinne, if you want to jump in the discussion, you're more than welcomed, you're hoped ;-)
(In reply to comment #7) > Is anyone thinking/working on getting this into Koha? yes, we are. We do not allow patrons to check out books, so the self-registration would be great.
*** Bug 8502 has been marked as a duplicate of this bug. ***
I have simply created a html and php page that speaks with the db. It would have been better if it was in perl and in the koha package, but I need to find some time to get more used with the koha implementation. Regardless, if you'd like, I could share the info here, i.e. the html and php pages. Cheers
This is sponsored now? Is there an RFC? Has there been communication with the library which has already implemented this feature?
(In reply to comment #12) > This is sponsored now? Is there an RFC? Has there been communication with > the library which has already implemented this feature? Yes, http://wiki.koha-community.org/wiki/Online_Borrower_Registration_RFC , and no. Hope this helps.
(In reply to comment #12) > This is sponsored now? Is there an RFC? Has there been communication with > the library which has already implemented this feature? I had some discussion with BULAC some weeks ago. They still plan to share the code of what they did, but not before the end of the year or something like that. Progilone does not the job without BULAC explicit request, and Corinne is in vacation until end of august
Created attachment 11960 [details] [review] Bug 7067 - OPAC Borrower Self Registration
Test Plan: 1) Enable PatronSelfRegistration 2) Set PatronSelfRegistrationExpireTemporaryAccountsDelay to a number of days 3) Create a self-registered borrower category 4) Set PatronSelfRegistrationUseTemporaryStatus 5) Set PatronSelfRegistrationVerifyByEmail to "Don't require" 6) Go to OPAC, log out if logged in. 7) You should see the "Register here" link below the login box 8) Attempt to register yourself 9) Verify you can log in with your temporary password. 10) Set PatronSelfRegistrationVerifyByEmail to "Require" 11) Attempt another self-registration 12) Check the messages table, you should see a new message with a verification link. 13) Copy and paste the link into a web browser to verify the registration 14) Log in with the given credentials to verify the account was created.
Created attachment 11961 [details] [review] Bug 7067 - OPAC Borrower Self Registration
Test Plan - Part 2 - Borrower Modifications 1) Log in to OPAC, go to "my personal details" tab. 2) Make some modifications to your details. 3) Repeat steps 1 and 2 for two more borrowers. 4) Log in to Koha intranet with a user that can modify borrowers. 5) At the bottom of mainpage.pl, you should see: Patrons requesting modifications: 3 6) Click the link 7) Approve one change, deny a different one, and ignore the third, then submit. 8) Check the records, you should see the changes take affect on the approved one, and no changes to the other two. You should also see "Patrons requesting modifications: 1" at the bottom of mainpage.pl now.
Created attachment 11962 [details] [review] Bug 7067 - OPAC Borrower Self Registration
I forgot to add, the system pref OPACBaseURL will need to be filled out if you are using the email verification.
Created attachment 11965 [details] [review] Bug 7067 - OPAC Borrower Self Registration
I'm still thinking it through, but I think maybe it would be good to have a separate preference for defining what fields are required for self-registration.
Created attachment 11966 [details] [review] Bug 7067 - OPAC Borrower Self Registration
Created attachment 11967 [details] [review] Bug 7067 - OPAC Borrower Self Registration
Created attachment 11968 [details] [review] Bug 7067 - OPAC Borrower Self Registration
Created attachment 11970 [details] [review] Bug 7067 - OPAC Borrower Self Registration
You are right. I was planning for that, then I forgot to add the prefs! Kyle (In reply to comment #22) > I'm still thinking it through, but I think maybe it would be good to have a > separate preference for defining what fields are required for > self-registration.
Hy Kyle, Here some remarks after a little test: - A new dependency (String::Random) is required and has to be added to the C4/Installer/PerlDependencies.pm file - The page opac-user.pl does not have a register link. - There is no verification on the date of birth entry and could be caused a warning - After sending the form, I get an error in my log DBD::mysql::st execute failed: Column 'user' cannot be null". But I don't know where does it come. - I can't continue after the step "13) Copy and paste the link into a web browser to verify the registration". Indeed, there are a lot of error in the page opac/opac-registration-verify.pl (variables not defined, etc.) + The qa tools say me a lot of failed qa points exist: * misc/cronjobs/delete_expired_opac_registrations.pl FAIL critic FAIL "require" statement with library name as string at line 28, column 12. Use a bareword instead. Code before strictures are enabled at line 23, column 1. See page 429 of PBP. * misc/cronjobs/delete_unverified_opac_registrations.pl FAIL critic FAIL "require" statement with library name as string at line 28, column 12. Use a bareword instead. Code before strictures are enabled at line 23, column 1. See page 429 of PBP. * opac/opac-registration-verify.pl FAIL valid FAIL Global symbol "$borrowernumber" requires explicit package name at opac/opac-registration-verify.pl line 58. Global symbol "$cookie" requires explicit package name at opac/opac-registration-verify.pl line 35. Global symbol "%borrower" requires explicit package name at opac/opac-registration-verify.pl line 52. Global symbol "$borrowernumber" requires explicit package name at opac/opac-registration-verify.pl line 35. Global symbol "$template" requires explicit package name at opac/opac-registration-verify.pl line 59. Global symbol "$cookie" requires explicit package name at opac/opac-registration-verify.pl line 69. opac/opac-registration-verify.pl had compilation errors. Global symbol "$template" requires explicit package name at opac/opac-registration-verify.pl line 44. Bareword "token" not allowed while "strict subs" in use at opac/opac-registration-verify.pl line 47. Global symbol "$template" requires explicit package name at opac/opac-registration-verify.pl line 69. Global symbol "$template" requires explicit package name at opac/opac-registration-verify.pl line 56. Global symbol "$template" requires explicit package name at opac/opac-registration-verify.pl line 35. Global symbol "$borrowernumber" requires explicit package name at opac/opac-registration-verify.pl line 52. Global symbol "$template" requires explicit package name at opac/opac-registration-verify.pl line 57.
Created attachment 11973 [details] [review] Bug 7067 - OPAC Borrower Self Registration
Step 9 fails (Verify you can log in with your temporary password.) I got: Username: Password: sHGrskYvtx with an error in my log: DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha_coderun`.`borrowers`, CONSTRAINT `borrowers_ibfk_1` FOREIGN KEY (`categorycode`) REFERENCES `categories` (`categorycode`)) at /home/koha/C4/SQLHelper.pm line 184., referer: http://opac.koha.localhost/cgi-bin/koha/opac-memberentry.pl [Wed Sep 05 14:17:55 2012] [error] [client 127.0.0.1] [Wed Sep 5 14:17:55 2012] opac-memberentry.pl: DBD::mysql::st execute failed: Column 'user' cannot be null at /home/koha/C4/Log.pm line 79., referer: http://opac.koha.localhost/cgi-bin/koha/opac-memberentry.pl I filled all fields into the form. And - The page opac-user.pl does not have a register link.
Created attachment 11978 [details] [review] Bug 7067 - OPAC Borrower Self Registration
> - A new dependency (String::Random) is required and has to be added to the > C4/Installer/PerlDependencies.pm file Fixed. > - The page opac-user.pl does not have a register link. Fixed > - There is no verification on the date of birth entry and could be caused a > warning Good catch. I told myself I would take care of it later and then forgot ; ) > > * misc/cronjobs/delete_expired_opac_registrations.pl > FAIL > critic FAIL > "require" statement with library name as string at line 28, column 12. Use > a bareword instead. > Code before strictures are enabled at line 23, column 1. See page 429 of > PBP. > > * misc/cronjobs/delete_unverified_opac_registrations.pl > FAIL > critic FAIL > "require" statement with library name as string at line 28, column 12. Use > a bareword instead. > Code before strictures are enabled at line 23, column 1. See page 429 of > PBP. These are not valid. I use 'use Modern::Perl' which enables both warnings and strictures. The rest of the errors you saw should be fixed.
Thanks for your work ! Some remarks again : 1/ I still have the sql error on submitting form. It seems caused by the action_log constraint: DBD::mysql::st execute failed: Column 'user' cannot be null at /home/koha/C4/Log.pm line 79., referer: http://opac.koha.localhost/cgi-bin/koha/opac-memberentry.pl 2/ > > - There is no verification on the date of birth entry and could be caused a > > warning > > Good catch. I told myself I would take care of it later and then forgot ; ) I don't know if it is configurable (in the jquery plugin) but now it's not possible to select a year < 2002 with just one click :-/ 3/ on opac-user.pl: "If you don't have a library card, stop by your local library to sign up or register here.. " There is 2 dots on the end of the line. 4/ Strange behaviour: modify your personnal details to replace a value with an other. Resubmit a change but replacing a value with an empty string. At intranet, you see an empty table (just with headers) in the 'changes' column. > These are not valid. I use 'use Modern::Perl' which enables both warnings > and strictures. The rest of the errors you saw should be fixed. Your first patch did not have any module use ;) This patch does not allow a librarian to validate or reject a self registration isn't it ?
Arg, please let me edit my posts! :) (I am sorry about my grammar...)
(In reply to comment #33) > This patch does not allow a librarian to validate or reject a self > registration isn't it ? Auto answer: In fact yes. In the issuing rules, we can restrict issues for the corresponding patron category. Like that we can simulate a pre-registration. A user is fully registrated when he has a "real" patron category.
(In reply to comment #35) > (In reply to comment #33) > > This patch does not allow a librarian to validate or reject a self > > registration isn't it ? > > Auto answer: In fact yes. In the issuing rules, we can restrict issues for > the corresponding patron category. > Like that we can simulate a pre-registration. > A user is fully registrated when he has a "real" patron category. That is exactly what I was going for. They idea is to create a TEMP patron category and when the patron comes in to the library, the patron can show whatever proof they need to and a library can change his or her patron category accordingly.
Created attachment 12006 [details] [review] Bug 7067 - OPAC Borrower Self Registration
> 1/ I still have the sql error on submitting form. It seems caused by the > action_log constraint: > DBD::mysql::st execute failed: Column 'user' cannot be null at > /home/koha/C4/Log.pm line 79., referer: > http://opac.koha.localhost/cgi-bin/koha/opac-memberentry.pl Fixed. It was a minor semantic error in Log.pm. It would get a borrowernumber from env if env was a hash, but wasn't bothering to check if there was actually a borrowernumber stored in env! > 2/ > > > I don't know if it is configurable (in the jquery plugin) but now it's not > possible to select a year < 2002 with just one click :-/ Another good catch! I've fixed it so it displays the last 120 years. That should far enough back for 99.999% of people. > 3/ on opac-user.pl: > "If you don't have a library card, stop by your local library to sign up or > register here.. " > There is 2 dots on the end of the line. Fixed. > 4/ Strange behaviour: > modify your personnal details to replace a value with an other. > Resubmit a change but replacing a value with an empty string. > At intranet, you see an empty table (just with headers) in the 'changes' > column. Also fixed. > > > These are not valid. I use 'use Modern::Perl' which enables both warnings > > and strictures. The rest of the errors you saw should be fixed. > Your first patch did not have any module use ;) Did not realize that! Better late than never! > > This patch does not allow a librarian to validate or reject a self > registration isn't it ? No it doesn't, that is meant to be 'interactive' where the patron comes in to the library with whatever documentation is necessary for membership.
Created attachment 12007 [details] [review] Bug 7067 - OPAC Borrower Self Registration
Created attachment 12013 [details] [review] Bug 7067 - OPAC Borrower Self Registration Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
A point for discussion: I think the registration confirmation messages should come from the email address of the library the user chose as their home library. The counter-argument is that the user may not have chosen the correct library, but I don't see that as much of a problem since either way the reply would be coming from an email address which wasn't their library's. The big issue I'm still seeing: New records are saved with NULL expiration date and NULL registration date. Will the delete_expired_opac_registrations cron job work with a NULL dateenrolled? Some small issues: - I think a note about pending patron modifications should appear on the patrons page (members/members-home.pl) in addition to the staff client home page, just as pending tag/comment/suggestions also show as pending on the Tools page. - Bug 8593 added unique IDs to the other "pending" notes. This patch should include one for the line it adds. - members/members-update.pl needs a descriptive page title and page header. If there are no pending operations it should display a message saying so rather than an empty table. - The table markup on members/members-update.pl is missing a </tr>, causing validation errors. - I think the random-character verification field should be styled like other required fields - Since the datepicker widget doesn't make it easy to pick a date far in the past (as are birthdays for some of us), I don't think the field should be read-only. - I think opac-memberentry-update-submitted.tt should include the user sidebar menu. - I see an error in the log when submitting the registration form in the OPAC: substr outside of string at /home/oleonard/kohaclone/opac/opac-memberentry.pl line 239 If PatronSelfRegistrationExpireTemporaryAccountsDelay is set to "0" does that mean never? If so the description should say so.
Ho, collision :) So I don't switch to signed off
Hm, only a note, but isn't PatronSelfRegistrationExpireTemporaryAccountsDelay a bit montrous for a syspref name?
(In reply to comment #43) > Hm, only a note, but isn't > PatronSelfRegistrationExpireTemporaryAccountsDelay a bit montrous for a > syspref name? Yes, yes it is ; ) But it was the shortest version that would describe it correctly.
(In reply to comment #41) > A point for discussion: I think the registration confirmation messages > should come from the email address of the library the user chose as their > home library. The counter-argument is that the user may not have chosen the > correct library, but I don't see that as much of a problem since either way > the reply would be coming from an email address which wasn't their library's. I suppose a followup for that would not be too much trouble. > The big issue I'm still seeing: New records are saved with NULL expiration > date and NULL registration date. Will the delete_expired_opac_registrations > cron job work with a NULL dateenrolled? This should be fixed in the current version of the patch. > Some small issues: > > - I think a note about pending patron modifications should appear on > the patrons page (members/members-home.pl) in addition to the staff > client home page, just as pending tag/comment/suggestions also show > as pending on the Tools page. I'll take a look and add a followup for that. > > - Bug 8593 added unique IDs to the other "pending" notes. This patch > should include one for the line it adds. I'll have to look into that. > - members/members-update.pl needs a descriptive page title and page > header. If there are no pending operations it should display a message > saying so rather than an empty table. I suppose we can do that, but if there are no pending operations, the link for the page disappears, so it is assumed no one will visit it when there are no pending operations. > - The table markup on members/members-update.pl is missing a </tr>, > causing validation errors. I'll look at it. > - I think the random-character verification field should be styled like > other required fields Good idea. > - Since the datepicker widget doesn't make it easy to pick a date far in > the past (as are birthdays for some of us), I don't think the field > should be read-only. I suppose we can do that. I'm a bit iffy about that one though. > - I think opac-memberentry-update-submitted.tt should include the user > sidebar menu. I'll see what I can do about that. > - I see an error in the log when submitting the registration form in > the OPAC: > > substr outside of string at > /home/oleonard/kohaclone/opac/opac-memberentry.pl line 239 I'll take a look at that too. > If PatronSelfRegistrationExpireTemporaryAccountsDelay is set to "0" does > that mean never? If so the description should say so. That is correct. I'll have to add it to the description.
(In reply to comment #45) > > The big issue I'm still seeing: New records are saved with NULL expiration > > date and NULL registration date. Will the delete_expired_opac_registrations > > cron job work with a NULL dateenrolled? > > This should be fixed in the current version of the patch. I don't find that to be the case. Another issue, which came up on IRC but should be noted here as well: The registration confirmation messages are sent to the message queue, but the message queue might not be processed in a timely manner. Others on IRC said theirs were set to process as infrequently as once an hour. I think a delay as long as one hour is contrary to the expectations many users will have based on other web applications. As a result I think repeated self-registrations are likely as people assume theirs didn't work. I think this is a pretty serious concern.
Another issue: The note about Patrons requesting modifications on the staff client home page does not show up if you are not a superlibrarian. The permissions check needs to be added to the conditional which controls the display of all pending operations (line 101 of intranet-main.tt).
If the "IndependantBranches" preference is turned on it should not be possible for a librarian from Library A to approve changes to the account of a patron with branchcode = Library B.
(In reply to comment #45) > > If there are no pending operations it should display a message > > saying so rather than an empty table. > > I suppose we can do that, but if there are no pending operations, the link > for the page disappears, so it is assumed no one will visit it when there > are no pending operations. Actually as I work with it more and test leaving some patron modifications to "ignore," it makes sense to me to have this page redirect back to itself rather than the home page. If some lines had been set to "ignore" then they re-display. If all are complete it shows a message that the queue is empty.
That makes a lot of sense. I'll add a followup for that. Kyle (In reply to comment #49) > (In reply to comment #45) > > > If there are no pending operations it should display a message > > > saying so rather than an empty table. > > > > I suppose we can do that, but if there are no pending operations, the link > > for the page disappears, so it is assumed no one will visit it when there > > are no pending operations. > > Actually as I work with it more and test leaving some patron modifications > to "ignore," it makes sense to me to have this page redirect back to itself > rather than the home page. If some lines had been set to "ignore" then they > re-display. If all are complete it shows a message that the queue is empty.
Created attachment 12104 [details] [review] Bug 7067 - OPAC Borrower Self Registration - Followup - Change redirect from mainpage.pl to members-update.pl
Another major bug: If I have PatronSelfRegistration turned off, clicking the "my personal details" page redirects to the OPAC home page. There needs to be several working combinations: - PatronSelfRegistration is ON, OPACPatronDetails is ON: New patrons can sign up on the OPAC, existing patrons can edit their details via the entry form. - PatronSelfRegistration is ON, OPACPatronDetails is OFF: New patrons can sign up on the OPAC but existing patrons can't edit their details via the entry form. [This option may not make much logical sense but since we can't link two preferences...] - PatronSelfRegistration is OFF, OPACPatronDetails is ON: New patrons can't sign up on the OPAC but existing patrons can edit their details via the entry form. - PatronSelfRegistration is OFF, OPACPatronDetails is OFF: Patrons can only view their details on the OPAC. Jonathan I hope you don't mind I'm removing the "signed off" status. I think there are too many issues with this for it to move forward just yet.
(En réponse au commentaire 52) > Jonathan I hope you don't mind I'm removing the "signed off" status. I think > there are too many issues with this for it to move forward just yet. No problem Owen :) If there are problems with this patch, it is normal to passed it as failed qa.
Created attachment 12109 [details] [review] Bug 7067 - OPAC Borrower Self Registration - Followup
Created attachment 12114 [details] [review] Bug 7067 - OPAC Borrower Self Registration - Followup
This new followup should address all the issues that Owen has brought up.
After the latest patch I find that if PatronSelfRegistration is ON and OPACPatronDetails is OFF I can submit a new registration. The check for OPACPatronDetails on the entry form disables the form. Turning OPACPatronDetails ON, I find that submitting changes to an existing patron record and approving them in the staff client results in NO changes to the patron record. Now when I submit a new patron registration it appears to work but the username field isn't pre-filled in the login form. In the log I see errors: - opac-memberentry.pl: Use of uninitialized value in pattern match (m//) at /home/oleonard/kohaclone/C4/SQLHelper.pm line 418., referer: http://localhost:88 - opac-memberentry.pl: DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha`.`borrowers`, CONSTRAINT `borrowers_ibfk_1` FOREIGN KEY (`categorycode`) REFERENCES `categories` (`categorycode`)) at /home/oleonard/kohaclone/C4/SQLHelper.pm line 168., referer: http://localhost:88/cgi-bin/koha/opac-memberentry.pl Other issues which still need to be addressed: > - The table markup on members/members-update.pl is missing a </tr>, > causing validation errors. > - I think the random-character verification field should be styled like > other required fields > - Since the datepicker widget doesn't make it easy to pick a date far in > the past (as are birthdays for some of us), I don't think the field > should be read-only. > - I see an error in the log when submitting the registration form in > the OPAC: > > substr outside of string at > /home/oleonard/kohaclone/opac/opac-memberentry.pl line 239 Also: Any thoughts on the message queue issue?
Created attachment 12116 [details] [review] Bug 7067 - OPAC Borrower Self Registration - Followup
(In reply to comment #57) > After the latest patch I find that if PatronSelfRegistration is ON and > OPACPatronDetails is OFF I can submit a new registration. The check for > OPACPatronDetails on the entry form disables the form. Doh! Fixed. > Turning OPACPatronDetails ON, I find that submitting changes to an existing > patron record and approving them in the staff client results in NO changes > to the patron record. Caused by a bug in master. Fixed. > Now when I submit a new patron registration it appears to work but the > username field isn't pre-filled in the login form. In the log I see errors: > > - opac-memberentry.pl: Use of uninitialized value in pattern match (m//) at > /home/oleonard/kohaclone/C4/SQLHelper.pm line 418., referer: > http://localhost:88 > > - opac-memberentry.pl: DBD::mysql::st execute failed: Cannot add or update a > child row: a foreign key constraint fails (`koha`.`borrowers`, CONSTRAINT > `borrowers_ibfk_1` FOREIGN KEY (`categorycode`) REFERENCES `categories` > (`categorycode`)) at /home/oleonard/kohaclone/C4/SQLHelper.pm line 168., > referer: http://localhost:88/cgi-bin/koha/opac-memberentry.pl I'm not getting this error. Please retest with the new followup. Either I fixed it incidentally, or it's a different in configuration perhaps? > > Other issues which still need to be addressed: > I know I had fixed these, the followup must have gone awol! > > - The table markup on members/members-update.pl is missing a </tr>, > > causing validation errors. Fixed > > - I think the random-character verification field should be styled like > > other required fields Fixed > > - Since the datepicker widget doesn't make it easy to pick a date far in > > the past (as are birthdays for some of us), I don't think the field > > should be read-only. Fixed > > - I see an error in the log when submitting the registration form in > > the OPAC: > > > > substr outside of string at > > /home/oleonard/kohaclone/opac/opac-memberentry.pl line 239 Fixed > Also: Any thoughts on the message queue issue? Is there any good reason the process_message_queue cron can't be run every minute? We need a "send it now" function, but we cannot until the email configuration is pulled out of the cron script parameters and put into Koha system preferences.
(In reply to comment #59) > Is there any good reason the process_message_queue cron can't be run every > minute? We need a "send it now" function, but we cannot until the email > configuration is pulled out of the cron script parameters and put into Koha > system preferences. There's no reason not to have the cronjob run once a minute, but if we start recommending that configuration, it will need to be patched to use a lockfile so that it doesn't allow simultaneous jobs to run if it takes longer than a minute to clear the message queue.
(In reply to comment #59) > > - opac-memberentry.pl: Use of uninitialized value in pattern match (m//) at > > /home/oleonard/kohaclone/C4/SQLHelper.pm line 418., referer: > > http://localhost:88 > > > > - opac-memberentry.pl: DBD::mysql::st execute failed: Cannot add or update a > > child row: a foreign key constraint fails (`koha`.`borrowers`, CONSTRAINT > > `borrowers_ibfk_1` FOREIGN KEY (`categorycode`) REFERENCES `categories` > > (`categorycode`)) at /home/oleonard/kohaclone/C4/SQLHelper.pm line 168., > > referer: http://localhost:88/cgi-bin/koha/opac-memberentry.pl I figured it out: I cleared the PatronSelfRegistration preferences for testing and didn't put in a patron category for PatronSelfRegistrationUseTemporaryStatus. How might we deal with this possible omission?
New issue: Submitting a change to an existing patron's home library works in that approving the change will correctly update the record, but the line doesn't show up in the list of fields which will be updated on members/members-update.pl.
What we really need is a 'system preference validator' in Koha. I recall there are other preference dependencies, so it would not only be useful for this. We could just add a warning to mainpage.pl for these things instead. So if PatronSelfRegistration is enabled, but PatronSelfRegistrationUseTemporaryStatus is empty, a warning will be written out on mainpage.pl. At the moment, these issues are only covered by documentation. Kyle (In reply to comment #61) > (In reply to comment #59) > > > - opac-memberentry.pl: Use of uninitialized value in pattern match (m//) at > > > /home/oleonard/kohaclone/C4/SQLHelper.pm line 418., referer: > > > http://localhost:88 > > > > > > - opac-memberentry.pl: DBD::mysql::st execute failed: Cannot add or update a > > > child row: a foreign key constraint fails (`koha`.`borrowers`, CONSTRAINT > > > `borrowers_ibfk_1` FOREIGN KEY (`categorycode`) REFERENCES `categories` > > > (`categorycode`)) at /home/oleonard/kohaclone/C4/SQLHelper.pm line 168., > > > referer: http://localhost:88/cgi-bin/koha/opac-memberentry.pl > > > I figured it out: I cleared the PatronSelfRegistration preferences for > testing and didn't put in a patron category for > PatronSelfRegistrationUseTemporaryStatus. How might we deal with this > possible omission?
Created attachment 12129 [details] [review] Bug 7067 - OPAC Borrower Self Registration - Followup
This should be fixed in my new followup. (In reply to comment #62) > New issue: Submitting a change to an existing patron's home library works in > that approving the change will correctly update the record, but the line > doesn't show up in the list of fields which will be updated on > members/members-update.pl.
(In reply to comment #63) > We could just add a warning to mainpage.pl for these things instead. So if > PatronSelfRegistration is enabled, but > PatronSelfRegistrationUseTemporaryStatus is empty, a warning will be written > out on mainpage.pl. Maybe we could make display of the "register" link in the OPAC dependent on PatronSelfRegistrationUseTemporaryStatus being defined? And now that I think about it, shouldn't this preference be "PatronSelfRegistrationDefaultCategory" ? It's required, so it's not as if we're saying "use this temporary category if you want to."
(In reply to comment #66) > (In reply to comment #63) > > > We could just add a warning to mainpage.pl for these things instead. So if > > PatronSelfRegistration is enabled, but > > PatronSelfRegistrationUseTemporaryStatus is empty, a warning will be written > > out on mainpage.pl. > > Maybe we could make display of the "register" link in the OPAC dependent on > PatronSelfRegistrationUseTemporaryStatus being defined? > > And now that I think about it, shouldn't this preference be > "PatronSelfRegistrationDefaultCategory" ? It's required, so it's not as if > we're saying "use this temporary category if you want to." That is a good idea, and a good rename as well.
Created attachment 12135 [details] [review] Bug 7067 - OPAC Borrower Self Registration - Followup - Rename PatronSelfRegistrationUseTemporaryStatus to PatronSelfRegistrationDefaultCategory
Created attachment 12136 [details] [review] Bug 7067 - allow patron self registration via the opac - Followup - Hide register link unless PatronSelfRegistrationDefaultCategory is set.
If you submit a new registration via the OPAC and then reload the confirmation page (whether you're asked for email confirmation or not), you get duplicate patron records. The patron who required email confirmation gets duplicate fully-valid activation links. The patron who didn't require email confirmation gets newly-generated passwords on each reload but it doesn't appear that reloaded ones are valid.
Created attachment 12138 [details] [review] Bug 7067 - allow patron self registration via the opac - Followup - Add invalid token page
(In reply to comment #70) > If you submit a new registration via the OPAC and then reload the > confirmation page (whether you're asked for email confirmation or not), you > get duplicate patron records. The patron who required email confirmation > gets duplicate fully-valid activation links. The patron who didn't require > email confirmation gets newly-generated passwords on each reload but it > doesn't appear that reloaded ones are valid. With email registration on, I'm not getting duplicate patrons. I just get the "Registration Complete" page with an empty username and password if I reload the page. That is safe, but it would be better to redirect to an 'invalid token' page instead. When not requiring email verification, I *do* get duplicate borrowers. I'm not sure if this situation has a solution. It's no different than the user hitting back and submitting the same data again. We could redirect to a new page, rather than loading the Registration Complete page directly from opac-memberentry.pl, but that just shifts the problem. Instead of reloading creating a duplicate patron, hitting back will. If anyone has any suggestions, please let me know. But as far as I can tell this is more of a HTTP problem than a Koha problem.
Created attachment 12153 [details] [review] Bug 7067 - OPAC Borrower Self Registration - Followup * Rename PatronSelfRegistrationUseTemporaryStatus to PatronSelfRegistrationDefaultCategory * Allow patron self registration via the opac - Followup - Hide register link unless PatronSelfRegistrationDefaultCategory is set. * Allow patron self registration via the opac - Followup - Add invalid token page
Created attachment 12154 [details] [review] Bug 7067 - OPAC Borrower Self Registration - Followup * Rename PatronSelfRegistrationUseTemporaryStatus to PatronSelfRegistrationDefaultCategory * Hide register link unless PatronSelfRegistrationDefaultCategory is set. * Add invalid token page
(In reply to comment #74) > * Add invalid token page This works great, thanks, As for the potential duplicate patrons when not requiring email notification, I guess the upside is that delete_unverified_opac_registrations.pl job will deal with them. However, that assumes that the patron is coming in to the library to confirm their identity (or whatever procedure changes them from a temp category to a permanent one), and that the library is running that cron job. It remains a potential problem for libraries which allow patrons to register for full-fledged accounts without confirmation (however unlikely that scenario seems to me).
(In reply to comment #75) > (In reply to comment #74) > > * Add invalid token page > > This works great, thanks, > > As for the potential duplicate patrons when not requiring email > notification, I guess the upside is that > delete_unverified_opac_registrations.pl job will deal with them. However, > that assumes that the patron is coming in to the library to confirm their > identity (or whatever procedure changes them from a temp category to a > permanent one), and that the library is running that cron job. It remains a > potential problem for libraries which allow patrons to register for > full-fledged accounts without confirmation (however unlikely that scenario > seems to me). Agreed. Perhaps just adding a caveat in the manual should suffice.
Hm, but delete_unverified_opac_registrations.pl doesn't do that does it? It deletes from borrower_modifications. But that's just for pending updates to existing patrons isn't it? Is that how it's supposed to work? Either way, that script needs help added to it, and should not run without setting an explicit flag to proceed with deletions.
(In reply to comment #77) > Hm, but delete_unverified_opac_registrations.pl doesn't do that does it? It > deletes from borrower_modifications. But that's just for pending updates to > existing patrons isn't it? Is that how it's supposed to work? > > Either way, that script needs help added to it, and should not run without > setting an explicit flag to proceed with deletions. It doesn't touch modifications to existing borrowers. If someone submits a registration from the opac, but never clicks the verification link, that row will just sit in the borrower_modifications table forever. This script deletes rows that have been sitting in the table for more than 24 hours. I'll add some help to it, and while I'm at it, I can make the hours part configurable.
(In reply to comment #77) > Hm, but delete_unverified_opac_registrations.pl doesn't do that does it? It > deletes from borrower_modifications. But that's just for pending updates to > existing patrons isn't it? Is that how it's supposed to work? > > Either way, that script needs help added to it, and should not run without > setting an explicit flag to proceed with deletions. I think the script you are thinking of is misc/cronjobs/delete_expired_opac_registrations.pl That script will delete patrons with the category set in PatronSelfRegistrationDefaultCategory after the specified delay.
Created attachment 12171 [details] [review] Bug 7067 - OPAC Borrower Self Registration - Followup * Rename PatronSelfRegistrationUseTemporaryStatus to PatronSelfRegistrationDefaultCategory * Hide register link unless PatronSelfRegistrationDefaultCategory is set. * Add invalid token page * Add documentation and switches to cron scripts
I guess I should note that no cron script deletes modification to existing borrowers. Those changes will sit in the borrower_modifications table until a librarian either approves or denies the changes, no matter how long that will take.
Thanks for the clarifications Kyle. Unfortunately I have to ask you to rebase this again since we got a new database revision and changes to SQLHelper.pm
Created attachment 12190 [details] [review] Bug 7067 - OPAC Borrower Self Registration Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 12191 [details] [review] Bug 7067 - OPAC Borrower Self Registration - Followup * Rename PatronSelfRegistrationUseTemporaryStatus to PatronSelfRegistrationDefaultCategory * Hide register link unless PatronSelfRegistrationDefaultCategory is set. * Add invalid token page * Add documentation and switches to cron scripts
Sorry: Applying: Bug 7067 - OPAC Borrower Self Registration error: patch failed: installer/data/mysql/sysprefs.sql:372 error: installer/data/mysql/sysprefs.sql: patch does not apply Patch failed at 0001 Bug 7067 - OPAC Borrower Self Registration
Created attachment 12196 [details] [review] Bug 7067 - OPAC Borrower Self Registration Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 12197 [details] [review] Bug 7067 - OPAC Borrower Self Registration - Followup * Rename PatronSelfRegistrationUseTemporaryStatus to PatronSelfRegistrationDefaultCategory * Hide register link unless PatronSelfRegistrationDefaultCategory is set. * Add invalid token page * Add documentation and switches to cron scripts
I'm ready to sign off on this, but I'm wondering if this issue is something which can be left to a follow-up or if it will fail QA without it: (In reply to comment #60) > There's no reason not to have the cronjob run once a minute, but if we start > recommending that configuration, it will need to be patched to use a > lockfile so that it doesn't allow simultaneous jobs to run if it takes > longer than a minute to clear the message queue. I have some template-related fixups but I'd like to get these patches signed off first.
Oops, new small glitch: If PatronSelfRegistrationVerifyByEmail is set to "require," the Primary email field in the patron record update form in the OPAC becomes mandatory even if it's not listed in PatronSelfRegistrationBorrowerMandatoryField. If it's not listed in PatronSelfRegistrationBorrowerMandatoryField and the patron is doing an update, not a new registration, the field should not be required.
Created attachment 12242 [details] [review] Bug 7067 - OPAC Borrower Self Registration - Followup * Rename PatronSelfRegistrationUseTemporaryStatus to PatronSelfRegistrationDefaultCategory * Hide register link unless PatronSelfRegistrationDefaultCategory is set. * Add invalid token page * Add documentation and switches to cron scripts * Add required fields check for editing exiting patrons * Don't force require email address for existing patrons when PatronSelfRegistrationVerifyByEmail is enabled.
Created attachment 12243 [details] [review] [SIGNED-OFF] Bug 7067 - OPAC Borrower Self Registration Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 12244 [details] [review] Whoops, duplicate attachment. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 12245 [details] [review] [SIGNED-OFF] Bug 7067 - OPAC Borrower Self Registration - Followup
Created attachment 12277 [details] [review] Bug 7067 [Follow-up: templates] allow patron self registration via the opac - Adding simple style to register link on home page - Adding unique ids to new pages and to blocks containing registration links - Adding better page titles and breadcrumb links to some pages - Correcting bug which meant incorrect message showed on registration page when OPACPatronDetails preferences is turned off - Passing patron details to opac-memberentry.tt so that patron's name can be displayed in breadcrumbs - Improving display in staff client of patron record updates waiting to be approved. - Adding a sort by name to output of pending patron record updates - Adding updated JqueryUI library files to include expanded widget options. The changes in this patch require the addition of the jQueryUI Accordion widget. Other pending patches are seeking to add enough of the other remaining missing widgets that it seems time to go ahead and add the rest. Future submissions which add usage of these widgets will have to be careful to make changes to Koha's CSS where necessary.
Created attachment 12278 [details] [review] [SIGNED-OFF] Bug 7067 [Follow-up: templates] allow patron self registration via the opac - Adding simple style to register link on home page - Adding unique ids to new pages and to blocks containing registration links - Adding better page titles and breadcrumb links to some pages - Correcting bug which meant incorrect message showed on registration page when OPACPatronDetails preferences is turned off - Passing patron details to opac-memberentry.tt so that patron's name can be displayed in breadcrumbs - Improving display in staff client of patron record updates waiting to be approved. - Adding a sort by name to output of pending patron record updates - Adding updated JqueryUI library files to include expanded widget options. The changes in this patch require the addition of the jQueryUI Accordion widget. Other pending patches are seeking to add enough of the other remaining missing widgets that it seems time to go ahead and add the rest. Future submissions which add usage of these widgets will have to be careful to make changes to Koha's CSS where necessary. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Changing target version so this bug report will show up in the list of 3.12-targeted features and doesn't get lost in the shuffle during feature freeze.
Created attachment 13038 [details] [review] Bug 7067 - OPAC Borrower Self Registration Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 13039 [details] [review] Bug 7067 - OPAC Borrower Self Registration - Followup * Rename PatronSelfRegistrationUseTemporaryStatus to PatronSelfRegistrationDefaultCategory * Hide register link unless PatronSelfRegistrationDefaultCategory is set. * Add invalid token page * Add documentation and switches to cron scripts * Add required fields check for editing exiting patrons * Don't force require email address for existing patrons when PatronSelfRegistrationVerifyByEmail is enabled. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 13040 [details] [review] Bug 7067 [Follow-up: templates] allow patron self registration via the opac - Adding simple style to register link on home page - Adding unique ids to new pages and to blocks containing registration links - Adding better page titles and breadcrumb links to some pages - Correcting bug which meant incorrect message showed on registration page when OPACPatronDetails preferences is turned off - Passing patron details to opac-memberentry.tt so that patron's name can be displayed in breadcrumbs - Improving display in staff client of patron record updates waiting to be approved. - Adding a sort by name to output of pending patron record updates - Adding updated JqueryUI library files to include expanded widget options. The changes in this patch require the addition of the jQueryUI Accordion widget. Other pending patches are seeking to add enough of the other remaining missing widgets that it seems time to go ahead and add the rest. Future submissions which add usage of these widgets will have to be careful to make changes to Koha's CSS where necessary. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 13048 [details] [review] Bug 7067: Followup FIX QA fails This patch fixes: * Koha/Borrower/Modifications.pm FAIL pod FAIL *** ERROR: Unknown command 'head' in file Koha/Borrower/Modifications.pm *** WARNING: Verbatim paragraph in NAME section in file Koha/Borrower/Modifications.pm * C4/Auth.pm FAIL forbidden patterns FAIL The patch introduces a forbidden pattern: tabulation character (983) The patch introduces a forbidden pattern: tabulation character (982) * misc/cronjobs/delete_expired_opac_registrations.pl FAIL critic FAIL "require" statement with library name as string at line 28, column 12. Use a bareword instead. * misc/cronjobs/delete_unverified_opac_registrations.pl FAIL critic FAIL "require" statement with library name as string at line 28, column 12. Use a bareword instead. QA tools raise too: * C4/Members.pm FAIL valid FAIL Subroutine AddMember_Opac redefined It is 'normal' (Circular dependencies and new routine).
Created attachment 13053 [details] [review] Bug 7067: Followup FIX merge updatedatabase
Well done :) Works as expected so it passes QA !
(In reply to comment #102) > Well done :) Works as expected so it passes QA ! Excellent! Thanks for the followups Janathan!
Could someone please sign off on the last two patches? Once they've been signed off, this can be set back to "Passed QA" and I will review it.
Created attachment 13259 [details] [review] [SIGNED-OFF] Bug 7067: Followup FIX QA fails Signed-off-by: Owen Leonard <oleonard@myacpl.org> Fixed minor conflict in C4/Installer/PerlDependencies.pm
Created attachment 13260 [details] [review] [SIGNED-OFF] Bug 7067: Followup FIX merge updatedatabase Signed-off-by: Owen Leonard <oleonard@myacpl.org>
(In reply to comment #104) > Could someone please sign off on the last two patches? Once they've been > signed off, this can be set back to "Passed QA" and I will review it. Jared, Sorry about that. I thought I could pass QA directly without requesting a signed off for my patch, given that it was simple modifications (merge marker, pod adjustement and tabulations characters). It was in order to avoid another SO and QA (by someone else looking at it my own patch). I will be more respecful toward the procedure next time.
(In reply to comment #107) > (In reply to comment #104) > > Could someone please sign off on the last two patches? Once they've been > > signed off, this can be set back to "Passed QA" and I will review it. > > Jared, > Sorry about that. I thought I could pass QA directly without requesting a > signed off for my patch, given that it was simple modifications (merge > marker, pod adjustement and tabulations characters). It was in order to > avoid another SO and QA (by someone else looking at it my own patch). > I will be more respecful toward the procedure next time. No problem. Since the patches meet the requirements set out by QA, I am marking this passed QA now and will review the patch.
Due to a conflict on the first patch, the third patch refuses to apply at all (sha1 information lacking or useless). Owen, since you have a branch with these patches could you please rebase the branch on latest master and re-upload?
Created attachment 13744 [details] [review] Bug 7067 [1] OPAC Borrower Self Registration
Created attachment 13745 [details] [review] Bug 7067 [2] OPAC Borrower Self Registration - Followup * Rename PatronSelfRegistrationUseTemporaryStatus to PatronSelfRegistrationDefaultCategory * Hide register link unless PatronSelfRegistrationDefaultCategory is set. * Add invalid token page * Add documentation and switches to cron scripts * Add required fields check for editing exiting patrons * Don't force require email address for existing patrons when PatronSelfRegistrationVerifyByEmail is enabled.
Created attachment 13746 [details] [review] Bug 7067 [3] [Follow-up: templates] allow patron self registration via the opac - Adding simple style to register link on home page - Adding unique ids to new pages and to blocks containing registration links - Adding better page titles and breadcrumb links to some pages - Correcting bug which meant incorrect message showed on registration page when OPACPatronDetails preferences is turned off - Passing patron details to opac-memberentry.tt so that patron's name can be displayed in breadcrumbs - Improving display in staff client of patron record updates waiting to be approved. - Adding a sort by name to output of pending patron record updates - Adding updated JqueryUI library files to include expanded widget options. The changes in this patch require the addition of the jQueryUI Accordion widget. Other pending patches are seeking to add enough of the other remaining missing widgets that it seems time to go ahead and add the rest. Future submissions which add usage of these widgets will have to be careful to make changes to Koha's CSS where necessary.
Created attachment 13747 [details] [review] Bug 7067 [4] Followup FIX QA fails
Created attachment 13748 [details] [review] Bug 7067 [5] Followup FIX merge updatedatabase
All patches rebased on current master.
Owen, do you think you could push your branch to gitorious (or somewhere)? I still can't seem to apply this, so I'll just cherry-pick directly off your branch.
(In reply to comment #116) > Owen, do you think you could push your branch to gitorious (or somewhere)? I > still can't seem to apply this, so I'll just cherry-pick directly off your > branch. Hi Jared, It's my fault, I proposed a dirty followup to avoid to resubmit an already signed-off patch. So the last patch just moves the new database entry to the end of the file. I pushed the branch with a new last patch in our git repository: http://git.biblibre.com/?p=kohac;a=shortlog;h=refs/heads/ft/Bug7067
Created attachment 13765 [details] [review] Bug 7067 - OPAC Borrower Self Registration This development will add the ability for a new patron to register himself or herself. The self-registration will attempt to match this newly inputted data to any existing patrons and if any possible matches are found, ask if the patron is sure he or she doesn't already have an account at the library. A system preference may be set to prevent patron self-registration if the system detects the possibility that the person may already have an account. Once the patron has registered, passing a captcha (or similar bot-stopper), the patron will then be optionally verified a second time via email. At this point, the patron will be able to print a temporary library card (optional by system preference), and will be provided any details necessary to access electronic resources (this body of text would be a template in the slips and notices system). At the library's choice, this new patron would either be set to a temporary patron status (patron type set via system preference), or a fully-fledged patron (allow patron type to be determined by age and/or other attributes). Assuming the library uses temporary patron types for OPAC registrations, this patron will next enter a queue and would need to physically enter the library to verify himself and become a fully-fledged patron (most likely by bringing in physical proof of address, etc.). The librarian would look up the patron record and modify the patron type. If a temporary patron has not been verified within a certain time frame (defined by a system preference), the patron record will be deleted from the system via a cron job. For registered patrons, the system will allow each person to also update his or her personal data via the OPAC. When a patron updates his or her information, the changes will be entered into a queue to be verified by a librarian (preventing a patron from inputting obviously bogus data). The staff client home page will display the number of patron records with changes awaiting approval. A librarian would then be able to click through a list of modification requests, and approve or deny each (with approval and denial alerts being sent to the patron via the standard messaging system). NEW SYSTEM PREFERENCES * PatronSelfRegistration * PatronSelfRegistrationDetectDuplicates * PatronSelfRegistrationVerifyByEmail * PatronSelfRegistrationPrintTemporaryCard * PatronSelfRegistrationUseTemporaryStatus * PatronSelfRegistrationExpireTemporaryAccountsDelay NEW NOTICE * Verify by email notice NEW SLIP * Temporary card slip NEW CRON JOB * delete_expired_opac_registrations.pl - Deletes patrons that have not been upgraded from the temporary status within the specified delay * delete_unverified_opac_registrations.pl - Deletes the unverified patrons based on the length of time specified in the PatronSelfRegistrationExpireTemporaryAccountsDelay The patron will register from self_registration.pl, linked off opac-main.pl if enabled. The registration page will be translatable to other languages in the same way that existing templates are. Test Plan: 1) Enable PatronSelfRegistration 2) Set PatronSelfRegistrationExpireTemporaryAccountsDelay to a number of days 3) Create a self-registered borrower category 4) Set PatronSelfRegistrationUseTemporaryStatus 5) Set PatronSelfRegistrationVerifyByEmail to "Don't require" 6) Go to OPAC, log out if logged in. 7) You should see the "Register here" link below the login box 8) Attempt to register yourself 9) Verify you can log in with your temporary password. 10) Set PatronSelfRegistrationVerifyByEmail to "Require" 11) Attempt another self-registration 12) Check the messages table, you should see a new message with a verification link. 13) Copy and paste the link into a web browser to verify the registration 14) Log in with the given credentials to verify the account was created. Test Plan - Part 2 - Borrower Modifications 1) Log in to OPAC, go to "my personal details" tab. 2) Make some modifications to your details. 3) Repeat steps 1 and 2 for two more borrowers. 4) Log in to Koha intranet with a user that can modify borrowers. 5) At the bottom of mainpage.pl, you should see: Patrons requesting modifications: 3 6) Click the link 7) Approve one change, deny a different one, and ignore the third, then submit. 8) Check the records, you should see the changes take affect on the approved one, and no changes to the other two. You should also see "Patrons requesting modifications: 1" at the bottom of mainpage.pl now. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 13766 [details] [review] Bug 7067 - OPAC Borrower Self Registration - Followup * Rename PatronSelfRegistrationUseTemporaryStatus to PatronSelfRegistrationDefaultCategory * Hide register link unless PatronSelfRegistrationDefaultCategory is set. * Add invalid token page * Add documentation and switches to cron scripts * Add required fields check for editing exiting patrons * Don't force require email address for existing patrons when PatronSelfRegistrationVerifyByEmail is enabled. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Passed-QA-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 13767 [details] [review] Bug 7067 [Follow-up: templates] allow patron self registration via the opac - Adding simple style to register link on home page - Adding unique ids to new pages and to blocks containing registration links - Adding better page titles and breadcrumb links to some pages - Correcting bug which meant incorrect message showed on registration page when OPACPatronDetails preferences is turned off - Passing patron details to opac-memberentry.tt so that patron's name can be displayed in breadcrumbs - Improving display in staff client of patron record updates waiting to be approved. - Adding a sort by name to output of pending patron record updates - Adding updated JqueryUI library files to include expanded widget options. The changes in this patch require the addition of the jQueryUI Accordion widget. Other pending patches are seeking to add enough of the other remaining missing widgets that it seems time to go ahead and add the rest. Future submissions which add usage of these widgets will have to be careful to make changes to Koha's CSS where necessary. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Passed-QA-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 13768 [details] [review] Bug 7067: Followup FIX QA fails Signed-off-by: Owen Leonard <oleonard@myacpl.org> Fixed minor conflict in C4/Installer/PerlDependencies.pm
Created attachment 13769 [details] [review] Bug 7067 [5] Followup FIX merge updatedatabase
There are no unit tests for Koha::Borrower::Modifications, nor for the AddMember_Opac routine (any other routines added should also have unit tests, of course). Starting with the 3.12 release cycle, unit tests are required for code added to the Koha:: and C4:: namespaces. Also, please use hashrefs rather than hashes as arguments in the Koha:: namespace. I could have sworn you did a follow-up changing all the hash arguments to hashrefs but I don't see it anywhere, even among the obsolete patches. Other notes: * Do not access the database in BEGIN {} blocks, especially not in the Koha:: namespace. * Use of C4::SQLHelper from the Koha:: namespace. Calling into the C4:: namespace from Koha:: is not supposed to be done. If that was the only objection, I would probably push it anyway, at least this time, but arguably I probably shouldn't. * Package-level my variables are verboten, since they break persistence, and replacing "my" with "our" should be done only under extreme duress, and never in new code (note: you can use our when it's called for by the code, just not as a workaround for my not working under Plack). Thank you for including a test plan on the bug. I copied it (and the original RFC) into the commit message for the first patch. I did not review it for accuracy yet, since I discovered the lack of unit tests before I got that far, so you may want to do that. Also, I noticed a few other issues that would not prevent me pushing this but you might want to keep in mind for the future: * The standard for help in command-line scripts is to use pod2usage. * Object-oriented classes do not export routines and therefore should not use Exporter. Even procedural classes that do not export any routines should not use Exporter. * When creating ->new() subroutines, the following idiom may be useful: return bless( { 'verification_token' => $args{'verification_token'}, ... }, $class ); Or even: return bless( $args, $class ); I find those two idioms make code easier to read, and certainly save typing, but they are by no means required.
Created attachment 13984 [details] [review] Bug 7067 [6] - QA Followup
(In reply to comment #123) > There are no unit tests for Koha::Borrower::Modifications, nor for the > AddMember_Opac routine (any other routines added should also have unit > tests, of course). Starting with the 3.12 release cycle, unit tests are > required for code added to the Koha:: and C4:: namespaces. Add a Unit Test. > Also, please use hashrefs rather than hashes as arguments in the Koha:: > namespace. I could have sworn you did a follow-up changing all the hash > arguments to hashrefs but I don't see it anywhere, even among the obsolete > patches. Fixed > Other notes: > * Do not access the database in BEGIN {} blocks, especially not in the > Koha:: namespace. Fixed > * Use of C4::SQLHelper from the Koha:: namespace. Calling into the C4:: > namespace from Koha:: is not supposed to be done. If that was the only > objection, I would probably push it anyway, at least this time, but arguably > I probably shouldn't. This is the only one I haven't changed. Once we have DBIx::Class support, I pledge to switch this module from C4::SQLHelper to that. > * Package-level my variables are verboten, since they break persistence, and > replacing "my" with "our" should be done only under extreme duress, and > never in new code (note: you can use our when it's called for by the code, > just not as a workaround for my not working under Plack). Fixed > * Object-oriented classes do not export routines and therefore should not > use Exporter. Even procedural classes that do not export any routines should > not use Exporter. Fixed > * When creating ->new() subroutines, the following idiom may be useful: > return bless( { 'verification_token' => $args{'verification_token'}, ... > }, $class ); > > Or even: > return bless( $args, $class ); Fixed
Setting back to 'Signed Off' as this is a QA Followup only.
Created attachment 14001 [details] [review] Bug 7067 [7] - QA Followup 2
We are getting very close, I think. There are four remaining problems: 1) Two of the unit tests fail: [...] not ok 7 # Failed test at t/db_dependent/Koha_borrower_modifications.t line 55. not ok 8 # Failed test at t/db_dependent/Koha_borrower_modifications.t line 56. [...] 2) The unit tests do not have labels that would indicate what they are testing. 3) The breadcrumb on /cgi-bin/koha/members/members-update.pl has an incorrect link for the "Patrons" section (/cgi-bin/koha/members-home.pl instead of /cgi-bin/koha/members/members-home.pl). 4) When I try to confirm a patron via e-mail, I get the following in my logs and the patron is not created: INSERT INTO borrowers SET borrowers.dateenrolled = ? , borrowers.userid = ? , borrowers.categorycode = ? , borrowers.dateexpiry = ? , borrowers.password = ? , borrowers.cardnumber = ? 2012-12-13,,PT,2021-03-13,4eTMRTxoZ2tV15HgDg4fXw,23529001223641 at /home/jcamins/kohaclone/C4/SQLHelper.pm line 182. DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha`.`borrowers`, CONSTRAINT `borrowers_ibfk_2` FOREIGN KEY (`branchcode`) REFERENCES `branches` (`branchcode`)) at /home/jcamins/kohaclone/C4/SQLHelper.pm line 184.DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha`.`borrowers`, CONSTRAINT `borrowers_ibfk_2` FOREIGN KEY (`branchcode`) REFERENCES `branches` (`branchcode`)) at /home/jcamins/kohaclone/C4/SQLHelper.pm line 184. Not blocking this patch, but just a comment: I am slightly dubious about the wisdom of allowing the public to register users on the OPAC without giving staff the ability to review those registrations. However, if this meets the needs of the library that requested the feature, I have no objection to including it in Koha as-is. Those libraries concerned with security can simply disable the feature.
Created attachment 14085 [details] [review] Bug 7067 - OPAC Borrower Self Registration This development will add the ability for a new patron to register himself or herself. The self-registration will attempt to match this newly inputted data to any existing patrons and if any possible matches are found, ask if the patron is sure he or she doesn't already have an account at the library. A system preference may be set to prevent patron self-registration if the system detects the possibility that the person may already have an account. Once the patron has registered, passing a captcha (or similar bot-stopper), the patron will then be optionally verified a second time via email. At this point, the patron will be able to print a temporary library card (optional by system preference), and will be provided any details necessary to access electronic resources (this body of text would be a template in the slips and notices system). At the library's choice, this new patron would either be set to a temporary patron status (patron type set via system preference), or a fully-fledged patron (allow patron type to be determined by age and/or other attributes). Assuming the library uses temporary patron types for OPAC registrations, this patron will next enter a queue and would need to physically enter the library to verify himself and become a fully-fledged patron (most likely by bringing in physical proof of address, etc.). The librarian would look up the patron record and modify the patron type. If a temporary patron has not been verified within a certain time frame (defined by a system preference), the patron record will be deleted from the system via a cron job. For registered patrons, the system will allow each person to also update his or her personal data via the OPAC. When a patron updates his or her information, the changes will be entered into a queue to be verified by a librarian (preventing a patron from inputting obviously bogus data). The staff client home page will display the number of patron records with changes awaiting approval. A librarian would then be able to click through a list of modification requests, and approve or deny each (with approval and denial alerts being sent to the patron via the standard messaging system). NEW SYSTEM PREFERENCES * PatronSelfRegistration * PatronSelfRegistrationDetectDuplicates * PatronSelfRegistrationVerifyByEmail * PatronSelfRegistrationPrintTemporaryCard * PatronSelfRegistrationUseTemporaryStatus * PatronSelfRegistrationExpireTemporaryAccountsDelay NEW NOTICE * Verify by email notice NEW SLIP * Temporary card slip NEW CRON JOB * delete_expired_opac_registrations.pl - Deletes patrons that have not been upgraded from the temporary status within the specified delay * delete_unverified_opac_registrations.pl - Deletes the unverified patrons based on the length of time specified in the PatronSelfRegistrationExpireTemporaryAccountsDelay The patron will register from self_registration.pl, linked off opac-main.pl if enabled. The registration page will be translatable to other languages in the same way that existing templates are. Test Plan: 1) Enable PatronSelfRegistration 2) Set PatronSelfRegistrationExpireTemporaryAccountsDelay to a number of days 3) Create a self-registered borrower category 4) Set PatronSelfRegistrationUseTemporaryStatus 5) Set PatronSelfRegistrationVerifyByEmail to "Don't require" 6) Go to OPAC, log out if logged in. 7) You should see the "Register here" link below the login box 8) Attempt to register yourself 9) Verify you can log in with your temporary password. 10) Set PatronSelfRegistrationVerifyByEmail to "Require" 11) Attempt another self-registration 12) Check the messages table, you should see a new message with a verification link. 13) Copy and paste the link into a web browser to verify the registration 14) Log in with the given credentials to verify the account was created. Test Plan - Part 2 - Borrower Modifications 1) Log in to OPAC, go to "my personal details" tab. 2) Make some modifications to your details. 3) Repeat steps 1 and 2 for two more borrowers. 4) Log in to Koha intranet with a user that can modify borrowers. 5) At the bottom of mainpage.pl, you should see: Patrons requesting modifications: 3 6) Click the link 7) Approve one change, deny a different one, and ignore the third, then submit. 8) Check the records, you should see the changes take affect on the approved one, and no changes to the other two. You should also see "Patrons requesting modifications: 1" at the bottom of mainpage.pl now. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Bug 7067 - OPAC Borrower Self Registration - Followup * Rename PatronSelfRegistrationUseTemporaryStatus to PatronSelfRegistrationDefaultCategory * Hide register link unless PatronSelfRegistrationDefaultCategory is set. * Add invalid token page * Add documentation and switches to cron scripts * Add required fields check for editing exiting patrons * Don't force require email address for existing patrons when PatronSelfRegistrationVerifyByEmail is enabled. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Passed-QA-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 14086 [details] [review] Bug 7067 [Follow-up: templates] allow patron self registration via the opac - Adding simple style to register link on home page - Adding unique ids to new pages and to blocks containing registration links - Adding better page titles and breadcrumb links to some pages - Correcting bug which meant incorrect message showed on registration page when OPACPatronDetails preferences is turned off - Passing patron details to opac-memberentry.tt so that patron's name can be displayed in breadcrumbs - Improving display in staff client of patron record updates waiting to be approved. - Adding a sort by name to output of pending patron record updates - Adding updated JqueryUI library files to include expanded widget options. The changes in this patch require the addition of the jQueryUI Accordion widget. Other pending patches are seeking to add enough of the other remaining missing widgets that it seems time to go ahead and add the rest. Future submissions which add usage of these widgets will have to be careful to make changes to Koha's CSS where necessary. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Passed-QA-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 14087 [details] [review] Bug 7067: Followup FIX QA fails Signed-off-by: Owen Leonard <oleonard@myacpl.org> Fixed minor conflict in C4/Installer/PerlDependencies.pm
Created attachment 14088 [details] [review] Bug 7067 Followup: FIX merge updatedatabase
Created attachment 14089 [details] [review] Bug 7067 QA Followup Adjusts calling conventions to use hashrefs and eliminate redundant procedural/OO mixed code.
Created attachment 14090 [details] [review] Bug 7067 QA Followup - Add descriptions to tests
Created attachment 14092 [details] [review] Bug 7067 - QA Followup - Fix failing tests
Created attachment 14093 [details] [review] Bug 7067 - Fix bad url in breadcrumbs
At this point the only thing blocking this patch is: 4) When I try to confirm a patron via e-mail, I get the following in my logs and the patron is not created (when I click on the link in the e-mail): INSERT INTO borrowers SET borrowers.dateenrolled = ? , borrowers.userid = ? , borrowers.categorycode = ? , borrowers.dateexpiry = ? , borrowers.password = ? , borrowers.cardnumber = ? 2012-12-13,,PT,2021-03-13,4eTMRTxoZ2tV15HgDg4fXw,23529001223641 at /home/jcamins/kohaclone/C4/SQLHelper.pm line 182. DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha`.`borrowers`, CONSTRAINT `borrowers_ibfk_2` FOREIGN KEY (`branchcode`) REFERENCES `branches` (`branchcode`)) at /home/jcamins/kohaclone/C4/SQLHelper.pm line 184.DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha`.`borrowers`, CONSTRAINT `borrowers_ibfk_2` FOREIGN KEY (`branchcode`) REFERENCES `branches` (`branchcode`)) at /home/jcamins/kohaclone/C4/SQLHelper.pm line 184.
Created attachment 14119 [details] [review] Bug 7067 - QA Followup - Fix error when confirming via email
Gets this error Not enough arguments for C4::Members::fixup_cardnumber at /usr/share/koha/lib/C4/Members.pm line 2361, near "()" Compilation failed in require at /usr/share/koha/lib/C4/Auth.pm line 149.
(In reply to comment #139) > Gets this error > > Not enough arguments for C4::Members::fixup_cardnumber at > /usr/share/koha/lib/C4/Members.pm line 2361, near "()" > Compilation failed in require at /usr/share/koha/lib/C4/Auth.pm line 149. What does?
(In reply to comment #140) > (In reply to comment #139) > > Gets this error > > > > Not enough arguments for C4::Members::fixup_cardnumber at > > /usr/share/koha/lib/C4/Members.pm line 2361, near "()" > > Compilation failed in require at /usr/share/koha/lib/C4/Auth.pm line 149. > > What does? Trying to make this in version 3.8.07 but gets this message on opac and intranet. In log is [Sat Dec 15 21:34:41 2012] [error] [client 127.0.0.1] (2)No such file or directory: exec of '/usr/share/koha/intranet/cgi-bin/mainpage.pl' failed [Sat Dec 15 21:34:41 2012] [error] [client 127.0.0.1] Premature end of script headers: mainpage.pl [Sat Dec 15 21:34:42 2012] [error] [client 127.0.0.1] [Sat Dec 15 21:34:42 2012] 500.pl: Not enough arguments for C4::Members::fixup_cardnumber at /usr/share/koha/lib/C4/Members.pm line 2361, near "()" [Sat Dec 15 21:34:42 2012] [error] [client 127.0.0.1] [Sat Dec 15 21:34:42 2012] 500.pl: Compilation failed in require at /usr/share/koha/lib/C4/Auth.pm line 149.
(In reply to comment #141) > Trying to make this in version 3.8.07 but gets this message on opac and > intranet. In log is These patches are for 3.12+ only. They were not intended for backporting to 3.8 (and, as you can see, rebasing causes syntax errors). Resetting the status to "Pushed to Master." If you have further questions about backporting, I suggest you take them to the koha-devel mailing list, as bugzilla is not really the place for that sort of discussion and many people who might be able to offer advice would not read your messages.
One problem with this. opac-memberentry.pl says 'mobile phone' for the third phone - it should be 'other phone' like in the staff client cause the primary phone might be your mobile phone.
Also the captcha is case sensitive which is unusual. Usually I just type the letters and case doesn't matter - this might confuse people. Nicole
Created attachment 14190 [details] no username Sorry for all the updates to this - I was about to document it and when I registered via the OPAC I got the attached. It shows me a password but no username ... the username wasn't generated. Nicole
Created attachment 14191 [details] [review] Bug 7067 - Followup - Change 'mobile phone' to 'other phone', run typed in captcha through uc before comparing.
Nicole, I cannot recreate this issue. AFAIK, there are no system preferences related to the automatic generation of user names, are there? (In reply to comment #145) > Created attachment 14190 [details] > no username > > Sorry for all the updates to this - I was about to document it and when I > registered via the OPAC I got the attached. It shows me a password but no > username ... the username wasn't generated. > > Nicole
Hmm, There is no preference and when I create a patron in the staff client the username is generated fine: http://screencast.com/t/OPRjsR80EAi
Hi Nicole, Did you fill the syspref PatronSelfRegistrationDefaultCategory with an existing patron category? There are errors in your log file?
(In reply to comment #149) > Hi Nicole, > Did you fill the syspref PatronSelfRegistrationDefaultCategory with an > existing patron category? > There are errors in your log file? BINGO! I had an invalid default category :) Thanks Jonathan
Created attachment 14193 [details] [review] Bug 7067 - Followup - Add note to circulation.pl if borrower has pending modifications
Created attachment 14194 [details] [review] Bug 7067 - Followup - Add note to circulation.pl if borrower has pending modifications
Created attachment 14282 [details] [review] Bug 7067 - Followup - Change 'mobile phone' to 'other phone', run typed in captcha through uc before comparing. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Label changed and captcha is not case sensitive
Created attachment 14283 [details] [review] Bug 7067 - Followup - Add note to circulation.pl if borrower has pending modifications Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> New link "patron modifications" (if there are) on the checkout page.
If these two patches are intended for inclusion in 3.12, a new bug needs to reported for them, to which they need to be attached with their subject lines changed to reflect their new bug number. If the follow-ups are unrelated, two separate bugs would make sense.
*** Bug 4848 has been marked as a duplicate of this bug. ***
Patches moved to http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9393 (In reply to comment #155) > If these two patches are intended for inclusion in 3.12, a new bug needs to > reported for them, to which they need to be attached with their subject > lines changed to reflect their new bug number. If the follow-ups are > unrelated, two separate bugs would make sense.
RFC: http://wiki.koha-community.org/wiki/Online_Borrower_Registration_RFC