Bug 33431 - Make code use C4::Context->yaml_preference
Summary: Make code use C4::Context->yaml_preference
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: Tomás Cohen Arazi
QA Contact: Testopia
URL:
Keywords:
Depends on: 23580 30403 35293
Blocks:
  Show dependency treegraph
 
Reported: 2023-04-06 13:41 UTC by Tomás Cohen Arazi
Modified: 2024-03-21 09:42 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00


Attachments
Bug 33431: Make C4::Circulation use C4::Context->yaml_preference (4.13 KB, patch)
2023-04-06 14:45 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33431: Make C4::Record use C4::Context->yaml_preference (1.45 KB, patch)
2023-04-06 14:45 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33431: Fix remaining cases (4.73 KB, patch)
2023-04-06 14:45 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33431: Make C4::Circulation use C4::Context->yaml_preference (5.54 KB, patch)
2023-12-22 15:27 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33431: Make C4::Record use C4::Context->yaml_preference (1.45 KB, patch)
2023-12-22 15:27 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33431: Fix remaining cases (4.17 KB, patch)
2023-12-22 15:27 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33431: Make C4::Circulation use C4::Context->yaml_preference (5.59 KB, patch)
2023-12-23 08:45 UTC, David Nind
Details | Diff | Splinter Review
Bug 33431: Make C4::Record use C4::Context->yaml_preference (1.49 KB, patch)
2023-12-23 08:45 UTC, David Nind
Details | Diff | Splinter Review
Bug 33431: Fix remaining cases (4.21 KB, patch)
2023-12-23 08:45 UTC, David Nind
Details | Diff | Splinter Review
Bug 33431: Make C4::Circulation use C4::Context->yaml_preference (5.64 KB, patch)
2024-01-02 10:30 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33431: Make C4::Record use C4::Context->yaml_preference (1.55 KB, patch)
2024-01-02 10:30 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33431: Fix remaining cases (4.27 KB, patch)
2024-01-02 10:30 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33431: Make C4::Circulation use C4::Context->yaml_preference (6.20 KB, patch)
2024-02-23 15:02 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33431: Make C4::Record use C4::Context->yaml_preference (1.55 KB, patch)
2024-02-23 15:02 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33431: Fix remaining cases (4.27 KB, patch)
2024-02-23 15:02 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2023-04-06 13:41:47 UTC

    
Comment 1 Tomás Cohen Arazi 2023-04-06 14:45:13 UTC
Created attachment 149236 [details] [review]
Bug 33431: Make C4::Circulation use C4::Context->yaml_preference

This patch removes manual YAML handling for sysprefs in C4::Circulation.

It also makes C4::Context->yaml_preference not warn when undef is
retrieved from the sysprefs.

To test:
1. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Circulation*
=> SUCCESS: Tests pass!
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests pass!
4. Sign off :-D
Comment 2 Tomás Cohen Arazi 2023-04-06 14:45:16 UTC
Created attachment 149237 [details] [review]
Bug 33431: Make C4::Record use C4::Context->yaml_preference

This patch makes what the title says.

To test:
1. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Rec*
=> SUCCESS: Tests pass
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests pass!
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 Tomás Cohen Arazi 2023-04-06 14:45:19 UTC
Created attachment 149238 [details] [review]
Bug 33431: Fix remaining cases

This patch tweaks three remaining cases, that are not covered by tests.

To test:
1. Apply this patch
2. Make use of those places
=> SUCCESS: No behavior change
Comment 4 David Nind 2023-04-07 07:02:34 UTC
The tests for the first two patches pass, both before and after the patches are applied.

I'm not sure for the third patch what features/things to test to make sure they work as expected.
Comment 5 Tomás Cohen Arazi 2023-12-22 15:27:22 UTC
Created attachment 160245 [details] [review]
Bug 33431: Make C4::Circulation use C4::Context->yaml_preference

This patch removes manual YAML handling for sysprefs in C4::Circulation.

It also makes C4::Context->yaml_preference not warn when undef is
retrieved from the sysprefs.

To test:
1. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Circulation*
=> SUCCESS: Tests pass!
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests pass!
4. Sign off :-D
Comment 6 Tomás Cohen Arazi 2023-12-22 15:27:25 UTC
Created attachment 160246 [details] [review]
Bug 33431: Make C4::Record use C4::Context->yaml_preference

This patch makes what the title says.

To test:
1. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Rec*
=> SUCCESS: Tests pass
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests pass!
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 7 Tomás Cohen Arazi 2023-12-22 15:27:28 UTC
Created attachment 160247 [details] [review]
Bug 33431: Fix remaining cases

This patch tweaks three remaining cases, that are not covered by tests.

