Bug 30848 - Introduce Koha::Filter::ExpandCodedFields
Summary: Introduce Koha::Filter::ExpandCodedFields
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Testopia
URL:
Keywords: rel_21_05_candidate, rel_21_11_candidate
Depends on:
Blocks: 30744 30921
  Show dependency treegraph
 
Reported: 2022-05-25 14:19 UTC by Martin Renvoize
Modified: 2023-06-08 22:26 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.11.00, 22.05.04, 21.11.11, 21.05.18


Attachments
Bug 30848: Add an AuthorizedValues RecordProcessor filter (5.22 KB, patch)
2022-05-25 14:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30848: Add an AuthorizedValues RecordProcessor filter (5.36 KB, patch)
2022-05-25 16:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30848: Unit test (4.55 KB, patch)
2022-05-26 10:06 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30848: Add an AuthorizedValues RecordProcessor filter (5.42 KB, patch)
2022-05-26 10:06 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30848: Unit test (4.55 KB, patch)
2022-05-26 15:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30848: Add an AuthorizedValues RecordProcessor filter (5.42 KB, patch)
2022-05-26 15:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30848: Caching for performance (6.28 KB, patch)
2022-05-26 15:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30848: Ensure we flush the cache as necessary (4.15 KB, patch)
2022-05-27 10:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30848: Unit test (4.55 KB, patch)
2022-05-27 13:16 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30848: Add an AuthorizedValues RecordProcessor filter (5.42 KB, patch)
2022-05-27 13:16 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30848: Caching for performance (6.59 KB, patch)
2022-05-27 13:16 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30848: Ensure we flush the cache as necessary (4.15 KB, patch)
2022-05-27 13:16 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30848: Move caching level (11.09 KB, patch)
2022-05-27 13:16 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30848: Unit test (4.55 KB, patch)
2022-06-06 10:26 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30848: Add an AuthorizedValues RecordProcessor filter (5.42 KB, patch)
2022-06-06 10:27 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30848: Caching for performance (6.59 KB, patch)
2022-06-06 10:27 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30848: Ensure we flush the cache as necessary (4.15 KB, patch)
2022-06-06 10:27 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30848: Move caching level (11.09 KB, patch)
2022-06-06 10:27 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30848: Ensure we flush the cache as necessary (8.77 KB, patch)
2022-06-06 10:27 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30848: Rename to ExpandCodedFields (2.95 KB, patch)
2022-06-06 10:35 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30848: Unit test (4.55 KB, patch)
2022-06-06 10:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30848: Add an AuthorizedValues RecordProcessor filter (5.42 KB, patch)
2022-06-06 10:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30848: Caching for performance (19.54 KB, patch)
2022-06-06 10:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30848: Rename to ExpandCodedFields (3.17 KB, patch)
2022-06-06 10:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30848: Add an ExpandCodedFields RecordProcessor filter (12.72 KB, patch)
2022-06-07 13:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30848: Add an ExpandCodedFields RecordProcessor filter (12.72 KB, patch)
2022-06-13 13:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30848: Expand unit tests (3.67 KB, patch)
2022-06-13 13:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30848: Fix issue exposed by unit tests (1.80 KB, patch)
2022-06-13 13:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30848: Add an ExpandCodedFields RecordProcessor filter (12.77 KB, patch)
2022-06-17 13:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 30848: Expand unit tests (3.73 KB, patch)
2022-06-17 13:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 30848: Fix issue exposed by unit tests (1.85 KB, patch)
2022-06-17 13:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 30848: Add exec flag to test (597 bytes, patch)
2022-06-17 13:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 30848: Add an ExpandCodedFields RecordProcessor filter (12.83 KB, patch)
2022-06-30 13:58 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 30848: Expand unit tests (3.78 KB, patch)
2022-06-30 13:58 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 30848: Fix issue exposed by unit tests (1.91 KB, patch)
2022-06-30 13:58 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 30848: Add exec flag to test (652 bytes, patch)
2022-06-30 13:58 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2022-05-25 14:19:31 UTC
I think we should introduce a Koha::Filter for replacing AV codes with AV values in the MARC::Record.. we already do this using C4::XSLT::transformMARCXML4XSLT but I believe a RecordProcessor makes more sense than to hide the function away in the defunct XSLT module.

