Bug 33170 - Refactor MarcItemFieldsToOrder code to make adding more fields simpler
Summary: Refactor MarcItemFieldsToOrder code to make adding more fields simpler
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks: 33171 34355 35913 35912
  Show dependency treegraph
 
Reported: 2023-03-08 16:10 UTC by Kyle M Hall
Modified: 2024-02-08 18:38 UTC (History)
4 users (show)

See Also:
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


Attachments
Bug 33170: Refactor MARCItemFieldsToOrder to make adding more fields trivial (6.93 KB, patch)
2023-03-08 16:13 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 33170: Refactor MARCItemFieldsToOrder to make adding more fields trivial (6.99 KB, patch)
2023-05-22 15:51 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 33170: Refactor MARCItemFieldsToOrder to make adding more fields trivial (7.09 KB, patch)
2023-09-08 07:10 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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