Bug 40934

Summary: process_message_queue.pl add ability to exclude some letter code
Product: Koha Reporter: Marion Durand <marion.durand>
Component: Command-line UtilitiesAssignee: Martin Renvoize (ashimema) <martin.renvoize>
Status: Needs Signoff --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: david, martin.renvoize, robin, ryan.henderson
Version: unspecified   
Hardware: All   
OS: All   
GIT URL: Initiative type: ---
Sponsorship status: Sponsored Comma delimited list of Sponsors: OpenFifth <https://openfifth.co.uk/>
Crowdfunding goal: 0 Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Attachments: Bug 40934: Add tests for exclude_letter_code in process_message_queue.pl
Bug 40934: Add --exclude-code option to process_message_queue.pl
Bug 40934: Add tests for exclude_letter_code in process_message_queue.pl
Bug 40934: Add --exclude-code option to process_message_queue.pl
Bug 40934: Add tests for exclude_letter_code in process_message_queue.pl
Bug 40934: Add --exclude-code option to process_message_queue.pl

Description Marion Durand 2025-10-02 09:52:57 UTC
It would help to add an argument to process_message_queue.pl to exclude some letter-code (like a blacklist, send all letter-code except these ones)

Use case : 
Some library would like to send digest once per day (say at 20:05) but all others message every hour.
It is currently possible to do that by having 2 lines process_message_queue.pl that way :
```
00 */1 * * * process_message_queue.pl --code ACQ_NOTIF_ON_RECEIV --code CART ... # list all message except digest
05 20 * * * process_message_queue.pl #send all message including digest
```
The problem with this method is that we have to list almost all letter-code in the first line (around 85), there is a high chance that we forget some notification in the every hour line (thus these notifications will not be sent every hour as wanted)

