Bug 37869 - Plugin hook before_send_messages not triggered for any messages sent without use of process_message_queue.pl
Summary: Plugin hook before_send_messages not triggered for any messages sent without ...
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal
Assignee: Kyle M Hall (khall)
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 37885
  Show dependency treegraph
 
Reported: 2024-09-09 13:38 UTC by Kyle M Hall (khall)
Modified: 2024-10-02 17:02 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 37869: Move plugin hook before_send_messages to SendQueuedMessages (3.52 KB, patch)
2024-09-09 16:51 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 37869: Tidy code (4.16 KB, patch)
2024-09-09 16:51 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 37869: Move plugin hook before_send_messages to SendQueuedMessages (3.52 KB, patch)
2024-10-02 17:01 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 37869: Tidy code (4.16 KB, patch)
2024-10-02 17:02 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 37869: Add unit tests (5.19 KB, patch)
2024-10-02 17:02 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall (khall) 2024-09-09 13:38:15 UTC
Most messages in Koha are handled by process_message_queue.pl. This script calls before_send_messages which calls the plugin hook before_send_messages. This works for nearly all messages, but some messages like WELCOME notices are triggered immediately and do not way to for process_message_queue.pl to run. We should move the triggering of before_send_messages into SendQueuedMessages so it is always handled.
Comment 1 Kyle M Hall (khall) 2024-09-09 16:51:46 UTC
Created attachment 171217 [details] [review]
Bug 37869: Move plugin hook before_send_messages to SendQueuedMessages

Most messages in Koha are handled by process_message_queue.pl. This script calls before_send_messages which calls the plugin hook before_send_messages. This works for nearly all messages, but some messages like WELCOME notices are triggered immediately and do not way to for process_message_queue.pl to run. We should move the triggering of before_send_messages into SendQueuedMessages so it is always handled.

Test Plan:
1) Install version 2.5.1 of the kitchen sink plugin
2) Run process_messsage_queue.pl and also trigger a WELCOME notice
3) Note the "Plugin hook before_send_message called with the params" message in the logs for the former but not the latter
4) Apply this patch
5) Repeat step 2, note there the message also shows in the logs for the WELCOME notice!
Comment 2 Kyle M Hall (khall) 2024-09-09 16:51:53 UTC
Created attachment 171218 [details] [review]
Bug 37869: Tidy code
Comment 3 David Nind 2024-09-09 22:20:29 UTC
I had a go at testing, but I'm not seeing any messages in the logs. Any specific log to look in, or do I need to change the logging level?

Testing notes (using KTD):

1. Plugin installed from https://github.com/bywatersolutions/dev-koha-plugin-kitchen-sink/releases/tag/v2.5.1

2. Added an email address (mary@example.com) to Mary (for the welcome message), and added an SMTP server. (Not sure if this was necessary or not).

3. Before the patch, when running misc/cronjobs/process_message_queue.pl:

Plugin hook before_send_message called with the params: $VAR1 = {
          'letter_code' => [],
          'verbose' => 0,
          'where' => undef,
          'type' => [],
          'limit' => undef
        };

4. After the patch, when misc/cronjobs/process_message_queue.pl:

Plugin hook before_send_message called with the params: $VAR1 = {
          'exit_on_plugin_failure' => 0,
          'type' => [],
          'limit' => undef,
          'password' => undef,
          'letter_code' => [],
          'verbose' => 0,
          'where' => undef,
          'method' => 'LOGIN',
          'username' => undef
        };
