Bug 30921

Summary: Replace use of C4::XSLT::transformMARCXML4XSLT with RecordProcessor
Product: Koha Reporter: Martin Renvoize (ashimema) <martin.renvoize>
Component: Architecture, internals, and plumbingAssignee: Martin Renvoize (ashimema) <martin.renvoize>
Status: CLOSED FIXED QA Contact: Joonas Kylmälä <joonas.kylmala>
Severity: enhancement    
Priority: P5 - low CC: david, dcook, joonas.kylmala, lucas, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.11.00
Circulation function:
Bug Depends on: 30744, 30848    
Bug Blocks:    
Attachments: Bug 30921: Replace C4::XSLT::transformMARCXML4XSLT with RecordProcessor
Bug 30921: Replace C4::XSLT::transformMARCXML4XSLT with RecordProcessor
Bug 30921: Replace C4::XSLT::transformMARCXML4XSLT with RecordProcessor
Bug 30921: Replace C4::XSLT::transformMARCXML4XSLT with RecordProcessor
Bug 30921: (QA follow-up) Remove unnecessary if

Description Martin Renvoize (ashimema) 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 (ashimema) 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 (ashimema) 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 (ashimema) 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 (tcohen) 2022-09-12 19:42:23 UTC
Pushed to master for 22.11.

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