Bug 16891 - Move MoveMemberToDeleted to Koha::Patron->move_to_deleted
Summary: Move MoveMemberToDeleted to Koha::Patron->move_to_deleted
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 16890
Blocks: 16846 16907
  Show dependency treegraph
 
Reported: 2016-07-09 15:12 UTC by Jonathan Druart
Modified: 2017-12-07 22:21 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 16891: Move C4::Members::MoveMemberToDeleted to Koha::Patron->move_to_deleted (8.94 KB, patch)
2016-07-09 15:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16891: Move C4::Members::MoveMemberToDeleted to Koha::Patron->move_to_deleted (8.99 KB, patch)
2016-07-20 14:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16891: Move C4::Members::MoveMemberToDeleted to Koha::Patron->move_to_deleted (9.18 KB, patch)
2016-08-09 03:10 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 16891: Move C4::Members::MoveMemberToDeleted to Koha::Patron->move_to_deleted (9.00 KB, patch)
2016-10-13 10:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
[PASSED QA] Bug 16891: Move C4::Members::MoveMemberToDeleted to Koha::Patron->move_to_deleted (9.07 KB, patch)
2016-10-13 15:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 16891: Move C4::Members::MoveMemberToDeleted to Koha::Patron->move_to_deleted (9.05 KB, patch)
2016-10-13 15:23 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2016-07-09 15:12:53 UTC

    
Comment 1 Jonathan Druart 2016-07-09 15:22:26 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2016-07-20 14:09:19 UTC Comment hidden (obsolete)
Comment 3 Owen Leonard 2016-08-08 18:16:11 UTC
(In reply to Jonathan Druart from comment #2)
> 3/ Same as previously but using the cronjob
> misc/cronjobs/delete_patrons.pl.

At this step I'm getting "No method move_to_deleted! at misc/cronjobs/delete_patrons.pl line 77."
Comment 4 Tomás Cohen Arazi 2016-08-09 03:10:14 UTC
Created attachment 54178 [details] [review]
Bug 16891: Move C4::Members::MoveMemberToDeleted to Koha::Patron->move_to_deleted

This patch removes the C4::Members::MoveMemberToDeleted subroutine in
order to replace it with the Koha::Patron->move_to_deleted method.
Next after this change, we will move C4::Members::HandleDelBorrower and
C4::Members::DelMember to the same module to simplify the code in
members/deletemem.pl and misc/cronjobs/delete_patrons.pl

Test plan:
1/ Delete a patron from the staff interface and make sure (s)he has been moved to
the deletedborrowers table.
2/ Use the "Batch patron deletion" tool (tools/cleanborrowers.pl) to
remove patron. Make sure the "Permanently delete these patrons" and "Move
these patrons to the trash" options work as before
3/ Same as previously but using the cronjob
misc/cronjobs/delete_patrons.pl.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Tested the delete_patrons.pl script and cleanborrowers.pl too.
Tests (are relevant and) pass and the qa scripts are happy too :-D
Comment 5 Jonathan Druart 2016-08-09 08:45:46 UTC
(In reply to Owen Leonard from comment #3)
> (In reply to Jonathan Druart from comment #2)
> > 3/ Same as previously but using the cronjob
> > misc/cronjobs/delete_patrons.pl.
> 
> At this step I'm getting "No method move_to_deleted! at
> misc/cronjobs/delete_patrons.pl line 77."

The patch has been signed off in the meanwhile.
Could you confirm that it correctly works please?
I have just retested and the cronjob script seems to work as expected.
Comment 6 Owen Leonard 2016-08-09 11:51:13 UTC
(In reply to Jonathan Druart from comment #5)

> Could you confirm that it correctly works please?
> I have just retested and the cronjob script seems to work as expected.

I defer to Tomás on the signoff, since I'm never confident that errors like that aren't a configuration problem with my gitified package installation. However, if there is more I can do to test I would be happy to do so.
Comment 7 Jonathan Druart 2016-10-13 10:24:11 UTC
Created attachment 56339 [details] [review]
Bug 16891: Move C4::Members::MoveMemberToDeleted to Koha::Patron->move_to_deleted

This patch removes the C4::Members::MoveMemberToDeleted subroutine in
order to replace it with the Koha::Patron->move_to_deleted method.
Next after this change, we will move C4::Members::HandleDelBorrower and
C4::Members::DelMember to the same module to simplify the code in
members/deletemem.pl and misc/cronjobs/delete_patrons.pl

Test plan:
1/ Delete a patron from the staff interface and make sure (s)he has been moved to
the deletedborrowers table.
2/ Use the "Batch patron deletion" tool (tools/cleanborrowers.pl) to
remove patron. Make sure the "Permanently delete these patrons" and "Move
these patrons to the trash" options work as before
3/ Same as previously but using the cronjob
misc/cronjobs/delete_patrons.pl.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Tested the delete_patrons.pl script and cleanborrowers.pl too.
Tests (are relevant and) pass and the qa scripts are happy too :-D
Comment 8 Martin Renvoize 2016-10-13 15:14:01 UTC
Created attachment 56415 [details] [review]
[PASSED QA] Bug 16891: Move C4::Members::MoveMemberToDeleted to Koha::Patron->move_to_deleted

This patch removes the C4::Members::MoveMemberToDeleted subroutine in
order to replace it with the Koha::Patron->move_to_deleted method.
Next after this change, we will move C4::Members::HandleDelBorrower and
C4::Members::DelMember to the same module to simplify the code in
members/deletemem.pl and misc/cronjobs/delete_patrons.pl

Test plan:
1/ Delete a patron from the staff interface and make sure (s)he has been moved to
the deletedborrowers table.
2/ Use the "Batch patron deletion" tool (tools/cleanborrowers.pl) to
remove patron. Make sure the "Permanently delete these patrons" and "Move
these patrons to the trash" options work as before
3/ Same as previously but using the cronjob
misc/cronjobs/delete_patrons.pl.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Tested the delete_patrons.pl script and cleanborrowers.pl too.
Tests (are relevant and) pass and the qa scripts are happy too :-D

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Nick Clemens 2016-10-13 15:23:37 UTC
Created attachment 56418 [details] [review]
Bug 16891: Move C4::Members::MoveMemberToDeleted to Koha::Patron->move_to_deleted

This patch removes the C4::Members::MoveMemberToDeleted subroutine in
order to replace it with the Koha::Patron->move_to_deleted method.
Next after this change, we will move C4::Members::HandleDelBorrower and
C4::Members::DelMember to the same module to simplify the code in
members/deletemem.pl and misc/cronjobs/delete_patrons.pl

Test plan:
1/ Delete a patron from the staff interface and make sure (s)he has been moved to
the deletedborrowers table.
2/ Use the "Batch patron deletion" tool (tools/cleanborrowers.pl) to
remove patron. Make sure the "Permanently delete these patrons" and "Move
these patrons to the trash" options work as before
3/ Same as previously but using the cronjob
misc/cronjobs/delete_patrons.pl.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Tested the delete_patrons.pl script and cleanborrowers.pl too.
Tests (are relevant and) pass and the qa scripts are happy too :-D

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 10 Kyle M Hall 2016-10-21 16:59:40 UTC
Pushed to master for 16.11, thanks Jonathan!
Comment 11 Mason James 2017-04-23 22:43:14 UTC
Enhancement, skipping for 16.05.x