Created attachment 30754 [details] [review] Bug 12754: Add XSLT action to MARC modification templates This is the dbrev for this report. It adds enum run_xslt for the action column. It adds a new columns run_xslt for the XSLT file to run. Test plan: Upgrade via the webinstaller and check table marc_modification_template_actions.
Created attachment 30756 [details] [review] Bug 12754: Add XSLT action to MARC modification templates This is the dbrev for this report. It adds enum run_xslt for the action column. It adds a new columns run_xslt for the XSLT file to run. Test plan: Upgrade via the webinstaller and check table marc_modification_template_actions.
Created attachment 30757 [details] [review] Bug 12754: [TESTING ONLY] dbix update for actions table This patch should not be pushed. The RM will update the DBIx schema.
Created attachment 30758 [details] [review] Bug 12754: Add run_xslt to MarcModificationTemplates.pm
Created attachment 30759 [details] [review] Bug 12754: Template changes for run_xslt in marc mod templates Adds action Run XSLT to the admin template and script. Test plan: Add, edit and delete actions with/without xslt actions.
Still one patch forthcoming that performs the actual xslt action..
Created attachment 30782 [details] [review] Bug 12754: Template changes for run_xslt in marc mod templates Adds action 'Run XSLT' to the admin template and script. Test plan: Go to Manage MARC modification templates. Select a template. Add, edit and delete actions, including the Run XSLT type.
Created attachment 30783 [details] [review] Bug 12754: Perform the actual XSLT action for marc mod template This patch modifies MarcModificationTemplates.pm. A local subroutine _RunXSLTAction is added to perform the actual xslt transformation using the XSLTHandler object. Test plan: Add some XSLT actions to a marc mod template. Run Stage for Import and select the correct marc mod template. Check your staged records for the expected changes (you can also view these reservoir records in Cataloging search).
Created attachment 30784 [details] [review] Bug 12754: [TESTING ONLY] Sample XSLT file for testing This test XSLT adds a 500 tag to your record.
Very promising! I have a few suggestions before I would sign it off though: 1) Change the database column name from "run_xslt" to just "xslt". To me "run_xslt" sounds like a boolean, and could cause confusion for future developers. 2) Change the phrase "Run XSLT" to "Apply XSLT", which is a more proper way to describe this in English. 3) The form input for the XSLT code should be a textarea, rather than a single line text input. 4) Please add a formal step by step test plan With those changes, I think this will make an excellent addition for the modification templates!
Looks like you did add a test plan, I just missed it, so you can scratch point 4!
(In reply to Kyle M Hall from comment #10) > Very promising! I have a few suggestions before I would sign it off though: > 1) Change the database column name from "run_xslt" to just "xslt". To me > "run_xslt" sounds like a boolean, and could cause confusion for future > developers. > 2) Change the phrase "Run XSLT" to "Apply XSLT", which is a more proper way > to describe this in English. > 3) The form input for the XSLT code should be a textarea, rather than a > single line text input. > 4) Please add a formal step by step test plan > > With those changes, I think this will make an excellent addition for the > modification templates! Will do that. About point 3: The single line of text refers to a filename instead of the actual xslt code.
(In reply to M. de Rooy from comment #12) > (In reply to Kyle M Hall from comment #10) > > Very promising! I have a few suggestions before I would sign it off though: > > 1) Change the database column name from "run_xslt" to just "xslt". To me > > "run_xslt" sounds like a boolean, and could cause confusion for future > > developers. > > 2) Change the phrase "Run XSLT" to "Apply XSLT", which is a more proper way > > to describe this in English. > > 3) The form input for the XSLT code should be a textarea, rather than a > > single line text input. > > 4) Please add a formal step by step test plan > > > > With those changes, I think this will make an excellent addition for the > > modification templates! > > Will do that. > About point 3: The single line of text refers to a filename instead of the > actual xslt code. Is this a reference to a file on the server? Would it not be easier to store the code in the database so it can be viewed and edited from the template editor?
(In reply to Kyle M Hall from comment #13) > Is this a reference to a file on the server? Would it not be easier to store > the code in the database so it can be viewed and edited from the template > editor? Yes, it refers to a file. But we could offer both approaches..
(In reply to M. de Rooy from comment #14) > (In reply to Kyle M Hall from comment #13) > > Is this a reference to a file on the server? Would it not be easier to store > > the code in the database so it can be viewed and edited from the template > > editor? > > Yes, it refers to a file. But we could offer both approaches.. I think that would be for the best!
Status back to Assigned. Will send some changes soon.
This will require rebasing since Bug 15867 was pushed.