Comment 4 Kyle M Hall (khall) 2024-09-10 13:44:04 UTC
(In reply to David Nind from comment #3)
> I had a go at testing, but I'm not seeing any messages in the logs. Any
> specific log to look in, or do I need to change the logging level?

I'm using KTD with all the defaults.
I set up Gmail as my SMTP server. 

When I create a new patron with a primary email address I see the following in
/var/log/koha/kohadev/plack-intranet-error.log

[2024/09/10 13:41:27] [WARN] Plugin hook before_send_message called with the params: $VAR1 = {
  'message_id' => 8
};

The only parameter will be the message_id and this will let the plugin know that it is not running from process_messages_queue.pl and instead is acting on a single message!
Comment 5 David Nind 2024-09-29 18:29:03 UTC
(In reply to Kyle M Hall (khall) from comment #4)
> (In reply to David Nind from comment #3)
> > I had a go at testing, but I'm not seeing any messages in the logs. Any
> > specific log to look in, or do I need to change the logging level?
> 
> I'm using KTD with all the defaults.
> I set up Gmail as my SMTP server. 
> 
> When I create a new patron with a primary email address I see the following
> in
> /var/log/koha/kohadev/plack-intranet-error.log
> 
> [2024/09/10 13:41:27] [WARN] Plugin hook before_send_message called with the
> params: $VAR1 = {
>   'message_id' => 8
> };
> 
> The only parameter will be the message_id and this will let the plugin know
> that it is not running from process_messages_queue.pl and instead is acting
> on a single message!

I had another go at testing using a default KTD.

I'm still not seeing any warning messages in /var/log/koha/kohadev/plack-intranet-error.log when running the message queue script.

8-(..

How I tested (using KTD):

1. Set up KTD to send email using a Gmail account.[1]

2. Install version 2.5.1 of the kitchen sink plugin (download from https://github.com/bywatersolutions/dev-koha-plugin-kitchen-sink/releases/tag/v2.5.1):
   2.1 Administration > Plugins > Manage plugins > Upload plugin
   2.2 No restart required (although I did do this as well)

3. Add a new patron with a primary email address.

4. Before the patch, run the process message queue script:

   misc/cronjobs/process_message_queue.pl 
   Plugin hook before_send_message called with the params: $VAR1 = {
             'verbose' => 0,
             'limit' => undef,
             'where' => undef,
             'letter_code' => [],
             'type' => []
           };

5. Check the /var/log/koha/kohadev/plack-intranet-error.log
   => There should be a warning (for me, there was no warning message)

   [2024/09/10 13:41:27] [WARN] Plugin hook before_send_message called with the params: $VAR1 = {
     'message_id' => 8
   };

6. Send the patron you created a welcome message and process the message queue as per step 4. There should be no warning message in the log. (This was the case for me.)

[1] To test sending emails using a Google account:
    - Set up an App password for your Google Account.
    - Edit /etc/koha/sites/kohadev/koha-conf.xml file and add this
      configuration near the end (where <user_name> = your Google email
      address; <password> = your APP password, not your Google account
      password):

      <smtp_server>
        <host>smtp.gmail.com</host>
        <port>587</port>
        <timeout>5</timeout>
        <ssl_mode>STARTTLS</ssl_mode>
        <user_name>GOOGLEACCOUNTUSER</user_name>
        <password>GOOGLEAPPPASSWORD</password>
        <debug>1</debug>
      </smtp_server>
   - Restart everything: restart_all
   - Test by:
     . Adding an email address to an existing patron
     . Sending that patron a welcome message (Patron account > More > Send welcome message)
     . Run the message queue processing script: misc/cronjobs/process_message_queue.pl
Comment 6 Martin Renvoize (ashimema) 2024-10-01 06:32:42 UTC
Now that we've moved this into a module method, any chance of getting a unit test for it Kyle?

I've not tested myself yet, I hope to take a look today.
Comment 7 Kyle M Hall (khall) 2024-10-01 11:39:57 UTC
(In reply to Martin Renvoize (ashimema) from comment #6)
> Now that we've moved this into a module method, any chance of getting a unit
> test for it Kyle?
> 
> I've not tested myself yet, I hope to take a look today.

Capital idea. I'll see what I can do!
Comment 8 Kyle M Hall (khall) 2024-10-02 17:01:31 UTC
Created attachment 172323 [details] [review]
Bug 37869: Move plugin hook before_send_messages to SendQueuedMessages

Most messages in Koha are handled by process_message_queue.pl. This script calls before_send_messages which calls the plugin hook before_send_messages. This works for nearly all messages, but some messages like WELCOME notices are triggered immediately and do not way to for process_message_queue.pl to run. We should move the triggering of before_send_messages into SendQueuedMessages so it is always handled.

Test Plan:
1) Install version 2.5.1 of the kitchen sink plugin
2) Run process_messsage_queue.pl and also trigger a WELCOME notice
3) Note the "Plugin hook before_send_message called with the params" message in the logs for the former but not the latter
4) Apply this patch
5) Repeat step 2, note there the message also shows in the logs for the WELCOME notice!
Comment 9 Kyle M Hall (khall) 2024-10-02 17:02:06 UTC
Created attachment 172324 [details] [review]
Bug 37869: Tidy code
Comment 10 Kyle M Hall (khall) 2024-10-02 17:02:09 UTC
Created attachment 172325 [details] [review]
Bug 37869: Add unit tests