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.
Created attachment 57266 [details] [review] Bug 17569: Move tests to the patron module test file
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
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.
Created attachment 57294 [details] [review] Bug 17569: Remove C4::Members::GetUpcomingMembershipExpires
Created attachment 57295 [details] [review] Bug 17569: Do not limit by branch if option is not passed
Fixed Josef, thanks for testing!
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>
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>
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>
Created attachment 57318 [details] [review] [SIGNED-OFF] Bug 17569: Remove C4::Members::GetUpcomingMembershipExpires Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
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>
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.
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>
Created attachment 57895 [details] [review] Bug 17569: Move tests to the patron module test file Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
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>
Created attachment 57897 [details] [review] Bug 17569: Remove C4::Members::GetUpcomingMembershipExpires Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
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>
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>
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>
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>
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>
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>
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>
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.)
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>
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>
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>
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>
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>
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>
Pushed to master for 17.05, thanks Jonathan, Marcel!
This won't get ported back to 16.11.x as it is an enhancement.