Bug 16891

Summary: Move MoveMemberToDeleted to Koha::Patron->move_to_deleted
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: kyle, mtj, nick
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 16890    
Bug Blocks: 16846, 16907    
Attachments: Bug 16891: Move C4::Members::MoveMemberToDeleted to Koha::Patron->move_to_deleted
Bug 16891: Move C4::Members::MoveMemberToDeleted to Koha::Patron->move_to_deleted
Bug 16891: Move C4::Members::MoveMemberToDeleted to Koha::Patron->move_to_deleted
Bug 16891: Move C4::Members::MoveMemberToDeleted to Koha::Patron->move_to_deleted
[PASSED QA] Bug 16891: Move C4::Members::MoveMemberToDeleted to Koha::Patron->move_to_deleted
Bug 16891: Move C4::Members::MoveMemberToDeleted to Koha::Patron->move_to_deleted

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 (kidclamp) 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