[Koha-bugs] [Bug 20478] Advance notices: send separate digest messages per branch

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20478

Katrin Fischer <katrin.fischer at bsz-bw.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |katrin.fischer at bsz-bw.de
             Status|NEW                         |Needs Signoff

--- Comment #4 from Katrin Fischer <katrin.fischer at bsz-bw.de> ---
Hi Andreas,

thx for your patches! Some things:

1) Once you submitted your patches it's important to switch the status to
'Needs Signoff' in order to get the attention of the testers, otherwise patches
can get easily lost in the saved searches.

2) The QA test tools can help you to find problems with your code easily. For
your patches the report is:

 FAIL   misc/cronjobs/advance_notices.pl
   OK     critic
   FAIL   forbidden patterns
                forbidden pattern: tab char (line 367)
                forbidden pattern: tab char (line 479)
                forbidden pattern: tab char (line 366)
                forbidden pattern: tab char (line 374)
                forbidden pattern: tab char (line 357)
                forbidden pattern: tab char (line 373)
                forbidden pattern: tab char (line 372)
                forbidden pattern: tab char (line 348)
                forbidden pattern: tab char (line 356)
                forbidden pattern: tab char (line 350)
                forbidden pattern: tab char (line 368)
                forbidden pattern: tab char (line 358)
                forbidden pattern: tab char (line 347)
                forbidden pattern: tab char (line 349)
                forbidden pattern: tab char (line 355)
                forbidden pattern: tab char (line 365)
                forbidden pattern: tab char (line 353)
                forbidden pattern: tab char (line 369)
                forbidden pattern: tab char (line 371)
                forbidden pattern: tab char (line 352)
                forbidden pattern: tab char (line 354)
                forbidden pattern: tab char (line 364)
                forbidden pattern: tab char (line 351)
                forbidden pattern: tab char (line 370)

 FAIL   t/db_dependent/cronjobs/advance_notices_digest.t
   FAIL   critic
                Expression form of "eval" at line 167, column 1. See page 161
of PBP.
                Bareword file handle opened at line 158, column 1. See pages
202,204 of PBP.
   FAIL   forbidden patterns
                forbidden pattern: tab char (line 67)


3) This can all be fixed, but I see another problem here: libraries are
differently organized and we support a lot of different use cases with Koha.
Some libraries want Koha to only send one email instead of many, others like
yours don't want this behaviour. So we need to find a compromise here and a way
to make this configurable. I think a system preference could work here as I
don't see how to add it to the notices configuration in a good way.

4) We usually don't test .pl files yet... but not opposed to more tsts :) Tests
are failing for me right now:
t/db_dependent/cronjobs/advance_notices_digest.t .. 1/5 
#   Failed test 'There are two messages in the queue'
#   at t/db_dependent/cronjobs/advance_notices_digest.t line 175.
#          got: '0'
#     expected: '2'
# Looks like you planned 5 tests but ran 1.
# Looks like you failed 1 test of 1 run.
t/db_dependent/cronjobs/advance_notices_digest.t .. Dubious, test returned 1
(wstat 256, 0x100)
Failed 5/5 subtests 

5) If you want, you could also add an individual Sponsord-By line for each
sponsoring library. It depends on how you want the output in the release notes
later on.

-- 
You are receiving this mail because:
You are watching all bug changes.
You are the assignee for the bug.