We must not pass $dbh but retrieve it when needed instead
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This patch updates SIP patron code to use account methods to calculate balances
over the patronflags returns. It also checks if patron should be blocked for each
'No Issues charge' preference
Tests are added for NoIssuesChargeGuarantees
To test:
1 - Set noissuescharge preference to 5
2 - Add a $10 charge to a patron
3 - perl misc/sip_cli_emulator.pl -a localhost -p 6001 -su term1 -sp term1 -l CPL -m patron_information --patron BARCODE
4 - Note the 64 message starts with Y's that mean patron is blocked
5 - Set noissuescharge to 11
6 - Repeat 3, patron is no longer blocked
7 - Set NoIssuesChargeGuarantees to 8
8 - Repeat 3, patron is blocked
9 - Pay $3 on patron so they owe 7
10 - Repeat 3, patron is not blocked
11 - Add a child account with patron as guarantor
12 - Repeat 3, patron is not blocked
13 - Add a $4 charge to child
14 - Repeat 3, patron is blocked
15 - Repeat 3, but with child barcode, child is not blocked
16 - Set NoIssuesChargeGuarantorsWithGuarantees to 10
17 - Repeat 3, patron is blocked
18 - Repeat 3 with child barcode, child is blocked
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Currently, on SIP checkout, Koha only returns "Patron Blocked" when there is
a problem with the patron.
This patch adds more specific informations, with the following messages:
- "Patron expired"
- "Patron debarred"
- "Patron has fines" (see system preference "noissuescharge")
- "Patron blocked" (see system preference "OverduesBlockCirc")
Test plan:
- Try to do a SIP checkout with a patron that is in one of the above situations.
- Check that the displayed message matches the patron's situation.
Signed-off-by: Sonia <sonia.bouis@univ-lyon3.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch removes the flag to fetch the patrons own debts as we have already
calculated them
This test plan highlights another issue - the value of NoIssuesChargeGuarantorsWithGuarantees
is not checked in SIP code - we merely get the total owed including guarantees and compares to
noissuescharge
See bug 29755
To test:
1 - Apply first patch only
2 - prove t/db_dependent/SIP/Patron.t
3 - It fails
4 - Enable NoIssuesChargeGuarantorsWithGuarantees, you can set to 1
5 - Create/find a patron
6 - Add a $10 charge to parons account
7 - Add a guarantee to patron
8 - Add a $5 charge to guarantee
9 - Set noissuescharge to 15.01
10 - perl misc/sip_cli_emulator.pl -a localhost -p 6001 -su term1 -sp term1 -l CPL -m patron_information --patron BARCODE
11 - Note the 64 message starts with Y's that mean patron is blocked
12 - Patron shouldn't be blocked as under 15.01
13 - Apply patch
14 - Restart all
15 - repeat 10
16 - Patron is no longer blocked
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
and some more...
There are lot of inconsistencies in our ->search calls. We could
simplify some of them, but not in this patch. Here we want to prevent
regressions as much as possible and so don't add unecessary changes.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
We are using Koha::Logger when it makes sense to keep the info,
otherwise we simply remove it
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Bug 28572: Replace missing occurrence in misc/admin/koha-preferences
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This reverts commit ca0e7541ec.
It was part of a patch set that removed actively used data and must be
therefore reverted.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Bug 14570 added a borrower_relationships table but it didn't remove the relationship column from the following tables:
- deletedborrowers
- borrowers
- borrower_modifications
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Caused by bug 23403 - when performing a checkin we lookup the SIP patron
using the borrowernumber, however, SIP only knows how to find a patron
via cardnumber or userid
The change on 23403 was to avoid using an 'id' that didn't always exist
(as some users don't have a userid or cardnumber
When checking in, however, we are not passed a user cardnumber or
borrowernumber, so we don't have those on hand to get the patron.
Test Plan:
1) Check in an item via SIP, note patron is not found
2) Apply this patch
3) Restart all the things!
4) Check in an item via SIP, patron should be found!
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
There are two primary issues I've identified with requesting line items
for fees via SIP:
1) The end boundary is incorrect. For example, if send a request with a
BP ( starting item) of 1, and a BQ (end item) of 1, I should get just
the first item. Instead I will get two items
2) Our SIP server does not check bounds. For example, if I have 3 fines,
but I send a BP of 1 and a BQ of 5, I will get back 5 AVs, two of
them being "empty" because the patron only has 3 accountlines!
Test Plan:
1) Apply the unit test patch
2) prove t/db_dependent/SIP/Patron.t
3) Note the failures
4) Apply the second patch
5) prove t/db_dependent/SIP/Patron.t
6) All tests should pass!
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Some libraries would like to not only block the circulation of a guarantor based on fines owed by guarantees, but would also like to block circulation for all guarantees as well. Basically, if a family as a whole reaches a certain threshold of fines, the entire family will be blocked from checking out items.
Test Plan:
1) Apply this patch
2) Set NoIssuesChargeGuarantorsWithGuarantees to $14
3) Create a family of four ( 2 guarantors, 2 guarantees ) where the parents guarantee both children
4) Give 3 of the 4 a $5 fine
5) None of them should be able to check out items
Signed-off-by: Barbara Johnson <barbara.johnson@bedfordtx.gov>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Now that we have Koha::Logger, we should use it in our SIP server. This
has the potential to make debugging SIP issue much easier. We should add
the userid for the sipuser to the namespace so we can allow for separate
files per sip user if wanted.
Test Plan:
1) Apply this patch set
2) Update the modififed log4perl.conf to your system
3) Restart your sip server
4) Tail your sip2.log, run some queries
5) Note you still get the same output messages as before, with the
addition of the ip address and username ( if available )
prefixing the message.
Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
To test:
1 - You will need to enable SIP on your testing instance
cp etc/SIPconfig.xml /etc/koha/sites/kohadev/
sudo koha-start-sip
add a user listed in the SIPconfig to your system and give them permissions (superlibrarian works)
on koha-testing-docker you should be able to start sip with user koha/koha without any adjustments
2 - If you copied the above file you should be set to get custom field DE with dateexpiry
Otherwise edit the sip login for the user to have a custom section like:
<login id="koha" password="koha" delimiter="|" error-detect="enabled" institution="kohalibrary" encoding="utf8" >
<custom_patron_field field="DE" template="[% patron.dateexpiry %]" />
</login>
3 - send a status test using the sip cli tester:
perl misc/sip_cli_emulator.pl -a 127.0.0.1 -p 6001 -su koha -sp koha -l kohalibrary --patron 23529001000463 -m patron_status_request
4 - send an information test using the sip cli tester:
perl misc/sip_cli_emulator.pl -a 127.0.0.1 -p 6001 -su koha -sp koha -l kohalibrary --patron 23529001000463 -m patron_information
5 - confirm you receive the DE field with a dateexpiry
6 - Add your own custom fields and confirm it works with several
<custom_patron_field field="EW" template="Phone: [% patron.phone %] Email: [% patron.email %]" />
7 - prove -v t/db_dependent/SIP/Patron.t
8 - prove -v t/db_dependent/SIP/
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
The drop_hold routine was using the wrong parameter for item info in the patron's
array of holds. We store it as 'barcode' not 'item_id'. This is true for both waiting and unavailable
holds
To test:
1 - Apply unit tests patch
2 - prove -v t/db_dependent/SIP/ILS.t
3 - It fails
4 - Apply second patch
5 - prove -v t/db_dependent/SIP/ILS.t
6 - It passes!
You can also see bug 24175 - you cna hack sip_cli_emulator to allow cancelling a hold
and check the messages, test coverage is improved by this patch and should be sufficient
for testing
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Caused by
commit bddfed7591
Bug 19933: Remove patronflags - tricky ones
Test plan:
1 - Setup SIP in your testing environment
https://wiki.koha-community.org/wiki/Koha_SIP2_server_setup
2 - Set a patron to have a restriction
3 - Send a patron status request or patron information command via
the SIP cli emulator (see link above)
4 - Note the comment on the restriction is sent only once
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
To test:
1 - Have sip running and use the cli to get info for a patron
example: perl misc/sip_cli_emulator.pl -a localhost -p 6001 -su koha -sp koha -l CPL -m patron_status_request --patron koha
2 - Checkout an item to the patron, backdated so it is overdue
3 - Set syspref 'OverduesBlockCirc' to anything but "Don't Block"
4 - Get the patron status via SIP, note circ blocked flag is set, but too_many_overdue is not
5 - Apply patch
6 - Restart all the things
7 - Get the patron status via SIP
8 - Note the too_many_overdue flag is now triggered and there is a message
example: READ: 24Y Y 00120200117 142716AEEdna Acosta|AA23529001000463|BLY|CQN|BV0.25|AFGreetings from Koha. -- Patron has overdues -- Patron owes 0.25|AOCPL|
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment needs to be corrected, and now applies to the whole code
paragraph following it.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This patch adds the ability to set an unlimited number of guarantors
for a given patron. As before, each guarantor may be linked to another
Koha patron, and all the behavior that applies to a given guarantor
remains the same.
Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Find some patrons with guarantors, verify the still have their guarantor
4) Test adding and removing guarantors on a patron record, both Koha users and not
5) Verify the "Add child" button works
6) Verify NoIssuesChargeGuarantees still works
7) Verify tools/cleanborrowers.pl will not delete a guarantor
8) Verify the guarantors are displayed on moremember.pl
9) Verify the guarantor is removed by members/update-child.pl
10) Verify the guarantor is removed by misc/cronjobs/j2a.pl
11) Verify import patrons converts guarantor_id, relationship, contactfirstname,
and contactsurname into a guarantor
12) prove t/Patron.t
13) prove t/db_dependent/Circulation.t
14) prove t/db_dependent/Circulation/NoIssuesChargeGuarantees.t
15) prove t/db_dependent/Items.t
16) prove t/db_dependent/Koha/Patrons.t
17) prove t/db_dependent/Members.t
18) prove t/db_dependent/Patron/Relationships.t
Signed-off-by: Kim Peine <kmpeine@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Agustin Moyano <agustinmoyano@theke.io>
Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Some libraries would like to limit the amount of personal information a SIP server sends
to arbitrary parties on a per-login basis.
Test Plan:
1) Add a new key/value pair to one of your existing login stanzas in your SIP config file
For example: hide_fields="BD,BE,BF,PB"
2) Restart SIP
3) Send a SIP message that would normally return those fields ( in this example, a Patron Information Request )
4) Note the response has had those fields removed
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Christopher Davis <tubaclarinet@protonmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Now that we have Koha::Logger, we should use it in our SIP server. This
has the potential to make debugging SIP issue much easier. We should add
the userid for the sipuser to the namespace so we can allow for separate
files per sip user if wanted.
Also modifies the log4perl.conf to lazy-open filehandles to log files,
so the same config can be used with log-files needing different
permissions.
Test Plan:
1) Apply this patch set
2) Update the modififed log4perl.conf to your system
3) Restart your sip server
4) Tail your sip2.log, run some queries
5) Note you still get the same output messages as before, with the
addition of the ip address and username ( if available )
prefixing the message.
Based on original patches by Kyle Hall and additions by Olli-Antti
Kivilahti.
Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
and avoid unecessary processing
Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com>
Tested and found correct AU data is returned, and passes QA test tool
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com>
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Misspelled variable causes _get_outstanding_holds to crash
if patron is queued on biblio level
Caused by
commit 62a075987a
Bug 21184: Replace C4::Items::GetBarcodeFromItemnumber calls
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Those calls to C4::Items::GetItemnumbersForBiblio can be replaced with
my @itemnumbers = Koha::Items->search({ biblionumber => $biblionumber})->get_column("itemnumber")
Test plan:
- Use the GetAvailability service of ILS-DI
- Try to place a hold on an item that is available and another one
- Use the batch record deletion tool to remove record with and without items.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Test plan:
1) Set up your SIP: https://wiki.koha-community.org/wiki/Koha_SIP2_server_setup
2) Run:
misc/sip_cli_emulator.pl -a localhost -p 6001 -su <login> -sp <passsword> -l <library_code> --patron <borrowernumber> -s "Y " -m patron_information
the -s parameter says what kind of additional information you want to see
3) Check the response: if the requested patron has waiting hold, there
is field "AS" with the barcode of waiting item
The right response looks like this:
64 00120180817
052434000100000000000000000000AOCPL|AA1|AEKoha Admin|BLY|CC5|AS39999000004410|PCS|PIY|AFGreetings from Koha. |
More reading ;)
http://multimedia.3m.com/mws/media/355361O/sip2-protocol.pdf
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Those calls to C4::Items::GetBarcodeFromItemnumber can be replaced with
my $barcode = Koha::Items->find($itemnumber)->barcode;
But if we are not sure that the item exists, we should test the return
of ->find before ->barcode
Test plan:
- Edit an item
- Check an item in
- Test SIP - I do not really know how to trigger that code, apparently
misc/sip_cli_emulator.pl does not deal with holds. Any ideas?
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
No need to check the pref.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Many services use SIP patron req info to validate a patron against
the koha database. If recording lastseen sites will
also want to record that the user has accessed these
library facilities although they have not logged into koha
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Verified that Patron Info request (63) updates lastseen when the pref has
been enabled.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Sounds like we do not need related fields or 'overdue' flag here.
No idea how to confirm there is no regression here.
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Here we are, patronflags is used in a couple of places where (almost) all flags
were really useful: C4::SIP::ILS::Patron->new and circulation.pl
This patch only deal with the circulation code as I am not convident
enough with SIP code.
The change does not seems trivial because of the complexity of the
existing code, but the logic is the same. We send a variable to the
template depending on the situation of the patron.
I guess only code eyes ball could catch issue in this patch quickly.
Maybe we need to find a good place in a Koha module to move this code
and provide code coverage (especially when C4::SIP::ILS::Patron will
reuse it).
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Some libraries need to be able to send additional patron data from the
extended patron attributes in made up SIP2 fields for the patron
information and patron status responses.
Test Plan:
1) Apply this patch
2) Create 3 new patron attributes with the codes CODE1, CODE2, CODE3.
Make a least one repeatable.
3) Create a patron, add those attibutes for the patron, make sure there
are at least two instances of the repeatable code
4) Edit your SIP2 config file, add the following within the login stanza:
<patron_attribute field="XX" code="CODE1" />
<patron_attribute field="XY" code="CODE2" />
<patron_attribute field="XZ" code="CODE3" />
5) Using the sip cli emulator, run patron_information and
patron_status_request messages for the patron
6) Note the values you set for the patron attributes are sent in the
corrosponding fields!
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Daniel Mauchley <dmauchley@duchesne.utah.gov>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Amended: added parentheses on line 488 when assigning hashref to array.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Replaces "or" with "||" in variable assignment
second attempt to retrieve borrower was not being executed due
to changed precedence
Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
The GetMemberAccountRecords may be a perf killer, it retrieves all the
account lines of a patron and then the related item and biblio
information.
Most of the time we only want to know how much the patron owns to the
library (sum of amountoutstanding). We already have this information in
Koha::Patron->account->balance.
This patch replaces the occurrences of this subroutine by fetching only
the information we need, either the balance, the detail, or both.
It removes the formatting done in the module, to use the TT plugin
'Price' instead.
There is a very weird and error-prone behavior/feature in
GetMemberAccountBalance (FIXME): as the accountlines.accounttype is a
varchar(5), the value of the authorised value used for the
ManInvInNoissuesCharge pref (category MANUAL_INV) is truncated to the 5
first characters. That could lead to unexpected behaviors.
On the way, this patchset also replace the GetMemberAccountBalance
subroutine, which returns the balance, the non issues charges and the
other charges. We only need to have the balance and the non issues
charges to calcul the third one.
Test plan:
Add several fees for a patron and play with HoldsInNoissuesCharge,
RentalsInNoissuesCharge and ManInvInNoissuesCharge.
The information (biblio and item info, as well as the account line) must
be correctly displayed on the different screens: 'Fines' module, fine
slips, circulation module
Note that this patchset could introduce regression on price formatting,
but will be easy to fix using the TT plugin.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
To test:
1 - Set 'OverduesBlockCirc' to block
2 - Find or create a patron with overdues
3 - Perform a SIP patron lookup on that patron
misc/sip_cli_emulator.pl -a 127.0.0.1 -p 6001 -su term1 -sp term1 -l CPL
--patron {userid or cardnumber} --password {pass} -m patron_information
4 - Note the first character of response is a ' '
5 - Apply patch
6 - Restart memcached, apache, and plack
7 - Perform SIP patron lookup
8 - Note the first character of response is 'Y'
9 - prove t/db_dependent/SIP/Patron.t
10 - Test should return green
Signed-off-by: Chris Kirby <chris.kirby@ilsleypubliclibrary.org>
Works as advertised
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>
Koha's SIP2 server sends the patron's name in the format "Firstname
Surname" which is not very good for machine reading. We need to allow
the format of the patron name to be customized in a manner similar to
what is done with the DA field on bug 16755.
Test Plan:
1) Apply this patch, start or restart your SIP server
2) Find a patron with a first and last name
3) Send a patron information request via the sip2 cli tool
4) Note the AE field has the format "<firstname> <surname>" ( i.e. the current behavior )
5) Add this parameter to the login stanza you are using:
ae_field_template="[% patron.surname %][% IF patron.firstname %], [% patron.firstname %][% END %]"
6) Restart your SIP server
7) Repeat step 3
8) Note the AE field now has the format "<surname>, <firstname>"
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Benjamin Daeuber <BDaeuber@cityoffargo.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch replace the different calls to GetReservesFromBorrowernumber
with a calls to Koha::Patron->get_holds.
In some places we need to get a restricted set of holds, that's why we
process a search on this holds returned by ->get_holds (on the found
status for instance).
The changes are quite trivial and reading the diff should be enough to
catch bugs.
Test plan:
I would suggest to test this patch with patches from bug 17736 and bug 17737,
to place different kind of holds (biblio and item level, future and
past).
Then do a whole workflow to detect bug, view a record, delete record,
order, place a hold on an item which has been ordered, etc.
The hold's informations should always be the same without or without
these patches.
Tested both patches together, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
All the values different from the ones GetMember returned has been
managed outside of GetMemberDetails.
It looks safe to replace all the occurrences of GetMemberDetails with
GetMember.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Same as authflags, a flags key is set containing all the patron flags.
It is only used in a few places and it's better to call
C4::Members::patronflags when we need it.
Test plan:
Look at the diff and confirm that the change make sense
Use git grep to confirm we do not use the flags somewhere else.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This is the fourth and last patch set to remove C4::Branch.
The real purpose of this patch is to standardise and refactor some code
which is related to the libraries selection/display.
Its unconfessed purpose is to remove the C4::Branch package.
Before this patch set, only 6 subroutines still existed in the C4::Branch
package:
- GetBranchName
- GetBranchesLoop
- mybranch
- onlymine
- GetBranches
- GetBranch
GetBranchName basically returns the branchname for a given branchcode.
The branchname is only used for a display purpose and we don't need to
retrieve it in package or pl scripts (unless for a few exceptions).
We have a `Branches` template plugin with a `GetName` method which does
exactly this job.
To achieve this removal, we will use this template plugin and delete the
GetBranchName from pl and pm files.
The `Branches.all()` will now select the library of the logged in user
if no `selected` parameter has been passed.
This new behavior could cause regressions, for instance there are some
places where we do not want an option preselected (batch item
modification for instance), keep that in mind when testing.
GetBranchesLoop took 3 parameters: $branch and $onlymine.
The first one was used to set a "selected" flag, for a display purpose:
select an option in the libraries dropdown lists.
The second one was useless: If not passed or set to 0, the
`C4::Branch::onlymine` subroutine was called.
This onlymine flag was use to know if the logged in user was able to see
other libraries infos.
A patron can see the infos from other libraries if IndependentBranches
is not set OR if he has the superlibrarian permission.
Prior to this patch set, the "onlymine test" was done on different
places (neworderempty.pl, additem.pl, holidays.pl, etc.), including the
Branches TT plugin. In this patch set, this test is only done on one
place (C4::Context::only_my_library, code moved from
C4::Branch::onlymine).
To accomplish the same job as this subroutine, we just need to call the
`Branches.all()` method from the `Branches` TT plugin. It already
accepts a `selected` parameter to set a flag on the option to select.
To avoid the repetitive
[% IF selected %]<option selected="selected">[% ELSE %]<option>[% END %]
pattern, a new `html_helpers` TT include file has been created, it
defines an `options_for_libraries` block, which takes a `selected`
parameter. We could imagine to use this include file for other
selects.
The 'mybranch` and `onlymine` subroutines of the C4::Branch package have
been moved to C4::Context. onlymine has been renamed with
only_my_library. There are only 4 occurrences of it, against 11 before
this patch set.
There 2 subroutines are Context-centric and it makes sense to put them
in `C4::Context` (at least it's the least worst place!)
GetBranches is the tricky part of this patch set: It retrieves all the
libraries, independently of the value of IndependentBranches.
To keep the same way as the existing calls of `Branches.all()`, I have
added a `unfiltered` parameter. If set, the `Branches.all()` will call
a usual Koha::Libraries->search method, otherwise
Koha::Libraries->search_filtered will be called. This new method will
check if the logged in user is allowed to see other libraries or only
its library.
Note that this `GetBranches` subroutine also created a `category` key:
it allowed to get the list of groups (of libraries) where this library
existed. Thanks to a previous patch set (bug 15295), this value was
not used anymore (I may have missed something!).
Note that the only use of `GetBranch` was buggy (see bug 15746).
Test plan (for the whole patch set):
The best way to test this whole patch set is to test with 2 instances: 1
with the patch set applied, 1 using master, to be sure there is no
regression.
It would be good to test the same with `IndependentBranches` and the
without `IndependentBranches`.
No difference should be found.
The tester must focus on the library dropdowns on as many forms as
possible.
You will notice changes in the order of the options: the libraries will
now be ordered by branchname (instead of branchcode in some places).
A special attention will be given to the following page:
- acqui/neworderempty.pl
- catalogue/search.pl
- members/members-home.pl (header?)
- opac/opac-topissues.pl
- tools/holidays.pl
- admin/branch_transfer_limits.pl
- admin/item_circulation_alerts.pl
- rotating_collections/transferCollection.pl
- suggestion/suggestion.pl
- tools/export.pl
Notes for QA:
- There are 2 FIXMEs in the patch set, I have kept the existing behavior,
but I am not sure it's the good one. Feel free to open a bug report and
I will fill a patch if you think it's not correct. Otherwise, remove the
FIXME lines in a follow-up patch.
- The whole patch set is huge and makes a lot of changes.
But it finally will tremendously reduce the number of lines:
716 insertions for 1910 deletions
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 14507 introduced the use of checkpw in C4::SIP::ILS::Patron so that
non-Koha internal authentication processes would be able to function via
SIP ( LDAP et al ).
The problem is that checkpw changes the userenv to that of the patron!
This is not usually an issue in Koha because most of the time that
patron running through checkpw is the one to be logged in.
Aside from SIP2 the only other area where this may be an issue is in SCO
when using SelfCheckoutByLogin.
Test Plan:
1) On master, check out an item to a patron via SIP2
2) Note the checkout lists the item as having been checked out
from the patron's home library not matter which library is was
supposed to be checked out from.
3) Apply this patch
4) Re-checkout the item
5) The item should now be checked out as if it was checked out from
the library as defined in the SIP configuration file.
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Previous to bug 14507, SIP2 only did internal authentication. A change
to the way we check empty passwords has caused any empty password to
send back a CQ of Y. Previous to that patch set, a CQ of Y would only be
sent back of the patron password column was NULL. Now, an empty AD field
*always* returns a CQ of Y.
Test Plan:
1) Send a patron information request with an empty AD field
Note: You must send the AD field or you won't get back a CQ field
2) Note you get back a CQ of Y
3) Apply this patch
4) Repeat step 1
5) Note you now get back a CQ of N
Signed-off-by: Trent Roby <troby@bclib.info>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Jonathan Druart
Nick Clemens
Matthias Meusburger
Joonas Kylmälä
Kyle M Hall
Marcel de Rooy
David Gustafsson
Josef Moravec
Colin Campbell
Kyle M Hall
Jose Martin