[Bug 31457] New: Move Koha::Patron::Debarment into Koha::Object based classes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31457 Bug ID: 31457 Summary: Move Koha::Patron::Debarment into Koha::Object based classes Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: martin.renvoize@ptfs-europe.com QA Contact: testopia@bugs.koha-community.org Collector bug for removing the Koha::Patron::Debarment which is written in C4 style and replacing it with the new Koha::Object based class pair, Koha::Patron::Restriction(s). -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31457 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |31095 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31095 [Bug 31095] Move Koha::Patron::Debarment into Koha::Object based classes -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31457 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|31095 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31095 [Bug 31095] Remove Koha::Patron::Debarment::GetDebarments and use $patron->restrictions in preference -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31457 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |31095 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31095 [Bug 31095] Remove Koha::Patron::Debarment::GetDebarments and use $patron->restrictions in preference -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31457 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |31458 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31458 [Bug 31458] Remove Koha::Patron::Debarment::AddDebarment and use $patron->add_restriction in preference -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31457 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|31095 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31095 [Bug 31095] Remove Koha::Patron::Debarment::GetDebarments and use $patron->restrictions in preference -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31457 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com --- Comment #1 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- *** Bug 19903 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31457 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | Status|NEW |ASSIGNED -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31457 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff Patch complexity|--- |Medium patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31457 --- Comment #2 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205245 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205245&action=edit Bug 31457: Add OO restriction methods to Koha::Patron::Restriction(s) and Koha::Patron This patch adds an object-oriented API for managing patron restrictions, replacing the procedural Koha::Patron::Debarments functions. The old module is kept as a thin compatibility shim delegating to the new API so existing callers continue to work with no behavior change. Changes: - Koha::Patron::Restriction: store() override (sets manager_id, updates debarred/debarredcomment quick-flags, logs CREATE/MODIFY_RESTRICTION); delete() override (logs DELETE_RESTRICTION, updates flags); patron() accessor - Koha::Patron::Restrictions: filter_by_unique(), lift_after_payment() - Koha::Patron::Restriction::Types: UNIQUE_TYPES constant (OVERDUES, SUSPENSION, FINES), filter_by_unique() - Koha::Patron: add_restriction(), add_or_update_restriction() (upsert keyed on type, never shortens expiration), _update_debarred_flags() (raw SQL to avoid Koha::Patron->store side effects) - Koha::Patron::Debarments: rewritten as deprecated shim delegating all exported functions to the OO API Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Patron/Restriction.t \ t/db_dependent/Koha/Patron/Restrictions.t \ t/db_dependent/Koha/Patron/Restriction/Types.t \ t/db_dependent/Koha/Patron.t \ t/db_dependent/Patron/Borrower_Debarments.t => SUCCESS: All tests pass! 3. The existing Borrower_Debarments.t (40 tests) proves the shim preserves exact behavior through the new OO layer 4. New tests cover: store (create/modify/flags/logging), delete (remove/flags/logging), patron(), filter_by_unique, UNIQUE_TYPES, lift_after_payment, add_restriction, add_or_update_restriction 5. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31457 --- Comment #3 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205246 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205246&action=edit Bug 31457: Use the OO restriction API in all callers This patch migrates all in-tree callers of the procedural Koha::Patron::Debarments functions to the new object-oriented API on Koha::Patron and Koha::Patron::Restriction(s). Changes: - C4::Circulation: SUSPENSION now uses $patron->add_or_update_restriction; the two OVERDUES removals reuse the $overdue_restrictions resultset already in scope ($overdue_restrictions->delete) - Koha::Account::Line: $self->patron->restrictions->lift_after_payment - Koha::Patron::merge_with: $self->_update_debarred_flags (and dropped the now-unneeded use Koha::Patron::Debarments) - Koha::Patrons::Import, Koha::Patron::Discharge, Koha::Notice::Message: use $patron->add_restriction - members/mod_debarment.pl, members/memberentry.pl, tools/modborrowers.pl: use $patron->add_restriction and $restriction->delete - misc/cronjobs/overdue_notices.pl, debar_patrons_with_fines.pl: use $patron->add_or_update_restriction - misc/cronjobs/cleanup_database.pl: Koha::Patron::Restrictions->find->delete (only a restriction id is available there) - Migrated two incidental test calls that relied on Koha::Patron transitively loading Koha::Patron::Debarments (Restriction/Type.t and Borrower_Discharge.t) The convention followed is to go through $patron->restrictions->search/find (or a resultset already in scope), only using the class-level Koha::Patron::Restrictions->find where just an id is available. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Patron/Borrower_Debarments.t \ t/db_dependent/Circulation.t \ t/db_dependent/Circulation/MarkIssueReturned.t \ t/db_dependent/Circulation/maxsuspensiondays.t \ t/db_dependent/Koha/Patron.t \ t/db_dependent/Koha/Patron/Restriction/Type.t \ t/db_dependent/Koha/Account/Lines.t \ t/db_dependent/Members.t \ t/db_dependent/Koha/Notices.t => SUCCESS: All tests pass! 3. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31457 --- Comment #4 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205247 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205247&action=edit Bug 31457: Remove the deprecated Koha::Patron::Debarments shim Now that all callers use the object-oriented restriction API, this patch removes the Koha::Patron::Debarments compatibility shim and migrates the last remaining (incidental) callers, which were test setups. Changes: - Remove Koha/Patron/Debarments.pm - Remove t/db_dependent/Patron/Borrower_Debarments.t (its coverage now lives in Restriction.t, Restrictions.t, Restriction/Types.t and Koha/Patron.t) - Migrate incidental test callers to the OO API: Circulation.t, Circulation/MarkIssueReturned.t, Circulation/maxsuspensiondays.t, Members.t, Koha/Patron.t, api/v1/patrons.t, api/v1/holds.t - misc/cronjobs/cleanup_database.pl: require Koha::Patron::Restrictions (the class actually used) instead of the removed module - Koha::Patron::Restriction::Type: refresh an outdated comment (the raw SQL bulk type reassignment is kept on purpose to avoid per-row store triggers) Test plan: 1. Apply patch 2. Confirm nothing loads the removed module: $ git grep "Koha::Patron::Debarments" -- ":!misc/release_notes" => SUCCESS: no matches 3. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Patron/Restriction.t \ t/db_dependent/Koha/Patron/Restrictions.t \ t/db_dependent/Koha/Patron/Restriction/Type.t \ t/db_dependent/Koha/Patron/Restriction/Types.t \ t/db_dependent/Koha/Patron.t \ t/db_dependent/Circulation.t \ t/db_dependent/Circulation/MarkIssueReturned.t \ t/db_dependent/Circulation/maxsuspensiondays.t \ t/db_dependent/Members.t \ t/db_dependent/api/v1/patrons.t \ t/db_dependent/api/v1/holds.t => SUCCESS: All tests pass! 4. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31457 Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |baptiste.wojtkowski@biblibr | |e.com --- Comment #5 from Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> --- Looks like the two last tests are broken on main by Bug 39658, thus I tested on yesterday's main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31457 Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #205245|0 |1 is obsolete| | Attachment #205246|0 |1 is obsolete| | Attachment #205247|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31457 --- Comment #6 from Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> --- Created attachment 205384 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205384&action=edit Bug 31457: Add OO restriction methods to Koha::Patron::Restriction(s) and Koha::Patron This patch adds an object-oriented API for managing patron restrictions, replacing the procedural Koha::Patron::Debarments functions. The old module is kept as a thin compatibility shim delegating to the new API so existing callers continue to work with no behavior change. Changes: - Koha::Patron::Restriction: store() override (sets manager_id, updates debarred/debarredcomment quick-flags, logs CREATE/MODIFY_RESTRICTION); delete() override (logs DELETE_RESTRICTION, updates flags); patron() accessor - Koha::Patron::Restrictions: filter_by_unique(), lift_after_payment() - Koha::Patron::Restriction::Types: UNIQUE_TYPES constant (OVERDUES, SUSPENSION, FINES), filter_by_unique() - Koha::Patron: add_restriction(), add_or_update_restriction() (upsert keyed on type, never shortens expiration), _update_debarred_flags() (raw SQL to avoid Koha::Patron->store side effects) - Koha::Patron::Debarments: rewritten as deprecated shim delegating all exported functions to the OO API Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Patron/Restriction.t \ t/db_dependent/Koha/Patron/Restrictions.t \ t/db_dependent/Koha/Patron/Restriction/Types.t \ t/db_dependent/Koha/Patron.t \ t/db_dependent/Patron/Borrower_Debarments.t => SUCCESS: All tests pass! 3. The existing Borrower_Debarments.t (40 tests) proves the shim preserves exact behavior through the new OO layer 4. New tests cover: store (create/modify/flags/logging), delete (remove/flags/logging), patron(), filter_by_unique, UNIQUE_TYPES, lift_after_payment, add_restriction, add_or_update_restriction 5. Sign off :-D Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31457 --- Comment #7 from Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> --- Created attachment 205385 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205385&action=edit Bug 31457: Use the OO restriction API in all callers This patch migrates all in-tree callers of the procedural Koha::Patron::Debarments functions to the new object-oriented API on Koha::Patron and Koha::Patron::Restriction(s). Changes: - C4::Circulation: SUSPENSION now uses $patron->add_or_update_restriction; the two OVERDUES removals reuse the $overdue_restrictions resultset already in scope ($overdue_restrictions->delete) - Koha::Account::Line: $self->patron->restrictions->lift_after_payment - Koha::Patron::merge_with: $self->_update_debarred_flags (and dropped the now-unneeded use Koha::Patron::Debarments) - Koha::Patrons::Import, Koha::Patron::Discharge, Koha::Notice::Message: use $patron->add_restriction - members/mod_debarment.pl, members/memberentry.pl, tools/modborrowers.pl: use $patron->add_restriction and $restriction->delete - misc/cronjobs/overdue_notices.pl, debar_patrons_with_fines.pl: use $patron->add_or_update_restriction - misc/cronjobs/cleanup_database.pl: Koha::Patron::Restrictions->find->delete (only a restriction id is available there) - Migrated two incidental test calls that relied on Koha::Patron transitively loading Koha::Patron::Debarments (Restriction/Type.t and Borrower_Discharge.t) The convention followed is to go through $patron->restrictions->search/find (or a resultset already in scope), only using the class-level Koha::Patron::Restrictions->find where just an id is available. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Patron/Borrower_Debarments.t \ t/db_dependent/Circulation.t \ t/db_dependent/Circulation/MarkIssueReturned.t \ t/db_dependent/Circulation/maxsuspensiondays.t \ t/db_dependent/Koha/Patron.t \ t/db_dependent/Koha/Patron/Restriction/Type.t \ t/db_dependent/Koha/Account/Lines.t \ t/db_dependent/Members.t \ t/db_dependent/Koha/Notices.t => SUCCESS: All tests pass! 3. Sign off :-D Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31457 --- Comment #8 from Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> --- Created attachment 205386 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205386&action=edit Bug 31457: Remove the deprecated Koha::Patron::Debarments shim Now that all callers use the object-oriented restriction API, this patch removes the Koha::Patron::Debarments compatibility shim and migrates the last remaining (incidental) callers, which were test setups. Changes: - Remove Koha/Patron/Debarments.pm - Remove t/db_dependent/Patron/Borrower_Debarments.t (its coverage now lives in Restriction.t, Restrictions.t, Restriction/Types.t and Koha/Patron.t) - Migrate incidental test callers to the OO API: Circulation.t, Circulation/MarkIssueReturned.t, Circulation/maxsuspensiondays.t, Members.t, Koha/Patron.t, api/v1/patrons.t, api/v1/holds.t - misc/cronjobs/cleanup_database.pl: require Koha::Patron::Restrictions (the class actually used) instead of the removed module - Koha::Patron::Restriction::Type: refresh an outdated comment (the raw SQL bulk type reassignment is kept on purpose to avoid per-row store triggers) Test plan: 1. Apply patch 2. Confirm nothing loads the removed module: $ git grep "Koha::Patron::Debarments" -- ":!misc/release_notes" => SUCCESS: no matches 3. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Patron/Restriction.t \ t/db_dependent/Koha/Patron/Restrictions.t \ t/db_dependent/Koha/Patron/Restriction/Type.t \ t/db_dependent/Koha/Patron/Restriction/Types.t \ t/db_dependent/Koha/Patron.t \ t/db_dependent/Circulation.t \ t/db_dependent/Circulation/MarkIssueReturned.t \ t/db_dependent/Circulation/maxsuspensiondays.t \ t/db_dependent/Members.t \ t/db_dependent/api/v1/patrons.t \ t/db_dependent/api/v1/holds.t => SUCCESS: All tests pass! 4. Sign off :-D Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31457 Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org