To test:
1. Apply this patch
2. Make use of those places
=> SUCCESS: No behavior change
Comment 8 Tomás Cohen Arazi 2023-12-22 15:34:14 UTC
(In reply to David Nind from comment #4)
> The tests for the first two patches pass, both before and after the patches
> are applied.
> 
> I'm not sure for the third patch what features/things to test to make sure
> they work as expected.

Hi, this changes are pretty trivial code-wise.

For the third patch you would need to:

* Have something set on `RisExportAdditionalFields` and export a record as RIS- It should still work as usual.
* Have something set on `MarcFieldsToOrder` and add an order using an ISO2709 (mrc) file. Should work as usual
* Have something set for `UpdateItemWhenLostFromHoldList` and a couple pending holds. I'm not sure how to trigger it but it seems to be possible to mark a hold as lost from the pending list, and it should honor the setting.
Comment 9 David Nind 2023-12-23 08:45:38 UTC
Created attachment 160281 [details] [review]
Bug 33431: Make C4::Circulation use C4::Context->yaml_preference

This patch removes manual YAML handling for sysprefs in C4::Circulation.

It also makes C4::Context->yaml_preference not warn when undef is
retrieved from the sysprefs.

To test:
1. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Circulation*
=> SUCCESS: Tests pass!
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests pass!
4. Sign off :-D

Signed-off-by: David Nind <david@davidnind.com>
Comment 10 David Nind 2023-12-23 08:45:40 UTC
Created attachment 160282 [details] [review]
Bug 33431: Make C4::Record use C4::Context->yaml_preference

This patch makes what the title says.

To test:
1. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Rec*
=> SUCCESS: Tests pass
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests pass!
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Comment 11 David Nind 2023-12-23 08:45:44 UTC
Created attachment 160283 [details] [review]
Bug 33431: Fix remaining cases

This patch tweaks three remaining cases, that are not covered by tests.

To test:
1. Apply this patch
2. Make use of those places
=> SUCCESS: No behavior change

Signed-off-by: David Nind <david@davidnind.com>
Comment 12 David Nind 2023-12-23 08:52:59 UTC
Testing notes (using KTD) - for patch 3 (see comment #8):

1. Add LC: 010$a to RisExportAdditionalFields and export a record as RIS. It should still work as usual and be the same before and after the patch. I used record 262 (Perl programming).

2. Have something set on `MarcFieldsToOrder` and add an order using an ISO2709 (mrc) file. Should work as usual.
   2.1 I used the steps from comment 22 and sample files for bug 34645

3. Have something set for `UpdateItemWhenLostFromHoldList` and a couple pending holds. See this ByWater Solutions tutorial "Alert Patrons When Hold is Cancelled"  https://bywatersolutions.com/education/monday-minutes-alert-patrons-when-hold-is-cancelled
Comment 13 Pedro Amorim 2024-01-02 10:30:49 UTC
Created attachment 160392 [details] [review]
Bug 33431: Make C4::Circulation use C4::Context->yaml_preference

This patch removes manual YAML handling for sysprefs in C4::Circulation.

It also makes C4::Context->yaml_preference not warn when undef is
retrieved from the sysprefs.

To test:
1. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Circulation*
=> SUCCESS: Tests pass!
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests pass!
4. Sign off :-D

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 14 Pedro Amorim 2024-01-02 10:30:51 UTC
Created attachment 160393 [details] [review]
Bug 33431: Make C4::Record use C4::Context->yaml_preference

This patch makes what the title says.

To test:
1. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Rec*
=> SUCCESS: Tests pass
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests pass!
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 15 Pedro Amorim 2024-01-02 10:30:54 UTC
Created attachment 160394 [details] [review]
Bug 33431: Fix remaining cases

This patch tweaks three remaining cases, that are not covered by tests.

To test:
1. Apply this patch
2. Make use of those places
=> SUCCESS: No behavior change

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 16 Katrin Fischer 2024-01-31 13:09:53 UTC
I am sorry, this no longer applies. I'll put it on my list to check and push with higher priority:


Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 33431: Make C4::Circulation use C4::Context->yaml_preference
Using index info to reconstruct a base tree...
M	C4/Circulation.pm
Falling back to patching base and 3-way merge...
Auto-merging C4/Circulation.pm
CONFLICT (content): Merge conflict in C4/Circulation.pm
error: Failed to merge in the changes.
Patch failed at 0001 Bug 33431: Make C4::Circulation use C4::Context->yaml_preference
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-33431-Make-C4Circulation-use-C4Context-yamlpre-issx6rg8.patch
Comment 17 Tomás Cohen Arazi 2024-02-23 14:39:27 UTC
(In reply to Katrin Fischer from comment #16)
> I am sorry, this no longer applies. I'll put it on my list to check and push
> with higher priority:

Caused by bug 35293.
Comment 18 Tomás Cohen Arazi 2024-02-23 15:02:40 UTC
Created attachment 162381 [details] [review]
Bug 33431: Make C4::Circulation use C4::Context->yaml_preference

This patch removes manual YAML handling for sysprefs in C4::Circulation.

It also makes C4::Context->yaml_preference not warn when undef is
retrieved from the sysprefs.

To test:
1. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Circulation*
=> SUCCESS: Tests pass!
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests pass!
4. Sign off :-D

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 19 Tomás Cohen Arazi 2024-02-23 15:02:44 UTC
Created attachment 162382 [details] [review]
Bug 33431: Make C4::Record use C4::Context->yaml_preference

This patch makes what the title says.

To test:
1. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Rec*
=> SUCCESS: Tests pass
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests pass!
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 20 Tomás Cohen Arazi 2024-02-23 15:02:48 UTC
Created attachment 162384 [details] [review]
Bug 33431: Fix remaining cases

This patch tweaks three remaining cases, that are not covered by tests.

To test:
1. Apply this patch
2. Make use of those places
=> SUCCESS: No behavior change

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 21 Katrin Fischer 2024-03-18 09:05:07 UTC
Nice code clean-up!
Comment 22 Katrin Fischer 2024-03-18 10:05:13 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 23 Fridolin Somers 2024-03-21 09:42:41 UTC
Enhancement not pushed to 23.11.x