Some libraries are paperless and require all payment receipts to be emailed. Koha should give libraries the option to send email receipts if a patron has an email address. If a notice for the type of "credit" exists ( payment or writeoff ), then an email receipt will be sent.
Created attachment 66554 [details] [review] Bug 19191 - Add ability to email receipts for account payments and write-offs Some libraries are paperless and require all payment receipts to be emailed. Koha should give libraries the option to send email receipts if a patron has an email address. If a notice for the type of "credit" exists ( payment or writeoff ), then an email receipt will be sent. These notices only support Template Toolkit syntax. Test Plan: 1) Apply this patch and dependencies 2) Run updatedatabase.pl 3) Note two new notices exist in the notices editor, ACCOUNT_PAYMENT and ACCOUNT_WRITEOFF 4) Find or create a patron with an email address that owes some amount of money 5) Make a payment for one or more fees 6) Note a new email is queued for the patron 7) Make a writeoff for one or more fees 8) Note a new new email is queued for the patron
This patch also introduces a new method of passing objects directly through to the notice template without processing. This is incredibly efficient for notices that only support Template Toolkit syntax, as it does not require refetching the data from the database multiple times as the existing parameters may.
Hey, I tried to test this patch, and couldn't apply it (Dependency patch 14826 applied just fine): Applying: Bug 19191 - Add ability to email receipts for account payments and write-offs Using index info to reconstruct a base tree... M C4/Letters.pm M Koha/Account/Offset.pm M t/db_dependent/Letters/TemplateToolkit.t Falling back to patching base and 3-way merge... Auto-merging t/db_dependent/Letters/TemplateToolkit.t Auto-merging Koha/Account/Offset.pm CONFLICT (content): Merge conflict in Koha/Account/Offset.pm Auto-merging C4/Letters.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 19191 - Add ability to email receipts for account payments and write-offs
Kyle, thanks for this nice feature, just couple of thoughts: > 2) Run updatedatabase.pl You probably forget to add atomic update file - there is just sample_notices.sql addition Will it be possible to NOT to send e-mail about payment? Let's say configurable for patrons or patron categories? Somehow similire to current "Enhanced Messaging" feature?
Created attachment 68605 [details] [review] Bug 19191 - Add ability to email receipts for account payments and write-offs Some libraries are paperless and require all payment receipts to be emailed. Koha should give libraries the option to send email receipts if a patron has an email address. If a notice for the type of "credit" exists ( payment or writeoff ), then an email receipt will be sent. These notices only support Template Toolkit syntax. Test Plan: 1) Apply this patch and dependencies 2) Run updatedatabase.pl 3) Note two new notices exist in the notices editor, ACCOUNT_PAYMENT and ACCOUNT_WRITEOFF 4) Find or create a patron with an email address that owes some amount of money 5) Make a payment for one or more fees 6) Note a new email is queued for the patron 7) Make a writeoff for one or more fees 8) Note a new new email is queued for the patron
(In reply to Josef Moravec from comment #4) > Kyle, thanks for this nice feature, just couple of thoughts: > > > 2) Run updatedatabase.pl > > You probably forget to add atomic update file - there is just > sample_notices.sql addition Good catch! > Will it be possible to NOT to send e-mail about payment? Let's say > configurable for patrons or patron categories? Somehow similire to current > "Enhanced Messaging" feature? That would be an excellent followup feature, but is outside the scope of this particular work. I'd love to see it! For now the feature can be turned on and off for each notice by simply having the notice exist or not.
Created attachment 68812 [details] [review] Bug 19191 - Add ability to email receipts for account payments and write-offs Some libraries are paperless and require all payment receipts to be emailed. Koha should give libraries the option to send email receipts if a patron has an email address. If a notice for the type of "credit" exists ( payment or writeoff ), then an email receipt will be sent. These notices only support Template Toolkit syntax. Test Plan: 1) Apply this patch and dependencies 2) Run updatedatabase.pl 3) Note two new notices exist in the notices editor, ACCOUNT_PAYMENT and ACCOUNT_WRITEOFF 4) Find or create a patron with an email address that owes some amount of money 5) Make a payment for one or more fees 6) Note a new email is queued for the patron 7) Make a writeoff for one or more fees 8) Note a new new email is queued for the patron Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 70465 [details] [review] Bug 19191 [QA Followup] - Remove db update so feature is disabled by default for existing installations
Created attachment 70466 [details] [review] Bug 19191 [QA Followup] - Remove db update so feature is disabled by default for existing installations
Created attachment 70471 [details] [review] Bug 19191 - Add ability to email receipts for account payments and write-offs Some libraries are paperless and require all payment receipts to be emailed. Koha should give libraries the option to send email receipts if a patron has an email address. If a notice for the type of "credit" exists ( payment or writeoff ), then an email receipt will be sent. These notices only support Template Toolkit syntax. Test Plan: 1) Apply this patch and dependencies 2) Add the two new notices, you can find them in installer/data/mysql/en/mandatory/sample_notices.sql 3) Note two new notices exist in the notices editor, ACCOUNT_PAYMENT and ACCOUNT_WRITEOFF 4) Find or create a patron with an email address that owes some amount of money 5) Make a payment for one or more fees 6) Note a new email is queued for the patron 7) Make a writeoff for one or more fees 8) Note a new new email is queued for the patron Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
BLOCKED by bug 19966 still needing a sign-off!
Created attachment 72366 [details] [review] Bug 19191: Add ability to email receipts for account payments and write-offs Some libraries are paperless and require all payment receipts to be emailed. Koha should give libraries the option to send email receipts if a patron has an email address. If a notice for the type of "credit" exists ( payment or writeoff ), then an email receipt will be sent. These notices only support Template Toolkit syntax. Test Plan: 1) Apply this patch and dependencies 2) Add the two new notices, you can find them in installer/data/mysql/en/mandatory/sample_notices.sql 3) Note two new notices exist in the notices editor, ACCOUNT_PAYMENT and ACCOUNT_WRITEOFF 4) Find or create a patron with an email address that owes some amount of money 5) Make a payment for one or more fees 6) Note a new email is queued for the patron 7) Make a writeoff for one or more fees 8) Note a new new email is queued for the patron Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 72367 [details] [review] Bug 19191: Remove need for bug 19966
Created attachment 72370 [details] [review] Bug 19191: Remove need for bug 19966
Needs a real quick rebase!
We'll also need a follow up to include the new sample notices in the the translated web installers and in a database update!
Created attachment 73924 [details] [review] Bug 19191: Add ability to email receipts for account payments and write-offs Some libraries are paperless and require all payment receipts to be emailed. Koha should give libraries the option to send email receipts if a patron has an email address. If a notice for the type of "credit" exists ( payment or writeoff ), then an email receipt will be sent. These notices only support Template Toolkit syntax. Test Plan: 1) Apply this patch and dependencies 2) Add the two new notices, you can find them in installer/data/mysql/en/mandatory/sample_notices.sql 3) Note two new notices exist in the notices editor, ACCOUNT_PAYMENT and ACCOUNT_WRITEOFF 4) Find or create a patron with an email address that owes some amount of money 5) Make a payment for one or more fees 6) Note a new email is queued for the patron 7) Make a writeoff for one or more fees 8) Note a new new email is queued for the patron Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Continuing here, sample notices can be fixed once the base patch passes.
Question: - How can you control the sending of the emails? (delete the notices?) To fix after initial testing is finished: - Fix capitalization on notice templates: Account Writeoff, Account Payment - Add database update - Add notices to other installers
Continuing here.
(In reply to Katrin Fischer from comment #19) > Question: > - How can you control the sending of the emails? (delete the notices?) Yes, removing the notice disables the feature.
Ok, big problem: - notice templates are there - homebranch library has an email - patron has an email But no notice is generated on paying/writing off. > To fix after initial testing is finished: > - Fix capitalization on notice templates: Account Writeoff, Account Payment > - Add database update > - Add notices to other installers Maybe related: ERROR: nothing to substitute - both 'tables', 'loops' and 'substitute' are empty at /home/vagrant/kohaclone/Koha/Account.pm line 265. Use of uninitialized value in string eq at /home/vagrant/kohaclone/members/paycollect.pl line 194. Use of uninitialized value in string eq at /home/vagrant/kohaclone/members/pay.pl line 224. Please fix all notes in one patch (installers now too, notices are ok afaikt if they work ;) )
Created attachment 74092 [details] [review] Bug 19191: Add ability to email receipts for account payments and write-offs Some libraries are paperless and require all payment receipts to be emailed. Koha should give libraries the option to send email receipts if a patron has an email address. If a notice for the type of "credit" exists ( payment or writeoff ), then an email receipt will be sent. These notices only support Template Toolkit syntax. Test Plan: 1) Apply this patch and dependencies 2) Add the two new notices, you can find them in installer/data/mysql/en/mandatory/sample_notices.sql 3) Note two new notices exist in the notices editor, ACCOUNT_PAYMENT and ACCOUNT_WRITEOFF 4) Find or create a patron with an email address that owes some amount of money 5) Make a payment for one or more fees 6) Note a new email is queued for the patron 7) Make a writeoff for one or more fees 8) Note a new new email is queued for the patron Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 74093 [details] [review] Bug 19191: Add ability to email receipts for account payments and write-offs Some libraries are paperless and require all payment receipts to be emailed. Koha should give libraries the option to send email receipts if a patron has an email address. If a notice for the type of "credit" exists ( payment or writeoff ), then an email receipt will be sent. These notices only support Template Toolkit syntax. Test Plan: 1) Apply this patch and dependencies 2) Add the two new notices, you can find them in installer/data/mysql/en/mandatory/sample_notices.sql 3) Note two new notices exist in the notices editor, ACCOUNT_PAYMENT and ACCOUNT_WRITEOFF 4) Find or create a patron with an email address that owes some amount of money 5) Make a payment for one or more fees 6) Note a new email is queued for the patron 7) Make a writeoff for one or more fees 8) Note a new new email is queued for the patron Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 74094 [details] [review] Bug 19191: Add ability to email receipts for account payments and write-offs Some libraries are paperless and require all payment receipts to be emailed. Koha should give libraries the option to send email receipts if a patron has an email address. If a notice for the type of "credit" exists ( payment or writeoff ), then an email receipt will be sent. These notices only support Template Toolkit syntax. Test Plan: 1) Apply this patch and dependencies 2) Add the two new notices, you can find them in installer/data/mysql/en/mandatory/sample_notices.sql 3) Note two new notices exist in the notices editor, ACCOUNT_PAYMENT and ACCOUNT_WRITEOFF 4) Find or create a patron with an email address that owes some amount of money 5) Make a payment for one or more fees 6) Note a new email is queued for the patron 7) Make a writeoff for one or more fees 8) Note a new new email is queued for the patron Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 74095 [details] [review] Bug 19191: Remove need for bug 19966
Created attachment 74104 [details] [review] Bug 19191: Add correct unit tests
Created attachment 74105 [details] [review] Bug 19191: Add correct unit tests
Hi Kyle, it works now! Can you please fix the remaining issues in a follow-up? - Fix capitalization on notice templates: Account Writeoff, Account Payment - Add notices to other installers
Created attachment 74423 [details] [review] Bug 19191: Fix capitalization of notice titles
Created attachment 74424 [details] [review] Bug 19191: Add non-english notices
Hi Kyle, didn't sign them yet, will check the follow-up tonight!
(In reply to Katrin Fischer from comment #32) > Hi Kyle, didn't sign them yet, will check the follow-up tonight! My bad! Thanks!
Created attachment 74504 [details] [review] Bug 19191: Add ability to email receipts for account payments and write-offs Some libraries are paperless and require all payment receipts to be emailed. Koha should give libraries the option to send email receipts if a patron has an email address. If a notice for the type of "credit" exists ( payment or writeoff ), then an email receipt will be sent. These notices only support Template Toolkit syntax. Test Plan: 1) Apply this patch and dependencies 2) Add the two new notices, you can find them in installer/data/mysql/en/mandatory/sample_notices.sql 3) Note two new notices exist in the notices editor, ACCOUNT_PAYMENT and ACCOUNT_WRITEOFF 4) Find or create a patron with an email address that owes some amount of money 5) Make a payment for one or more fees 6) Note a new email is queued for the patron 7) Make a writeoff for one or more fees 8) Note a new new email is queued for the patron Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 74505 [details] [review] Bug 19191: Remove need for bug 19966 Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 74506 [details] [review] Bug 19191: Add correct unit tests Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 74507 [details] [review] Bug 19191: Fix capitalization of notice titles Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 74508 [details] [review] Bug 19191: Add non-english notices Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Puh, ok, I found a problem: When we push this as is, we will immediately start sending out payment and writeoff notices after update. Libraries might not expect that to happen and it smells trouble, especially since you'd want to translate/adapt the notice. I think the easisest solution would be to not add them with a database update. We could put the sample notices on a wiki page instead, so people can easily add them. Or into the manual. Or: We could add a system preference to determine when to send them.
Created attachment 74625 [details] [review] Bug 19191: Add ability to email receipts for account payments and write-offs Some libraries are paperless and require all payment receipts to be emailed. Koha should give libraries the option to send email receipts if a patron has an email address. If a notice for the type of "credit" exists ( payment or writeoff ), then an email receipt will be sent. These notices only support Template Toolkit syntax. Test Plan: 1) Apply this patch and dependencies 2) Add the two new notices, you can find them in installer/data/mysql/en/mandatory/sample_notices.sql 3) Note two new notices exist in the notices editor, ACCOUNT_PAYMENT and ACCOUNT_WRITEOFF 4) Find or create a patron with an email address that owes some amount of money 5) Make a payment for one or more fees 6) Note a new email is queued for the patron 7) Make a writeoff for one or more fees 8) Note a new new email is queued for the patron Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 74626 [details] [review] Bug 19191: Add ability to email receipts for account payments and write-offs Some libraries are paperless and require all payment receipts to be emailed. Koha should give libraries the option to send email receipts if a patron has an email address. If a notice for the type of "credit" exists ( payment or writeoff ), then an email receipt will be sent. These notices only support Template Toolkit syntax. Test Plan: 1) Apply this patch and dependencies 2) Add the two new notices, you can find them in installer/data/mysql/en/mandatory/sample_notices.sql 3) Note two new notices exist in the notices editor, ACCOUNT_PAYMENT and ACCOUNT_WRITEOFF 4) Find or create a patron with an email address that owes some amount of money 5) Make a payment for one or more fees 6) Note a new email is queued for the patron 7) Make a writeoff for one or more fees 8) Note a new new email is queued for the patron Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 74627 [details] [review] Bug 19191: Remove need for bug 19966 Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 74628 [details] [review] Bug 19191: Add correct unit tests Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 74629 [details] [review] Bug 19191: Fix capitalization of notice titles Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 74630 [details] [review] Bug 19191: Add non-english notices Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 74631 [details] [review] Bug 19191: Add syspref to control use of email receipts, defaulting to disabled
Created attachment 74686 [details] [review] Bug 19191: Add syspref to control use of email receipts, defaulting to disabled Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Please rebase CONFLICT (content): Merge conflict in t/db_dependent/Accounts.t CONFLICT (content): Merge conflict in Koha/Account/Offset.pm
Created attachment 75066 [details] [review] Bug 19191: Add ability to email receipts for account payments and write-offs Some libraries are paperless and require all payment receipts to be emailed. Koha should give libraries the option to send email receipts if a patron has an email address. If a notice for the type of "credit" exists ( payment or writeoff ), then an email receipt will be sent. These notices only support Template Toolkit syntax. Test Plan: 1) Apply this patch and dependencies 2) Add the two new notices, you can find them in installer/data/mysql/en/mandatory/sample_notices.sql 3) Note two new notices exist in the notices editor, ACCOUNT_PAYMENT and ACCOUNT_WRITEOFF 4) Find or create a patron with an email address that owes some amount of money 5) Make a payment for one or more fees 6) Note a new email is queued for the patron 7) Make a writeoff for one or more fees 8) Note a new new email is queued for the patron Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 75067 [details] [review] Bug 19191: Add ability to email receipts for account payments and write-offs Some libraries are paperless and require all payment receipts to be emailed. Koha should give libraries the option to send email receipts if a patron has an email address. If a notice for the type of "credit" exists ( payment or writeoff ), then an email receipt will be sent. These notices only support Template Toolkit syntax. Test Plan: 1) Apply this patch and dependencies 2) Add the two new notices, you can find them in installer/data/mysql/en/mandatory/sample_notices.sql 3) Note two new notices exist in the notices editor, ACCOUNT_PAYMENT and ACCOUNT_WRITEOFF 4) Find or create a patron with an email address that owes some amount of money 5) Make a payment for one or more fees 6) Note a new email is queued for the patron 7) Make a writeoff for one or more fees 8) Note a new new email is queued for the patron Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 75068 [details] [review] Bug 19191: Remove need for bug 19966 Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 75069 [details] [review] Bug 19191: Add correct unit tests Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 75070 [details] [review] Bug 19191: Fix capitalization of notice titles Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 75071 [details] [review] Bug 19191: Add non-english notices Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 75072 [details] [review] Bug 19191: Add syspref to control use of email receipts, defaulting to disabled Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
1. There is 'Account Writeoff' vs 'Account writeoff' 2. In Koha::Account->pay, why require C4::Letters; instead of use? 3. Koha/Account.pm:273 lang => Koha::Patrons->find( $self->{patron_id} )->lang, No need to refetch the patron object, you did it already at the beginning of the method. 4. Koha/Account.pm:280 offsets => scalar Koha::Account::Offsets->search( { id => { -in => [ map { $_->id } @account_offsets ] } } ), I do not understand this line, we are refetching what we already have?
Created attachment 75093 [details] [review] Bug 19191 [QA Followup] - Remove double fetches from database
(In reply to Jonathan Druart from comment #56) > 1. There is 'Account Writeoff' vs 'Account writeoff' > > 2. In Koha::Account->pay, why require C4::Letters; instead of use? > > 3. Koha/Account.pm:273 > lang => Koha::Patrons->find( $self->{patron_id} )->lang, > No need to refetch the patron object, you did it already at the beginning of > the method. > > 4. Koha/Account.pm:280 > offsets => scalar Koha::Account::Offsets->search( { id => { -in => [ map { > $_->id } @account_offsets ] } } ), > > I do not understand this line, we are refetching what we already have? Follow addresses everything but 2. I used require instead of use because use is hoisted and it would import it even if the feature is not being used. Basically it's a small efficiency improvement.
(In reply to Kyle M Hall from comment #58) > (In reply to Jonathan Druart from comment #56) > > 1. There is 'Account Writeoff' vs 'Account writeoff' > > > > 2. In Koha::Account->pay, why require C4::Letters; instead of use? > > > > 3. Koha/Account.pm:273 > > lang => Koha::Patrons->find( $self->{patron_id} )->lang, > > No need to refetch the patron object, you did it already at the beginning of > > the method. > > > > 4. Koha/Account.pm:280 > > offsets => scalar Koha::Account::Offsets->search( { id => { -in => [ map { > > $_->id } @account_offsets ] } } ), > > > > I do not understand this line, we are refetching what we already have? > > Follow addresses everything but 2. I used require instead of use because use > is hoisted and it would import it even if the feature is not being used. > Basically it's a small efficiency improvement. It's usually a bad idea to require instead of use, to catch errors at compilation time. It's too late for 18.05 so I will let next RM decides. Moreover the title of the last patch is not formatted correctly.
Created attachment 75134 [details] [review] Bug 19191 [QA Followup]: Remove double fetches from database
(In reply to Jonathan Druart from comment #59) > (In reply to Kyle M Hall from comment #58) > > (In reply to Jonathan Druart from comment #56) > > > 1. There is 'Account Writeoff' vs 'Account writeoff' > > > > > > 2. In Koha::Account->pay, why require C4::Letters; instead of use? > > > > > > 3. Koha/Account.pm:273 > > > lang => Koha::Patrons->find( $self->{patron_id} )->lang, > > > No need to refetch the patron object, you did it already at the beginning of > > > the method. > > > > > > 4. Koha/Account.pm:280 > > > offsets => scalar Koha::Account::Offsets->search( { id => { -in => [ map { > > > $_->id } @account_offsets ] } } ), > > > > > > I do not understand this line, we are refetching what we already have? > > > > Follow addresses everything but 2. I used require instead of use because use > > is hoisted and it would import it even if the feature is not being used. > > Basically it's a small efficiency improvement. > > It's usually a bad idea to require instead of use, to catch errors at > compilation time. > It's too late for 18.05 so I will let next RM decides. > > Moreover the title of the last patch is not formatted correctly. We already load C4::Letters in plack.psgi anyway. I agree with Jonathan it is better to use the lib, to catch problems in compilation time.
Awesome work all! Pushed to master for 18.11
Enhancement, will not be backported to 18.05.x series.
(In reply to Jonathan Druart from comment #59) > It's usually a bad idea to require instead of use, to catch errors at > compilation time. See bug 21396.
This might be a nice candidate for 'new'.
*** Bug 13842 has been marked as a duplicate of this bug. ***