Bug 16912

Summary: Koha::Patrons - Move AddEnrolmentFeeIfNeeded to add_enrolment_fee_if_needed
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: aleisha, katrin.fischer, kyle, mtompset, 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: 16911    
Bug Blocks: 16846, 21133    
Attachments: Bug 16912: Koha::Patrons - Move and rewrite tests for AddEnrolmentFeeIfNeeded
Bug 16912: Koha::Patrons - Move AddEnrolmentFeeIfNeeded to ->add_enrolment_fee_if_needed
Bug 16912: Koha::Patrons - Move and rewrite tests for AddEnrolmentFeeIfNeeded
Bug 16912: Koha::Patrons - Move AddEnrolmentFeeIfNeeded to ->add_enrolment_fee_if_needed
[SIGNED-OFF] Bug 16912: Koha::Patrons - Move AddEnrolmentFeeIfNeeded to ->add_enrolment_fee_if_needed
Bug 16912: Update patron category enrolment fees
[SIGNED-OFF] Bug 16912: Koha::Patrons - Move and rewrite tests for AddEnrolmentFeeIfNeeded
Bug 16912: Koha::Patrons - Move AddEnrolmentFeeIfNeeded to ->add_enrolment_fee_if_needed
[SIGNED-OFF] Bug 16912: Update patron category enrolment fees
Bug 16912: Koha::Patrons - Move AddEnrolmentFeeIfNeeded to ->add_enrolment_fee_if_needed
Bug 16912: Koha::Patrons - Move and rewrite tests for AddEnrolmentFeeIfNeeded
Bug 16912: Update patron category enrolment fees
Bug 16912: Koha::Patrons - Move AddEnrolmentFeeIfNeeded to ->add_enrolment_fee_if_needed
Bug 16912: Koha::Patrons - Move and rewrite tests for AddEnrolmentFeeIfNeeded
Bug 16912: Update patron category enrolment fees
Bug 16912: Koha::Patrons - Move AddEnrolmentFeeIfNeeded to ->add_enrolment_fee_if_needed
Bug 16912: Koha::Patrons - Move and rewrite tests for AddEnrolmentFeeIfNeeded
Bug 16912: Update patron category enrolment fees

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!
Comment 29 Jonathan Druart 2023-08-01 06:40:04 UTC
*** Bug 13455 has been marked as a duplicate of this bug. ***