We can then use this anywhere we have a RecordProcessor instantiated and get the goodness of filtered MARC::Records.
Comment 1 Martin Renvoize 2022-05-25 14:37:34 UTC Comment hidden (obsolete)
Comment 2 Tomás Cohen Arazi 2022-05-25 15:47:22 UTC Comment hidden (obsolete)
Comment 3 Martin Renvoize 2022-05-25 16:19:14 UTC Comment hidden (obsolete)
Comment 4 Martin Renvoize 2022-05-25 16:20:58 UTC Comment hidden (obsolete)
Comment 5 Martin Renvoize 2022-05-25 16:21:14 UTC Comment hidden (obsolete)
Comment 6 Martin Renvoize 2022-05-26 10:06:24 UTC Comment hidden (obsolete)
Comment 7 Martin Renvoize 2022-05-26 10:06:30 UTC Comment hidden (obsolete)
Comment 8 Martin Renvoize 2022-05-26 15:46:55 UTC Comment hidden (obsolete)
Comment 9 Martin Renvoize 2022-05-26 15:47:00 UTC Comment hidden (obsolete)
Comment 10 Martin Renvoize 2022-05-26 15:47:05 UTC Comment hidden (obsolete)
Comment 11 Martin Renvoize 2022-05-26 16:24:46 UTC Comment hidden (obsolete)
Comment 12 Martin Renvoize 2022-05-26 16:26:07 UTC Comment hidden (obsolete)
Comment 13 Martin Renvoize 2022-05-27 10:23:23 UTC Comment hidden (obsolete)
Comment 14 Martin Renvoize 2022-05-27 10:24:25 UTC Comment hidden (obsolete)
Comment 15 Martin Renvoize 2022-05-27 10:42:00 UTC Comment hidden (obsolete)
Comment 16 Marcel de Rooy 2022-05-27 10:47:28 UTC Comment hidden (obsolete)
Comment 17 Marcel de Rooy 2022-05-27 10:48:10 UTC Comment hidden (obsolete)
Comment 18 Martin Renvoize 2022-05-27 11:40:24 UTC Comment hidden (obsolete)
Comment 19 Martin Renvoize 2022-05-27 13:16:32 UTC Comment hidden (obsolete)
Comment 20 Martin Renvoize 2022-05-27 13:16:37 UTC Comment hidden (obsolete)
Comment 21 Martin Renvoize 2022-05-27 13:16:41 UTC Comment hidden (obsolete)
Comment 22 Martin Renvoize 2022-05-27 13:16:45 UTC Comment hidden (obsolete)
Comment 23 Martin Renvoize 2022-05-27 13:16:50 UTC Comment hidden (obsolete)
Comment 24 Martin Renvoize 2022-05-27 13:17:59 UTC Comment hidden (obsolete)
Comment 25 Martin Renvoize 2022-06-06 10:26:59 UTC Comment hidden (obsolete)
Comment 26 Martin Renvoize 2022-06-06 10:27:03 UTC Comment hidden (obsolete)
Comment 27 Martin Renvoize 2022-06-06 10:27:07 UTC Comment hidden (obsolete)
Comment 28 Martin Renvoize 2022-06-06 10:27:11 UTC Comment hidden (obsolete)
Comment 29 Martin Renvoize 2022-06-06 10:27:15 UTC Comment hidden (obsolete)
Comment 30 Martin Renvoize 2022-06-06 10:27:18 UTC Comment hidden (obsolete)
Comment 31 Martin Renvoize 2022-06-06 10:35:42 UTC Comment hidden (obsolete)
Comment 32 Martin Renvoize 2022-06-06 10:44:06 UTC Comment hidden (obsolete)
Comment 33 Martin Renvoize 2022-06-06 10:44:10 UTC Comment hidden (obsolete)
Comment 34 Martin Renvoize 2022-06-06 10:44:15 UTC Comment hidden (obsolete)
Comment 35 Martin Renvoize 2022-06-06 10:44:21 UTC Comment hidden (obsolete)
Comment 36 Martin Renvoize 2022-06-06 10:48:45 UTC Comment hidden (obsolete)
Comment 37 Martin Renvoize 2022-06-07 13:37:14 UTC
Created attachment 135770 [details] [review]
Bug 30848: Add an ExpandCodedFields RecordProcessor filter

