[Bug 16819] New: C4::Members::DelMember should use Koha::Holds to delete holds
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16819 Bug ID: 16819 Summary: C4::Members::DelMember should use Koha::Holds to delete holds Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: gmcharlt@gmail.com Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16819 --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 52929 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=52929&action=edit Bug 16819: Unit tests for C4::Members::DelMember The C4::Members::DelMember function actually deletes reserves, and it is not properly tested. To test: - Apply the patch - Run: $ prove t/db_dependent/Members.t => SUCCESS: Tests pass - Sign off Sponsored-by: NEKLS -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16819 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 52930 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=52930&action=edit Bug 16819: C4::Members::DelMember should use Koha::Holds to delete holds This patch makes C4::Members::DelMember proprely use Koha::Holds to delete holds. This is important as holds actions are started to be logged. To reproduce: - Apply the patch - Run: $ prove t/db_dependent/Members.t => SUCCESS: Tests pass - Sign off :-D Sponsored-by: NEKLS -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16819 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|gmcharlt@gmail.com |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16819 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16819 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=14642 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16819 Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #52929|0 |1 is obsolete| | --- Comment #3 from Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> --- Created attachment 53030 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53030&action=edit [SIGNED-OFF]Bug 16819: Unit tests for C4::Members::DelMember The C4::Members::DelMember function actually deletes reserves, and it is not properly tested. To test: - Apply the patch - Run: $ prove t/db_dependent/Members.t => SUCCESS: Tests pass - Sign off Sponsored-by: NEKLS Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16819 Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #52930|0 |1 is obsolete| | --- Comment #4 from Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> --- Created attachment 53031 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53031&action=edit [SIGNED-OFF]Bug 16819: C4::Members::DelMember should use Koha::Holds to delete holds This patch makes C4::Members::DelMember proprely use Koha::Holds to delete holds. This is important as holds actions are started to be logged. To reproduce: - Apply the patch - Run: $ prove t/db_dependent/Members.t => SUCCESS: Tests pass - Sign off :-D Sponsored-by: NEKLS Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> All tests pass successfully -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16819 Héctor Eduardo Castro Avalos <hector.hecaxmmx@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hector.hecaxmmx@gmail.com Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16819 --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Comment on attachment 53031 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53031 [SIGNED-OFF]Bug 16819: C4::Members::DelMember should use Koha::Holds to delete holds Review of attachment 53031: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=16819&attachment=53031) ----------------------------------------------------------------- ::: C4/Members.pm @@ +1662,5 @@
#warn "in delmember with $borrowernumber"; return unless $borrowernumber; # borrowernumber is mandatory. + # Delete Patron's holds + my @holds = Koha::Holds->search({ borrowernumber => $borrowernumber }); + map { $_->delete } @holds;
I'd have written that `$_->delete for @holds`. Since map is supposed to return something I guess it can perturb a new developer. Do you agree? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16819 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16819 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #53030|0 |1 is obsolete| | Attachment #53031|0 |1 is obsolete| | --- Comment #6 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 53054 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53054&action=edit Bug 16819: Unit tests for C4::Members::DelMember The C4::Members::DelMember function actually deletes reserves, and it is not properly tested. To test: - Apply the patch - Run: $ prove t/db_dependent/Members.t => SUCCESS: Tests pass - Sign off Sponsored-by: NEKLS Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16819 --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 53055 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53055&action=edit Bug 16819: C4::Members::DelMember should use Koha::Holds to delete holds This patch makes C4::Members::DelMember proprely use Koha::Holds to delete holds. This is important as holds actions are started to be logged. To reproduce: - Apply the patch - Run: $ prove t/db_dependent/Members.t => SUCCESS: Tests pass - Sign off :-D Sponsored-by: NEKLS Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> All tests pass successfully Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16819 --- Comment #8 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 53056 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53056&action=edit Bug 16819: Remove use of map Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16819 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #9 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Jonathan Druart from comment #8)
Created attachment 53056 [details] [review] Bug 16819: Remove use of map
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Tomas or RM: feel free to obsolete this patch if you want to. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16819 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |14642 See Also|https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=14642 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14642 [Bug 14642] Add logging of hold modifications -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16819 --- Comment #10 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #9)
(In reply to Jonathan Druart from comment #8)
Created attachment 53056 [details] [review] [review] Bug 16819: Remove use of map
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Tomas or RM: feel free to obsolete this patch if you want to.
I like either, passing the ball to the RM :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16819 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |veron@veron.ch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16819 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |kyle@bywatersolutions.com --- Comment #11 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to master for 16.11, thanks Tomas, Jonathan! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16819 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable CC| |frederic@tamil.fr --- Comment #12 from Frédéric Demians <frederic@tamil.fr> --- Pushed in 16.05. Will be in 16.05.02. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org