Bug 17569 - Move GetUpcomingMembershipExpires to Koha::Patrons
Summary: Move GetUpcomingMembershipExpires to Koha::Patrons
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 17568
Blocks: 16846
  Show dependency treegraph
 
Reported: 2016-11-07 14:41 UTC by Jonathan Druart
Modified: 2017-12-07 22:15 UTC (History)
4 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 17569: Rewrite existing tests to make them reusable and more robust (6.28 KB, patch)
2016-11-07 14:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17569: Move tests to the patron module test file (9.31 KB, patch)
2016-11-07 14:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17569: Koha::Patrons - Move GetUpcomingMembershipExpires to search_upcoming_membership_expires (4.41 KB, patch)
2016-11-07 14:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17569: Remove C4::Members::GetUpcomingMembershipExpires (2.46 KB, patch)
2016-11-08 08:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17569: Do not limit by branch if option is not passed (1.15 KB, patch)
2016-11-08 08:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17569: Rewrite existing tests to make them reusable and more robust (6.35 KB, patch)
2016-11-08 12:38 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17569: Move tests to the patron module test file (9.37 KB, patch)
2016-11-08 12:38 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17569: Koha::Patrons - Move GetUpcomingMembershipExpires to search_upcoming_membership_expires (4.48 KB, patch)
2016-11-08 12:38 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17569: Remove C4::Members::GetUpcomingMembershipExpires (2.53 KB, patch)
2016-11-08 12:38 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17569: Do not limit by branch if option is not passed (1.22 KB, patch)
2016-11-08 12:38 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17569: Rewrite existing tests to make them reusable and more robust (6.33 KB, patch)
2016-12-02 08:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17569: Move tests to the patron module test file (9.36 KB, patch)
2016-12-02 08:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17569: Koha::Patrons - Move GetUpcomingMembershipExpires to search_upcoming_membership_expires (4.47 KB, patch)
2016-12-02 08:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17569: Remove C4::Members::GetUpcomingMembershipExpires (2.52 KB, patch)
2016-12-02 08:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17569: Do not limit by branch if option is not passed (1.20 KB, patch)
2016-12-02 08:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17569: Rewrite existing tests to make them reusable and more robust (6.43 KB, patch)
2016-12-02 09:59 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17569: Move tests to the patron module test file (9.46 KB, patch)
2016-12-02 09:59 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17569: Koha::Patrons - Move GetUpcomingMembershipExpires to search_upcoming_membership_expires (4.56 KB, patch)
2016-12-02 09:59 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17569: Remove C4::Members::GetUpcomingMembershipExpires (2.61 KB, patch)
2016-12-02 09:59 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17569: Do not limit by branch if option is not passed (1.30 KB, patch)
2016-12-02 09:59 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17569: [QA Follow-up] Small changes (2.97 KB, patch)
2016-12-02 09:59 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17569: Rewrite existing tests to make them reusable and more robust (6.39 KB, patch)
2016-12-29 09:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17569: Move tests to the patron module test file (9.39 KB, patch)
2016-12-29 09:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17569: Koha::Patrons - Move GetUpcomingMembershipExpires to search_upcoming_membership_expires (4.52 KB, patch)
2016-12-29 09:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17569: Remove C4::Members::GetUpcomingMembershipExpires (2.59 KB, patch)
2016-12-29 09:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17569: Do not limit by branch if option is not passed (1.26 KB, patch)
2016-12-29 09:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17569: [QA Follow-up] Small changes (2.94 KB, patch)
2016-12-29 09:32 UTC, Jonathan Druart
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-11-07 14:41:05 UTC

    
Comment 1 Jonathan Druart 2016-11-07 14:53:08 UTC
Created attachment 57265 [details] [review]
Bug 17569: Rewrite existing tests to make them reusable and more robust

In order not to cheat you, I am rewriting the tests in a separate
commit.
You can confirm that they pass with the other patches.
Comment 2 Jonathan Druart 2016-11-07 14:53:11 UTC
Created attachment 57266 [details] [review]
Bug 17569: Move tests to the patron module test file
Comment 3 Jonathan Druart 2016-11-07 14:53:14 UTC
Created attachment 57267 [details] [review]
Bug 17569: Koha::Patrons - Move GetUpcomingMembershipExpires to search_upcoming_membership_expires

