Test Plan:
1) Apply this patch
2) Run updatedatabase
3) Create a new patron
4) Note the new column date_renewed is NULL
5) Renew the patron
6) Note the date in the column date_renewed is today's date
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
If the patron categories J, K, YA would not exist, Patrons.t would fail.
Test plan:
[1] Remove one of these patron categories.
[2] Run t/db_dependent/Koha/Patrons.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The test files do not need to return 1
Patch generated with:
perl -p -i -e "s/^1;\n//xsm" t/**/*.t
Test plan:
git grep '^1;$' t/**/*.t
should not return any results
NOTE: does not fix C4/SIP/t, nor xt tests.
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Test plan:
prove -v t/db_dependent/Koha/Patrons.t
Subtest: renew_account
1..30
ok 1 - 2016-02-29T00:00:00 + 12 months must be 2017-02-28T00:00:00
ok 2 - 2016-02-29T00:00:00 + 12 months must be 2017-02-28T00:00:00
ok 3 - With BorrowerLogs, Koha::Patron->renew_account should have logged
ok 4 - today + 12 months must be 2017-03-31T00:00:00
ok 5 - today + 12 months must be 2017-03-31T00:00:00
ok 6 - Without BorrowerLogs, Koha::Patron->renew_account should not have logged
ok 7 - today + 12 months must be 2017-03-31T00:00:00
ok 8 - today + 12 months must be 2017-03-31T00:00:00
ok 9 - 2016-04-30T00:00:00 + 12 months must be 2017-04-30T00:00:00
ok 10 - 2016-04-30T00:00:00 + 12 months must be 2017-04-30T00:00:00
ok 11 - 2016-10-30T00:00:00 + 12 months must be 2017-10-30T00:00:00
ok 12 - 2016-10-30T00:00:00 + 12 months must be 2017-10-30T00:00:00
ok 13 - With BorrowerLogs, Koha::Patron->renew_account should have logged
ok 14 - today + 12 months must be 2017-11-30T00:00:00
ok 15 - today + 12 months must be 2017-11-30T00:00:00
ok 16 - Without BorrowerLogs, Koha::Patron->renew_account should not have logged
ok 17 - today + 12 months must be 2017-11-30T00:00:00
ok 18 - today + 12 months must be 2017-11-30T00:00:00
ok 19 - 2016-12-30T00:00:00 + 12 months must be 2017-12-30T00:00:00
ok 20 - 2016-12-30T00:00:00 + 12 months must be 2017-12-30T00:00:00
ok 21 - 2017-06-30T00:00:00 + 12 months must be 2018-06-30T00:00:00
ok 22 - 2017-06-30T00:00:00 + 12 months must be 2018-06-30T00:00:00
ok 23 - With BorrowerLogs, Koha::Patron->renew_account should have logged
ok 24 - today + 12 months must be 2018-07-31T00:00:00
ok 25 - today + 12 months must be 2018-07-31T00:00:00
ok 26 - Without BorrowerLogs, Koha::Patron->renew_account should not have logged
ok 27 - today + 12 months must be 2018-07-31T00:00:00
ok 28 - today + 12 months must be 2018-07-31T00:00:00
ok 29 - 2017-08-31T00:00:00 + 12 months must be 2018-08-31T00:00:00
ok 30 - 2017-08-31T00:00:00 + 12 months must be 2018-08-31T00:00:00
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
From DateTime::Duration pod:
""
For positive durations, the "end_of_month" parameter defaults to wrap.
For negative durations, the default is "limit". This should match how
most people "intuitively" expect datetime math to work.
""""
We need end_of_month => limit for positive durations as well.
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Add problematic cases to highlight the problem.
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
GetMember returned a patron given a borrowernumber, cardnumber or
userid.
All of these 3 attributes are defined as a unique key at the DB level
and so we can use Koha::Patrons->find to replace this subroutine.
Additionaly GetMember set category_type and description.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Jonathan Field <jonathan.field@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Updating to use they/them and skipping the ones changed to it
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comments throughout the Koha codebase assume that
all librarians or borrowers are male by using the
pronoun 'he' universally. This patch changes to
'he or she' / 'him or hers'.
Testing plan:
- ensuring modifying tests still pass:
+ C4/SIP/t/06patron_enable.t
+ t/db_dependent/Circulation.t
+ t/db_dependent/Koha/Patrons.t
+ t/db_dependent/Reserves.t
Sponsored-By: California College of the Arts
No code changes detected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
From opac-privacy.pl:
# delete all reading records for items returned
# uses a hardcoded date ridiculously far in the future
my $rows = eval {
Koha::Patrons->search({ 'me.borrowernumber' => $borrowernumber })->anonymise_issue_history( { before => '2999-12-12' } );
};
It sounds better to make this before parameter not mandatory, and remove the condition from the sql query if it is not passed.
Test plan:
1. Anonymise your reading history at the OPAC.
2. Confirm that all your reading history has been anonymised
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
The next patch will move C4::Circulation::AnonymiseIssueHistory and
C4::Members::GetBorrowersWithIssuesHistoryOlderThan to Koha::Patrons
This patch move the history anonymisation code to the Patrons.t test
file and the entire subtest related to StoreLastBorrower to
StoreLastBorrower.t
It just moves and add some minor adjustements.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Without this patch get_age return actually 0 instead of
Signed-off-by: Dobrica Pavlinusic <dpavlin@rot13.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Fix tests
prove t/db_dependent/Koha/Patrons.t
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
The goal of this method is to replace
C4::Reserves::GetReservesFromBorrowernumber but could be reused for
something else, that's why it has its own bug report.
Test plan:
prove t/db_dependent/Koha/Patrons.t
should return green
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Rebased. Edited two test descriptions.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Patron.pm: Adds two missing semicolons at the last statement. Not strictly
needed, but strongly recommended.
Patrons.t: Add a test description, remove two comments that refer to
previously hardcoded dates.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Test plan:
prove t/db_dependent/Koha/Patrons.t
should return green
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Koha::Issues and Koha::Checkouts have been added to the codebase to
represent the same thing.
In ODLIS the word Issue is never used in the sense we use it. Another
problem with Issue is it has so many meaning in English (such as
problem/bug)
The word Checkout *is* in ODLIS, closer to what we use:
http://www.abc-clio.com/ODLIS/odlis_c.aspx#checkoutslip
Test plan:
git grep Koha::Issue
should not return any occurrences and the tests must still pass
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Test plan:
prove t/db_dependent/Koha/Patrons.t
should return green
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This method will be convenient when moving code to Koha::Patrons
Test plan:
prove t/db_dependent/Koha/Patrons.t
should return green
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
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>
This method will be used by several patches later.
Test plan:
prove t/db_dependent/Koha/Patrons.t
should return green
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Number of tests in Patrons.t corrected.
Method is_going_to_expired (no english!) renamed to is_going_to_expire.
Adding a negative duration replaced by a subtract. Reads easier.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
In order to be consistent, we need to create this method as well.
Test plan:
Make sure the pref NotifyBorrowerDeparture works as expected
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Without this patch, the tests are not testing the same things as what
happens on the interface.
We need to refresh the object to make sure the date set into dateexpiry
is the one in DB.
Without this patch, ->is_expired test a datetime object, with this patch
it compares with a date oject
Without the changes made in Koha::Patron->is_expired, a patron which has
a dateexpiry set to today was marked as expired on the interface. This
is a change in the behavior, what this refactoring does not want to do.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This new method will be handy for further uses
Test plan:
prove t/db_dependent/Koha/Patrons.t
should return green
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
We need to use the DBIx::Class relationship to retrieve the patron
category.
It is more convenient to have a Koha::Patron->category method to
retrieve the category of a given patron.
Test plan:
Make sure that the tests in t/db_dependent/Koha/Patron* return green
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Test plan:
prove t/db_dependent/Koha/Patrons.t
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This job should be done each time patron data are deleted. It's better
to do it just before deleting the patron than assuming the caller did
the job by itself.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch moves the C4::Members::DelMember subroutine to the
Koha::Patron module.
The delete method must be overwritten to permit handling of patron's
holds.
Test plan:
(With the 2 patches applied)
1/ Create a patron with holds and owner of lists
2/ Delete patrons using the web interface:
- More > Delete on a patron page
- Batch patron deletion tools
3/ and the cronjob script
- perl misc/cronjobs/delete_patrons.pl -c [more options]
The patron should have been moved to the deletedborrowers table, his/her
holds and lists should have been deleted.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch removes the C4::Members::MoveMemberToDeleted subroutine in
order to replace it with the Koha::Patron->move_to_deleted method.
Next after this change, we will move C4::Members::HandleDelBorrower and
C4::Members::DelMember to the same module to simplify the code in
members/deletemem.pl and misc/cronjobs/delete_patrons.pl
Test plan:
1/ Delete a patron from the staff interface and make sure (s)he has been moved to
the deletedborrowers table.
2/ Use the "Batch patron deletion" tool (tools/cleanborrowers.pl) to
remove patron. Make sure the "Permanently delete these patrons" and "Move
these patrons to the trash" options work as before
3/ Same as previously but using the cronjob
misc/cronjobs/delete_patrons.pl.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Tested the delete_patrons.pl script and cleanborrowers.pl too.
Tests (are relevant and) pass and the qa scripts are happy too :-D
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This patch just move C4::Members::HasOverdues to
Koha::Patron->has_overdues and updated callers
Test plan:
No change in behavior is expected.
1/ If a patron is debarred and does not have overdues and
AutoRemoveOverduesRestrictions is on, the debarment will be removed on
checkin
2/ Add overdues and make sure the patron cannot renew
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch moves the code from C4::Members::changepassword to
Koha::Patron->update_password
Test plan:
Change your password at the OPAC and the staff interface
This should work as before
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
I rebased this on top of 16849 because they were conflicting.
Tests pass, code looks good (as usual) and I checked both OPAC
and staff password change work as expected.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Note:
QA question: Does the Koha::Patron->siblings method should return undef
if there is no guarantor?
It would avoid the weird != undef, = $borrowernumber conditions.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
2 subroutines of C4::Members deal with guarantor/guarantees:
GetGuarantees and GetMemberRelatives.
Since we already have a Koha::Patron->guarantor method, it makes sense
to move these 2 subroutines to this module.
This first patch deals with GetGuarantees.
Test plan for the entire patch set:
1/ Create 5 patrons A (adult), B (child), C (child), D (child), E
(child), F (adult)
2/ Add relation between them: A is father of B, C and D.
E does not have a guarantor
F does not have guarantees
3/ Check some items out for all of these patrons
4/ On the "Check out" and "Details" tabs, you should not see any
differences with these patch applied : The "Relatives' checkouts" tabs
should list all of the guarantor/guarantee/siblings checkouts
Note:
$template->param('C' => 1);
I have not found any reference of this 'C' in the template.
It seems it's an old c/p from members/memberentrygen.tt
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com