[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

Jonathan Druart <jonathan.druart at bugs.koha-community.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jonathan.druart at bugs.koha-c
                   |                            |ommunity.org

--- Comment #18 from Jonathan Druart <jonathan.druart at bugs.koha-community.org> ---
Great to see tests for this code, even if done in an unusual way.

Some quick remarks:
1. Transactions must be done using txn_begin (also note that you are setting
AutoCommit off in the sub and rollback at the end => no sense)

2. $ENV{"OVERRIDE_SYSPREF_dateformat"} = 'metric';
Why not set_preference? Note that for test we usually use mock_preference (from
t::lib::Mocks) to avoid to mess with the cache.

3. You could use build_sample_item from bug 21971 to simplify the objects
creation (not pushed yet)

4. use Koha::DateUtils instead of DateTime directly

5. You could use File::Slurp::read_file to simplify a bit the code

I would have preferred to see the code moved to a module. It would have eased
the write of the tests and made the code reusable.

Keeping the SO status to get other QA opinions.

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