This patchset moves the C4::Members::GetUpcomingMembershipExpires
subroutine code to the Koha::Patrons->search_upcoming_membership_expires
method.
This subroutine was used from only 1 place, so it's an easier to move.

Test plan:
Use the membership_expiry.pl cronjob script using the different
options.
The behavior should be the same as prior to this patch.

  prove t/db_dependent/Koha/Patrons.t
should return green
Comment 4 Josef Moravec 2016-11-07 20:22:29 UTC
Are you going to remove C4::Members::GetUpcomingMembershipExpires sub? I can't see it in patchset.

Also, the membership_expiry.pl script works fine, when you specify branch, but without branch, it should search for patrons in all branches, but always returns 0.
Comment 5 Jonathan Druart 2016-11-08 08:08:00 UTC
Created attachment 57294 [details] [review]
Bug 17569: Remove C4::Members::GetUpcomingMembershipExpires
Comment 6 Jonathan Druart 2016-11-08 08:08:04 UTC
Created attachment 57295 [details] [review]
Bug 17569: Do not limit by branch if option is not passed
Comment 7 Jonathan Druart 2016-11-08 08:08:34 UTC
Fixed Josef, thanks for testing!
Comment 8 Josef Moravec 2016-11-08 12:38:31 UTC
Created attachment 57315 [details] [review]
[SIGNED-OFF] Bug 17569: Rewrite existing tests to make them reusable and more robust

In order not to cheat you, I am rewriting the tests in a separate
commit.
You can confirm that they pass with the other patches.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 9 Josef Moravec 2016-11-08 12:38:38 UTC
Created attachment 57316 [details] [review]
[SIGNED-OFF] Bug 17569: Move tests to the patron module test file

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 10 Josef Moravec 2016-11-08 12:38:42 UTC
Created attachment 57317 [details] [review]
[SIGNED-OFF] Bug 17569: Koha::Patrons - Move GetUpcomingMembershipExpires to search_upcoming_membership_expires

This patchset moves the C4::Members::GetUpcomingMembershipExpires
subroutine code to the Koha::Patrons->search_upcoming_membership_expires
method.
This subroutine was used from only 1 place, so it's an easier to move.

Test plan:
Use the membership_expiry.pl cronjob script using the different
options.
The behavior should be the same as prior to this patch.

  prove t/db_dependent/Koha/Patrons.t
should return green

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 11 Josef Moravec 2016-11-08 12:38:46 UTC
Created attachment 57318 [details] [review]
[SIGNED-OFF] Bug 17569: Remove C4::Members::GetUpcomingMembershipExpires

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 12 Josef Moravec 2016-11-08 12:38:50 UTC
Created attachment 57319 [details] [review]
[SIGNED-OFF] Bug 17569: Do not limit by branch if option is not passed

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 13 Marcel de Rooy 2016-12-02 07:57:20 UTC
Applied dependency 17568 first.

Applying: Bug 17569: Move tests to the patron module test file
fatal: sha1 information is lacking or useless (t/db_dependent/Koha/Patrons.t).
Repository lacks necessary blobs to fall back on 3-way merge.
Comment 14 Jonathan Druart 2016-12-02 08:36:25 UTC
Created attachment 57894 [details] [review]
Bug 17569: Rewrite existing tests to make them reusable and more robust

In order not to cheat you, I am rewriting the tests in a separate
commit.
You can confirm that they pass with the other patches.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 15 Jonathan Druart 2016-12-02 08:36:33 UTC
Created attachment 57895 [details] [review]
Bug 17569: Move tests to the patron module test file

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 16 Jonathan Druart 2016-12-02 08:36:46 UTC
Created attachment 57896 [details] [review]
Bug 17569: Koha::Patrons - Move GetUpcomingMembershipExpires to search_upcoming_membership_expires

This patchset moves the C4::Members::GetUpcomingMembershipExpires
subroutine code to the Koha::Patrons->search_upcoming_membership_expires
method.
This subroutine was used from only 1 place, so it's an easier to move.

Test plan:
Use the membership_expiry.pl cronjob script using the different
options.
The behavior should be the same as prior to this patch.

  prove t/db_dependent/Koha/Patrons.t
