Bug 16912 - Koha::Patrons - Move AddEnrolmentFeeIfNeeded to add_enrolment_fee_if_needed
Summary: Koha::Patrons - Move AddEnrolmentFeeIfNeeded to add_enrolment_fee_if_needed
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)
Target Milestone: ---
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 16911
Blocks: 16846 21133
  Show dependency treegraph
 
Reported: 2016-07-12 18:57 UTC by Jonathan Druart
Modified: 2018-07-31 15:39 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 16912: Koha::Patrons - Move and rewrite tests for AddEnrolmentFeeIfNeeded (5.92 KB, patch)
2016-07-12 19:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16912: Koha::Patrons - Move AddEnrolmentFeeIfNeeded to ->add_enrolment_fee_if_needed (5.30 KB, patch)
2016-07-12 19:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16912: Koha::Patrons - Move and rewrite tests for AddEnrolmentFeeIfNeeded (5.99 KB, patch)
2016-07-20 14:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16912: Koha::Patrons - Move AddEnrolmentFeeIfNeeded to ->add_enrolment_fee_if_needed (5.25 KB, patch)
2016-07-20 14:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 16912: Koha::Patrons - Move AddEnrolmentFeeIfNeeded to ->add_enrolment_fee_if_needed (5.32 KB, patch)
2016-08-23 23:58 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 16912: Update patron category enrolment fees (1.40 KB, patch)
2016-08-24 08:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 16912: Koha::Patrons - Move and rewrite tests for AddEnrolmentFeeIfNeeded (6.06 KB, patch)
2016-08-24 20:58 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 16912: Koha::Patrons - Move AddEnrolmentFeeIfNeeded to ->add_enrolment_fee_if_needed (5.31 KB, patch)
2016-08-24 20:58 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 16912: Update patron category enrolment fees (1.47 KB, patch)
2016-08-24 21:00 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 16912: Koha::Patrons - Move AddEnrolmentFeeIfNeeded to ->add_enrolment_fee_if_needed (5.48 KB, patch)
2016-10-14 09:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16912: Koha::Patrons - Move and rewrite tests for AddEnrolmentFeeIfNeeded (6.07 KB, patch)
2016-10-14 09:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16912: Update patron category enrolment fees (1.46 KB, patch)
2016-10-14 09:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16912: Koha::Patrons - Move AddEnrolmentFeeIfNeeded to ->add_enrolment_fee_if_needed (5.53 KB, patch)
2016-10-14 13:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 16912: Koha::Patrons - Move and rewrite tests for AddEnrolmentFeeIfNeeded (6.12 KB, patch)
2016-10-14 13:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 16912: Update patron category enrolment fees (1.51 KB, patch)
2016-10-14 13:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 16912: Koha::Patrons - Move AddEnrolmentFeeIfNeeded to ->add_enrolment_fee_if_needed (5.12 KB, patch)
2016-10-24 13:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16912: Koha::Patrons - Move and rewrite tests for AddEnrolmentFeeIfNeeded (6.09 KB, patch)
2016-10-24 13:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16912: Update patron category enrolment fees (1.51 KB, patch)
2016-10-24 13: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-07-12 18:57:51 UTC

    
Comment 1 Jonathan Druart 2016-07-12 19:08:01 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2016-07-12 19:08:04 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2016-07-20 14:00:15 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2016-07-20 14:00:18 UTC Comment hidden (obsolete)
Comment 5 Sandre Cunha 2016-08-06 20:17:14 UTC
I d't understand why this is under Acquisitions. Shouldn't it be under Patrons or circulation
Comment 6 Katrin Fischer 2016-08-06 20:18:20 UTC
I think it was a mistake - well spotted!
Comment 7 Aleisha Amohia 2016-08-23 02:19:03 UTC
fatal: sha1 information is lacking or useless (t/db_dependent/Koha/Patrons.t)
Comment 8 Jonathan Druart 2016-08-23 10:02:52 UTC
(In reply to Aleisha Amohia from comment #7)
> fatal: sha1 information is lacking or useless (t/db_dependent/Koha/Patrons.t)

Apply cleanly on top of bug 16911.
Comment 9 Aleisha Amohia 2016-08-23 23:28:52 UTC
(In reply to Jonathan Druart from comment #3)
> Created attachment 53519 [details] [review] [review]
> Bug 16912: Koha::Patrons - Move and rewrite tests for AddEnrolmentFeeIfNeeded

Tests failed:

prove t/db_dependent/Koha/Patrons.t 
t/db_dependent/Koha/Patrons.t .. 1/8 Violation of unique constraint in Category at /home/vagrant/kohaclone/t/lib/TestBuilder.pm line 201.
Violation of unique constraint in Category at /home/vagrant/kohaclone/t/lib/TestBuilder.pm line 201.
Violation of unique constraint in Category at /home/vagrant/kohaclone/t/lib/TestBuilder.pm line 201.

    #   Failed test 'New kid pay 5'
    #   at t/db_dependent/Koha/Patrons.t line 200.
    #          got: '0'
    #     expected: '5'

    #   Failed test 'Kid growing and become a juvenile, but shouldn't pay for the upgrade '
    #   at t/db_dependent/Koha/Patrons.t line 206.
    #          got: '0'
    #     expected: '5'

    #   Failed test 'Kid growing and become a juvenile, he should pay 15'
    #   at t/db_dependent/Koha/Patrons.t line 215.
    #          got: '0'
    #     expected: '15'

    #   Failed test 'Juvenile growing and become an young adult, he should pay 35'
    #   at t/db_dependent/Koha/Patrons.t line 222.
    #          got: '0'
    #     expected: '35'
    # Looks like you failed 4 tests of 4.

#   Failed test 'add_enrolment_fee_if_needed'
#   at t/db_dependent/Koha/Patrons.t line 228.
# Looks like you failed 1 test of 8.
t/db_dependent/Koha/Patrons.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/8 subtests 

Test Summary Report
-------------------
t/db_dependent/Koha/Patrons.t (Wstat: 256 Tests: 8 Failed: 1)
  Failed test:  7
  Non-zero exit status: 1
Files=1, Tests=8,  2 wallclock secs ( 0.03 usr  0.00 sys +  2.12 cusr  0.09 csys =  2.24 CPU)
Result: FAIL

The enrolment fee behaviour all works as expected, so happy to sign off once the tests pass.
Comment 10 Aleisha Amohia 2016-08-23 23:58:20 UTC
Created attachment 54798 [details] [review]
[SIGNED-OFF] Bug 16912: Koha::Patrons - Move AddEnrolmentFeeIfNeeded to ->add_enrolment_fee_if_needed

This patch moves the code of the C4::Members::AddEnrolmentFeeIfNeeded
subroutine to the Koha::Patron->add_enrolment_fee_if_needed method.
The behavior should be unchanged.

Test plan:
1/ Define enrolment fee for 2 patron categories
2/ Create a patron using the first category
=> The patron should be charged
3/ Change the patron category to the other one
=> The patron should be charged again

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 11 Jonathan Druart 2016-08-24 08:14:06 UTC Comment hidden (obsolete)
Comment 12 Aleisha Amohia 2016-08-24 20:58:10 UTC
Created attachment 54836 [details] [review]
[SIGNED-OFF] Bug 16912: Koha::Patrons - Move and rewrite tests for AddEnrolmentFeeIfNeeded

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 13 Aleisha Amohia 2016-08-24 20:58:24 UTC Comment hidden (obsolete)
Comment 14 Aleisha Amohia 2016-08-24 21:00:23 UTC
Created attachment 54838 [details] [review]
[SIGNED-OFF] Bug 16912: Update patron category enrolment fees

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 15 Aleisha Amohia 2016-08-24 21:01:26 UTC
All tests pass and patch works as described, signing off.
Comment 16 Nick Clemens 2016-10-14 08:27:46 UTC
16911 is in master:

Applying: Bug 16912: Koha::Patrons - Move AddEnrolmentFeeIfNeeded to ->add_enrolment_fee_if_needed
fatal: sha1 information is lacking or useless (C4/Members.pm).
Repository lacks necessary blobs to fall back on 3-way merge.
Comment 17 Jonathan Druart 2016-10-14 09:25:16 UTC
Created attachment 56493 [details] [review]
Bug 16912: Koha::Patrons - Move AddEnrolmentFeeIfNeeded to ->add_enrolment_fee_if_needed

This patch moves the code of the C4::Members::AddEnrolmentFeeIfNeeded
subroutine to the Koha::Patron->add_enrolment_fee_if_needed method.
The behavior should be unchanged.

Test plan:
1/ Define enrolment fee for 2 patron categories
2/ Create a patron using the first category
=> The patron should be charged
3/ Change the patron category to the other one
=> The patron should be charged again

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 18 Jonathan Druart 2016-10-14 09:25:19 UTC
Created attachment 56494 [details] [review]
Bug 16912: Koha::Patrons - Move and rewrite tests for AddEnrolmentFeeIfNeeded

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 19 Jonathan Druart 2016-10-14 09:25:23 UTC
Created attachment 56495 [details] [review]
Bug 16912: Update patron category enrolment fees

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Comment 20 Nick Clemens 2016-10-14 13:12:15 UTC
Created attachment 56526 [details] [review]
Bug 16912: Koha::Patrons - Move AddEnrolmentFeeIfNeeded to ->add_enrolment_fee_if_needed

This patch moves the code of the C4::Members::AddEnrolmentFeeIfNeeded
subroutine to the Koha::Patron->add_enrolment_fee_if_needed method.
The behavior should be unchanged.

Test plan:
1/ Define enrolment fee for 2 patron categories
2/ Create a patron using the first category
=> The patron should be charged
3/ Change the patron category to the other one
=> The patron should be charged again

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 21 Nick Clemens 2016-10-14 13:12:21 UTC
Created attachment 56527 [details] [review]
Bug 16912: Koha::Patrons - Move and rewrite tests for AddEnrolmentFeeIfNeeded

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 22 Nick Clemens 2016-10-14 13:12:26 UTC
Created attachment 56528 [details] [review]
Bug 16912: Update patron category enrolment fees

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 23 Kyle M Hall 2016-10-21 17:04:00 UTC
Looks like this may need a rebase and a unit test fix up:

t/db_dependent/Koha/Patrons.t .. 8/11 Violation of unique constraint in Category at /home/vagrant/kohaclone/t/lib/TestBuilder.pm line 201.
Violation of unique constraint in Category at /home/vagrant/kohaclone/t/lib/TestBuilder.pm line 201.
Violation of unique constraint in Category at /home/vagrant/kohaclone/t/lib/TestBuilder.pm line 201.

    #   Failed test 'New kid pay 5'
    #   at t/db_dependent/Koha/Patrons.t line 276.
    #          got: '0'
    #     expected: '5'

    #   Failed test 'Kid growing and become a juvenile, but shouldn't pay for the upgrade '
    #   at t/db_dependent/Koha/Patrons.t line 282.
    #          got: '0'
    #     expected: '5'

    #   Failed test 'Kid growing and become a juvenile, he should pay 15'
    #   at t/db_dependent/Koha/Patrons.t line 291.
    #          got: '0'
    #     expected: '15'

    #   Failed test 'Juvenile growing and become an young adult, he should pay 35'
    #   at t/db_dependent/Koha/Patrons.t line 298.
    #          got: '0'
    #     expected: '35'
    # Looks like you failed 4 tests of 4.

#   Failed test 'add_enrolment_fee_if_needed'
#   at t/db_dependent/Koha/Patrons.t line 304.
# Looks like you failed 1 test of 11.
t/db_dependent/Koha/Patrons.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/11 subtests

Test Summary Report
-------------------
t/db_dependent/Koha/Patrons.t (Wstat: 256 Tests: 11 Failed: 1)
  Failed test:  10
  Non-zero exit status: 1
Files=1, Tests=11,  2 wallclock secs ( 0.04 usr  0.00 sys +  1.80 cusr  0.15 csys =  1.99 CPU)
Result: FAIL
Comment 24 Jonathan Druart 2016-10-24 13:32:22 UTC
Created attachment 56771 [details] [review]
Bug 16912: Koha::Patrons - Move AddEnrolmentFeeIfNeeded to ->add_enrolment_fee_if_needed

This patch moves the code of the C4::Members::AddEnrolmentFeeIfNeeded
subroutine to the Koha::Patron->add_enrolment_fee_if_needed method.
The behavior should be unchanged.

Test plan:
1/ Define enrolment fee for 2 patron categories
2/ Create a patron using the first category
=> The patron should be charged
3/ Change the patron category to the other one
=> The patron should be charged again

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 25 Jonathan Druart 2016-10-24 13:32:27 UTC
Created attachment 56772 [details] [review]
Bug 16912: Koha::Patrons - Move and rewrite tests for AddEnrolmentFeeIfNeeded

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 26 Jonathan Druart 2016-10-24 13:32:31 UTC
Created attachment 56773 [details] [review]
Bug 16912: Update patron category enrolment fees

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 27 Jonathan Druart 2016-10-24 13:34:21 UTC
(In reply to Kyle M Hall from comment #23)
> Looks like this may need a rebase and a unit test fix up:

I got conflicts applying these patches and I cannot recreate the failures.
Could you please retest with these patches again?
I think I already got these failures with this test file. It looks like random failures caused by existing data and not directly linked with these changes.
Comment 28 Kyle M Hall 2016-10-28 15:32:10 UTC
Pushed to master for 16.11, thanks Jonathan!