|
Description
Lari Taskula
2015-08-25 13:27:35 UTC
Created attachment 42053 [details] [review] Bug 14723 - Additional delivery notes to messages This patch adds additional delivery notes to messages in message queue. They are useful, because there can be multiple reasons for a delivery to fail. To test template modifications: 1. Find a borrower with notices at members/notices.pl 2. Observe that there is no column for Delivery notes 3. Apply patch and run the given database update 4. Repeat step 1. 5. Observe that there is now a column for Delivery notes Created attachment 42074 [details] [review] Bug 14723: (follow-up) Example of handling custom SMS::Send Driver reports This patch presents an example of how SMS::Send driver can be configured to receive reports from your SMS Gateway provider if they support such feature. With Bug 14723 adding a delivery note column to message_queue, you are now able to update the error status provided by your SMS gateway provider to the message. It uses REST API, so make sure to include dependencies. Koha conf gets a new block "smsProviders", where you are able to define your SMS Gateway login data and the report URL that our provider will use. Koha httpd conf gets an example for blocking all traffic to the REST API address for modifying the message status, apart from our provider IPs. Here is a description of the protocol and common actions with the example driver. 1. message_queue gets processed as usual. 2. Letters.pm calls our example driver with phone number, content and message_id (new!). 3. Our example driver sends a POST request to the SMS Gateway provider as follows: - user: username, loaded from koha conf smsProviders->labyrintti - password: password, loaded from koha conf smsProviders->labyrintti - dests: destination phone number - text: content of the message - report: the URL that SMS Gateway provider will update the status of this delivery, loaded from koha conf (with REST API, it should be http://yourhost.com/v1/messages) smsProviders->labyrintti 4. We get (hopefully) an OK message from our SMS provider. It will already let us know if there was an error with the phone number. If so, we return it to Letters.pm via HASH: { status => 1/0, delivery_note => the message of error/success } 5. When the delivery is complete, our SMS Gateway will contact us via the report URL we provided. 6. Via Swagger definitions, we will use Koha::REST::V1::Messages delivery_report to handle the report. If the given status is "ERROR" (this is our example provider's protocol), we will update the message status (and add delivery note) in message_queue for the given message_id. 7. All done, we have now handled the report given by our SMS Gateway provider :) Created attachment 42159 [details] [review] Bug 14723 - Additional delivery notes to messages Test plan: 1. Find a borrower with notices at members/notices.pl 2. Observe that there is no column for Delivery notes 3. Apply patch and run the given database update 4. Repeat step 1. 5. Observe that there is now a column for Delivery notes Includes PageObject Selenium test for delivery failures in C4::Letters. Created attachment 42170 [details] [review] Bug 14723 - Additional delivery notes to messages Selenium tests are using PageObjects and Authentication rewrite (therefore dependencey on Bug 7174). The patch only covers delivery notes at failure cases in C4::Letters. If you wish to see how delivery notes can be taken further with custom SMS::Send driver and SMS Gateway reports, please see Bug 14767. (In reply to Lari Taskula from comment #3) > Created attachment 42074 [details] [review] [review] > Bug 14723: (follow-up) Example of handling custom SMS::Send Driver reports > Patch moved to bug 14767. Created attachment 42227 [details] [review] Bug 14723 - Additional delivery notes to messages Created attachment 42228 [details] [review] Bug 14723 - Additional delivery notes to messages Created attachment 42384 [details] [review] Bug 14723 - Additional delivery notes to messages Fixed PageObject test by enabling EnhancedMessagingPreferences system preference via SystemPreferenceFactory. Without this preference, the Notices tab will not be shown in left navigation menu. Test plan: 1. Enable EnhancedMessagingPreferences syspref 2. Find a borrower with notices at members/notices.pl 3. Observe that there is no column for Delivery notes 4. Apply patch and run the given database update 5. Repeat step 1. 6. Observe that there is now a column for Delivery notes Created attachment 42453 [details] [review] Bug 14723 - Additional delivery notes to messages Added unit tests. Created attachment 42455 [details] [review] Bug 14723 - Additional delivery notes to messages Created attachment 42466 [details] [review] Bug 14723 - Additional delivery notes to messages Created attachment 42505 [details] [review] Bug 14723 - Additional delivery notes to messages Database updates are now using the AtomicUpdater (Bug 14698) Created attachment 42553 [details] [review] Bug 14723 - Additional delivery notes to messages Fixes MessageQueueFactory parameters to match database format. Created attachment 42574 [details] [review] Bug 14723 - Additional delivery notes to messages Created attachment 42668 [details] [review] Bug 14723 - Additional delivery notes to messages Adds delivery_note to SELECT in GetMessage() (bug 12426). Includes PageObject Selenium test for delivery failures in C4::Letters. Created attachment 42669 [details] [review] Bug 14723 - Additional delivery notes to messages Created attachment 44367 [details] [review] Bug 14723 - Additional delivery notes to messages - fixed missing kohastructure Created attachment 44413 [details] [review] Bug 14723 - Additional delivery notes to messages - fixed missing kohastructure Get this: fatal: sha1 information is lacking or useless (C4/Letters.pm). error: could not build fake ancestor Created attachment 62080 [details] [review] Bug 14723: Additional delivery notes to messages This patch adds additional delivery notes to messages in message queue as there can be multiple reasons for a delivery to fail. Currently in message_queue we are given only two delivery statuses for messages, "sent" and "failed". When the status becomes failed, we have no idea why it fails. This feature can be useful with SMS gateway providers. Many SMS gateways inform the application the reason of SMS delivery failure. With this feature, this information can now be stored. As well as for emails, instead of simply logging failures, we can now store the reason of failure directly into the message row of message_queue. Test plan: 1. Enable EnhancedMessagingPreferences syspref 2. Find a borrower with notices at members/notices.pl 3. Observe that there is no column for Delivery notes 4. Apply patch and run the given database update 5. Repeat step 1. 6. Observe that there is now a column for Delivery notes (In reply to Frédéric Demians from comment #21) > Get this: > > fatal: sha1 information is lacking or useless (C4/Letters.pm). > error: could not build fake ancestor I removed some dependencies and rebased this on master. Should now apply. Depends on Bug 18413 (otherwise Letters.t fails) Created attachment 62138 [details] [review] Bug 14723: Additional delivery notes to messages This patch adds additional delivery notes to messages in message queue as there can be multiple reasons for a delivery to fail. Currently in message_queue we are given only two delivery statuses for messages, "sent" and "failed". When the status becomes failed, we have no idea why it fails. This feature can be useful with SMS gateway providers. Many SMS gateways inform the application the reason of SMS delivery failure. With this feature, this information can now be stored. As well as for emails, instead of simply logging failures, we can now store the reason of failure directly into the message row of message_queue. Test plan: 1. Enable EnhancedMessagingPreferences syspref 2. Find a borrower with notices at members/notices.pl 3. Observe that there is no column for Delivery notes 4. Apply patch and run the given database update 5. Repeat step 1. 6. Observe that there is now a column for Delivery notes Hi Lari When applying your patch I got the following merge error on Letters.t file, (note: as I am using an up to date master branch patch 18413 is already applied (I double checked to make sure)): Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 14723: Additional delivery notes to messages Using index info to reconstruct a base tree... M C4/Letters.pm M installer/data/mysql/kohastructure.sql M koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt M t/db_dependent/Letters.t Falling back to patching base and 3-way merge... Auto-merging t/db_dependent/Letters.t CONFLICT (content): Merge conflict in t/db_dependent/Letters.t Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt Auto-merging installer/data/mysql/kohastructure.sql Auto-merging C4/Letters.pm Failed to merge in the changes. Patch failed at 0001 Bug 14723: Additional delivery notes to messages The copy of the patch that failed is found in: /home/vagrant/kohaclone/.git/rebase-apply/patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-14723-Additional-delivery-notes-to-messages-Zqwu8p.patch Created attachment 78313 [details] [review] Bug 14723: Additional delivery notes to messages This patch adds additional delivery notes to messages in message queue as there can be multiple reasons for a delivery to fail. Currently in message_queue we are given only two delivery statuses for messages, "sent" and "failed". When the status becomes failed, we have no idea why it fails. This feature can be useful with SMS gateway providers. Many SMS gateways inform the application the reason of SMS delivery failure. With this feature, this information can now be stored. As well as for emails, instead of simply logging failures, we can now store the reason of failure directly into the message row of message_queue. Test plan: 1. Enable EnhancedMessagingPreferences syspref 2. Find a borrower with notices at members/notices.pl 3. Observe that there is no column for Delivery notes 4. Apply patch and run the given database update 5. Repeat step 1. 6. Observe that there is now a column for Delivery notes Failed to apply patch Bug 14723 - Additional delivery notes to messages 78313 - Bug 14723: Additional delivery notes to messages Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 14723: Additional delivery notes to messages Using index info to reconstruct a base tree... M C4/Letters.pm M installer/data/mysql/kohastructure.sql M koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt M t/db_dependent/Letters.t Falling back to patching base and 3-way merge... Auto-merging t/db_dependent/Letters.t CONFLICT (content): Merge conflict in t/db_dependent/Letters.t Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt Auto-merging installer/data/mysql/kohastructure.sql Auto-merging C4/Letters.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 14723: Additional delivery notes to messages The copy of the patch that failed is found in: .git/rebase-apply/patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-14723-Additional-delivery-notes-to-messages-vyvgwK.patch Created attachment 87525 [details] [review] Bug 14723: Additional delivery notes to messages This patch adds additional delivery notes to messages in message queue as there can be multiple reasons for a delivery to fail. Currently in message_queue we are given only two delivery statuses for messages, "sent" and "failed". When the status becomes failed, we have no idea why it fails. This feature can be useful with SMS gateway providers. Many SMS gateways inform the application the reason of SMS delivery failure. With this feature, this information can now be stored. As well as for emails, instead of simply logging failures, we can now store the reason of failure directly into the message row of message_queue. Test plan: 1. Enable EnhancedMessagingPreferences syspref 2. Find a borrower with notices at members/notices.pl 3. Observe that there is no column for Delivery notes 4. Apply patch and run the given database update 5. Repeat step 1. 6. Observe that there is now a column for Delivery notes Sponsored-by: Hypernova Oy Created attachment 87531 [details] [review] Bug 14723: Additional delivery notes to messages This patch adds additional delivery notes to messages in message queue as there can be multiple reasons for a delivery to fail. Currently in message_queue we are given only two delivery statuses for messages, "sent" and "failed". When the status becomes failed, we have no idea why it fails. This feature can be useful with SMS gateway providers. Many SMS gateways inform the application the reason of SMS delivery failure. With this feature, this information can now be stored. As well as for emails, instead of simply logging failures, we can now store the reason of failure directly into the message row of message_queue. Test plan: 1. Enable EnhancedMessagingPreferences syspref 2. Find a borrower with notices at members/notices.pl 3. Observe that there is no column for Delivery notes 4. Apply patch and run the given database update 5. Repeat step 1. 6. Observe that there is now a column for Delivery notes Sponsored-by: Hypernova Oy Signed-off-by: Liz Rea <wizzyrea@gmail.com> Comment on attachment 87531 [details] [review] Bug 14723: Additional delivery notes to messages Review of attachment 87531 [details] [review]: ----------------------------------------------------------------- ::: C4/Letters.pm @@ +1272,5 @@ > unless ($patron) { > warn "FAIL: No 'to_address' and INVALID borrowernumber ($message->{borrowernumber})"; > _set_message_status( { message_id => $message->{'message_id'}, > + status => 'failed', > + delivery_note => 'Invalid borrowernumber '.$message->{borrowernumber} } ); These notes are untranslatable. Comment on attachment 87531 [details] [review] Bug 14723: Additional delivery notes to messages Review of attachment 87531 [details] [review]: ----------------------------------------------------------------- I took another look at the code and Joseph is right. Made some additional notes too. ::: C4/Letters.pm @@ +912,2 @@ > VALUES > +( ?, ?, ?, ?, ?, ?, ?, NOW(), ?, ?, ?, ? ) I think NOW() might be a MySQLism from my research. @@ +1281,5 @@ > # warn "FAIL: No 'to_address' and no email for " . ($member->{surname} ||'') . ", borrowernumber ($message->{borrowernumber})"; > # warning too verbose for this more common case? > _set_message_status( { message_id => $message->{'message_id'}, > + status => 'failed', > + delivery_note => 'Unable to find an email address for this borrower' } ); See above, these need to be dealt with in the template. ::: installer/data/mysql/atomicupdate/Bug-14723_-_Additional_delivery_notes_to_messages.perl @@ +1,3 @@ > +$DBversion = 'XXX'; # will be replaced by the RM > +if( CheckVersion( $DBversion ) ) { > + $dbh->do("ALTER TABLE message_queue ADD delivery_note TEXT"); Please use BEFORE or AFTER column to specify where the new column will be added exactly. This will help to keep things the same between new and old installations. I'm no longer able to work on this, so I'm setting assignee to default. Feel free to continue this work. Created attachment 105776 [details] [review] Bug 14723: Additional delivery notes to messages This patch adds additional delivery notes to messages in message queue as there can be multiple reasons for a delivery to fail. Currently in message_queue we are given only two delivery statuses for messages, "sent" and "failed". When the status becomes failed, we have no idea why it fails. This feature can be useful with SMS gateway providers. Many SMS gateways inform the application the reason of SMS delivery failure. With this feature, this information can now be stored. As well as for emails, instead of simply logging failures, we can now store the reason of failure directly into the message row of message_queue. Test plan: 1. Enable EnhancedMessagingPreferences syspref 2. Find a borrower with notices at members/notices.pl 3. Observe that there is no column for Delivery notes 4. Apply patch and run the given database update 5. Repeat step 1. 6. Observe that there is now a column for Delivery notes Sponsored-by: Hypernova Oy Signed-off-by: Liz Rea <wizzyrea@gmail.com> Created attachment 105777 [details] [review] Bug 14723: Make delivery notes translatable Delivery notes weren't translatable. This patch adds them in notices.tt. If note is Mail::Sendmail error generic 'Error occured while sending email' is shown. Also fixes MySQLisms and adds AFTER to atomicupdate file. Sponsored-by: Koha-Suomi Oy can you please add a test plan? Applying: Bug 14723: Additional delivery notes to messages Using index info to reconstruct a base tree... M C4/Letters.pm M installer/data/mysql/kohastructure.sql M koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt M t/db_dependent/Letters.t Falling back to patching base and 3-way merge... Auto-merging t/db_dependent/Letters.t CONFLICT (content): Merge conflict in t/db_dependent/Letters.t Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt Auto-merging installer/data/mysql/kohastructure.sql CONFLICT (content): Merge conflict in installer/data/mysql/kohastructure.sql Auto-merging C4/Letters.pm CONFLICT (content): Merge conflict in C4/Letters.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 14723: Additional delivery notes to messages The copy of the patch that failed is found in: .git/rebase-apply/patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-14723-Additional-delivery-notes-to-messages-_b90Al.patch Patch doesn't apply Failed to apply the patch in step 4 $ git bz apply 14723 Bug 14723 - Additional delivery notes to messages 105776 - Bug 14723: Additional delivery notes to messages 105777 - Bug 14723: Make delivery notes translatable Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 14723: Additional delivery notes to messages Using index info to reconstruct a base tree... M C4/Letters.pm M installer/data/mysql/kohastructure.sql M koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt M t/db_dependent/Letters.t Falling back to patching base and 3-way merge... Auto-merging t/db_dependent/Letters.t CONFLICT (content): Merge conflict in t/db_dependent/Letters.t Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt Auto-merging installer/data/mysql/kohastructure.sql CONFLICT (content): Merge conflict in installer/data/mysql/kohastructure.sql Auto-merging C4/Letters.pm CONFLICT (content): Merge conflict in C4/Letters.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 14723: Additional delivery notes to messages The copy of the patch that failed is found in: .git/rebase-apply/patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-14723-Additional-delivery-notes-to-messages-h__pEr.patch Created attachment 116642 [details] [review] Bug 14723: Additional delivery notes to messages This patch adds additional delivery notes to messages in message queue as there can be multiple reasons for a delivery to fail. Currently in message_queue we are given only two delivery statuses for messages, "sent" and "failed". When the status becomes failed, we have no idea why it fails. This feature can be useful with SMS gateway providers. Many SMS gateways inform the application the reason of SMS delivery failure. With this feature, this information can now be stored. As well as for emails, instead of simply logging failures, we can now store the reason of failure directly into the message row of message_queue. Test plan: 1. Enable EnhancedMessagingPreferences syspref 2. Find a borrower with notices at members/notices.pl 3. Observe that there is no column for Delivery notes 4. Apply patch and run the given database update 5. Repeat step 1. 6. Observe that there is now a column for Delivery notes Sponsored-by: Hypernova Oy Signed-off-by: Liz Rea <wizzyrea@gmail.com> Created attachment 116643 [details] [review] Bug 14723: Make delivery notes translatable Delivery notes weren't translatable. This patch adds them in notices.tt. If note is Mail::Sendmail error generic 'Error occured while sending email' is shown. Also fixes MySQLisms and adds AFTER to atomicupdate file. To test prove t/db_dependent/Letters.t Sponsored-by: Koha-Suomi Oy Created attachment 118225 [details] [review] Bug 14723: Make delivery notes translatable Delivery notes weren't translatable. This patch adds them in notices.tt. If note is Mail::Sendmail error generic 'Error occured while sending email' is shown. Also fixes MySQLisms and adds AFTER to atomicupdate file. To test prove t/db_dependent/Letters.t Sponsored-by: Koha-Suomi Oy Signed-off-by: Stina Hallin <stina.hallin@ub.lu.se> Can you change the English strings with translatable codes? For example, "Missing SMS number" could be "MISSING_SMS". Martin just did work like this for the accounts tables. Created attachment 120252 [details] [review] Bug 14723: Add error codes and replace strings with them in template This patch adds error codes for delivery notes and replaces English strings with them in template. To test confirm that everything works according to the first test plan. The patch didn't apply: CONFLICT (content): Merge conflict in t/db_dependent/Letters.t Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt Auto-merging installer/data/mysql/kohastructure.sql Auto-merging C4/Letters.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 14723: Additional delivery notes to messages The copy of the patch that failed is found in: .git/rebase-apply/patch Created attachment 120309 [details] [review] Bug 14723: Additional delivery notes to messages This patch adds additional delivery notes to messages in message queue as there can be multiple reasons for a delivery to fail. Currently in message_queue we are given only two delivery statuses for messages, "sent" and "failed". When the status becomes failed, we have no idea why it fails. This feature can be useful with SMS gateway providers. Many SMS gateways inform the application the reason of SMS delivery failure. With this feature, this information can now be stored. As well as for emails, instead of simply logging failures, we can now store the reason of failure directly into the message row of message_queue. Test plan: 1. Enable EnhancedMessagingPreferences syspref 2. Find a borrower with notices at members/notices.pl 3. Observe that there is no column for Delivery notes 4. Apply patch and run the given database update 5. Repeat step 1. 6. Observe that there is now a column for Delivery notes Sponsored-by: Hypernova Oy Signed-off-by: Liz Rea <wizzyrea@gmail.com> Created attachment 120310 [details] [review] Bug 14723: Make delivery notes translatable Delivery notes weren't translatable. This patch adds them in notices.tt. If note is Mail::Sendmail error generic 'Error occured while sending email' is shown. Also fixes MySQLisms and adds AFTER to atomicupdate file. To test prove t/db_dependent/Letters.t Sponsored-by: Koha-Suomi Oy Signed-off-by: Stina Hallin <stina.hallin@ub.lu.se> Created attachment 120312 [details] [review] Bug 14723: Add error codes and replace strings with them in template This patch adds error codes for delivery notes and replaces English strings with them in template. To test confirm everything works as intended. prove t/db_dependent/Letters.t t/db_dependent/Letters.t .. 6/86 # Failed test 'Time status changed equals time queued when created in message_queue table' # at t/db_dependent/Letters.t line 148. # got: '2021-04-29 13:50:54' # expected: '2021-04-29 00:00:00' t/db_dependent/Letters.t .. 70/86 # Looks like you failed 1 test of 86. t/db_dependent/Letters.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/86 subtests Test Summary Report ------------------- t/db_dependent/Letters.t (Wstat: 256 Tests: 86 Failed: 1) Failed test: 21 Non-zero exit status: 1 Files=1, Tests=86, 4 wallclock secs ( 0.03 usr 0.01 sys + 2.48 cusr 0.39 csys = 2.91 CPU) Result: FAIL Created attachment 120314 [details] [review] Bug 14723: (QA follow-up) Fix typo Created attachment 120376 [details] [review] Bug 14723: Use DATETIME instead DATE Letters.t tests failed since time_queued didn't include timecode. Use DATETIME when inserting into message_queue. To test prove t/db_dependent/Letters.t Created attachment 120487 [details] [review] Bug 14723: Additional delivery notes to messages This patch adds additional delivery notes to messages in message queue as there can be multiple reasons for a delivery to fail. Currently in message_queue we are given only two delivery statuses for messages, "sent" and "failed". When the status becomes failed, we have no idea why it fails. This feature can be useful with SMS gateway providers. Many SMS gateways inform the application the reason of SMS delivery failure. With this feature, this information can now be stored. As well as for emails, instead of simply logging failures, we can now store the reason of failure directly into the message row of message_queue. Test plan: 1. Enable EnhancedMessagingPreferences syspref 2. Find a borrower with notices at members/notices.pl 3. Observe that there is no column for Delivery notes 4. Apply patch and run the given database update 5. Repeat step 1. 6. Observe that there is now a column for Delivery notes Sponsored-by: Hypernova Oy Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 120488 [details] [review] Bug 14723: Additional delivery notes to messages This patch adds additional delivery notes to messages in message queue as there can be multiple reasons for a delivery to fail. Currently in message_queue we are given only two delivery statuses for messages, "sent" and "failed". When the status becomes failed, we have no idea why it fails. This feature can be useful with SMS gateway providers. Many SMS gateways inform the application the reason of SMS delivery failure. With this feature, this information can now be stored. As well as for emails, instead of simply logging failures, we can now store the reason of failure directly into the message row of message_queue. Test plan: 1. Enable EnhancedMessagingPreferences syspref 2. Find a borrower with notices at members/notices.pl 3. Observe that there is no column for Delivery notes 4. Apply patch and run the given database update 5. Repeat step 1. 6. Observe that there is now a column for Delivery notes Sponsored-by: Hypernova Oy Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 120489 [details] [review] Bug 14723: Make delivery notes translatable Delivery notes weren't translatable. This patch adds them in notices.tt. If note is Mail::Sendmail error generic 'Error occured while sending email' is shown. Also fixes MySQLisms and adds AFTER to atomicupdate file. To test prove t/db_dependent/Letters.t Sponsored-by: Koha-Suomi Oy Signed-off-by: Stina Hallin <stina.hallin@ub.lu.se> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 120490 [details] [review] Bug 14723: Add error codes and replace strings with them in template This patch adds error codes for delivery notes and replaces English strings with them in template. To test confirm everything works as intended. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 120491 [details] [review] Bug 14723: (QA follow-up) Fix typo Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 120492 [details] [review] Bug 14723: Use DATETIME instead DATE Letters.t tests failed since time_queued didn't include timecode. Use DATETIME when inserting into message_queue. To test prove t/db_dependent/Letters.t Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 120667 [details] [review] Bug 14723: Make DBrev idempotent Pushed to master for 21.05, thanks to everybody involved! Enhancement not pushed to 20.11.x Ouch.. there's stuff missing here. We don't update kohastructure at all and we don't do a DB update to add the error_code field to the database either. Finally, we don't add error_code to the _set_message_status routine. (In reply to Martin Renvoize from comment #60) > Ouch.. there's stuff missing here. > > We don't update kohastructure at all and we don't do a DB update to add the > error_code field to the database either. Finally, we don't add error_code > to the _set_message_status routine. I don't understand what you mean with error_code? We add delivery_note column where fail message is added and it's also added to _set_message_status routine. Also kohastructure is updated on first patch? See bug 28813 where I fixed the issues. To clarify, the followups here that converted from string note to translatable code broke the feature entirely ;) Gosh, I didn't realise I had made that change in follow up and hadn't tested it properly. My bad, sorry. Thanks Martin for fixing this :) No worries at all, we all do it sometimes :) *** Bug 15417 has been marked as a duplicate of this bug. *** No release notes provided in the expected delay. |