should return green

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 17 Jonathan Druart 2016-12-02 08:36:58 UTC
Created attachment 57897 [details] [review]
Bug 17569: Remove C4::Members::GetUpcomingMembershipExpires

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 18 Jonathan Druart 2016-12-02 08:37:12 UTC
Created attachment 57898 [details] [review]
Bug 17569: Do not limit by branch if option is not passed

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 19 Marcel de Rooy 2016-12-02 09:59:00 UTC
Created attachment 57908 [details] [review]
Bug 17569: Rewrite existing tests to make them reusable and more robust

In order not to cheat you, I am rewriting the tests in a separate
commit.
You can confirm that they pass with the other patches.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 20 Marcel de Rooy 2016-12-02 09:59:04 UTC
Created attachment 57909 [details] [review]
Bug 17569: Move tests to the patron module test file

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 21 Marcel de Rooy 2016-12-02 09:59:08 UTC
Created attachment 57910 [details] [review]
Bug 17569: Koha::Patrons - Move GetUpcomingMembershipExpires to search_upcoming_membership_expires

This patchset moves the C4::Members::GetUpcomingMembershipExpires
subroutine code to the Koha::Patrons->search_upcoming_membership_expires
method.
This subroutine was used from only 1 place, so it's an easier to move.

Test plan:
Use the membership_expiry.pl cronjob script using the different
options.
The behavior should be the same as prior to this patch.

  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 22 Marcel de Rooy 2016-12-02 09:59:11 UTC
Created attachment 57911 [details] [review]
Bug 17569: Remove C4::Members::GetUpcomingMembershipExpires

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 23 Marcel de Rooy 2016-12-02 09:59:18 UTC
Created attachment 57912 [details] [review]
Bug 17569: Do not limit by branch if option is not passed

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 24 Marcel de Rooy 2016-12-02 09:59:23 UTC
Created attachment 57913 [details] [review]
Bug 17569: [QA Follow-up] Small changes

Patron.pm: Adds two missing semicolons at the last statement. Not strictly
needed, but strongly recommended.
Patrons.t: Add a test description, remove two comments that refer to
previously hardcoded dates.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 25 Marcel de Rooy 2016-12-02 10:00:12 UTC
QA Comment:
sub search_upcoming_membership_expires
The mix of before/after and search options in the parameters does not really look good to me. (No blocker, but source for future problems when someone adds stuff and is not careful.)
Comment 26 Jonathan Druart 2016-12-29 09:32:16 UTC
Created attachment 58478 [details] [review]
Bug 17569: Rewrite existing tests to make them reusable and more robust

In order not to cheat you, I am rewriting the tests in a separate
commit.
You can confirm that they pass with the other patches.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 27 Jonathan Druart 2016-12-29 09:32:20 UTC
Created attachment 58479 [details] [review]
Bug 17569: Move tests to the patron module test file

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 28 Jonathan Druart 2016-12-29 09:32:25 UTC
Created attachment 58480 [details] [review]
Bug 17569: Koha::Patrons - Move GetUpcomingMembershipExpires to search_upcoming_membership_expires

This patchset moves the C4::Members::GetUpcomingMembershipExpires
subroutine code to the Koha::Patrons->search_upcoming_membership_expires
method.
This subroutine was used from only 1 place, so it's an easier to move.

Test plan:
Use the membership_expiry.pl cronjob script using the different
options.
The behavior should be the same as prior to this patch.

  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 29 Jonathan Druart 2016-12-29 09:32:29 UTC
Created attachment 58481 [details] [review]
Bug 17569: Remove C4::Members::GetUpcomingMembershipExpires

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 30 Jonathan Druart 2016-12-29 09:32:34 UTC
Created attachment 58482 [details] [review]
Bug 17569: Do not limit by branch if option is not passed

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 31 Jonathan Druart 2016-12-29 09:32:38 UTC
Created attachment 58483 [details] [review]
Bug 17569: [QA Follow-up] Small changes

Patron.pm: Adds two missing semicolons at the last statement. Not strictly
needed, but strongly recommended.
Patrons.t: Add a test description, remove two comments that refer to
previously hardcoded dates.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 32 Kyle M Hall (khall) 2016-12-30 11:55:40 UTC
Pushed to master for 17.05, thanks Jonathan, Marcel!
Comment 33 Katrin Fischer 2017-01-04 21:59:33 UTC
This won't get ported back to 16.11.x as it is an enhancement.