Description
Kelly McElligott
2019-09-04 16:53:28 UTC
This is something that we've implemented locally for clients. I have other priorities at the moment, but I could look at upstreaming that code when I have a chance. Hi all, I've implemented functionality very similar to what is requested on this bug report in Bug 25090 Our client wanted to be able to moderate which patron self registrations got OPAC logins (i.e. a record in the borrowers table) due to wanting to restrict paid for access to online resources (for which users need a Koha OPAC login) to only those self registrations from within their organisation. With PatronSelfRegistrationVerifybyEmail syspref patrons do not get a patron record (OPAC login) until they've clicked the verification link in the email. However it gave the library no control over which self registrations got OPAC logins. The functionality in bug 25090 allows librarians to control who receives the verification email, (much the same way as: EmailPurchaseSuggestions and EmailAddressForSuggestions) if set to anything other than 'email address of registering patron' then a notice is sent to librarians listing some patron details. If the library decides that registration is valid (because the user is in the same organisation for example) they can click the verification link and a password reset link is sent to the registering patron to reset their credentials and login to the OPAC. I'd really appreciate any thoughts/feedback on bug 25090 :) Many thanks, Alex Created attachment 133050 [details] [review] Bug 23538: Database changes Sponsored-by: Catalyst IT Created attachment 133051 [details] [review] Bug 23538: Email library when new patron self-registers Test plan: 1. Apply patchset 2. Update database: - cd installer/data/mysql - sudo koha-shell <instance> - ./updatedatabase.pl 3. Restart services 4. Set email addresses in: - KohaAdminEmailAddress syspref - EmailAddressForPatronRegistrations syspref - Email address in the library branch 5. Enable PatronSelfRegistration syspref 6. Test the following use cases: 6.a. EmailPatronRegistrations syspref = 'none'. Submit OPAC registration. = OUTCOME: Confirm no OPAC_REG notice in message_queue table 6.b. EmailPatronRegistrations syspref = 'email address of branch'. Submit OPAC registration. = OUTCOME: Confirm OPAC_REG notice in message_queue with to_address of branch address 6.c. EmailPatronRegistrations syspref = 'EmailAddressForPatronRegistrations'. Submit OPAC registration. = OUTCOME: Confirm OPAC_REG notice in message_queue with to_address equalling EmailAddressForPatronRegistrations syspref 6.d. EmailPatronRegistrations syspref = 'KohaAdminEmailAddress'. Submit OPAC registration. = OUTCOME: Confirm OPAC_REG notice in message_queue with to_address equalling KohaAdminEmaiLAddress syspref 7. Enable PatronSelfRegistrationVerifyByEmail syspref 8. Repeat steps 6.a, 6.b, 6.c, 6.d but this time confirm the expected outcomes only happen AFTER you have clicked the verification link in the OPAC_REG_VERIFY notice Sponsored-by: Catalyst IT Created attachment 133052 [details] [review] Bug 23538: Unit tests Test plan: 1. Run unit tests: sudo koha-shell <instance> cd t/db_dependent/Koha prove -v Patron.t Sponsored-by: Catalyst IT Created attachment 133053 [details] [review] Bug 23538: (follow-up) Fix up notices - Add toolkit template syntax to notices - Add translated notice Sponsored-by: Catalyst IT Created attachment 133097 [details] [review] Bug 23538: (follow-up) Fix up notices - Add toolkit template (TT) syntax to notices - Add TT conditionals to notices - Add translated notice Sponsored-by: Catalyst IT Created attachment 133098 [details] [review] Bug 23538: Email library when new patron self-registers Test plan: 1. Apply patchset 2. Update database: - cd installer/data/mysql - sudo koha-shell <instance> - ./updatedatabase.pl 3. Restart services 4. Set email addresses in: - KohaAdminEmailAddress syspref - EmailAddressForPatronRegistrations syspref - Email address in the library branch 5. Enable PatronSelfRegistration syspref 6. Test the following use cases: 6.a. EmailPatronRegistrations syspref = 'none'. Submit OPAC registration. = OUTCOME: Confirm no OPAC_REG notice in message_queue table 6.b. EmailPatronRegistrations syspref = 'email address of branch'. Submit OPAC registration. = OUTCOME: Confirm OPAC_REG notice in message_queue with to_address of branch address 6.c. EmailPatronRegistrations syspref = 'EmailAddressForPatronRegistrations'. Submit OPAC registration. = OUTCOME: Confirm OPAC_REG notice in message_queue with to_address equalling EmailAddressForPatronRegistrations syspref 6.d. EmailPatronRegistrations syspref = 'KohaAdminEmailAddress'. Submit OPAC registration. = OUTCOME: Confirm OPAC_REG notice in message_queue with to_address equalling KohaAdminEmaiLAddress syspref 7. Enable PatronSelfRegistrationVerifyByEmail syspref 8. Repeat steps 6.a, 6.b, 6.c, 6.d but this time confirm the expected outcomes only happen AFTER you have clicked the verification link in the OPAC_REG_VERIFY notice Sponsored-by: Catalyst IT Created attachment 133099 [details] [review] Bug 23538: Unit tests Test plan: 1. Run unit tests: sudo koha-shell <instance> cd t/db_dependent/Koha prove -v Patron.t Sponsored-by: Catalyst IT Created attachment 133100 [details] [review] Bug 23538: (follow-up) Fix up notices - Add toolkit template (TT) syntax to notices - Add TT conditionals to notices - Add translated notice Sponsored-by: Catalyst IT Created attachment 133101 [details] [review] Bug 23538: Email library when new patron self-registers Test plan: 1. Apply patchset 2. Update database: - cd installer/data/mysql - sudo koha-shell <instance> - ./updatedatabase.pl 3. Restart services 4. Set email addresses in: - KohaAdminEmailAddress syspref - EmailAddressForPatronRegistrations syspref - Email address in the library branch 5. Enable PatronSelfRegistration syspref 6. Test the following use cases: 6.a. EmailPatronRegistrations syspref = 'none'. Submit OPAC registration. = OUTCOME: Confirm no OPAC_REG notice in message_queue table 6.b. EmailPatronRegistrations syspref = 'email address of branch'. Submit OPAC registration. = OUTCOME: Confirm OPAC_REG notice in message_queue with to_address of branch address 6.c. EmailPatronRegistrations syspref = 'EmailAddressForPatronRegistrations'. Submit OPAC registration. = OUTCOME: Confirm OPAC_REG notice in message_queue with to_address equalling EmailAddressForPatronRegistrations syspref 6.d. EmailPatronRegistrations syspref = 'KohaAdminEmailAddress'. Submit OPAC registration. = OUTCOME: Confirm OPAC_REG notice in message_queue with to_address equalling KohaAdminEmaiLAddress syspref 7. Enable PatronSelfRegistrationVerifyByEmail syspref 8. Repeat steps 6.a, 6.b, 6.c, 6.d but this time confirm the expected outcomes only happen AFTER you have clicked the verification link in the OPAC_REG_VERIFY notice Sponsored-by: Catalyst IT Created attachment 133102 [details] [review] Bug 23538: Unit tests Test plan: 1. Run unit tests: sudo koha-shell <instance> cd t/db_dependent/Koha prove -v Patron.t Sponsored-by: Catalyst IT Created attachment 133103 [details] [review] Bug 23538: (follow-up) Fix up notices - Add toolkit template (TT) syntax to notices - Add TT conditionals to notices - Add translated notice Sponsored-by: Catalyst IT I linked bug 25090 in comment #2 but I think that will need me to do some re-work to get it working better. So what I've done now is write this patchset, so librarians can at least choose if they are notified when a new patron self-registers. This is similar functionality to emails sent to the library when an OPAC purchase suggestion is made. Ready to test! Hi Alex, it looks like there is a small conflict in sample_notices.yml in current master. Can this be rebased? Created attachment 137997 [details] [review] Bug 23538: Database changes Sponsored-by: Catalyst IT Created attachment 137998 [details] [review] Bug 23538: Email library when new patron self-registers Test plan: 1. Apply all patches 2. Update database 3. Restart services 4. Set email addresses in: - KohaAdminEmailAddress syspref - EmailAddressForPatronRegistrations syspref - Email address in the library branch 5. Enable PatronSelfRegistration syspref 6. Test the following use cases: - EmailPatronRegistrations syspref = 'none'. Submit OPAC registration. = OUTCOME: Confirm no OPAC_REG notice in message_queue table --- - EmailPatronRegistrations syspref = 'email address of branch'. Submit OPAC registration. = OUTCOME: Confirm OPAC_REG notice in message_queue with to_address equalling the branch address --- - EmailPatronRegistrations syspref = 'EmailAddressForPatronRegistrations'. Submit OPAC registration. = OUTCOME: Confirm OPAC_REG notice in message_queue with to_address equalling EmailAddressForPatronRegistrations syspref --- - EmailPatronRegistrations syspref = 'KohaAdminEmailAddress'. Submit OPAC registration. = OUTCOME: Confirm OPAC_REG notice in message_queue with to_address equalling KohaAdminEmailAddress syspref 7. Enable PatronSelfRegistrationVerifyByEmail syspref 8. Repeat testing all use cases. Confirm this time the expected outcomes only happen AFTER you have clicked the verification link in the OPAC_REG_VERIFY notice Sponsored-by: Catalyst IT Created attachment 137999 [details] [review] Bug 23538: Email library when new patron self-registers Test plan: 1. Apply all patches 2. Update database 3. Restart services 4. Set email addresses in: - KohaAdminEmailAddress syspref - EmailAddressForPatronRegistrations syspref - Email address in the library branch 5. Enable PatronSelfRegistration syspref 6. Test the following use cases: - EmailPatronRegistrations syspref = 'none'. Submit OPAC registration. = OUTCOME: Confirm no OPAC_REG notice in message_queue table --- - EmailPatronRegistrations syspref = 'email address of branch'. Submit OPAC registration. = OUTCOME: Confirm OPAC_REG notice in message_queue with to_address equalling the branch address --- - EmailPatronRegistrations syspref = 'EmailAddressForPatronRegistrations'. Submit OPAC registration. = OUTCOME: Confirm OPAC_REG notice in message_queue with to_address equalling EmailAddressForPatronRegistrations syspref --- - EmailPatronRegistrations syspref = 'KohaAdminEmailAddress'. Submit OPAC registration. = OUTCOME: Confirm OPAC_REG notice in message_queue with to_address equalling KohaAdminEmailAddress syspref 7. Enable PatronSelfRegistrationVerifyByEmail syspref 8. Repeat testing all use cases. Confirm this time the expected outcomes only happen AFTER you have clicked the verification link in the OPAC_REG_VERIFY notice Sponsored-by: Catalyst IT Created attachment 138000 [details] [review] Bug 23538: Unit tests Test plan: 1. Run unit tests: sudo koha-shell <instance> cd t/db_dependent/Koha prove -v Patron.t Sponsored-by: Catalyst IT Created attachment 138001 [details] [review] Bug 23538: (follow-up) Fix up notices - Add toolkit template (TT) syntax to notices - Add TT conditionals to notices - Add translated notice Sponsored-by: Catalyst IT Hi Lucas, Apologies for the delay rebasing. This is now rebased and is ready for testing again. Thanks, Alex Created attachment 138237 [details] [review] Bug 23538: Database changes Sponsored-by: Catalyst IT Signed-off-by: David Nind <david@davidnind.com> Created attachment 138238 [details] [review] Bug 23538: Email library when new patron self-registers Test plan: 1. Apply all patches 2. Update database 3. Restart services 4. Set email addresses in: - KohaAdminEmailAddress syspref - EmailAddressForPatronRegistrations syspref - Email address in the library branch 5. Enable PatronSelfRegistration syspref 6. Test the following use cases: - EmailPatronRegistrations syspref = 'none'. Submit OPAC registration. = OUTCOME: Confirm no OPAC_REG notice in message_queue table Signed-off-by: David Nind <david@davidnind.com> Created attachment 138239 [details] [review] Bug 23538: Unit tests Test plan: 1. Run unit tests: sudo koha-shell <instance> cd t/db_dependent/Koha prove -v Patron.t Sponsored-by: Catalyst IT Signed-off-by: David Nind <david@davidnind.com> Created attachment 138240 [details] [review] Bug 23538: (follow-up) Fix up notices - Add toolkit template (TT) syntax to notices - Add TT conditionals to notices - Add translated notice Sponsored-by: Catalyst IT Signed-off-by: David Nind <david@davidnind.com> Not sure what happened to the test plan for the 'Email library when new patron self-registers' patch after I signed it off - step 6 and onwards are cut off. Minor suggested changes to the new EmailPatronRegistrations system preference wording: - Revised wording for the first sentence: Choose the email address that new patron self-registration notifications are sent to: [options]. - Option - 'email address of branch': I think the terminology guidelines are to use library instead of branch. - Remove the : at the end of the last sentence and replace with a full stop. I've also suggested some revised wording. ==> Choose the email address that new patron self-registration notifications are sent to: [options]. A valid email address is required for EmailAddressForPatronRegistrations if that option is selected. Created attachment 138383 [details] [review] Bug 23538: Database changes Sponsored-by: Catalyst IT Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 138384 [details] [review] Bug 23538: Email library when new patron self-registers Test plan: 1. Apply all patches 2. Update database 3. Restart services 4. Set email addresses in: - KohaAdminEmailAddress syspref - EmailAddressForPatronRegistrations syspref - Email address in the library branch 5. Enable PatronSelfRegistration syspref 6. Test the following use cases: - EmailPatronRegistrations syspref = 'none'. Submit OPAC registration. = OUTCOME: Confirm no OPAC_REG notice in message_queue table Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 138385 [details] [review] Bug 23538: Unit tests Test plan: 1. Run unit tests: sudo koha-shell <instance> cd t/db_dependent/Koha prove -v Patron.t Sponsored-by: Catalyst IT Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 138386 [details] [review] Bug 23538: (follow-up) Fix up notices - Add toolkit template (TT) syntax to notices - Add TT conditionals to notices Sponsored-by: Catalyst IT Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 138387 [details] [review] Bug 23538: (QA follow-up) Group system preferences into one entry This makes the new perferences EmailPatronRegistrations and EmailAddressForPatronRegistrations follow a little more closely the pattern set by the very similar EmailPurchaseSuggestions and EmailAddressforSuggestions. * Group both preferences into one entry * Add line break between preferences * Switch branch for library in pull down Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Very happy to see inbound_email_address and ReplyToDefault used here! installer/data/mysql/fr-FR/1-Obligatoire/sample_notices.sql is no longer necessary. I have amended David's patch removing the file and adjusting the commit message. Thanks very much for testing and feedback David and Katrin! Great to see this Passed QA Pushed to master for 22.11. Nice work everyone, thanks! + t::lib::Mocks::mock_preference( 'KohaAdminEmailAddress', 'root@localhost' ); + t::lib::Mocks::mock_preference( 'EmailAddressForPatronRegistrations', 'library@localhost' ); + + # Test when EmailPatronRegistrations equals BranchEmailAddress + t::lib::Mocks::mock_preference( 'EmailPatronRegistrations', 'BranchEmailAddress' ); + is( $patron->notify_library_of_registration(C4::Context->preference('EmailPatronRegistrations')), 1, 'OPAC_REG email is queued if EmailPatronRegistration syspref equals BranchEmailAddress'); + my $sth = $dbh->prepare("SELECT to_address FROM message_queue where borrowernumber = ?"); + $sth->execute( $patron->borrowernumber ); + my $to_address = $sth->fetchrow_array; + is( $to_address, 'to@mybranch.com', 'OPAC_REG email queued to go to branchreplyto address when EmailPatronRegistration equals BranchEmailAddress' ); + $dbh->do(q|DELETE FROM message_queue|); + + # Test when EmailPatronRegistrations equals EmailAddressForPatronRegistrations + t::lib::Mocks::mock_preference( 'EmailPatronRegistrations', 'EmailAddressForPatronRegistrations' ); + is( $patron->notify_library_of_registration(C4::Context->preference('EmailPatronRegistrations')), 1, 'OPAC_REG email is queued if EmailPatronRegistration syspref equals EmailAddressForPatronRegistrations'); + $sth->execute( $patron->borrowernumber ); + $to_address = $sth->fetchrow_array; + is( $to_address, 'library@localhost', 'OPAC_REG email queued to go to EmailAddressForPatronRegistrations syspref when EmailPatronRegistration equals EmailAddressForPatronRegistrations' ); + $dbh->do(q|DELETE FROM message_queue|); + + # Test when EmailPatronRegistrations equals KohaAdminEmailAddress + t::lib::Mocks::mock_preference( 'EmailPatronRegistrations', 'KohaAdminEmailAddress' ); + is( $patron->notify_library_of_registration(C4::Context->preference('EmailPatronRegistrations')), 1, 'OPAC_REG email is queued if EmailPatronRegistration syspref equals KohaAdminEmailAddress'); + $sth->execute( $patron->borrowernumber ); + $to_address = $sth->fetchrow_array; + is( $to_address, 'root@localhost', 'OPAC_REG email queued to go to KohaAdminEmailAddress syspref when EmailPatronRegistration equals KohaAdminEmailAddress' ); + $dbh->do(q|DELETE FROM message_queue|); This part of the test fails with me: not ok 6 - OPAC_REG email queued to go to KohaAdminEmailAddress syspref when EmailPatronRegistration equals KohaAdminEmailAddress # Failed test 'OPAC_REG email queued to go to KohaAdminEmailAddress syspref when EmailPatronRegistration equals KohaAdminEmailAddress' # at t/db_dependent/Koha/Patron.t line 1347. # got: 'bibliotheek@master.rijkskoha.nl' # expected: 'root@localhost' # Looks like you failed 1 test of 6. Also note that you'd better not use localhost as an email address in your tests. And that you include several SQL statements that should be Koha objects. Koha::Patron elsif ( $email_patron_registrations eq "KohaAdminEmailAddress" ) { $to_address = C4::Context->preference('ReplytoDefault') || C4::Context->preference('KohaAdminEmailAddress'); } This is the cause of the failing test. Why do you use ReplytoDefault here. The value of the pref says KohaAdmin. This is quite confusing. Created attachment 141223 [details] [review] Bug 23538: (follow-up) Workaround to fix Patron.t See comment36 and comment37. This needs more attention but serves as a quick fix only. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Tomas: Last patch only please. Pushed to master for 22.11. Nice work everyone, thanks! (In reply to Marcel de Rooy from comment #39) > Tomas: Last patch only please. Thank you, Marcel. Good job! Enhancement will not be backported to 22.05.x series Added to manual. |