It would be great to be able to do it by excluding some letter code 
example: 
```
00 */1 * * * process_message_queue.pl --xcode DUEDGST --xcode PREDUEDGST --xcode HOLDDGST --xcode AUTO_RENEWALS_DGST # send all exepct digest
05 20 * * * process_message_queue.pl # send all including digest
```
(In this example, argument is xcode for exclude code but I'm not sure about the name)
Comment 1 Martin Renvoize (ashimema) 2026-01-07 15:53:14 UTC
Created attachment 190995 [details] [review]
Bug 40934: Add tests for exclude_letter_code in process_message_queue.pl

This patch adds unit tests for the new exclude_letter_code functionality
in SendQueuedMessages, which will allow excluding specific letter codes
when processing the message queue.

Test coverage includes:
- Excluding a single letter code (array format)
- Excluding multiple letter codes
- Excluding with scalar parameter format
Comment 2 Martin Renvoize (ashimema) 2026-01-07 15:53:15 UTC
Created attachment 190996 [details] [review]
Bug 40934: Add --exclude-code option to process_message_queue.pl

Some libraries want to send digest messages once per day (e.g., at 20:05)
but send all other messages every hour. Currently, this requires listing
almost all letter codes (~85) in the hourly cron job, which is error-prone
and makes it easy to forget notifications at upgrades.

This patch adds a new --exclude-code (-x) option to process_message_queue.pl
that allows excluding specific letter codes from processing, making it much
easier to configure digest scheduling.

Changes:
- Added --exclude-code option to process_message_queue.pl (repeatable)
- Modified C4::Letters::SendQueuedMessages to handle exclude_letter_code parameter
- Supports both scalar and array reference formats

Example usage:
  # Send all messages except digests every hour
  00 */1 * * * process_message_queue.pl --exclude-code DUEDGST \
    --exclude-code PREDUEDGST --exclude-code HOLDDGST \
    --exclude-code AUTO_RENEWALS_DGST

  # Send all messages including digests once per day
  05 20 * * * process_message_queue.pl

Test plan:
1. Apply both patches
2. Run: ktd --shell --run 'prove t/db_dependent/Letters.t'
3. Verify all tests pass (should show 105 tests passing)
4. Create test messages in message_queue with different letter codes:
   - Insert messages with codes: DUEDGST, PREDUEDGST, ACQ_NOTIF, HOLD
5. Test single exclusion:
   misc/cronjobs/process_message_queue.pl --exclude-code DUEDGST
   - Verify DUEDGST messages remain pending
   - Verify other messages are processed
6. Test multiple exclusions:
   misc/cronjobs/process_message_queue.pl --exclude-code DUEDGST \
     --exclude-code PREDUEDGST
   - Verify both digest messages remain pending
   - Verify non-digest messages are processed
7. Test combined with --code option (should work together):
   misc/cronjobs/process_message_queue.pl --code ACQ_NOTIF \
     --exclude-code DUEDGST
   - Verify only ACQ_NOTIF messages processed (DUEDGST excluded anyway)
8. Test help output:
   misc/cronjobs/process_message_queue.pl --help
   - Verify --exclude-code option is documented
Comment 3 Marion Durand 2026-01-08 08:24:40 UTC
Hello,

Thank you for your patch.
I tried to test but in the first step of the test plan, some test failed. Is it 

On a brand new ktd on main, I applied both patches and used the command 
`ktd --shell --run 'prove t/db_dependent/Letters.t'`
I have some test failed, I tried to restart_all then launch the test again but the results is the same. Is it a problem on my setup ?

Here is the full test results :
```

    #   Failed test 'PREDUEDGST message processed'
    #   at t/db_dependent/Letters.t line 1916.
    #          got: 'sent'
    #     expected: 'failed'

    #   Failed test 'ACQ_NOTIF message processed'
    #   at t/db_dependent/Letters.t line 1917.
    #          got: 'sent'
    #     expected: 'failed'

    #   Failed test 'TEST_MESSAGE message processed'
    #   at t/db_dependent/Letters.t line 1918.
    #          got: 'sent'
    #     expected: 'failed'

    #   Failed test 'ACQ_NOTIF message processed'
    #   at t/db_dependent/Letters.t line 1935.
    #          got: 'sent'
    #     expected: 'failed'

    #   Failed test 'TEST_MESSAGE message processed'
    #   at t/db_dependent/Letters.t line 1936.
    #          got: 'sent'
    #     expected: 'failed'

    #   Failed test 'ACQ_NOTIF message processed'
    #   at t/db_dependent/Letters.t line 1950.
    #          got: 'sent'
    #     expected: 'failed'
    # Looks like you failed 6 tests of 10.

#   Failed test 'Test exclude_letter_code parameter for SendQueuedMessages'
#   at t/db_dependent/Letters.t line 1952.
# Looks like you planned 105 tests but ran 106.
# Looks like you failed 1 test of 106 run.
t/db_dependent/Letters.t .. 
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/105 subtests 

Test Summary Report
-------------------
t/db_dependent/Letters.t (Wstat: 256 (exited 1) Tests: 106 Failed: 2)
  Failed tests:  105-106
  Non-zero exit status: 1
  Parse errors: Bad plan.  You planned 105 tests but ran 106.
Files=1, Tests=106,  4 wallclock secs ( 0.02 usr  0.01 sys +  3.17 cusr  0.61 csys =  3.81 CPU)
Result: FAIL

```
Comment 4 David Nind 2026-01-08 20:42:58 UTC
(In reply to Marion Durand from comment #3)
> Hello,
> 
> Thank you for your patch.
> I tried to test but in the first step of the test plan, some test failed. Is
> it 
> 
> On a brand new ktd on main, I applied both patches and used the command 
> `ktd --shell --run 'prove t/db_dependent/Letters.t'`
> I have some test failed, I tried to restart_all then launch the test again
> but the results is the same. Is it a problem on my setup ?

The tests fail for me as well, so it is not your setup.

You can happily change the status to Failed QA, for Martin to fix up when he gets a chance 8-).
Comment 5 Martin Renvoize (ashimema) 2026-01-12 17:26:18 UTC
Created attachment 191304 [details] [review]
Bug 40934: Add tests for exclude_letter_code in process_message_queue.pl

This patch adds unit tests for the new exclude_letter_code functionality
in SendQueuedMessages, which will allow excluding specific letter codes
when processing the message queue.

Test coverage includes:
- Excluding a single letter code (array format)
- Excluding multiple letter codes
- Excluding with scalar parameter format
Comment 6 Martin Renvoize (ashimema) 2026-01-12 17:26:20 UTC
Created attachment 191305 [details] [review]
Bug 40934: Add --exclude-code option to process_message_queue.pl

Some libraries want to send digest messages once per day (e.g., at 20:05)
but send all other messages every hour. Currently, this requires listing
almost all letter codes (~85) in the hourly cron job, which is error-prone
and makes it easy to forget notifications at upgrades.

This patch adds a new --exclude-code (-x) option to process_message_queue.pl
that allows excluding specific letter codes from processing, making it much
easier to configure digest scheduling.

Changes:
- Added --exclude-code option to process_message_queue.pl (repeatable)
- Modified C4::Letters::SendQueuedMessages to handle exclude_letter_code parameter
- Supports both scalar and array reference formats

Example usage:
  # Send all messages except digests every hour
  00 */1 * * * process_message_queue.pl --exclude-code DUEDGST \
    --exclude-code PREDUEDGST --exclude-code HOLDDGST \
    --exclude-code AUTO_RENEWALS_DGST

  # Send all messages including digests once per day
  05 20 * * * process_message_queue.pl

Test plan:
1. Apply both patches
2. Run: ktd --shell --run 'prove t/db_dependent/Letters.t'
3. Verify all tests pass (should show 105 tests passing)
4. Create test messages in message_queue with different letter codes:
   - Insert messages with codes: DUEDGST, PREDUEDGST, ACQ_NOTIF, HOLD
5. Test single exclusion:
   misc/cronjobs/process_message_queue.pl --exclude-code DUEDGST
   - Verify DUEDGST messages remain pending
   - Verify other messages are processed
6. Test multiple exclusions:
   misc/cronjobs/process_message_queue.pl --exclude-code DUEDGST \
     --exclude-code PREDUEDGST
   - Verify both digest messages remain pending
   - Verify non-digest messages are processed
7. Test combined with --code option (should work together):
   misc/cronjobs/process_message_queue.pl --code ACQ_NOTIF \
     --exclude-code DUEDGST
   - Verify only ACQ_NOTIF messages processed (DUEDGST excluded anyway)
8. Test help output:
   misc/cronjobs/process_message_queue.pl --help
   - Verify --exclude-code option is documented
Comment 7 Martin Renvoize (ashimema) 2026-01-12 17:27:24 UTC
Created attachment 191306 [details] [review]
Bug 40934: Add tests for exclude_letter_code in process_message_queue.pl

This patch adds unit tests for the new exclude_letter_code functionality
in SendQueuedMessages, which will allow excluding specific letter codes
when processing the message queue.

Test coverage includes:
- Excluding a single letter code (array format)
- Excluding multiple letter codes
- Excluding with scalar parameter format

Sponsored-by: OpenFifth <https://openfifth.co.uk/>
Comment 8 Martin Renvoize (ashimema) 2026-01-12 17:27:26 UTC
Created attachment 191307 [details] [review]
Bug 40934: Add --exclude-code option to process_message_queue.pl

Some libraries want to send digest messages once per day (e.g., at 20:05)
but send all other messages every hour. Currently, this requires listing
almost all letter codes (~85) in the hourly cron job, which is error-prone
and makes it easy to forget notifications at upgrades.

This patch adds a new --exclude-code (-x) option to process_message_queue.pl
that allows excluding specific letter codes from processing, making it much
easier to configure digest scheduling.

Changes:
- Added --exclude-code option to process_message_queue.pl (repeatable)
- Modified C4::Letters::SendQueuedMessages to handle exclude_letter_code parameter
- Supports both scalar and array reference formats

Example usage:
  # Send all messages except digests every hour
  00 */1 * * * process_message_queue.pl --exclude-code DUEDGST \
    --exclude-code PREDUEDGST --exclude-code HOLDDGST \
    --exclude-code AUTO_RENEWALS_DGST

  # Send all messages including digests once per day
  05 20 * * * process_message_queue.pl

Test plan:
1. Apply both patches
2. Run: ktd --shell --run 'prove t/db_dependent/Letters.t'
3. Verify all tests pass (should show 105 tests passing)
4. Create test messages in message_queue with different letter codes:
   - Insert messages with codes: DUEDGST, PREDUEDGST, ACQ_NOTIF, HOLD
5. Test single exclusion:
   misc/cronjobs/process_message_queue.pl --exclude-code DUEDGST
   - Verify DUEDGST messages remain pending
   - Verify other messages are processed
6. Test multiple exclusions:
   misc/cronjobs/process_message_queue.pl --exclude-code DUEDGST \
     --exclude-code PREDUEDGST
   - Verify both digest messages remain pending
   - Verify non-digest messages are processed
7. Test combined with --code option (should work together):
   misc/cronjobs/process_message_queue.pl --code ACQ_NOTIF \
     --exclude-code DUEDGST
   - Verify only ACQ_NOTIF messages processed (DUEDGST excluded anyway)
8. Test help output:
   misc/cronjobs/process_message_queue.pl --help
   - Verify --exclude-code option is documented

Sponsored-by: OpenFifth <https://openfifth.co.uk/>
Comment 9 Martin Renvoize (ashimema) 2026-01-12 17:28:28 UTC
My mistake on the tests.. I commited too early..

They should all be passing now.
Comment 10 Marion Durand 2026-01-14 08:33:56 UTC
Thank you for your work :) Everything works well for me except for the point 7.

If I try to use both --code and --exclude-code, the order of the parameter matters.

My message queue table contain 4 messages with status pending (restored between each run of process_mesage_queue).
- PREDUE
- DUEDGST
- WELCOME
- CHECKOUT

a- If I use the command
```
./misc/cronjobs/process_message_queue.pl --code WELCOME --exclude-code DUEDGST
```
The WELCOME notice is processed and nothing else is processed (expected for --code, the exclude-code is not needed as it will only process WELCOME notice)

b- If I use the command
```
./misc/cronjobs/process_message_queue.pl --exclude-code DUEDGST --code WELCOME
```
Everything is processed except DUEDGST (expected for --exclude-code, the --code is not taken into account (only the WELCOME should be processed if it was))

So the script is not warning, but only the first one is taken into account.


To me, it makes no sens to use both argument --code and --exclude-code in the same line. (either I send everything except a few (--excludecode) or I send only those listed (--code)).
In other word (because I'm not sure if I'm clear)
The --code argument is used to send only a list of message (so no need to exclude some other, just list what you need)
The --exclude-code argument is used to send everything except a list (so no need to list what you want to send).

I think it should work the same as --category and --skip-category in cronjob longoverdue.pl (they are incompatible with each other).