This patch introduces a RecordProcessor filter for MARC::Record objects
that replaces Koha codes with descriptions in the MARC::Record passed to the processor.

Target usage:

  my $biblio = Koha::Biblios->find(
      $biblio_id,
      { prefetch => [ metadata ] }
  );

  my $record = $biblio->metadata->record;

  my $processor = Koha::RecordProcessor->new(
    {
        filters => ('ExpandCodedFields'),
        options => {
            interface     => 'opac',
            frameworkcode => $biblio->frameworkcode
        }
    }
  );

  $processor->process( $record );

Test plan
* Read the included unit test and confirm it makes sense and passes
Comment 38 Martin Renvoize 2022-06-07 13:38:55 UTC
Patchset reduced to just the introduction of the ExpandCodedFields filter.. description caching moved to a subsequent bug for self-contained testing etc.
Comment 39 Martin Renvoize 2022-06-07 13:52:46 UTC
This code is intended to eventually replace C4::XSLT::transformMARCXML4XSLT and the private function C4::XSLT::getAuthorisedValues4MARCSubfields whilst also introducing a more standardised way of achieving code substitutions for Marc records.
Comment 40 Tomás Cohen Arazi 2022-06-08 12:45:43 UTC
I've always liked the RecordProcessor pattern myself. And love what I'm seeing.
Is this missing branches, itemtypes, etc?
Comment 41 Tomás Cohen Arazi 2022-06-08 12:51:44 UTC
(In reply to Tomás Cohen Arazi from comment #40)
> Is this missing branches, itemtypes, etc?

This intended to be: 'Is this missing tests for branches, itemtypes, etc?'
Comment 42 Martin Renvoize 2022-06-13 13:28:09 UTC
Created attachment 136025 [details] [review]
Bug 30848: Add an ExpandCodedFields RecordProcessor filter

This patch introduces a RecordProcessor filter for MARC::Record objects
that replaces Koha codes with descriptions in the MARC::Record passed to the processor.

Target usage:

  my $biblio = Koha::Biblios->find(
      $biblio_id,
      { prefetch => [ metadata ] }
  );

  my $record = $biblio->metadata->record;

  my $processor = Koha::RecordProcessor->new(
    {
        filters => ('ExpandCodedFields'),
        options => {
            interface     => 'opac',
            frameworkcode => $biblio->frameworkcode
        }
    }
  );

  $processor->process( $record );

Test plan
* Read the included unit test and confirm it makes sense and passes
Comment 43 Martin Renvoize 2022-06-13 13:28:14 UTC
Created attachment 136026 [details] [review]
Bug 30848: Expand unit tests

Add to the unit tests to test Library and Itemtype expansions as well as
linking multiple subfields of the same marc field to such expansions.
Comment 44 Martin Renvoize 2022-06-13 13:28:18 UTC
Created attachment 136027 [details] [review]
Bug 30848: Fix issue exposed by unit tests

This patch changes the 'map' to a simple for loop so that we can easily
map multiple subfields to a field without overwriting the first previous
subfields in the structure.
Comment 45 Nick Clemens 2022-06-17 13:12:34 UTC
Created attachment 136259 [details] [review]
Bug 30848: Add an ExpandCodedFields RecordProcessor filter

This patch introduces a RecordProcessor filter for MARC::Record objects
that replaces Koha codes with descriptions in the MARC::Record passed to the processor.

Target usage:

  my $biblio = Koha::Biblios->find(
      $biblio_id,
      { prefetch => [ metadata ] }
  );

  my $record = $biblio->metadata->record;

  my $processor = Koha::RecordProcessor->new(
    {
        filters => ('ExpandCodedFields'),
        options => {
            interface     => 'opac',
            frameworkcode => $biblio->frameworkcode
        }
    }
  );

  $processor->process( $record );

Test plan
* Read the included unit test and confirm it makes sense and passes

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 46 Nick Clemens 2022-06-17 13:12:37 UTC
Created attachment 136260 [details] [review]
Bug 30848: Expand unit tests

Add to the unit tests to test Library and Itemtype expansions as well as
linking multiple subfields of the same marc field to such expansions.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 47 Nick Clemens 2022-06-17 13:12:41 UTC
Created attachment 136261 [details] [review]
Bug 30848: Fix issue exposed by unit tests

This patch changes the 'map' to a simple for loop so that we can easily
map multiple subfields to a field without overwriting the first previous
subfields in the structure.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 48 Nick Clemens 2022-06-17 13:12:45 UTC
Created attachment 136262 [details] [review]
Bug 30848: Add exec flag to test

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 49 Tomás Cohen Arazi 2022-06-30 13:58:24 UTC
Created attachment 136774 [details] [review]
Bug 30848: Add an ExpandCodedFields RecordProcessor filter

This patch introduces a RecordProcessor filter for MARC::Record objects
that replaces Koha codes with descriptions in the MARC::Record passed to the processor.

Target usage:

  my $biblio = Koha::Biblios->find(
      $biblio_id,
      { prefetch => [ metadata ] }
  );

  my $record = $biblio->metadata->record;

  my $processor = Koha::RecordProcessor->new(
    {
        filters => ('ExpandCodedFields'),
        options => {
            interface     => 'opac',
            frameworkcode => $biblio->frameworkcode
        }
    }
  );

  $processor->process( $record );

Test plan
* Read the included unit test and confirm it makes sense and passes

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 50 Tomás Cohen Arazi 2022-06-30 13:58:29 UTC
Created attachment 136775 [details] [review]
Bug 30848: Expand unit tests

Add to the unit tests to test Library and Itemtype expansions as well as
linking multiple subfields of the same marc field to such expansions.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 51 Tomás Cohen Arazi 2022-06-30 13:58:35 UTC
Created attachment 136776 [details] [review]
Bug 30848: Fix issue exposed by unit tests

This patch changes the 'map' to a simple for loop so that we can easily
map multiple subfields to a field without overwriting the first previous
subfields in the structure.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 52 Tomás Cohen Arazi 2022-06-30 13:58:40 UTC
Created attachment 136777 [details] [review]
Bug 30848: Add exec flag to test

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 53 Tomás Cohen Arazi 2022-07-01 12:20:15 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 54 Lucas Gass 2022-07-29 17:18:15 UTC
Backported to 22.05.x for 22.05.04
Comment 55 Arthur Suzuki 2022-08-11 13:59:59 UTC
pushed to 21.11.x for 21.11.11
Comment 56 Victor Grousset/tuxayo 2022-08-15 20:03:55 UTC
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed.

Nothing to document, marking resolved.
Comment 57 Victor Grousset/tuxayo 2022-08-15 20:39:20 UTC
Will backport because it's a dependency of 30744
Comment 58 Victor Grousset/tuxayo 2022-08-15 20:42:03 UTC
There is a remaining diag() in the tests, shall it stay here?
Comment 59 Victor Grousset/tuxayo 2022-08-15 20:45:55 UTC
For 21.05.x In C4::Biblio I have to export GetMarcSubfieldStructure (like in other branches) so the tests compile. No problem to do that, right?
Comment 60 Victor Grousset/tuxayo 2022-08-15 20:48:41 UTC
Backported: Pushed to 21.05.x branch for 21.05.18