Bug 17741 - Use Koha::Patron->holds in Koha::Patron->delete
Summary: Use Koha::Patron->holds in Koha::Patron->delete
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 17740
Blocks:
  Show dependency tree / graph
 
Reported: 2016-12-07 16:33 UTC by Jonathan Druart
Modified: 2017-12-07 22:17 UTC (History)
3 users (show)

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

Attachments
Bug 17741: Use Koha::Patron->holds when deleting a patron (1.14 KB, patch)
2016-12-07 16:35 UTC, Jonathan Druart 		Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17741: Use Koha::Patron->holds when deleting a patron (1.21 KB, patch)
2016-12-07 22:03 UTC, Josef Moravec 		Details | Diff | Splinter Review
Bug 17741: Use Koha::Patron->holds when deleting a patron (1.29 KB, patch)
2017-01-06 09:20 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2016-12-07 16:33:39 UTC 
There is a TODO in Koha::Patron->delete, this method should call the Koha::Patron->holds to retrieve the holds made by the patron we are removing instead of calling Koha::Holds->search
Comment 1 Jonathan Druart 2016-12-07 16:35:21 UTC 
Created attachment 58022 [details] [review]
Bug 17741: Use Koha::Patron->holds when deleting a patron

There is a TODO in Koha::Patron->delete, this method should call the
Koha::Patron->holds to retrieve the holds made by the patron we are
removing instead of calling Koha::Holds->search

Test plan:
  prove t/db_dependent/Koha/Patrons.t
Should return green
Comment 2 Josef Moravec 2016-12-07 22:03:09 UTC 
Created attachment 58034 [details] [review]
[SIGNED-OFF] Bug 17741: Use Koha::Patron->holds when deleting a patron

There is a TODO in Koha::Patron->delete, this method should call the
Koha::Patron->holds to retrieve the holds made by the patron we are
removing instead of calling Koha::Holds->search

Test plan:
  prove t/db_dependent/Koha/Patrons.t
Should return green

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 3 Chris Kirby 2016-12-08 18:33:15 UTC 
I tested this too, but I get "Compilation failed in require at Koha/ArticleRequest.pm" and "Compilation failed in require at Koha/Patron.pm" errors. And the tests fail.
Comment 4 Chris Kirby 2016-12-09 17:52:21 UTC 
I have updated my packages. Now I get "Parse errors: Bad plan. You planned 15 tests but ran 9." "Failed test 'No tests run for subtest "is_going_to_expire"'
The method is_going_to_expire_expire is not covered by tests!"
Comment 5 Chris Kirby 2016-12-09 17:56:37 UTC 
Within the staff client, when I delete a patron with a hold, I get a "The method holds is not covered by tests!" error message.
Comment 6 Jonathan Druart 2016-12-12 10:02:26 UTC 
Everything looks ok for me, please apply the dependencies first.
Comment 7 Marcel de Rooy 2017-01-06 09:20:43 UTC 
Created attachment 58627 [details] [review]
Bug 17741: Use Koha::Patron->holds when deleting a patron

There is a TODO in Koha::Patron->delete, this method should call the
Koha::Patron->holds to retrieve the holds made by the patron we are
removing instead of calling Koha::Holds->search

Test plan:
  prove t/db_dependent/Koha/Patrons.t
Should return green

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 8 Kyle M Hall (khall) 2017-01-13 11:40:16 UTC 
Pushed to master for 17.05, thanks Jonathan!
Comment 9 Katrin Fischer 2017-01-15 17:24:19 UTC 
This won't get ported back to 16.11.x as it is an enhancement.