It would be nice to be able to combine several types in a single run, but exclude others, without having to have multiple cron lines
Created attachment 114522 [details] [review] Bug 27265: Enable message queue cron to accept multiple codes and types It would be nice to be able to combine several types in a single run, but exclude others, without having to have multiple cron lines Test Plan: 1) Apply this patch 2) Run process_message_queue.pl with a single -c parameter 3) Note behavior is unchanged 4) Run process_message_queue.pl with multiple -c parameters 5) Note all the codes specified are processed 6) Repeat 2-5 with -t for type limits
Created attachment 114523 [details] [review] Bug 27265: Enable message queue cron to accept multiple codes and types It would be nice to be able to combine several types in a single run, but exclude others, without having to have multiple cron lines Test Plan: 1) Apply this patch 2) Run process_message_queue.pl with a single -c parameter 3) Note behavior is unchanged 4) Run process_message_queue.pl with multiple -c parameters 5) Note all the codes specified are processed 6) Repeat 2-5 with -t for type limits
Created attachment 114525 [details] [review] Bug 27265: Enable message queue cron to accept multiple codes and types It would be nice to be able to combine several types in a single run, but exclude others, without having to have multiple cron lines Test Plan: 1) Apply this patch 2) Run process_message_queue.pl with a single -c parameter 3) Note behavior is unchanged 4) Run process_message_queue.pl with multiple -c parameters 5) Note all the codes specified are processed 6) Repeat 2-5 with -t for type limits
This isn't working for me. Here's what I see: 1- have a patron with email enabled for CHECKOUT, CHECKIN notices and an email address on their account 2- check item out to patron, check item in again 3- confirm on patron notices page that they have pending CHECKOUT and CHECKIN notices 4- perl misc/cronjobs/process_message_queue.pl -c CHECKOUT -c CHECKIN 5- notices are not sent 6- perl misc/cronjobs/process_message_queue.pl -c CHECKOUT 7- one notice is sent
Created attachment 115430 [details] [review] Bug 27265: Enable message queue cron to accept multiple codes and types It would be nice to be able to combine several types in a single run, but exclude others, without having to have multiple cron lines Test Plan: 1) Apply this patch 2) Run process_message_queue.pl with a single -c parameter 3) Note behavior is unchanged 4) Run process_message_queue.pl with multiple -c parameters 5) Note all the codes specified are processed 6) Repeat 2-5 with -t for type limits
Patch no longer applies 8-(.. root@kohadevbox:koha(bz27265)$ git bz apply 27265 Bug 27265 - Process message queue cron should be able to take multiple types as a parameter 115430 - Bug 27265: Enable message queue cron to accept multiple codes and types Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 27265: Enable message queue cron to accept multiple codes and types Using index info to reconstruct a base tree... M C4/Letters.pm M misc/cronjobs/process_message_queue.pl Falling back to patching base and 3-way merge... Auto-merging misc/cronjobs/process_message_queue.pl 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 27265: Enable message queue cron to accept multiple codes and types
Created attachment 121717 [details] [review] Bug 27265: Enable message queue cron to accept multiple codes and types It would be nice to be able to combine several types in a single run, but exclude others, without having to have multiple cron lines Test Plan: 1) Apply this patch 2) Run process_message_queue.pl with a single -c parameter 3) Note behavior is unchanged 4) Run process_message_queue.pl with multiple -c parameters 5) Note all the codes specified are processed 6) Repeat 2-5 with -t for type limits
I get this message when running the script after the patch is applied (koha-testing-docker): root@kohadevbox:koha(bz27265)$ perl misc/cronjobs/process_message_queue.pl -c HOLD Global symbol "$type" requires explicit package name (did you forget to declare "my $type"?) at misc/cronjobs/process_message_queue.pl line 89. Global symbol "$letter_code" requires explicit package name (did you forget to declare "my $letter_code"?) at misc/cronjobs/process_message_queue.pl line 90. Type of arg 1 to Try::Tiny::catch must be block or sub {} (not reference constructor) at misc/cronjobs/process_message_queue.pl line 96, near "};" Type of arg 1 to Try::Tiny::try must be block or sub {} (not reference constructor) at misc/cronjobs/process_message_queue.pl line 96, near "};" Execution of misc/cronjobs/process_message_queue.pl aborted due to compilation errors. It worked with a single letter type before the patch was applied.
I was going to write a very different comment here based on what I read in the "Text to go in the release notes", but after re-reading the comments and code, I realized I totally misunderstood this change. You might consider changing the phrasing to something like "This patch allows one to specify several types or letter codes, instead of only one type or letter code, when running the process_message_queue script. This allows libraries to consolidate calls when some messages types or letter codes are scheduled differently than others." I thought this patch added the ability to specify types or letter codes in general, and I was going to comment on that ability, but that ability was added in a previous ticket...
Created attachment 130476 [details] [review] Bug 27265: Enable message queue cron to accept multiple codes and types It would be nice to be able to combine several types in a single run, but exclude others, without having to have multiple cron lines Test Plan: 1) Apply this patch 2) Run process_message_queue.pl with a single -c parameter 3) Note behavior is unchanged 4) Run process_message_queue.pl with multiple -c parameters 5) Note all the codes specified are processed 6) Repeat 2-5 with -t for type limits
I'm getting this error when trying to run process_message_queue with this patch: Global symbol "$letter_code" requires explicit package name (did you forget to declare "my $letter_code"?) at misc/cronjobs/process_message_queue.pl line 84. Type of arg 1 to Try::Tiny::catch must be block or sub {} (not reference constructor) at misc/cronjobs/process_message_queue.pl line 90, near "};" Type of arg 1 to Try::Tiny::try must be block or sub {} (not reference constructor) at misc/cronjobs/process_message_queue.pl line 90, near "};" Execution of misc/cronjobs/process_message_queue.pl aborted due to compilation errors.
Created attachment 131434 [details] [review] Bug 27265: Enable message queue cron to accept multiple codes and types It would be nice to be able to combine several types in a single run, but exclude others, without having to have multiple cron lines Test Plan: 1) Apply this patch 2) Run process_message_queue.pl with a single -c parameter 3) Note behavior is unchanged 4) Run process_message_queue.pl with multiple -c parameters 5) Note all the codes specified are processed 6) Repeat 2-5 with -t for type limits
Created attachment 138712 [details] [review] Bug 27265: Enable message queue cron to accept multiple codes and types It would be nice to be able to combine several types in a single run, but exclude others, without having to have multiple cron lines Test Plan: 1) Apply this patch 2) Run process_message_queue.pl with a single -c parameter 3) Note behavior is unchanged 4) Run process_message_queue.pl with multiple -c parameters 5) Note all the codes specified are processed 6) Repeat 2-5 with -t for type limits
andrewfh@dubcolib.org 2022-08-05 17:53:58 UTC Status Needs Signoff Signed Off Normally we would also expect your signoff line?
At scope borders: sub SendQueuedMessages my $which_unsent_messages = { 'message_id' => $params->{'message_id'}, 'limit' => $params->{'limit'} // 0, 'borrowernumber' => $params->{'borrowernumber'} // q{}, 'letter_code' => $params->{'letter_code'} // q{}, 'type' => $params->{'type'} // q{}, my $unsent_messages = _get_unsent_messages( $which_unsent_messages ); The POD of the following sub does not mention the undocumented 'type' sub _get_unsent_messages { if ( $params->{'message_transport_type'} ) { $statement .= ' AND mq.message_transport_type = ? '; push @query_params, $params->{'message_transport_type'}; } AND if ( $params->{'type'} ) { my @types = ref $params->{'type'} eq "ARRAY" ? @{$params->{'type'}} : $params->{'type'}; if ( @types ) { my $q = join( ",", ("?") x @types ); $statement .= " AND message_transport_type IN ( $q ) "; push @query_params, @types; } } We could better use one parameter and use the one mentioned in POD.
Created attachment 141129 [details] [review] Bug 27265: Enable message queue cron to accept multiple codes and types It would be nice to be able to combine several types in a single run, but exclude others, without having to have multiple cron lines Test Plan: 1) Apply this patch 2) Run process_message_queue.pl with a single -c parameter 3) Note behavior is unchanged 4) Run process_message_queue.pl with multiple -c parameters 5) Note all the codes specified are processed 6) Repeat 2-5 with -t for type limits Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Created attachment 141130 [details] [review] Bug 27265: (follow-up) Use existing parameter
(In reply to Marcel de Rooy from comment #14) > andrewfh@dubcolib.org 2022-08-05 17:53:58 UTC Status Needs Signoff Signed Off > > Normally we would also expect your signoff line? Oh! I'm not sure how that happened; thanks for catching it.
Created attachment 141966 [details] [review] Bug 27265: Enable message queue cron to accept multiple codes and types It would be nice to be able to combine several types in a single run, but exclude others, without having to have multiple cron lines Test Plan: 1) Apply this patch 2) Run process_message_queue.pl with a single -c parameter 3) Note behavior is unchanged 4) Run process_message_queue.pl with multiple -c parameters 5) Note all the codes specified are processed 6) Repeat 2-5 with -t for type limits Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 141967 [details] [review] Bug 27265: (follow-up) Use existing parameter Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 141968 [details] [review] Bug 27265: (QA follow-up) Typo on L99 cron script Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Pushed to master for 22.11. Nice work everyone, thanks!
Backported to 22.05.x for upcoming 22.05.08
applied to 21.11.x for 21.11.14 (backported from 22.05.x)
Not backported to 21.05.x