There is no need for this code to have a hard coded list of fields directly in the code. Any invalid keys would be skipped anyway. If we refactor this code then adding new fields will be much simpler.
Created attachment 147963 [details] [review] Bug 33170: Refactor MARCItemFieldsToOrder to make adding more fields trivial There is no need for this code to have a hard coded list of fields directly in the code. Any invalid keys would be skipped anyway. If we refactor this code then adding new fields will be much simpler. Test Plan: 1) Set up your MARCItemFieldsToOrder, verify everything is working 2) Apply this patch 3) Restart all the things! 4) Verify there has been no change to the MARCItemFieldsToOrder functionality
Created attachment 151534 [details] [review] Bug 33170: Refactor MARCItemFieldsToOrder to make adding more fields trivial There is no need for this code to have a hard coded list of fields directly in the code. Any invalid keys would be skipped anyway. If we refactor this code then adding new fields will be much simpler. Test Plan: 1) Set up your MARCItemFieldsToOrder, verify everything is working 2) Apply this patch 3) Restart all the things! 4) Verify there has been no change to the MARCItemFieldsToOrder functionality Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Looks like this line might need similar attention somewhere too: my $infos = get_infos_syspref('MarcFieldsToOrder', $marcrecord, ['price', 'quantity', 'budget_code', 'discount', 'sort1', 'sort2','replacementprice']);
This waited much too long already, so I wont complain about moving such routines to module level.. (Making them testable etc. etc.)
(In reply to Kyle M Hall from comment #1) > > Test Plan: > 1) Set up your MARCItemFieldsToOrder, verify everything is working We could call this a lazy test plan :) The generic form is: Set it up and verify that all works. LOL
Created attachment 155363 [details] [review] Bug 33170: Refactor MARCItemFieldsToOrder to make adding more fields trivial There is no need for this code to have a hard coded list of fields directly in the code. Any invalid keys would be skipped anyway. If we refactor this code then adding new fields will be much simpler. Test Plan: 1) Set up your MARCItemFieldsToOrder, verify everything is working 2) Apply this patch 3) Restart all the things! 4) Verify there has been no change to the MARCItemFieldsToOrder functionality Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Pushed to master for 23.11. Nice work everyone, thanks!
Enhancement not pushed to 23.05.x