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>
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>
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>
== Test plan ==
1 - Have two patrons with userids and no cardnumber
2 - Note which of these has the higher borrower number
3 - Use the SIP cli emulator to connect and checkout a book to the patron with higher borrowernumber
See example after
4 - Note the book may checkout to the wrong patron!
5 - Apply patch
6 - Checkout to both patrons via sip
7 - The patrons get the correct checkouts
=== SIP CLI emulator ===
./misc/sip_cli_emulator.pl -a 127.0.0.1 -p 6001 -su term1 -sp term1 \
-l CPL --patron 23529001000463 -m checkout --item 39999000001259
translation: via the koha user term1, checkout item 39999000001259 to
patron 23529001000463
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
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>
* We should not call Log::Log4perl directly
* Not sure it is correct as I get from (comment 77):
% koha-sip --restart
[2020/04/23 11:23:27] [ERROR] [undef]@[undef]: Argument "0.33_01" isn't
numeric in numeric lt (<) at /usr/share/perl5/Net/Server/Log/Sys/Syslog.pm
line 42.
C4::SIP::Trapper::PRINT /kohadevbox/koha/C4/SIP/Trapper.pm (24)
Why "ERROR" when it's a warning?
The [undef]@[undef] seems wrong here.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
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>
This patch fixes 2 other occurrences. The first one is in POD of
AddReserve, the other one fixes SIP code
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
At some point some calls to maybe_add got an extra $server var in their
parameter lists. This doubled parameter does nothing and should be
removed.
Test Plan: No change in behavior should be noted
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Even if a library allows returns of lost items, the SIP server returns the error message "Item lost, return not allowed" if the checkin was not ok for any reason other than it being withdrawn ( and withdrawn items not being returnable ).
The most clear example of this is that when a lost item is not checked out to a patron and is returned. SIP returns that message even though lost items *can* be returned. The actual problem being that the item was not checked out.
Test Plan:
1) Ensure you can return lost items
2) Mark an item as lost
3) Check it in via SIP
4) Note the message you get back is "Item lost, return not allowed"
5) Apply this patch
6) Restart your SIP server
7) Repeat steps 2 and 3
8) Note you no longer get the incorrect message!
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
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>
It is used in list context, but we need a scalar value.
Can be fixed by adding scalar's, or returning empty string as here.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
See bug 24800 comment 0 for a description of the problem.
We do not want the SIP server to crash if it receives a checkin request
with a return date that is not given.
The option this patch chose is to parse it only if provided.
Signed-off-by: Clemens Elmlinger <clemens.elmlinger@bsz-bw.de>
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>
It appears that a debugging statement was accidentally left in FeePayment.pm by bug 5605.
Signed-off-by: Devinim <kohadevinim@devinim.com.tr>
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 ablity to specify a cash register id to link to
payments taken via SIP2 clients.
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>
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>
If UpdateItemLocationOnCheckin, it triggers an alert flag but cannot be acted on in the same way as LocalUse and thus should be removed in the same fashion.
Test Plan:
1) Enable UpdateItemLocationOnCheckin and set it up to trigger at
checkin for an item
2) Return that item via SIP2
3) Note the alert flag is set
4) Apply this patch
5) Repeat steps 1 and 2
6) The alert flag should not be set
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Kevin Cook <ILS@cedarparktexas.gov>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This patch adds handling to catch transfers that may have been triggered
on item return.
Test Plan:
1) Enable AutomaticItemReturn and accompanying system preference
2) Return an item to the wrong branch and note the transfer
3) Check the database for the new trigger value
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Rebasing was a nightmare, so I'm squashing the sign off follow-ups to
ease the pain with any future rebases
Includes:
Bug 23051: (follow-up) Refactor renewal code
As per Nick's first point in comment #20, the code that tests for
renewability and renews items has been refactored into it's own
function.
Bug 23051: (follow-up) Provide feedback
For renewals that fail when a fine is being paid off, this patch causes
any errors to be passed back to the template for display.
Addresses the second point in Nick's comment #20
Bug 23051: (follow-up) Fix unit tests
As raised by Nick in comment #35
Bug 23051: (follow-up) Fix/improve feedback
This follow up patch addresses the following parts of Nick's feedback in
comment #35:
- it would be nice to get feedback on what was successfully renewed as well
- In general I think I would prefer to see 'ok' and 'not_ok' returned as
a single 'renewal_results' array
- There is no listing of errors if I use the 'pay' button on an
individual fine
Bug 23051: (follow-up) Refactor methods
This follow up patch addresses the following parts of Nick's feedback in
comment #35:
- I don't really like that the functions are internal functions and then
exported
- I think the pref description should highlight that if 'RenewalPeriodBase'
is set to due date, there may be doubled charges
Bug 23051: (follow-up) Add SIP summary
This follow up patch addresses the following parts of Nick's feedback in
comment #35:
- Ideally SIP would get feedback in a screen message
Bug 23051: (follow-up) Renewing in OPAC
This follow up patch addresses the following parts of Nick's feedback in
comment #35:
- I am also not sure about the code path if a patron paid fines on the
opac (via paypal etc.) but renewals are not allowed on the opac.
We've introduced the syspref RenewAccruingItemInOpac (default is off)
which, when enabled, will cause items attached to fines that are paid
off in the OPAC (via payment plugins), to be automatically renewed.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
$patron->{items} contains the borrowers checkouts as an array of hashes
{ barcode => $item->barcode }
When printing to log we assumed these were only barcodes
This patch pushes the current checkout as a hash and maps the values retrieved to a string
To test:
1 - Enable SIP debug mode
2 - Perform multiple checkouts for a patron
3 - Note the messages like:
koha koha_sip_koha[13575]: ILS::Checkout: patron 123 has checked out HASH(0x55a5b187f858), HASH(0x55a5b1896ad0), HASH(0x55a5b18a6cf0), 7826832
4 - Apply patch
5 - Restart all the things
6 - Do some checkouts via SIP
6 - Messages should now have barcodes
Signed-off-by: Magnus Enger <magnus@libriotech.no>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
The SIP server does not calculate and pass a priority to AddReserve, which causes the hold to be given priority 1.
Test Plan:
1) Place a hold via SIP for a record with existing holds
2) Note the new hold is top priority
3) Apply this patch
4) Restart SIP
5) Repeat step 1
6) New hold should be last priority
Signed-off-by: Christofer Zorn <Christofer.Zorn@ajaxlibrary.ca>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
The number of parameters of AddReserve makes it hard to read and
maintain.
This patch replace it with a hashref, which will make the calls more
readable.
Moreover the bibitems has been removed as it was not used by the
subroutine.
Test plan:
- Make sure the tests pass
- Read the diff and search for typos
- Place a hold on few items
Note for QA: reservation_date and expiration_date do not match the DB column's names,
should we?
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This patch flushes the L1 cache on each SIP connection
(ie every invocation of process_request).
This means each SIP connection will fetch values from the
L2 cache (ie memcached) and persist the L1 cache (in Perl
memory cache) only until a new SIP connection comes in.
Without this patch, the L1 cache persists for the length
of the server process, which means the L1 cache will become
stale very quickly, which can lead to unexpected behaviour.
Test plan:
1. Enable IssueLog
2. Start SIP server
2a. change to Koha git directory
2b. koha-shell kohadev
2c. perl C4/SIP/SIPServer.pm /etc/koha/sites/kohadev/SIPconfig.xml
3. Issue book via SIP
perl misc/sip_cli_emulator.pl -a 127.0.0.1 -p 6001 -su term1 -sp term1 -l CPL --patron 1 --item 3999900000001 -m checkout
4. Check action_logs for "CIRCULATION ISSUE <borrowernumber> <biblionumber>" line
http://localhost:8081/cgi-bin/koha/tools/viewlog.pl
5. Disable IssueLog
6. Check in the book and then check it out again
perl misc/sip_cli_emulator.pl -a 127.0.0.1 -p 6001 -su term1 -sp term1 -l CPL --patron 1 --item 3999900000001 -m checkin
perl misc/sip_cli_emulator.pl -a 127.0.0.1 -p 6001 -su term1 -sp term1 -l CPL --patron 1 --item 3999900000001 -m checkout
7. Check action_logs for "CIRCULATION ISSUE <borrowernumber> <biblionumber>" line
http://localhost:8081/cgi-bin/koha/tools/viewlog.pl
8. Note results
Without the patch, you'll see a 2nd checkout.
With the patch, you won't see the 2nd checkout.
(Note: After applying the patch, you have to restart the SIP server.)
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Nick Clemens <nick@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>
and remove 'scalar' keyword in calls where it's not needed.
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
To test:
1 - place a hold for a patron
2 - Get SIP running
https://wiki.koha-community.org/wiki/Koha_SIP2_server_setup
3 - edit the sip_cli_emulator.pl for hold_mode from '+' to '-'
4 - Attempt to cancel hold
5 - It fails
6 - Apply patch
7 - Try again
8 - It succeeds
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
The system preference AllowItemsOnHoldCheckout only affects SIP based checkouts. I believe we should rename it AllowItemsOnHoldCheckoutSIP so it matches AllowItemsOnHoldCheckoutSCO. We should also update the description as well.
As far as I know AllowItemsOnHoldCheckout has never affected anything except SIP traffic. That does not preclude the addition of another AllowItemsOnHoldCheckout syspref that would affect the staff interface, but that would be an enhancement and is outside the scope of this bug report.
Test Plan:
1) Test AllowItemsOnHoldCheckout via SIP
2) Apply this patch
3) Test AllowItemsOnHoldCheckoutSIP via SIP
4) Note there is no difference!
5) Note the syspref description has been update to mention SIP
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
borrowernumber is not an accessor for C4::SIP::ILS::Patron, access is
via hashref
Add ability to specify the pickup location using a command line
parameter
Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: frederik chenier <frederik.chenier@inlibro.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
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>
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>
We should be able to set system preference overrides for SIP in a
similar manner that we do in Apache. It would be great if we could
specify those overrides on both a config level, and login level basis.
Test Plan:
1) Apply this patch
2) Start your SIP server
3) Enable the syspref AllFinesNeedOverride
4) Find or create a patron with a small fine ( less than noissuescharge )
5) Attempt to check out an item to the patron, it should fail
6) Add the global syspref override from the bottom of the example SIP config file
7) Restart your SIP server
8) Attempt to check out an item to the patron again, this time it should work
9) Now, add the login level syspref override section as it appears in
the eaxmple SIP config file. Make sure to add it to the login you are using
10) Attempt to check out another item to the patron, this time is should
again fail
Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
The SIP2 interface has long used a set of accounttypes to denote payment
types. Now we have an authorised list of payment_types and a
payment_type field in accountlines this patch alters the logic to use
it.
Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Matha Fuerst <mfuerst@hmcpl.org>
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: Matha Fuerst <mfuerst@hmcpl.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This patch fixes a bug from Bug 15221 and hopefully also makes it so that the alert flag
is suppressed when checked_in_ok is set and an item is not checked out.
To test:
0) Create patron in web interface with a cardnumber and userid of "staff" with a password
that matches the account in SIPconfig.xml. Also set their branch to CPL
(also matching SIPconfig.xml).
1) Create an item with a barcode of 'test'
2) Choose a patron to check out to and record their borrowernumber
In one terminal:
3) cd to your git directory (e.g. /home/koha/koha)
4) perl ./C4/SIP/SIPServer.pm ~/koha-dev/etc/SIPconfig.xml
In another terminal:
5) cd to your git directory (e.g. /home/koha/koha)
6) perl ./misc/sip_cli_emulator.pl -l CPL -su staff -sp <password> --port=6001 --address=localhost --item test -m checkin --patron <borrowernumber>
NOTE: You need to replace <borrowernumber> with the borrowernumber from Step 2, and <password> with the password from Step 0.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Matha Fuerst <mfuerst@hmcpl.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
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>
If CheckPrevCheckout is set to Do, then checkouts are blocked at the SIPServer
To test:
- Do not apply this patch
- Set CheckPrevCheckout to "Do", or "Unless overridden, do"
Verify the SIP server is blocking checkouts with this setting set
- Apply this patch
- Note that the checkout is now allowed, with a screen message telling
the borrower they've issued the item before.
Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
The checked in response from the ACS always has the alert bit set to
true, even when using the checked_in_ok option.
This patch sets the alert bit to 0 when checked_in_ok is used and
the item is checked in without a preceding checkout.
To test:
0) Create patron in web interface with a cardnumber and userid of "staff" with a password
that matches the account in SIPconfig.xml. Also set their branch to CPL
(also matching SIPconfig.xml).
1) Create an item with a barcode of 'test'
2) Choose a patron to check out to and record their borrowernumber
In one terminal:
3) cd to your git directory (e.g. /home/koha/koha)
4) perl ./C4/SIP/SIPServer.pm ~/koha-dev/etc/SIPconfig.xml
In another terminal:
5) cd to your git directory (e.g. /home/koha/koha)
6) perl ./misc/sip_cli_emulator.pl -l CPL -su staff -sp <password> --port=6001 --address=localhost --item test -m checkin --patron <borrowernumber>
NOTE: You need to replace <borrowernumber> with the borrowernumber from Step 2, and <password> with the password from Step 0.
Before patch:
koha@9ec653ab82ce:~/koha$ perl ./misc/sip_cli_emulator.pl -l CPL -su staff -sp <password> --port=6001 --address=localhost --item test -m checkin --patron <borrowernumber>
Attempting socket connection to localhost:6001...connected!
SEND: 9300CNstaff|COStaff1|CPCPL|
READ: 941
Trying 'checkin'
SEND: 09N20190521 00310320190521 003103APCPL|AOCPL|ABtest|AC<password>|BIN|
READ: 101YNY20190521 003106AOCPL|ABtest|AQCPL|AJTest|
After patch:
koha@9ec653ab82ce:~/koha$ perl ./misc/sip_cli_emulator.pl -l CPL -su staff -sp <password> --port=6001 --address=localhost --item test -m checkin --patron <borrowernumber>
Attempting socket connection to localhost:6001...connected!
SEND: 9300CNstaff|CO<password>|CPCPL|
READ: 941
Trying 'checkin'
SEND: 09N20190521 01000020190521 010000APCPL|AOCPL|ABtest|AC<password>|BIN|
READ: 101YNN20190521 010004AOCPL|ABtest|AQCPL|AJTest|
Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
The CM field was all set up for implementation, but never completed.
We should fully implement the CM ( Hold Pickup Date ), which according
to the SIP2 protocol specification is "The date that the hold expires".
Test Plan:
1) Set up a waiting hold for an item
2) Using the sip cli emulator, send an item information request for that item
3) Note the CM field does not show up
4) Apply this patch
5) Restart all the things!
6) Repeat the item information request
7) Note the CM field is now transmitted!
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>
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>
This patch adds the 'interface' field to the accountlines table and
updates all Koha::Object routines and calls to use it.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Rescued-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Geeta Halley <ghalley@roundrocktexas.gov>
Rescued-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
If an item that is withdrawn is checked in via SIP2, Koha does not process the checkin, but still indicates the checkin succeeded via the ok flag in the checkin response message.
To recreate:
1 - Mark an item withdrawn
2 - Set up sip server:
https://wiki.koha-community.org/wiki/Koha_SIP2_server_setup
3 - Set 'BlockReturnOfWithdrawnItems' to block
4 - Use the sip_cli_emulator to checkin the withdrawn item
5 - There is no error in the checkin
Test Plan:
1) Mark an item withdrawn
2) Set BlockReturnOfWithdrawnItems to block
3) Use the sip cli emulator to checkin the withdrawn item
4) Note the response begins with 101
5) Apply this patch
6) Restart all the things ( including SIP server )
7) Repeat checkin with sip cli emulator
8) Note the checkin now beings with 100
Signed-off-by: Geeta Halley <ghalley@roundrocktexas.gov>
Rescued-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Branch transfer limits are respected for placing holds in the OPAC but nowhere else. This should be remedied.
Test Plan:
1) Set up a branch transfer limit from Library A to Library B
2) Verify you cannot set up a hold for an item from Library A for pickup at Library B from the staff interface ( without overriding )
3) Verify you cannot place that hold via ILS-DI
4) Verify you cannot place that hold via SIP
4) Verify a forced hold from Library A to Library B will not show up in the holds queue
Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
If a checkin has any messages at all, even informational like LocalUse, the alert flag ( 4th character of the response ) will be set to Y, even if we do not prove a corresponding alert code in the CV field. We should only set the alert flag if we are going to send a value in the CV field.If a checkin has any messages at all, even informational like LocalUse, the alert flag ( 4th character of the response ) will be set to Y, even if we do not prove a corresponding alert code in the CV field. We should only set the alert flag if we are going to send a value in the CV field.
Test Plan:
1) Set the checked_in_ok sip account flag to 1 for an account
2) Check in an already checked in item via SIP
3) Note the alert flag is set to Y
4) Apply this patch
5) Restart SIP
6) Check the item in again
7) Note the alert flag is set to N
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jill Kleven <jill.kleven@pueblolibrary.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Some SIP devices ( in this particular case, bin sorting machines from RFID Library Solutions ) require a CT field to be sent, even if that field is empty. Koha should be able to support this behavior.
Test Plan:
1) Apply this patch
2) Enable the new option ct_always_send for a SIP2 account
3) Restart SIP
4) Check in an item successfully via SIP, which will not be transferred
5) Note the response contains a CT field with no value
Sponsored-by: Pueblo City-County Library District
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jill Kleven <jill.kleven@pueblolibrary.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Some SIP devices ( in this particular case, bin sorting machines from RFID Library Solutions ) require a checkin success to return a CV field of the value "00" rather than no CV field at all. Koha should be able to support this behavior.
Test Plan:
1) Apply this patch
2) Enable the new option cv_send_00_on_success for a SIP2 account
3) Restart SIP
4) Check in an item successfully via SIP
5) Note the response contains a CV field with the value '00'
Sponsored-by: Pueblo City-County Library District
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jill Kleven <jill.kleven@pueblolibrary.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Many SIP services send an empty password field (AD). Even if allow_empty_passwords is enabled for the given SIP account, this empty password is run though Koha's password checker which increments the number of login attempts for a patron. Thus repeated patron information requests can lock a patron out! Empty password fields in SIP should not call for a password check if allow_empty_passwords is enabled.
Test Plan:
1) Enable a patron password attempt with a limit of 3
2) Send 4 patron information requests with an empty AD field
3) Note the patron's account is now locked
4) Apply this patch
5) Repeat step 2 with a different patron
6) Note the patron's account does not get locked!
Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Only use of $pending was removed in the former patch.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
AddIssue already checks for holds in the item being issued and deals with them
The sip code, additionally, does not consider that multiple holds per bib
is allowed
To test:
0 - Apply only first patch (unit tests)
1 - Place two holds for a patron on a bib
2 - Checkout that item via sip_cli_emulator
3 - Note all holds are cancelled/filled
4 - prove -v t/db_dependent/SIP/Transaction.t
5 - it fails
6 - Apply second patch
7 - prove tests, all green
8 - Place two holds on a single bib for one patron again
9 - Checkout the item via sip_cli_emulator
10 - Only one holds is filled
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
To test:
1 - Define a rule to deny renewal
2 - Checkout an item meeting this rule
3 - Attempt renew an item via the OPAC
4 - Attempt renewal via SIP
5 - Attempt renewal via staff client
6 - Verify errors are reasonable
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
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>
Caused by
commit 546379cc92
Bug 17680: C4::Circulation - Remove GetItemIssue, simple calls
Since this commit AddReturn return a Koha::Issue object, not a hashref
with item's info.
Test plan:
1 - Enable SIP server on your test instance
For kohadevbox:
cp /etc/koha/SIPConfig.xml /etc/koha/sites/kohadev/
create patron with user/pass term1/term1 and grant circulation
privileges
2 - enable AutomaticItemReturn
3 - Checkin an item which will generate a transfer via SIP2:
perl /home/vagrant/kohaclone/misc/sip_cli_emulator.pl -a localhost -p
6001 -su term1 -sp term1 -l LPL --item {BARCODE} -m checkin -t
CR
4 - Note no CT field
5 - Apply patch
6 - Repeate checkin via SIP2
7 - Transfer destination is correctly returned in CT
Signed-off-by: Nick Clemens <nick@bywatersolutions.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>
Feature using it is completely undocumented as far as my research has
shown.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Benjamin Rokseth <bensinober@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Amended $item->biblio by $biblio.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Some SIP services ( such as Comprise ) require that an attempt at
over-paying a patron's account via SIP2 should fail, rather than create
a credit on the account. We should make this a configurable option on a
per-login basis in the SIP2 config file.
Test Plan:
1) Apply this patch
2) Enable the new parameter
disallow_overpayment="1"
for the login to be used in this test.
3) Restart your SIP server
4) Create or find a patron with fines
5) Attempt to send a payment via SIP for more than what the
patron's balance is
6) Note the response indicates a payment failure
7) Attempt to send a payment via SIP for the account balance or
less
8) Note the response indicates the payment has succeeded
9) Verify in Koha that the payment was processed
Signed-off-by: Rhonda Kuiper <kuiper@roundrocktexas.gov>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch adds a new Koha::Hold->cancel method and replaces the calls
to C4::Reserves::CancelReserve with it.
Test plan:
- Add and cancel holds
- Change priority of holds
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>
The returns from C4::Circulation::CanBookBeIssued used
to be structured as a hashref of entries like
REASON => {
data => 'foo',
moredata => 'bar',
};
Some entries still are. But many are now
REASON => 1,
data => 'foo',
moredata => 'bar',
The sip Checkout routine still assumed the former, as it
reports any causes it was not aware of (to maintain support for
a changing api) The data fields could leak into the screen message
field of the response. e.g. the borrowernumber or surname of the
borrower who has a hold on an issued title. Some real messages were
getting obscured by this
This patch sanatizes the return from from CanBookBeIssued
by removing keys which are not all uppercase
It also fixes a case where the key's data element was used
for the screen message when we should use the key itself
Updated the documentation of CanBookBeIssued to flag up
the assumption re case and the fact that 3 elements rather
than two may be returned
The loop through the returned keys was a bit bogus
so we now explicitly jump out if noerror is unset
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested quite extensively. Test results put on Bugzilla.
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>
From the syslog call on line 107/108. The field $item->{title} has not
been initialized. When catching the warns in a test script with Test::Warn,
I got:
Use of uninitialized value $args[1] in sprintf at /usr/lib/x86_64-linux-gnu/perl/5.20/Sys/Syslog.pm line 423.
This simply resolves the warning. We might have a closer look to this title
field on another report though.
Test plan:
Run Message.t from the next patch without this fix. It will fail since the
additional warning makes warnings_like return false.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
As per bug 18966 AddReturn returns false with a DataCorrupted message
when the issue_id at hand already occurs in the old_issues table.
This should be handled when returning an item via SIP too. SIP should
not pretend that the checkin was successful, since this item needs
special care.
The following six different situations are handled:
[1] An attempt to checkin an invalid barcode is handled as before. The
ok flag is false; the screen message shows 'Invalid Item'.
[2] We receive a DataCorrupted message: the alert type is set to the
unused 98 code to indicate this new error condition. The ok flag
is false; the screen message shows 'Checkin failed: data problem'.
[3] The item checked in was not checked out AND the option checked_in_ok
is active. The ok flag is set to true; no screen message.
[4] The item checked in was not checked out AND the option checked_in_ok
is not active. The ok flag is not changed (normally false); the screen
message shows 'Item not checked out'.
[5] (The regular checkin) The item was checked out and AddReturn returned
true. The ok flag is true; no screen message.
[6] ("Otherwise") The item was checked out, but AddReturn returned false.
The ok flag is false; the screen message shows 'Checkin failed'.
Note: Currently this case only refers to the Wrongbranch and withdrawn
messages from AddReturn (where doreturn==0).
Note: Situation 1 and 5 are unchanged. Behavior is slightly changed for
situation 3 and 4; the option is only used when the item was not checked
out. Situation 2 and situation 6 are changed. After bug 18966 SIP
returned OK in case of data corruption (while checkin failed). In the
remaining cases under [6] SIP also returned OK while checkin failed.
Test plan:
[1] Test all six cases listed above. In order to simulate data corruption
insert the issue_id at hand in old_issues before you check in.
In cases 3 and 4 you need to toggle the option in your SIPconfig.xml and
restart the SIP server.
Case 6 can be tested by checking out at branch A, adjusting the value
of pref AllowReturnToBranch and checking in at branch B.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This is a trivial fix, but finding the cause for a failing checkin in
SIPServer surely was not.
Without this simple change, I could not checkin an item while the SIP server
just closed the connection and syslog contained nothing useful.
The error can be easily reproduced without this patch by:
use Modern::Perl;
use C4::SIP::ILS::Item;
my $item = C4::SIP::ILS::Item->new('some_barcode');
where some_barcode should refer to an item checked out.
You should trigger the error (with your due date):
2017-07-28 23:59:00 is not an object at line 3.
Why the ILS/Item.pm module still compiles without a warning, is not clear
though.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com>
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>
FAIL pod Apparent command =cut not preceded by blank line in file misc/cronjobs/advance_notices.pl
FAIL pod Apparent command =cut not preceded by blank line in file C4/SIP/ILS/Item.pm
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The subroutine C4::Biblio::GetBiblioFromItemNumber was wrong for several
reasons:
- badly named, we can get biblio info from a barcode
- SELECT * from items, biblio and biblioitems
makes things hard to follow and debug, we never know where do come from
the value we display
- sometimes called only for trivial information such as biblionumber,
author or title
This patchset suggests to replace it with calls to:
- Koha::Items->find for item's info
- $item->biblio for biblio's info
- $item->biblio->biblioitem for biblioitem's info
Test plan:
Item's info should correctly be displayed on the following pages:
- circulation history
- transfer book
- checkin
- waiting holds
QA will check the other changes reading the code, it's trivial
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
C4::Circulation::GetItemIssue returned all the issue and item
informations for a given issue. Moveover it also did some date
manipulations. Most of the time this subroutine was called, there
additional information were useless as the caller usually just needed
the basic issue's infos 'from the issue table).
This first patch updates the simple calls, ie. the ones that just need
the issue's infomations.
Test plan:
The following operations should success:
- transfer a book
- create a rule for on-site checkouts and confirm that a patron cannot
check more items out that it's defined in the rule.
- Renew an issue using ILSDI
- Using SIP confirm that you are able to see your issues
Followed test plan, works as expected
Signed-off-by: Marc Véron <veron@veron.ch>
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>
The SIP2 DA field that Koha transmits is an odd and arbitrary format
that some SIP2 clients cannot handle. It would be best if this
format were customizable on a per-login basis in the same manner as
the AV field.
Test Plan:
1) Find an item that is checked out with holds
2) Return the item via SIP2 ( using the SIP2 cli emulator )
3) Note the value of the DA field
4) Apply this patch, restart your SIP2 server
5) Repeat step 2
6) Note the DA field value has not changed
7) Add this parameter to the login stanza you are using:
da_field_template="[% patron.surname %][% IF patron.firstname %], [% patron.firstname %][% END %]"
8) Restart the SIP2 server again
9) Repeat step 2
10) Note the DA field returned is now in 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>
With this patch a parameter 'allow_empty_passwords="1" can be added to a
login in the SIP configuration file to allow the behaviour as was normal
before the patch for bug 16610 was applied. Some sip clients rely on
this behaviour sending an empty password field when they wish to
validate to user but do not have the password.
If a password is supplied it will be validated
A test has been added to Message.t to confirm this behaviour
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>
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>
If a record has any holds on it, the SIP2 item information response will
return a value of 08 "waiting on hold shelf" even if the item is not
actually a waiting hold. This is clearly a bug.
Test Plan:
1) Find an item that is not a waiting hold, but whose record has one or
more holds.
2) Issue a SIP2 item information request
3) Note in the response, the circulation status field is '08'
4) Apply this patch
5) Repeat the item informationr request
6) Note the code is now '03' ( available )
7) Check the item in to fill the hold
8) Repeat the item information request
9) Verify the circulation status is now '08'
Followed test plan, works as expected
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Resolves:
Use of uninitialized value $payment_type_writeoff in string eq at C4/SIP/Sip/MsgType.pm line 1072, <STDIN> line 2.
Would be raised if this parameter is not found in SIPconfig.xml.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Many SIP2 services such as those by Comprise Technologies are able to or
require that an ILS be able to accept writeoffs via SIP2. The SIP2
protocol specifies that payment type be a two digit number, but does not
specify a code for writeoffs. To this end we should allow the write-off
code to be specified in the SIP2 config on a per-account basis so that
if different vendors use different fixed codes for write-offs we can
handle that gracefully.
Test Plan:
1) Apply this patch
2) Modify your SIP2 config to include
payment_type_writeoff="06"
in the login portion of the account you will be using for the test.
3) Restart your SIP2 server
4) Create a fee for a patron
5) Send a SIP2 fee paid message specifying the payment type code we
defined earlier, with a payment amount that is *not* equal to the
amount outstanding for the fee.
6) Note the fee paid response indicates the payment failed
7) Repeat step 5, but this time send the amount outstanding as the
payment amount
8) Note that the fee paid response indicates a successful payment
9) Note in Koha that the fee has been written off!
Signed-off-by: Rhonda Kuiper <kuiper@roundrocktexas.gov>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
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>
This small patch corrects the SIP2 checkin response if branch transfer is needed.
It depends on bug 7981 and also removes an obsolete alert check in SIP2.
Test plan:
- apply bug 7981
- make sure syspref AutomaticItemReturn is false
- check 'Return policy' in 'Default checkout, hold and return policy'
- make a SIP2 checkout and checkin from a branch different than item's home branch.
- verify that SIP2 response gives alert CV04 (=send to different branch)
- also verify that response field CT gives correct branch according to 'Return policy'
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Followed test plan, works as expected
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
The SIP protocol allows for a Fee Paid message to specify a fee
identifier in the CG field. While this field is read and passed to
C4::SIP::ILS::pay_fee, it is not passed along to
C4::SIP::ILS::Transaction::FeePayment::pay and is thus not used. We
should enable this functionality in Koha now that accountlines each have
a unique identifier that can be passed along to the payment requestor
via the AV field line items added by bug 14512.
Test Plan:
1) Create a fee in Koha
2) Ensure your SIP2 server is running and configured correctly
3) Send a 37 Fee Paid message to Koha's SIP server with the
accountlines_id for that fee in the CG fee identifier field,
along with the other required fields.
4) Note that specific fee was paid in Koha
Signed-off-by: Rhonda Kuiper <kuiper@roundrocktexas.gov>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch updates SIP checkin to use reserve_id as param, to fix regression
introduced by Bug 14695.
To test:
Given 1 patron and 1 biblio with two items attached:
1) create two record level holds on one biblio on same user
Using SIP client or telnet:
2) checkin item x on pickup branch, observe item is waiting and hold 1 is filled
3) checkin item y on pickup branch, observe item y now is waiting and x is available
4) apply this patch and repeat 1-3, now expected behaviour should be
that both holds are filled and both items waiting
Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
It is not about when the hold was 'placed' but if the hold pertains to
the future or not.
Test plan:
[1] Git grep on holds_placed_before_today.
[2] Run t/db_dependent/Koha/Biblios.t
[3] Run t/db_dependent/Reserves.t
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>
The C4::Reserve::GetReservesFromBiblionumber took 3 parameters, the
biblionumber, an optional itemnumber and a "all_dates" flag.
If set, the subroutine returned all the holds placed on a given bibliographic
record, even the ones placed in the future. Almost all of the calls had this
flag set, they will be replaced with a call to Koha::Biblio->holds.
But 5 did not have it:
- C4::Biblio::DelBiblio
-tools/batch_delete_records.pl
=> These 2 were wrong, we want to retrieve the holds to cancel them
before deleting the record. We need to get all the holds, even the ones
placed in the future /!\ CHANGE IN THE BEHAVIOR
- acqui/parcel.pl
=> 1 call per item were made to this subroutine. They have been replaced
with only 1 call to the new method Koha::Biblios->holds_placed_before_today
Then we filter on the itemnumbers.
I think this is wrong: we need the number of holds to know if the record
can be deleted, so even if future holds exist, the deletion should not
be possible.
- serials/routing-preview.pl
- C4::ILSDI::Services::GetRecords
- C4::SIP::ILS::Item->new
=> Seems ok, we just one to display holds placed before today
Test plan:
I would suggest to test this patch with patches from bug 17737 and bug 17738,
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.
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>
Two discussions on koha-devel lead to the same conclusion:
biblioitems.marcxml should be moved out this table
- biblio and biblioitems
http://lists.koha-community.org/pipermail/koha-devel/2013-April/039239.html
- biblioitems.marcxml & biblioitems.marc / HUGE performance issue !
http://lists.koha-community.org/pipermail/koha-devel/2016-July/042821.html
There are several goals to do it:
- Performance
As Paul Poulain wrote, a simple query like
SELECT publicationyear, count(publicationyear) FROM biblioitems GROUP BY publicationyear;
takes more than 10min on a DB with more than 1M bibliographic records
but only 3sec (!) on the same DB without the biblioitems.marcxml field
Note that priori to this patch set, the biblioitems.marcxml was not
retrieved systematically, but was, at least, in
C4::Acquisition::GetOrdersByBiblionumber and C4::Acquisition::GetOrders
- Flexibility
Storing the marcxml in a specific table would allow use to store several
kind of metadata (USMARC, MARCXML, MIJ, etc.) and different formats (marcflavour)
- Clean code
It would be a first step toward Koha::MetadataRecord for bibliographic
records (not done in this patch set).
Test plan:
- Update the DBIC Schema
- Add / Edit / Delete / Import / Export bibliographic records
- Add items
- Reindex records using ES
- Confirm that the following scripts still work:
* misc/cronjobs/delete_records_via_leader.pl
* misc/migration_tools/build_oai_sets.pl
- Look at the reading history at the OPAC (opac-readingrecord.pl)
- At the OPAC, click on a tag, you must see the result
Note: Changes in Koha/OAI/Server/ListRecords.pm is planned on bug 15108.
Signed-off-by: Mason James <mtj@kohaaloha.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
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 script is not used from the Koha codebase and does not seem very
useful.
We could rewrite it if needed later (ie. if someone complains I will
rewrite it).
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>
Test plan:
1) Apply this patch
2) Make a payment via SIP2
3) The payment should succeed
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
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>
Although mainly hypothetical, it would still be possible to get
response from the server for an acs resend request. (This exception
is allowed in MsgType::handle.)
I also noticed that the response may well be a message from an older
session still.
This patch just removes that exception by only passing login requests
to sub handle in the raw_transport loop.
Test plan:
[1] Verify normal login procedure for raw.
[2] Check a few acs resend requests in raw. They should terminate without
a response.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Removed tabs and used spaces consistently
Used 'use base' to remove unnecessary BEGIN sub and
explicit setting of ISA at application level
Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Moving timeout logic to one routine (with unit test).
This further implements two suggestions from Kyle and Larry:
[1] You could use a client_timeout of 0 to specify no timeout at all.
[2] Have the client_timeout default to the timeout if not defined.
Test plan:
[1] Run t/db_dependent/SIP/SIPServer.t.
[2] Test login timeout for raw and telnet.
[3] Check ACS status message for timeout value. Should match policy
timeout from institution.
[4] Test client timeout (zero and non-zero).
[5] Remove client timeout. Test fallback to service.
[6] Remove service timeout too. Test fallback to 30 at login.
Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Amended to incorporate Srdjan's suggestion to move get_timeout to
SIPServer.pm; this requires some additional mocking in the unit test.
And even makes the test db dependent, as documented.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Remove the tabs causing inconsistent indentation
of sip_protocol_loop and replace with spaces
Reimplements the renaining parts of Marcel de Rooy's
original QA patch
No logic changes in this patch - layout only
Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
raw_connection was not behaving correctly if an invalid string was
passed or a login failed.
It was not checking that the login succeeded ( it checked that account
existed not that it contained data and it existed even if login failed)
and so failed logins instead of aborting immediately fell through into
the sip_protocol_loop, forcing that to timeout invalid connections.
It now checks that account has id set and returns if not.
The timeout alarm is now set on the while loop, in normal running this
should not be triggered as the socket is opened and the first data
should be a login message and the while loop should only iterate once,
but lets not go into an infinite loop due to unforeseen circumstances.
I have reindented the routine as the flow was not clear (the while was
not indented at all.
Also if using Net::Server::PreFork when a new connection comes in you
may be handed the the successful login parameters from a preceding call.
Because of this you could successfully transmit transactions and Koha
would carry them out without having received a valid login ( and
possibly with the wrong account details!) We now delete any existing
account for new connections.
NB: This patch requires that the patch for bug 13807 has been applied
Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Debugging various problems in SIPServer and control of it, found it
could loop on unread buffers (e.g. the LF of a CRLF if it was only
expecting CR) making it unresponsive to signals.
Reworked the input loop with an eye to removing unnecessary whiles
and replacing the while(1) by a while( connection valid)
Enhanced the timeout code by wapping in an eval.
Moved the logic from SIP_read_packet into the server itself
Hopefully this makes the already baroque code easier to navigate
and it did seem the server was the logical place for this
Removed no longer iused SIP_read_packet from Sip.pm
Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
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>
If you enable send_patron_home_library_in_af in your sip account, you
want a separate AF field for the home branch.
Test plan:
Send a 63 (Patron Info) and verify that you have an extra AF.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested Patron Status and Patron Info.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Removes a warn and some commented warns.
Test plan:
Nothing to do here. Will be covered later by additional unit test.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Koha's SIP2 server should have support for the AV field ( field items ).
The biggest problem with this field is that its' contents are not really
defined in SIP2 protocol specification. All it says is "this field
should be sent for each fine item". Due to this, I think the contents of
the field need to be configurable at the login level, so that the
contents can be defined based on the SIP2 devices requirements for the
AV field.
Test Plan:
1) Apply this patch
2) Find a patron with outstanding fines
3) Run a patron information request using misc/sip_cli_emulator.pl using the new -s option with the value " Y "
4) Note there is an AV field for each fee containing the description and amount
5) Edit your sip config, add an av_field_template parameter to the login you are using such as
av_field_template="TEST [% accountline.description %] [% accountline.amountoutstanding | format('%.2f') %]"
6) Restart your SIP server
7) Repeat the patron information request
8) Note your custom AV field is being used!
Signed-off-by: Chris Davis <cgdavis@uintah.utah.gov>
Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Mainly a
perl -p -i -e 's/^.*3.07.00.049.*\n//' **/*.pm
Then some adjustements
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
perl -p -i -e 's/^(use vars .*)\$VERSION\s?(.*)/$1$2/' **/*.pm
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
When you are ready, you still see that small detail.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Conform QA comment, Patron Info request is slightly adjusted to be
consistent with changes to Patron Status request.
If the cardnumber is ok and the password is wrong, BL=N is reported but
also add 'Invalid password' in AF.
Additionally, an invalid card number is reported in AF.
Test plan:
[1] Send patron info request for invalid card.
[2] Idem for valid card, no password.
[3] Idem for valid card, good password.
[4] Idem for valid card, wrong password.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Verified by telnetting SIP server.
And tested additionally with the new unit test of bug 15956.
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Conform QA comment on Bugzilla, we do this:
[1] Attribute for overdrive mode/invalid credentials is not really needed.
We can always pass a screen message that card or password is invalid.
[2] If the cardnumber is correct and the password is wrong, we should
still honour the request. The bad password is recognized by BLN and
an additional message in AF.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Verified by telnetting SIP server.
And tested additionally with the new unit test of bug 15956.
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
NOTE: apply this patch after the additional perltidy patch
this patch is basically a small work-around to fix some confusing login text,
when users enter incorrect auth details via Overdrive's website
with this option disabled (default) there is no change to SIP's behaviour
to test...
1/ configure your overdrive account to talk to your Koha's SIP service
2/ start Koha's SIP
3/ enter a correct username and correct password in overdrive
see overdrive display '(1) Greetings from Koha' (good)
4/ enter a correct username and *incorrect* password in overdrive
see overdrive display '(1) Greetings from Koha' (bad)
5/ enter an incorrect username in overdrive
see overdrive display '(1)' (badder)
6/ apply patch, enable 'overdrive-mode' in Koha's SIPConfig.xml
example...
---------------------
<accounts>
<login id="kohasip" password="xxxxx" delimiter="|"
error-detect="enabled" institution="YYY" overdrive-mode="1" />
</accounts>
---------------------
7/ restart SIP
8/ enter a correct username and correct password
see overdrive display '(1) Greetings from Koha'
9/ enter a correct username and *incorrect* password
see overdrive display '(1) Invalid patron or patron password'
10/ enter an incorrect username and incorrect password
see overdrive display '(1) Invalid patron or patron password'
http://bugs.koha-community.org/show_bug.cgi?id=1387
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
The original perltidy patch from Mason has been amended.
The perltidy itself has been moved to a separate patch with the current
perltidyrc applied.
As noted on Bugzilla, the original perltidy patch included some extra
changes:
[1] You prefix timestamp with Sip
This is not actually needed (it is imported), but if we should prefix it,
we should prefix now with C4::SIP::Sip. But you only changed two
occurrences (out of 26). So I remove these two changes.
[2] You remove the $server parameter from two calls of maybe_add:
A closer look at the remaining code tells me that $server is always
passed to maybe_add for FID_SCREEN_MSG. So this only left me the
current whitespace change.
But at least we documented what we did or did not, and why..
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Run perltidy pro=xt/perltidyrc on the file.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Some devices are using patron information responses to validate
patron passwords to govern access to facilities as we
were using C4::Auth::checkpw_hash this only worked in a db password
context not other authentication routines.
The C4::Auth routines are not very consistent and there isnt a dropin
replacement for checkpw_hash this calls checkpw instead.
In a password only environment this behaves as the old version did
returning field CQ as Y if a valid password or no password is passed in
the patron info request and N if an incorrect password is supplied
It should also test against the appropriate authentication sources if
othere autrhentication schemes are in use
Signed-off-by: Liz Rea <liz@catalyst.net.nz>
Tested this with a client who reports that this enables SIP authentication to work correctly with their LDAP server.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
This patch specifically adds a test for the sub added to
resolve Bug 15479. As no test script existed to add this two
added one which also tests that ILS objects can be instantiated
correctly and offer the required interface
NOTE: This does not apply without the first patch.
Comment #3 suggested a unit test that would fail
before and pass after. So while this code is good,
it missed the before case. -- mtompset
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
The borrowers table is defined COLLATE=utf8_unicode_ci which means
that cardnumbers may be saved in either lowercase or uppercase and
these are considered equivalent.
The server was performing a case sensitive comparison between
the incoming patron identifier and that retrieved from the db
As a result some renewals were rejected as being on loan to
another borrower if the stored cardnumber differed in case.
Make code comparison comply with db.
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
