Bug 33170

Summary: Refactor MarcItemFieldsToOrder code to make adding more fields simpler
Product: Koha Reporter: Kyle M Hall <kyle>
Component: Architecture, internals, and plumbingAssignee: Kyle M Hall <kyle>
Status: RESOLVED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: enhancement    
Priority: P5 - low CC: andrewfh, fridolin.somers, m.de.rooy, martin.renvoize
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
**Sponsored by** *ByWater Solutions*
Version(s) released in:
23.11.00
Bug Depends on:    
Bug Blocks: 33171, 34355, 35913, 35912    
Attachments: Bug 33170: Refactor MARCItemFieldsToOrder to make adding more fields trivial
Bug 33170: Refactor MARCItemFieldsToOrder to make adding more fields trivial
Bug 33170: Refactor MARCItemFieldsToOrder to make adding more fields trivial

Description Kyle M Hall 2023-03-08 16:10:08 UTC
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.
Comment 1 Kyle M Hall 2023-03-08 16:13:20 UTC
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
Comment 2 ByWater Sandboxes 2023-05-22 15:51:25 UTC
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>
Comment 3 Marcel de Rooy 2023-09-08 06:58:13 UTC
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']);
Comment 4 Marcel de Rooy 2023-09-08 06:59:03 UTC
This waited much too long already, so I wont complain about moving such routines to module level.. (Making them testable etc. etc.)
Comment 5 Marcel de Rooy 2023-09-08 07:09:42 UTC
(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
Comment 6 Marcel de Rooy 2023-09-08 07:10:27 UTC
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>
Comment 7 Tomás Cohen Arazi 2023-10-11 13:34:09 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 8 Fridolin Somers 2023-10-12 23:55:39 UTC
Enhancement not pushed to 23.05.x