Bug 36526 - Remove circular dependency from Koha::Objects
Summary: Remove circular dependency from Koha::Objects
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Julian Maurice
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-04-04 15:52 UTC by Julian Maurice
Modified: 2024-05-28 21:27 UTC (History)
3 users (show)

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


Attachments
Bug 36526: Remove circular dependency from Koha::Objects (7.97 KB, patch)
2024-04-04 15:53 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 36526: Remove circular dependency from Koha::Objects (7.97 KB, patch)
2024-04-04 15:56 UTC, Julian Maurice
Details | Diff | Splinter Review
Signed-off-by: Tadeusz „tadzik” Sośnierz <tadeusz@sosnierz.com> (8.04 KB, patch)
2024-05-03 09:35 UTC, HKS3 Tadeusz Sośnierz
Details | Diff | Splinter Review
Bug 36526: Remove circular dependency from Koha::Objects (8.10 KB, patch)
2024-05-03 10:59 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Maurice 2024-04-04 15:52:24 UTC
Similar to bug 36432

Koha::Objects depends on Koha::DateUtils, which depends on C4::Context, which depends on Koha::Config::SysPrefs, which depends on Koha::Objects
Comment 1 Julian Maurice 2024-04-04 15:53:56 UTC
Created attachment 164431 [details] [review]
Bug 36526: Remove circular dependency from Koha::Objects

Koha::Objects depends on Koha::DateUtils, which depends on C4::Context,
which depends on Koha::Config::SysPrefs, which depends on Koha::Objects

Apart from the circular dependency, the dependency on C4::Context alone
is problematic as it loads a bunch of modules that are not needed at all
in Koha::Objects (YAML::XS and ZOOM for instance).
As Koha::Objects is used as a base for a lot of modules, we should take
care to only load the minimum required.

This patch removes uses of Koha::DateUtils from Koha::Objects.
It was only used in Koha::Objects::filter_by_last_update

filter_by_last_update now requires that the 'from' and 'to' parameters
must be DateTime objects. Previously it would also allow date and
datetime strings. This possibility was only used in two places:
* misc/cronjobs/cleanup_database.pl
* tools/cleanborrowers.pl

Now they call dt_from_string first and pass a DateTime object to
filter_by_last_update

Test plan:
1. Run `perl -cw Koha/Objects.pm`. It should only say:
   "Koha/Objects.pm syntax OK" without warnings
2. Run `prove t/db_dependent/Koha/Objects.t`
3. Verify that misc/cronjobs/cleanup_database.pl works as before,
   especially with the options --pseudo-transactions,
   --pseudo-transactions-from and --pseudo-transactions-to
4. Go to Tools » Batch patron deletion and anonymization, check "Verify
   you want to anonymize patron checkout history" and enter a date in
   the text input below. Then click Next and verify that the correct
   count of borrowers is shown. Click on the "Finish" button and verify
   that the circulation history has been correctly anonymized

See also bug 36432
Comment 2 Julian Maurice 2024-04-04 15:56:36 UTC
Created attachment 164432 [details] [review]
Bug 36526: Remove circular dependency from Koha::Objects

Koha::Objects depends on Koha::DateUtils, which depends on C4::Context,
which depends on Koha::Config::SysPrefs, which depends on Koha::Objects

Apart from the circular dependency, the dependency on C4::Context alone
is problematic as it loads a bunch of modules that are not needed at all
in Koha::Objects (YAML::XS and ZOOM for instance).
As Koha::Objects is used as a base for a lot of modules, we should take
care to only load the minimum required.

This patch removes uses of Koha::DateUtils from Koha::Objects.
It was only used in Koha::Objects::filter_by_last_update

filter_by_last_update now requires that the 'from' and 'to' parameters
must be DateTime objects. Previously it would also allow date and
datetime strings. This possibility was only used in two places:
* misc/cronjobs/cleanup_database.pl
* tools/cleanborrowers.pl

Now they call dt_from_string first and pass a DateTime object to
filter_by_last_update

Test plan:
1. Run `perl -cw Koha/Objects.pm`. It should only say:
   "Koha/Objects.pm syntax OK" without warnings
2. Run `prove t/db_dependent/Koha/Objects.t`
3. Verify that misc/cronjobs/cleanup_database.pl works as before,
   especially with the options --pseudo-transactions,
   --pseudo-transactions-from and --pseudo-transactions-to
4. Go to Tools » Batch patron deletion and anonymization, check "Verify
   you want to anonymize patron checkout history" and enter a date in
   the text input below. Then click Next and verify that the correct
   count of borrowers is shown. Click on the "Finish" button and verify
   that the circulation history has been correctly anonymized

See also bug 36432
Comment 3 HKS3 Tadeusz Sośnierz 2024-05-03 09:35:32 UTC
Created attachment 166104 [details] [review]
Signed-off-by: Tadeusz „tadzik” Sośnierz <tadeusz@sosnierz.com>
Comment 4 Martin Renvoize (ashimema) 2024-05-03 10:59:07 UTC
Created attachment 166108 [details] [review]
Bug 36526: Remove circular dependency from Koha::Objects

Koha::Objects depends on Koha::DateUtils, which depends on C4::Context,
which depends on Koha::Config::SysPrefs, which depends on Koha::Objects

Apart from the circular dependency, the dependency on C4::Context alone
is problematic as it loads a bunch of modules that are not needed at all
in Koha::Objects (YAML::XS and ZOOM for instance).
As Koha::Objects is used as a base for a lot of modules, we should take
care to only load the minimum required.

This patch removes uses of Koha::DateUtils from Koha::Objects.
It was only used in Koha::Objects::filter_by_last_update

filter_by_last_update now requires that the 'from' and 'to' parameters
must be DateTime objects. Previously it would also allow date and
datetime strings. This possibility was only used in two places:
* misc/cronjobs/cleanup_database.pl
* tools/cleanborrowers.pl

Now they call dt_from_string first and pass a DateTime object to
filter_by_last_update

Test plan:
1. Run `perl -cw Koha/Objects.pm`. It should only say:
   "Koha/Objects.pm syntax OK" without warnings
2. Run `prove t/db_dependent/Koha/Objects.t`
3. Verify that misc/cronjobs/cleanup_database.pl works as before,
   especially with the options --pseudo-transactions,
   --pseudo-transactions-from and --pseudo-transactions-to
4. Go to Tools » Batch patron deletion and anonymization, check "Verify
   you want to anonymize patron checkout history" and enter a date in
   the text input below. Then click Next and verify that the correct
   count of borrowers is shown. Click on the "Finish" button and verify
   that the circulation history has been correctly anonymized

See also bug 36432

Signed-off-by: Tadeusz Sośnierz <tadeusz@sosnierz.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 5 Martin Renvoize (ashimema) 2024-05-03 11:00:11 UTC
Nice improvement here, thanks Julain and Tadeusz.

Passing QA
Comment 6 Katrin Fischer 2024-05-03 14:03:55 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 7 Fridolin Somers 2024-05-24 13:00:26 UTC
Pushed to 23.11.x for 23.11.06
Comment 8 Lucas Gass (lukeg) 2024-05-28 21:27:03 UTC
Doesn't apply cleanly to 23.05.x, no backport.