Bug 17557 - Move GetAge to Koha::Patron->get_age (and remove SetAge)
Summary: Move GetAge to Koha::Patron->get_age (and remove SetAge)
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 (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 17521
Blocks: 16846
  Show dependency treegraph
 
Reported: 2016-11-04 16:50 UTC by Jonathan Druart
Modified: 2017-12-07 22:16 UTC (History)
3 users (show)

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


Attachments
Bug 17557: Revised patron age calculation tests (6.77 KB, patch)
2016-11-04 17:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17557: Koha::Patrons - Move GetAge to ->set_age (and remove SetAge) (7.84 KB, patch)
2016-11-04 17:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17557: Revised patron age calculation tests (6.84 KB, patch)
2016-11-04 17:20 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17557: Koha::Patrons - Move GetAge to ->set_age (and remove SetAge) (7.91 KB, patch)
2016-11-04 17:21 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17557: Revised patron age calculation tests (6.91 KB, patch)
2016-12-01 17:40 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17557: Koha::Patrons - Move GetAge to ->set_age (and remove SetAge) (7.91 KB, patch)
2016-12-01 17:40 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17557: Revised patron age calculation tests (6.89 KB, patch)
2016-12-12 10:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17557: Koha::Patrons - Move GetAge to ->set_age (and remove SetAge) (7.85 KB, patch)
2016-12-12 10:09 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-04 16:50:59 UTC

    
Comment 1 Jonathan Druart 2016-11-04 17:04:15 UTC
Created attachment 57218 [details] [review]
Bug 17557: Revised patron age calculation tests

The SetAge and GetAge test coverage are excessive.
First the SetAge subroutine was only created for testing purpose.
The goal of GetAge is quite simple and it seems quite easy to provide
corect test coverage using DateTime->add using negative numbers.
Comment 2 Jonathan Druart 2016-11-04 17:04:19 UTC
Created attachment 57219 [details] [review]
Bug 17557: Koha::Patrons - Move GetAge to ->set_age (and remove SetAge)

As said in the previous commit, I considered SetAge as unnecessary and
removed it.

Test plan:
1/ Edit a patron using the different 'Edit' links
2/ Play with the patron category limited to age ranges, and date of
birth
3/ You should get the expected warning if the date of birth is inside
the patron category date range.

To finish:
  prove t/Circulation/AgeRestrictionMarkers.t t/db_dependent/Reserves.t \
        t/db_dependent/Koha/Patrons.t t/db_dependent/Members.t
should return green
Comment 3 Josef Moravec 2016-11-04 17:20:56 UTC
Created attachment 57220 [details] [review]
[SIGNED-OFF] Bug 17557: Revised patron age calculation tests

The SetAge and GetAge test coverage are excessive.
First the SetAge subroutine was only created for testing purpose.
The goal of GetAge is quite simple and it seems quite easy to provide
corect test coverage using DateTime->add using negative numbers.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 4 Josef Moravec 2016-11-04 17:21:02 UTC
Created attachment 57221 [details] [review]
[SIGNED-OFF] Bug 17557: Koha::Patrons - Move GetAge to ->set_age (and remove SetAge)

As said in the previous commit, I considered SetAge as unnecessary and
removed it.

Test plan:
1/ Edit a patron using the different 'Edit' links
2/ Play with the patron category limited to age ranges, and date of
birth
3/ You should get the expected warning if the date of birth is inside
the patron category date range.

To finish:
  prove t/Circulation/AgeRestrictionMarkers.t t/db_dependent/Reserves.t \
        t/db_dependent/Koha/Patrons.t t/db_dependent/Members.t
should return green

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 5 Tomás Cohen Arazi 2016-12-01 17:40:16 UTC
Created attachment 57862 [details] [review]
Bug 17557: Revised patron age calculation tests

The SetAge and GetAge test coverage are excessive.
First the SetAge subroutine was only created for testing purpose.
The goal of GetAge is quite simple and it seems quite easy to provide
corect test coverage using DateTime->add using negative numbers.

Edit: rebased so it applies

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 6 Tomás Cohen Arazi 2016-12-01 17:40:27 UTC
Created attachment 57863 [details] [review]
Bug 17557: Koha::Patrons - Move GetAge to ->set_age (and remove SetAge)

As said in the previous commit, I considered SetAge as unnecessary and
removed it.

Test plan:
1/ Edit a patron using the different 'Edit' links
2/ Play with the patron category limited to age ranges, and date of
birth
3/ You should get the expected warning if the date of birth is inside
the patron category date range.

To finish:
  prove t/Circulation/AgeRestrictionMarkers.t t/db_dependent/Reserves.t \
        t/db_dependent/Koha/Patrons.t t/db_dependent/Members.t
should return green

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 7 Tomás Cohen Arazi 2016-12-01 17:42:27 UTC
Passing rebased patches.
Comment 8 Jonathan Druart 2016-12-12 10:09:28 UTC
Created attachment 58104 [details] [review]
Bug 17557: Revised patron age calculation tests

The SetAge and GetAge test coverage are excessive.
First the SetAge subroutine was only created for testing purpose.
The goal of GetAge is quite simple and it seems quite easy to provide
corect test coverage using DateTime->add using negative numbers.

Edit: rebased so it applies

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 9 Jonathan Druart 2016-12-12 10:09:32 UTC
Created attachment 58105 [details] [review]
Bug 17557: Koha::Patrons - Move GetAge to ->set_age (and remove SetAge)

As said in the previous commit, I considered SetAge as unnecessary and
removed it.

Test plan:
1/ Edit a patron using the different 'Edit' links
2/ Play with the patron category limited to age ranges, and date of
birth
3/ You should get the expected warning if the date of birth is inside
the patron category date range.

To finish:
  prove t/Circulation/AgeRestrictionMarkers.t t/db_dependent/Reserves.t \
        t/db_dependent/Koha/Patrons.t t/db_dependent/Members.t
should return green

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 10 Kyle M Hall 2016-12-16 12:02:18 UTC
Pushed to master for 17.05, thanks Jonathan!
Comment 11 Katrin Fischer 2016-12-18 20:02:48 UTC
Should this go into 16.11?
Comment 12 Katrin Fischer 2016-12-18 20:39:07 UTC
Hm, missed the second patch looking at git - this is a true enhancement and not just new unit tests. So:

This won't get backported to 16.11.x as it is an enhancement.