Bug 11790 - C4::Charset should not depend on C4::Context
Summary: C4::Charset should not depend on C4::Context
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-02-18 22:01 UTC by Galen Charlton
Modified: 2016-12-05 21:22 UTC (History)
6 users (show)

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


Attachments
Bug 11790: Remove dependency C4::Context from C4::Charset (1.19 KB, patch)
2015-04-24 15:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 11790: Remove dependency C4::Context from C4::Charset (1.34 KB, patch)
2015-05-16 13:31 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[PASSED QA] Bug 11790: Remove dependency C4::Context from C4::Charset (1.40 KB, patch)
2015-06-07 14:58 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Galen Charlton 2014-02-18 22:01:20 UTC
The patch for bug 8437 added a system preference lookup to C4::Charset::SetMarcUnicodeFlag(), which in turn forces a dependency on C4::Context (c.f. bug 11730).

This is a bit of a wart, as there's nothing else in C4::Charset that is dependent on there being a Koha database.  It builds on yet another wart, introduced by commit 7eca37db, that overloads SetMarcUnicodeFlag() to set fields in the UNIMARC 100 that have nothing to do with the character encoding.

This should be split apart so that

- C4::Charset doesn't require a dependency on C4::Context
- code that handles anything beyond setting the character set flags is moved out of SetMarcUnicodeFlag
Comment 1 Jonathan Druart 2015-04-24 15:06:39 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2015-04-24 15:07:18 UTC
This is a quick patch and does not provide a complete fix for this issue.
Feel free to provide another one.
Comment 3 Bernardo Gonzalez Kriegel 2015-05-16 13:31:08 UTC Comment hidden (obsolete)
Comment 4 Katrin Fischer 2015-06-07 14:58:26 UTC
Created attachment 39958 [details] [review]
[PASSED QA] Bug 11790: Remove dependency C4::Context from C4::Charset

C4::Context is only used to retrieve a syspref value.
This patch moves the use of C4::Context to a require.

Test plan:
Try to reach the SetMarcUnicodeFlag subroutine (batchmod, add/update a biblio, etc.)

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Tested on French UNIMARC install
No errors adding/editing biblios
No koha-qa errors

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 5 Katrin Fischer 2015-06-07 14:59:46 UTC
Couldn't find any regressions.
Comment 6 Tomás Cohen Arazi 2015-06-08 14:09:28 UTC
Patch pushed to master.

Thanks Jonathan!
Comment 7 Chris Cormack 2015-06-08 20:28:02 UTC
Pushed to 3.20.x will be in 3.20.1
Comment 8 Liz Rea 2015-06-22 22:43:47 UTC
Patch pushed to 3.18.x will be in 3.18.08
Comment 9 Mason James 2015-07-25 10:20:59 UTC
Pushed to 3.16.x, will be in 3.16.13