We should remove C4::XSLT::transformMARCXML4XSLT and instead utilise the coherent interface provided by RecordProcessor and the new ExpandCodedValues filter.
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.
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
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.
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
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.
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>
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.
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>
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>
Pushed to master for 22.11. Nice work everyone, thanks!
Enhancement will not be backported to 22.05.x series unless requested