Bug 10481 - No enrollment fee when changing patron category
Summary: No enrollment fee when changing patron category
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Jonathan Druart
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-06-17 12:56 UTC by Jonathan Druart
Modified: 2014-12-07 20:02 UTC (History)
8 users (show)

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


Attachments
Bug 10481: FIX No enrollment fee when changing patron category. (2.27 KB, patch)
2013-06-17 13:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10481: FIX No enrollment fee when changing patron category. (4.64 KB, patch)
2013-07-11 09:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10481: Unit tests for C4::Members::AddEnrolmentFeeIfNeeded (2.57 KB, patch)
2013-07-11 09:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 10481: FIX No enrollment fee when changing patron category. (4.75 KB, patch)
2013-07-16 15:51 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 10337: Unit tests is based on an existing database (4.79 KB, patch)
2013-07-19 07:55 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 10481: Unit tests for C4::Members::AddEnrolmentFeeIfNeeded (2.63 KB, patch)
2013-07-19 15:19 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 10481: FIX No enrollment fee when changing patron category. (4.75 KB, patch)
2013-08-16 13:01 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 10481: Unit tests for C4::Members::AddEnrolmentFeeIfNeeded (2.69 KB, patch)
2013-08-16 13:01 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2013-06-17 12:56:05 UTC
When a patron changes to a category with enrollment fee, they are not generated.
Comment 1 Jonathan Druart 2013-06-17 13:01:05 UTC Comment hidden (obsolete)
Comment 2 Galen Charlton 2013-07-10 16:22:34 UTC
There are two other places in C4/Members.pm that calculate and add the enrollment fee.  Time for a new function rather than copy-pasta.
Comment 3 Jonathan Druart 2013-07-11 09:46:48 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2013-07-11 09:46:53 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2013-07-11 09:48:26 UTC
(In reply to Galen Charlton from comment #2)
> There are two other places in C4/Members.pm that calculate and add the
> enrollment fee.  Time for a new function rather than copy-pasta.

Yes of course, you are right. Last 2 patches refactores code into 1 routine and add unit tests for it.
Comment 6 Srdjan Jankovic 2013-07-16 05:24:39 UTC
prove t/db_dependent/Members/AddEnrolmentFeeIfNeeded.t
t/db_dependent/Members/AddEnrolmentFeeIfNeeded.t .. Use of uninitialized value in pattern match (m//) at C4/SQLHelper.pm line 407.
DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha`.`borrowers`, CONSTRAINT `borrowers_ibfk_1` FOREIGN KEY (`categorycode`) REFERENCES `categories` (`categorycode`)) at C4/SQLHelper.pm line 184.
Comment 7 Jonathan Druart 2013-07-16 07:45:26 UTC
(In reply to Srdjan Jankovic from comment #6)
> prove t/db_dependent/Members/AddEnrolmentFeeIfNeeded.t
> t/db_dependent/Members/AddEnrolmentFeeIfNeeded.t .. Use of uninitialized
> value in pattern match (m//) at C4/SQLHelper.pm line 407.
> DBD::mysql::st execute failed: Cannot add or update a child row: a foreign
> key constraint fails (`koha`.`borrowers`, CONSTRAINT `borrowers_ibfk_1`
> FOREIGN KEY (`categorycode`) REFERENCES `categories` (`categorycode`)) at
> C4/SQLHelper.pm line 184.

It seems you don't have the good category codes. You have to insert sample data before executing this unit tests file.
Comment 8 Srdjan Jankovic 2013-07-16 07:53:44 UTC
installer/data/mysql/sample_only_param_tables.sql?
Comment 9 Jonathan Druart 2013-07-16 08:18:49 UTC
(In reply to Srdjan Jankovic from comment #8)
> installer/data/mysql/sample_only_param_tables.sql?

The patch for bug 10337 creates a DB with all sample data.
Comment 10 Owen Leonard 2013-07-16 15:51:40 UTC Comment hidden (obsolete)
Comment 11 Srdjan Jankovic 2013-07-16 23:03:18 UTC
> The patch for bug 10337 creates a DB with all sample data.

Sorry, did not know of that. It is in discussion, what is the instruction for using it atm?
Comment 12 Jonathan Druart 2013-07-17 07:40:13 UTC
(In reply to Srdjan Jankovic from comment #11)
> > The patch for bug 10337 creates a DB with all sample data.
> 
> Sorry, did not know of that. It is in discussion, what is the instruction
> for using it atm?

There is no instruction atm. I sent an email to the mailing list, but I did not get an answer. The patch allows to create a new DB with sample data only. These sample data are required for some unit tests (indeed, it is really boring to create *all* data for each unit tests).
Comment 13 Jonathan Druart 2013-07-17 07:41:29 UTC
(In reply to Owen Leonard from comment #10)
> Created attachment 19694 [details] [review] [review]
> [...]
> Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Owen, did you forgot to change the status to Signed Off?
Comment 14 Jonathan Druart 2013-07-17 07:42:41 UTC
(In reply to Jonathan Druart from comment #13)
> (In reply to Owen Leonard from comment #10)
> > Created attachment 19694 [details] [review] [review] [review]
> > [...]
> > Signed-off-by: Owen Leonard <oleonard@myacpl.org>
> 
> Owen, did you forgot to change the status to Signed Off?

Ok, I understood after sending the comment, only 1 patch is signed off.
Comment 15 Srdjan Jankovic 2013-07-18 04:21:25 UTC
I still got the same result after applying 10337
Comment 16 Jonathan Druart 2013-07-18 07:54:14 UTC
(In reply to Srdjan Jankovic from comment #15)
> I still got the same result after applying 10337

the sql file installer/data/mysql/en/optional/patron_categories.sql inserts needed categories (K, J, YA).
Comment 17 Srdjan Jankovic 2013-07-19 03:45:15 UTC
prove t/db_dependent/Members/AddEnrolmentFeeIfNeeded.t
t/db_dependent/Members/AddEnrolmentFeeIfNeeded.t .. DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha`.`borrowers`, CONSTRAINT `borrowers_ibfk_2` FOREIGN KEY (`branchcode`) REFERENCES `branches` (`branchcode`)) at C4/SQLHelper.pm line 184.
DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha`.`accountlines`, CONSTRAINT `accountlines_ibfk_1` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE CASCADE ON UPDATE CASCADE) at C4/Accounts.pm line 474.
DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha`.`accountlines`, CONSTRAINT `accountlines_ibfk_1` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE CASCADE ON UPDATE CASCADE) at C4/Accounts.pm line 474.
# Looks like your test exited with 255 before it could output anything.
Comment 18 Jonathan Druart 2013-07-19 07:27:52 UTC
The branchcode "CPL" does not exist in your DB (added by installer/data/mysql/en/optional/sample_libraries.sql).
Comment 19 Srdjan Jankovic 2013-07-19 07:55:19 UTC Comment hidden (obsolete)
Comment 20 Jonathan Druart 2013-07-19 08:06:29 UTC
Srdjan, Thanks for signing off this patch! But you did not sent the good one :)
Comment 21 Srdjan Jankovic 2013-07-19 15:19:56 UTC Comment hidden (obsolete)
Comment 22 Srdjan Jankovic 2013-07-19 15:20:46 UTC
Oh what a mess... Terribly sorry.
Comment 23 Kyle M Hall (khall) 2013-08-16 13:01:26 UTC
Created attachment 20406 [details] [review]
Bug 10481: FIX No enrollment fee when changing patron category.

