The module C4::Members depends on module C4::SQLHelper. However, the module SQLHelper is obsolete, so the calls for InsertInTable, UpdateInTable and SearchInTable have to be replaced by the use of the module DBIx::Class.
Created attachment 30115 [details] [review] Bug 12633: SQLHelper replacement - C4::Members With this patch, the subroutines AddMember and Search uses DBIx::Class instead of C4::SQLHelper. In order to replace SQLHelper in the Search subroutine, 2 subroutines has been added, it seems a lot : replace 1 line with 2 subroutines, but in fact these 2 subroutines replace all the hidden work done by the SQLHelper. Test plan: 1) Apply the patch 12457 in order to get the last unit tests for Members 2) Apply the patch 3) Execute the unit tests by launching : prove t/db_dependent/Members.t 4) The result has to be a success without error or warning : t/db_dependent/Members.t .. ok All tests successful. Files=1, Tests=55, 2 wallclock secs ( 0.04 usr 0.01 sys + 2.27 cusr 0.09 csys = 2.41 CPU) Result: PASS 5) Sign in the intranet. 6) Verify you can add a member in the intranet. 7) Test of members/guarantor_search.pl : Edit a member which belongs to the cateogry type 'C' (Child), click on 'Set to patron' and verify the search of an adult member is correct. 8) Test of patroncards/members-search.pl : Go to Tools > Patron card creator > New batch > "Add item(s)" and verify the search of a member is correct. 9) Test of serials/member-search.pl : Go on the page serials/member-search.pl and verify the search of a member is correct. 10) Test of reserve/request.pl : Go on a biblio details then "Holds" and verify the search of a member is correct. 11) Test of circ/circulation.pl : Go on Circulation > Checkouts and verify the search of a member is correct.
Created attachment 30119 [details] [review] Bug 12633: SQLHelper replacement - C4::Members With this patch, the subroutines AddMember and Search uses DBIx::Class instead of C4::SQLHelper. In order to replace SQLHelper in the Search subroutine, 2 subroutines has been added, it seems a lot : replace 1 line with 2 subroutines, but in fact these 2 subroutines replace all the hidden work done by the SQLHelper. Test plan: 1) Apply the patch 12457 in order to get the last unit tests for Members 2) Apply the patch 3) Execute the unit tests by launching : prove t/db_dependent/Members.t 4) The result has to be a success without error or warning : t/db_dependent/Members.t .. ok All tests successful. Files=1, Tests=55, 2 wallclock secs ( 0.04 usr 0.01 sys + 2.27 cusr 0.09 csys = 2.41 CPU) Result: PASS 5) Sign in the intranet. 6) Verify you can add a member in the intranet. 7) Test of members/guarantor_search.pl : Edit a member which belongs to the cateogry type 'C' (Child), click on 'Set to patron' and verify the search of an adult member is correct. 8) Test of patroncards/members-search.pl : Go to Tools > Patron card creator > New batch > "Add item(s)" and verify the search of a member is correct. 9) Test of serials/member-search.pl : Go on the page serials/member-search.pl and verify the search of a member is correct. 10) Test of reserve/request.pl : Go on a biblio details then "Holds" and verify the search of a member is correct. 11) Test of circ/circulation.pl : Go on Circulation > Checkouts and verify the search of a member is correct.
Created attachment 30547 [details] [review] [Signed-off] Bug 12633: SQLHelper replacement - C4::Members With this patch, the subroutines AddMember and Search uses DBIx::Class instead of C4::SQLHelper. In order to replace SQLHelper in the Search subroutine, 2 subroutines has been added, it seems a lot : replace 1 line with 2 subroutines, but in fact these 2 subroutines replace all the hidden work done by the SQLHelper. Test plan: 1) Apply the patch 12457 in order to get the last unit tests for Members 2) Apply the patch 3) Execute the unit tests by launching : prove t/db_dependent/Members.t 4) The result has to be a success without error or warning : t/db_dependent/Members.t .. ok All tests successful. Files=1, Tests=55, 2 wallclock secs ( 0.04 usr 0.01 sys + 2.27 cusr 0.09 csys = 2.41 CPU) Result: PASS 5) Sign in the intranet. 6) Verify you can add a member in the intranet. 7) Test of members/guarantor_search.pl : Edit a member which belongs to the cateogry type 'C' (Child), click on 'Set to patron' and verify the search of an adult member is correct. 8) Test of patroncards/members-search.pl : Go to Tools > Patron card creator > New batch > "Add item(s)" and verify the search of a member is correct. 9) Test of serials/member-search.pl : Go on the page serials/member-search.pl and verify the search of a member is correct. 10) Test of reserve/request.pl : Go on a biblio details then "Holds" and verify the search of a member is correct. 11) Test of circ/circulation.pl : Go on Circulation > Checkouts and verify the search of a member is correct. Followed test plan. Patch behaves as expected. Signed-off-by: Marc Véron <veron@veron.ch>
When adding patrons to a routing list, this explodes: DBIx::Class::Storage::DBI::_dbh_execute(): Unknown column 'subscriptionid' in 'where clause' at /home/katrin/kohaclone/C4/Members.pm line 358 To test: - Go to serials - Create a subscription - Go to 'serial collection' - Create a routing list - Add recipients to routing list - Window for patron search opens and shows error from above
Created attachment 30937 [details] [review] Bug 12633: SQLHelper replacement - C4::Members With this patch, the subroutines AddMember and Search uses DBIx::Class instead of C4::SQLHelper. In order to replace SQLHelper in the Search subroutine, 2 subroutines has been added, it seems a lot : replace 1 line with 2 subroutines, but in fact these 2 subroutines replace all the hidden work done by the SQLHelper. Test plan: 1) Apply the patch 12457 in order to get the last unit tests for Members 2) Apply the patch 3) Execute the unit tests by launching : prove t/db_dependent/Members.t 4) The result has to be a success without error or warning : t/db_dependent/Members.t .. ok All tests successful. Files=1, Tests=55, 2 wallclock secs ( 0.04 usr 0.01 sys + 2.27 cusr 0.09 csys = 2.41 CPU) Result: PASS 5) Sign in the intranet. 6) Verify you can add a member in the intranet. 7) Test of members/guarantor_search.pl : Edit a member which belongs to the cateogry type 'C' (Child), click on 'Set to patron' and verify the search of an adult member is correct. 8) Test of patroncards/members-search.pl : Go to Tools > Patron card creator > New batch > "Add item(s)" and verify the search of a member is correct. 9) Test of serials/member-search.pl : Go on the page serials/member-search.pl and verify the search of a member is correct. 10) Test of reserve/request.pl : Go on a biblio details then "Holds" and verify the search of a member is correct. 11) Test of circ/circulation.pl : Go on Circulation > Checkouts and verify the search of a member is correct.
Enabling ExtendedPatronAttributes completely breaks patron searching with this patch.
Actually, it only breaks it for patroncards/members-search.pl
This ties in a bit with bug 12833. This bug is why this patch isn't breaking the main patron search. They aren't directly related though.
If I am not wrong, there are 3 occurrences of C4::Members::Search: 1/ acqui/aqbasketuser_search.pl 2/ members/guarantor_search.pl 3/ patroncards/members-search.pl 1 is removed by 12648, I have a patch for 2. 3 is todo. When done, I will completely remove the C4::Members::Search subroutine. Some code should be move from C4::Utils::DT::Members to C4::Members (or Koha::Members).
(In reply to Jonathan Druart from comment #9) > 1 is removed by 12648, I have a patch for 2. The call in guarantor_search is removed by 13021.
Do we need a new patch here? It looks like this is the last missing piece to get rid of SQLHelper.
(In reply to Katrin Fischer from comment #11) > Do we need a new patch here? It looks like this is the last missing piece to > get rid of SQLHelper. see comment 9.
No track of SQLHelper in C4::Members anymore :)
Woohoo!
Sorry, but there is one left in C4::Members::Search :-/
Ok so... The good news is that only 1 occurrence still exists in C4::Members::Search The bad one is that it is called from 3 different places (but I may have missed some others...): admin/aqbudget_user_search.pl serials/member-search.pl patroncards/members-search.pl I have a patch and will be able to submitted it on monday.
(In reply to Jonathan Druart from comment #16) > Ok so... > The good news is that only 1 occurrence still exists in C4::Members::Search > The bad one is that it is called from 3 different places (but I may have > missed some others...): > admin/aqbudget_user_search.pl > serials/member-search.pl > patroncards/members-search.pl > > I have a patch and will be able to submitted it on monday. Done on bug 13891.
circ/ysearch.pl should not use C4::Members::Search, done on bug 13892.
Created attachment 37751 [details] [review] Bug 12633: Remove SQLHelper in C4::Members This is the only places where SQLHelper is still called. The C4::Members::Search is not used anymore, but ModMember and AddMember. This patch replaced the calls to SQLHelper to use DBIX::Class. TODO: Move them to Koha::Borrower. Test plan: 1/ Make sure the patron search still works (no changes expected since the code was not in used). 2/ Add a patron with all fields filled 3/ Add another patron with some fields filled 4/ Update them with other values 5/ Delete them You should not get any errors.
I wanted to test it, but there is merge conflict. Could you please rebase?
(In reply to Josef Moravec from comment #20) > I wanted to test it, but there is merge conflict. Could you please rebase? A patch has been pushed to master and I think it is a mistake. I pingued Tomas on bug 13189. You can test it reverting a7822b0a32faa5ffe6e5a3b43c1cd2c58a8a802a before.
The patch has been reverted from master.
With this patch applied, leaving the »Date of birth« field empty results in a default value of 02/00/0, which, on a subsequent change of another field, leads to the error message »The following fields are wrong. Please fix them. Date of birth is invalid.« If Date of birth is empty, there should not be anything added on saving the record.
I don't get it, I created a new patron, didn't fill the date of birth, save I got null in DB and nothing in the input on editing.
Ok sorry, too quick, I get it when modifying a patron
Created attachment 38272 [details] [review] Bug 12633: Fix specific default values The dates should be set to undef if defined but empty, otherwise with an empty string, '0000-00-00' will be inserted into the DB.
Created attachment 38284 [details] [review] Bug 12633: Remove SQLHelper in C4::Members This is the only places where SQLHelper is still called. The C4::Members::Search is not used anymore, but ModMember and AddMember. This patch replaced the calls to SQLHelper to use DBIX::Class. TODO: Move them to Koha::Borrower. Test plan: 1/ Make sure the patron search still works (no changes expected since the code was not in used). 2/ Add a patron with all fields filled 3/ Add another patron with some fields filled 4/ Update them with other values 5/ Delete them You should not get any errors. Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Created attachment 38285 [details] [review] Bug 12633: Fix specific default values The dates should be set to undef if defined but empty, otherwise with an empty string, '0000-00-00' will be inserted into the DB. Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Hi Jonathan, I think I located another use of the Search routine, can you please check? members/nl-search.pl: my $local_results = Search( $identifier );
git grep ' Search(' result: members/nl-search.pl: my $local_results = Search( $identifier );
(In reply to Katrin Fischer from comment #29) > Hi Jonathan, > > I think I located another use of the Search routine, can you please check? > > members/nl-search.pl: my $local_results = Search( $identifier ); Good catch, bug 14038 opened.
Bug 14038 is signed off.
Created attachment 38697 [details] [review] [PASSED QA] Bug 12633: Remove SQLHelper in C4::Members This is the only places where SQLHelper is still called. The C4::Members::Search is not used anymore, but ModMember and AddMember. This patch replaced the calls to SQLHelper to use DBIX::Class. TODO: Move them to Koha::Borrower. Test plan: 1/ Make sure the patron search still works (no changes expected since the code was not in used). 2/ Add a patron with all fields filled 3/ Add another patron with some fields filled 4/ Update them with other values 5/ Delete them You should not get any errors. Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 38698 [details] [review] [PASSED QA] Bug 12633: Fix specific default values The dates should be set to undef if defined but empty, otherwise with an empty string, '0000-00-00' will be inserted into the DB. Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Habits.. this was only a sign off, fixing.
It's getting too late... this is PASSED QA.
Patches pushed to master. Thanks Jonathan, almost there!
Created attachment 57270 [details] [review] Bug 12633: Remove export line