Bug 30921 - Replace use of C4::XSLT::transformMARCXML4XSLT with RecordProcessor
Summary: Replace use of C4::XSLT::transformMARCXML4XSLT with RecordProcessor
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Joonas Kylmälä
URL:
Keywords:
Depends on: 30744 30848
Blocks:
  Show dependency treegraph
 
Reported: 2022-06-07 13:57 UTC by Martin Renvoize
Modified: 2023-06-08 22:26 UTC (History)
5 users (show)

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


Attachments
Bug 30921: Replace C4::XSLT::transformMARCXML4XSLT with RecordProcessor (8.25 KB, patch)
2022-06-07 14:25 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30921: Replace C4::XSLT::transformMARCXML4XSLT with RecordProcessor (8.30 KB, patch)
2022-09-08 16:07 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30921: Replace C4::XSLT::transformMARCXML4XSLT with RecordProcessor (8.35 KB, patch)
2022-09-08 19:49 UTC, David Nind
Details | Diff | Splinter Review
Bug 30921: Replace C4::XSLT::transformMARCXML4XSLT with RecordProcessor (8.49 KB, patch)
2022-09-12 16:30 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 30921: (QA follow-up) Remove unnecessary if (1008 bytes, patch)
2022-09-12 16:30 UTC, Joonas Kylmälä
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-06-07 13:57:27 UTC
We should remove C4::XSLT::transformMARCXML4XSLT and instead utilise the coherent interface provided by RecordProcessor and the new ExpandCodedValues filter.
Comment 1 Martin Renvoize 2022-06-07 14:25:39 UTC
Created attachment 135778 [details] [review]
Bug 30921: Replace C4::XSLT::transformMARCXML4XSLT with RecordProcessor

This patch removes the last remaining uses of transformMARCXML4XSLT and
replaced them with the equivilent RecordProcessor usage.
Comment 2 David Nind 2022-06-08 17:02:16 UTC
Hi Martin.

What is the best way to test this?

- prove t/db_dependent/XSLT.t passes before and after the patch is applied
- a git grep transformMARCXML4XSLT shows no occurrences after the patch is applied

Since this is tied in with all the other dependencies, is there an overall test plan?

David
Comment 3 Martin Renvoize 2022-09-08 11:38:32 UTC
Hi David, 

Yes, basically it's run prove as you suggest and then grep to ensure transfromMARCXML4XSLT hasn't crept back in anywhere.

In general, to confirm there are no regressions in the UI, you can look at search results or detailed views on records in your catalog.. anything that's tied to an authorized value should look the same before and after the patch.. things like locations for example.
Comment 4 David Nind 2022-09-08 12:21:52 UTC
Patch no longer applies 8-(...

Applying: Bug 30921: Replace C4::XSLT::transformMARCXML4XSLT with RecordProcessor
Using index info to reconstruct a base tree...
M	Koha/OAI/Server/Repository.pm
Falling back to patching base and 3-way merge...
Auto-merging Koha/OAI/Server/Repository.pm
CONFLICT (content): Merge conflict in Koha/OAI/Server/Repository.pm
error: Failed to merge in the changes.
Patch failed at 0001 Bug 30921: Replace C4::XSLT::transformMARCXML4XSLT with RecordProcessor
Comment 5 Martin Renvoize 2022-09-08 16:07:08 UTC
Created attachment 140350 [details] [review]
Bug 30921: Replace C4::XSLT::transformMARCXML4XSLT with RecordProcessor

This patch removes the last remaining uses of transformMARCXML4XSLT and
replaced them with the equivilent RecordProcessor usage.
Comment 6 David Nind 2022-09-08 19:49:12 UTC
Created attachment 140354 [details] [review]
Bug 30921: Replace C4::XSLT::transformMARCXML4XSLT with RecordProcessor

This patch removes the last remaining uses of transformMARCXML4XSLT and
replaced them with the equivilent RecordProcessor usage.

Signed-off-by: David Nind <david@davidnind.com>
Comment 7 David Nind 2022-09-08 19:54:02 UTC
Testing notes (using koha-testing-docker):

1. Ran prove t/db_dependent/XSLT.t - tests should pass before and after the patch is applied

2. Ran git grep transformMARCXML4XSLT - should show no occurrences after the patch is applied.

3. Reviewed search results and a record details page before and after patch is applied, for both the staff interface and OPAC. There should be no differences in how the search results and individual record details page are displayed. (I searched for 'perl', and selected one record to view - compared side-by side in a new browser window.)

Hope that is sufficient for testing.
Comment 8 Joonas Kylmälä 2022-09-12 16:30:54 UTC
Created attachment 140477 [details] [review]
Bug 30921: Replace C4::XSLT::transformMARCXML4XSLT with RecordProcessor

This patch removes the last remaining uses of transformMARCXML4XSLT and
replaced them with the equivilent RecordProcessor usage.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Comment 9 Joonas Kylmälä 2022-09-12 16:30:58 UTC
Created attachment 140478 [details] [review]
Bug 30921: (QA follow-up) Remove unnecessary if

The statement is already inside an if block with the same condition.

Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Comment 10 Tomás Cohen Arazi 2022-09-12 19:42:23 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 11 Lucas Gass 2022-10-31 21:24:09 UTC
Enhancement will not be backported to 22.05.x series unless requested