When a patron changes to a category with enrollment fee, they
are not generated.

Test plan:
- Choose a category without fee (e.g. Kid)
- Add an enrollment fee for another category (e.g. Young adult)
- Choose a kid and change his category to "Young adult".
- Note the warning message "Fees & Charges: Patron has Outstanding fees
  & charges of XX" on the check out page.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 24 Kyle M Hall (khall) 2013-08-16 13:01:38 UTC
Created attachment 20407 [details] [review]
Bug 10481: Unit tests for C4::Members::AddEnrolmentFeeIfNeeded

prove t/db_dependent/Members/AddEnrolmentFeeIfNeeded.t
t/db_dependent/Members/AddEnrolmentFeeIfNeeded.t .. ok
All tests successful.
Files=1, Tests=3,  0 wallclock secs ( 0.02 usr  0.00 sys +  0.38 cusr
0.01 csys =  0.41 CPU)
Result: PASS

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 25 Galen Charlton 2013-08-16 14:54:37 UTC
Comment on attachment 20407 [details] [review]
Bug 10481: Unit tests for C4::Members::AddEnrolmentFeeIfNeeded

Review of attachment 20407 [details] [review]:
-----------------------------------------------------------------

::: t/db_dependent/Members/AddEnrolmentFeeIfNeeded.t
@@ +50,5 @@
> +
> +# Check with calling directly AddEnrolmentFeeIfNeeded
> +C4::Members::AddEnrolmentFeeIfNeeded( 'YA', $borrowernumber );
> +( $total ) = C4::Members::GetMemberAccountRecords( $borrowernumber );
> +is( $total, $enrolmentfee_K + $enrolmentfee_J + $enrolmentfee_YA, "Juvenile growing and become an young adult, he should pay " . ( $enrolmentfee_K + $enrolmentfee_J + $enrolmentfee_YA ) );

If I were a kid in this library district, I would think twice about reporting my age! ;)
Comment 26 Galen Charlton 2013-08-16 15:09:18 UTC
Pushed to master.  Thanks, Jonathan!
Comment 27 Tomás Cohen Arazi (tcohen) 2013-09-11 17:56:58 UTC
This patch has been pushed to 3.12.x, will be in 3.12.5.

Thanks Jonathan!
Comment 28 Bernardo Gonzalez Kriegel 2013-09-14 22:37:35 UTC
Pushed to 3.10.x, will be in 3.10.11
Comment 29 Chris Hall 2013-10-20 07:23:03 UTC
Pushed to 3.8.x, will be in 3.8.19
Comment 30 Sophie MEYNIEUX 2014-01-14 13:16:21 UTC
This patch introduce a new bug in 3.8.x
It uses C4::Members::GetBorrowerCategorycode that is introduced only in 3.10 with http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5742

This patch should be reverted for 3.8.x version
Comment 31 Kyle M Hall (khall) 2014-01-15 12:10:14 UTC
(In reply to Sophie MEYNIEUX from comment #30)
> This patch introduce a new bug in 3.8.x
> It uses C4::Members::GetBorrowerCategorycode that is introduced only in 3.10
> with http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5742
> 
> This patch should be reverted for 3.8.x version

This patch has now been reverted in 3.8.x and the reversion will be part of 3.8.22

Kyle