Test plan (ktd):
1 - Have henry acevedo have no mail and set messaging preference "Hold filled to sms"
2 - Have edna acosta have not messaging preference
3 - Place an hold on two books for edna acosta and henry acevedo, check in both books and select the button to fill the hold
4 - Check in notices and see that both received a notice
5 - Apply tests and run tests -> they should fail
6 - Apply patch, run rest -> they should pass
7 - Unfill both holds and fill them back. Henry should keep receiving notices while Edna should not anymore.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
To test:
1. Set 'EasyAnalyticalRecords' to 'Show'.
2. Find or create a parent record that has at least 2 items on it. Note the itemnumbers and the framework.
3. Edit that framework so you can add a 773$0, 773$9, and 773$t
4. Find or create a child record.
5. Make at least 2 773 entries on the child record. Add the biblionumber of the host record to the 773$0.
6. In each entry for the 773$9 add one of the itemnumbers from the host record.
7. For the 773$t and the biblio title of the host record.
8. Save.
9. Now attempt to put a hold on the child record.
10. 500 error:
11. APPLY PATCH and restart_all
12. Try placing the hold again, this time it succeeds.
Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
The term 'If any unavailable' is not very clear but it is also
very hard to find something short and better.
A next available hold is possible when at least one item is
unavailable (checked out, on order, waiting/transit hold).
A item level hold is possible when the specific item is
unavailable.
Test plan:
Read the patch. The code did not change.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Note: tests have to be performed on addReturn and have therefore been
added in a separate test
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch adds the optional 'tranferbranch' parameter to the
CanBookBeReturned routine. This allows for passing the correct
'ReturnToX' transfer to be tested at return time for transfer limit
restrictions.
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
Signed-off-by: Mathieu Saby <mathsabypro@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This follow-up patch actually fixes the issue.
This patch forbids a patron to return a book to a library
from where the book may not return to its home or holding library.
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
Signed-off-by: Mathieu Saby <mathsabypro@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Test case:
* UseBranchTransferLimits must be set
* define your branch transfer limit. Refuse transfers from libraryA to libraryB
* checkout a book owned by libraryB, from libraryB, with a librarian located at libraryB
* move the librarian to libraryA ("Set Library" link top/right)
* check-in the book => it's possible whatever your setup
After the patch, the behaviour respect the branch transfer limit parameter:
you can check-in if you accept transfers, you can't if you refuse them.
(Note: IndependantBranches must be OFF, otherwise it's not possible to do the checkin whatever the branch transfer limits)
TEST PLAN:
Note: older signoffs did not validate this test plan
Considering you use a ktd-like database.
Log in at centerville
Allow transfers between centerville, Midway and Franklin, don't allow transfers between Fairfield, Liberty and all other libraries. In addition, allow the transfer between Liberty and Midway
Transfer as much books as you can from Centerville to Midway.
Log in as Midway, check out as many books from Centerville as you can to Henry Acevedo. If you don't have enough books during the test plan, feel free to check out books again. During the whole test, without further precision, items checkout out in such conditions should be checked in.
You will also need 6 checked out floating books at midway
Note:
'-> ok' means the checkin is allowed
'-> not ok' means the checkin is not allowed
Apply patch
I - Test AllowReturnToBranch
During this part of the test plan, you want to have all following configuration :
'CircControlReturnsBranch' -> 'The library the item is checked in at'
In CIRC rules: 'Default holds policy and return policy' -> 'Item returns home' (make sure you have no other holds policy on any library)
'AllowReturnToBranch' -> as follows
1 - Set to "Either at the library the item is from or at the library the item was checked out from"
X a - Check in to Centerville -> ok
X b - Check in to Midway -> ok
X c - Check in to Franklin -> not ok
X d - Check in to Fairfield -> not ok
2 - Set to "Only at the library the item was checked out from"
X a - Check in to Centerville -> not ok
X b - Check in to Midway -> ok
X c - Check in to Franklin -> not ok
X d - Check in to Fairfield -> not ok
3 - Set to "Only at the library the item is from"
X a - Check in to Centerville -> ok
X b - Check in to Midway -> not ok
X c - Check in to Franklin -> not ok
X d - Check in to Fairfield -> not ok
4 - Set to "Any library"
X a - Check in to Centerville -> ok
X b - Check in to Midway -> ok
X c - Check in to Franklin -> ok
X d - Check in to Fairfield -> not ok
II - Transfer and check out 6 floating items at midway.
1 -
X a - Check in to Centerville -> ok
X b - Check in to Midway -> ok
X c - Check in to Franklin -> ok
X d - Check in to Fairfield -> ok
2 - Set 'AllowReturnToBranch' to 'home library'
X a - Check in to Centerville -> ok
X b - Check in to Midway -> not ok
III - Pathological & asymetrical cases.
During this part of the test plan, you want to have all following configuration :
'CircControlReturnsBranch' -> 'The library the item is checked in at'
'Default holds policy and return policy' -> 'Item returns home' (make sure you have no other holds policy on any library)
'AllowReturnToBranch' -> 'Any library'
FOR THIS PART OF THE TESTS ONLY (until IV): Don't allow tranfers from midway to centerville, but allow from centerville to midway
X 1 - Check in a book at midway -> not ok
Set 'Default holds policy and return policy' -> 'Item returns to issuing library'
X 2 - Check in a book at centerville -> ok
FOR THIS PART OF THE TESTS ONLY (until IV): Don't allow tranfers from centerville to midway
X 3 - Check in a book at centerville -> not ok
X 4 - Set 'AllowReturnToBranch' to "Only at the library the item was checked out from"
X a - check in to Centerville -> not ok
X b - check in to Midway -> not ok
IV - Test 'CircControlReturnsBranch'
(remove the precedent transfer restriction)
During this part of the test plan, you want to have all following configuration :
'Default holds policy and return policy' -> 'Item returns home' (unchanged)
'Default checkout, hold and return policy for Midway' -> 'Item returns to issuing library'
'Default checkout, hold and return policy for Franklin' -> 'Item floats'
'Default checkout, hold and return policy for Fairfield' -> 'Item returns to issuing library'
'Default checkout, hold and return policy for Liberty' -> 'Item returns home'
'AllowReturnToBranch' -> "Any library" (unchanged)
'CircControlReturnsBranch' -> as follows
1 - 'The library the item is checked in at'
X a - Check in to Centerville -> ok
X b - Check in to Midway -> ok
X c - Check in to Franklin -> ok
X d - Check in to Fairfield -> not ok
X e - Check in to Liberty -> not ok
2 - 'The library the current item is held by
X a - Check in to Centerville -> ok
X b - Check in to Midway -> ok
X c - Check in to Franklin -> ok
X d - Check in to Fairfield -> not ok
X e - Check in to Liberty -> ok
3 - 'the library the item is owned by'
X a - Check in to Centerville -> ok
X b - Check in to Midway -> ok
X c - Check in to Franklin -> ok
X d - Check in to Fairfield -> not ok
X e - Check in to Liberty -> not ok
Sponsored-by: National Library of Finland
Signed-off-by: Arthur Suzuki <arthur.suzuki@biblibre.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
Signed-off-by: Mathieu Saby <mathsabypro@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch makes the plugin hook load plugins using `get_enabled_plugins()`. This has
the immediate effect of using the already cached plugin objects without reinstantiating them.
This way transactions don't get broken for schema reloads. Also, only enabled plugins
are loaded/attempted to be loaded.
To test:
1. Run:
$ ktd --shell
k$ prove t/db_dependent/Koha/Plugins/Notices_content_hook.t
=> SUCCESS: Tests pass
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests still pass: fundamental behavior remains unchanged.
4. Sign off :-D
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
1 - Create suggestion
2 - Display a suggestion
3 - Amend a suggestion
To do with and without patch
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Amended-by: Jonathan Druart
tidy
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
If HoldsQueueParallelLoopsCount is set to a value greater than 1, build_holds_queue.pl will fail with the message:
Can't locate object method "new" via package "Parallel::ForkManager" (perhaps you forgot to load "Parallel::ForkManager"?) at /kohadevbox/koha/C4/HoldsQueue.pm line 205.
Test Plan:
1) Run misc/cronjobs/holds/build_holds_queue.pl
2) Note the error message
3) Run misc/cronjobs/holds/build_holds_queue.pl
4) Note the error message is gone!
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Generated using
`perl misc/devel/tidy.pl --perl`
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
To test:
1. APPLY PATCH, updatedatabase, restart_all
2. Go to your circulation rules and do the following:
-Automatic renewal: Yes
-No renewal before: 5
-No automatic renewal before: 5
-Renewal period: 5
3. Set ManualRenewalPeriodBase to "the old due date of the checkout".
4. Set AutomaticRenewalPeriodBase to "current date".
5. Check out an item and set the due date to 2 days from today
6. Attempt to manually renewal the item.
7. Notice the new date is calculated based on the old checkout date.
8. Check out an item another item and set the due date to 2 days from today.
9. Run `perl /kohadevbox/koha/misc/cronjobs/automatic_renewals.pl -c -v`
10. Notice the new date is calculated based on the current date.
11. Set ManualRenewalPeriodBase to "current date".
12. Set AutomaticRenewalPeriodBase to "the old due date of the checkout".
13. Check out an item and set the due date to 2 days from today
14. Attempt to manually renewal the item.
15. Notice the new date is calculated based on the current date.
16. Check out an item another item and set the due date to 2 days from today.
17. Run `perl /kohadevbox/koha/misc/cronjobs/automatic_renewals.pl -c -v`
18. Notice the new date is calculated based on the old checkout date.
Signed-off-by: Trevor Diamond <trevor.diamond@mainlib.org>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Trevor Diamond <trevor.diamond@mainlib.org>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
0 days in advance means that the notice should not be sent.
This patch contains the pl/pm changes: we set to null if 0 is passed,
and do not insert a row in borrower_message_transport_preferences
Test plan:
0. Do not apply the patches
1. Create some entries with email and days in advance = 0
2. Create some entries with email and days in advance > 0
3. Apply the patches
4. Run the updatedatabase script
=> The rows in borrower_message_transport_preferences have been removed
for days in advance = 0
5. Edit some messaging preferences
=> 0 has been replace with a blank entry
=> checkboxes are disabled when blank is selected
6. Save
=> No new entry in borrower_message_transport_preferences is blank is
selected
7. advance_notices.pl still works correctly
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Ben Daeuber <bdaeuber@fargolibrary.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
To test:
Before applying patch
1. Create an authority record, fill field 400$w with "n"
2. Search for the authority you just created
3. Observe a warning has been logged
"substr outside of string"
4. Apply patch
5. Refresh the search results from step 2
6. Observe no warnings have been logged
Signed-off-by: Harrison Hawkins <harrisonhawkins@catalyst.net.nz>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Test plan:
1. APPLY patch, restart_all, updatedatabase
2. Search for the StoreLastBorrower system preference, it should now allow you to enter a number of borrowers to store
3. Set the pref to 3.
4. Check an item out to any patron. Then immediately check it back in.
5. On catalogue/moredetail.pl?biblionumber=X ( where X is you biblionumber ) you should see a "Last returned by:" entry with the correct cardnumber for the borrower.
7. Confirm this is right in the database with: SELECT * FROM items_last_borrower where itemnumber = X; ( Where X is your itemnumber )
8. Repeat step 4 to a different borrower.
9. Repeat steps 5 and 6. You should see 2 cardnumbers of each of the borrowers on catalogue/moredetail.pl?biblionumber=X
10. Repeat step 4 with another new borrower.
11. Repeat steps 5 and 6. You should see 3 cardnumbers of each of the borrowers on catalogue/moredetail.pl?biblionumber=X
12. Choose a different item and repeat steps 4 - 11 for this item.
13. Now set the StoreLastBorrower preference to 0.
14. Using the original item from step 4, check something out to a patron and immediately check it back in.
15. catalogue/moredetail.pl?biblionumber=X should NOT have an entry for "Last returned by:"
16. Check the database, all entries for the itemnumber from step 14 should be deleted.
Signed-off-by: Trevor Diamond <trevor.diamond@mainlib.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
In bug 39184 we set EVAL_PERL and ABSOLUTE to 0, which is the default. RELATIVE also defaults to 0. For security we should probably specify that as well in case the default is changed somehow.
Test Plan:
1) Apply this patch
2) Generate some notices
3) No change should be noted!
Signed-off-by: noah <noah@inlibro.com>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
CanBookBeRenewed will by default pass the skip_future_holds flag
to item->current_holds so that renewals are not blocked by future
holds.
The new pref FutureHoldsBlockRenewals allows you to change that
historical behavior.
Test plan:
[1] Set pref ConfirmFutureHolds=3, FutureHoldsBlockRenewals=Allow,
OPACAllowHoldDateInFuture=Allow
[2] Checkout some item to patron A.
[3] Place hold on same item for patron B, reserve date tomorrow.
[4] Try to renew item with patron A on account page. Should be
marked as Not renewable.
[5] Set pref FutureHoldsBlockRenewals=Dont
[6] Repeat step 4. Renew should be allowed.
[7] prove t/db_dependent/Holds.t t/db_dependent/Reserves.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Grouping all future hold prefs here too.
Test plan:
prove t/db_dependent/UsageStats.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Test plan:
Apply the patches and update the database
1) Create a new subscription with items, there should be a checkbox at the bottom of the form to pre-select collection table issues - make sure this is checked
2) Receive some items against the subscription
3) Navigate to the collections table
4) The checkboxes should be pre-selected as per previous behaviour in Koha
5) Edit the subscription and uncheck the checkbox
6) Return to the collections table and the checkboxes should all be unchecked
Signed-off-by: Nathalie <nathalie.savarit@univ-amu.fr>
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch adds a column to the table to link to the item
Test plan:
1) Add a new subscription to a serial record (with the option 'create an item record' selected)
2) Receive several issues and create the linked items
3) Navigate to the serials collection table
4) There should be a column for the linked item
5) Click on one of the links, it should display the item
Signed-off-by: Nathalie <nathalie.savarit@univ-amu.fr>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch adds a new parameter to ModReserve: skip_fix_priority
The page for altering priority ensures each hold has a unique rank, so doesn't allow for setting two
holds to the same rank, so we don't need to reorder everything until the end (in case of some cancellations etc)
This patch also makes FixPriority exported and updates the name as it is already called in a few places outside of
C4:Reserve
To test:
1 - Place many holds on a record, 50 - 150
https://github.com/kidclamp/handy-koha-script/blob/main/randhold.pl
2 - View the record in the staff interface:
http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=312
3 - Click on the 'Holds' tab
4 - Adjust the priority of several holds using the dropdowns
5 - Open the network tab in the browser console (F12) to view the timing
6 - Click 'Update hold(s)' and note the time
7 - Apply patch, restart all
8 - Repeat
9 - Confirm timing is better and numbering updates correctly
10 - Sign off!
Note: The script adds holds that may be marked as 'lowestPriority' these will always all group at the bottom of the record
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
When either the <listen id=biblioserver> entry or the
<server id=biblioserver listenref=biblioserver> section
is not present in koha-conf.xml, the Staff interface > About page
triggers a 500 Internal Server Error.
Test plan:
1. Edit koha-conf.xml and Comment out:
<!--<listen id="biblioserver">unix:/path/to/zebradb/bibliosocket</listen>-->
2. Restart Apache: sudo systemctl restart apache2 (or restart_all)
3. Go to Intranet > About
==> Internal Server Error (500)
4. Now restore the <listen> line and instead comment out the entire
<server id=biblioserver listenref=biblioserver>...</server> block.
(Note that the commenting around <!-- The stuff below is used to enable SRU...
interferes with adding a closing comment after </server> -
close the comment before that section and add a comment around </server>.)
5. Repeat step 2, 3
==> Internal Server Error (500)
6. Apply the patch
7. Repeat step 2, 3
==> There is no Error (500)
Note: Zebra status: Error message from Zebra: 10004
8. Uncomment <server id=biblioserver listenref=biblioserver> </server>
and Repeat Step 1
9. Repeat step 2, 3
==> There is no Error (500)
Note: Zebra status: Zebra server seems not to be available. Is it started?
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This follow-up adds proper security controls for the DISABLE_SYSPREF_*
URL parameter functionality introduced in Bug 14004. Previously, any
user could bypass system preferences by using URL parameters without
authorization.
Changes:
- Restricts DISABLE_SYSPREF_* parameter processing to users with debug permission
- Uses existing haspermission() method for consistent authorization checking
- Maintains superlibrarian access as expected
- Closes potential security loophole where unauthorized users could disable
system preferences like IntranetUserJS and IntranetUserCSS
The debug UI buttons will only appear for authorized users, and now the
underlying syspref override functionality requires the same permission,
ensuring both UI and functional security are aligned.
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch addresses two issues found in QA review:
1. Fixed HTML syntax error in i18n_l10n.pref where closing anchor tag
was incorrectly written as <a> instead of </a>
2. Changed C4/Search.pm to use elsif instead of separate if statement
for FacetOrder checking, matching the logic structure used in
Elasticsearch implementation and preventing potential double-execution
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This patch restores the previous perl sorting to restore speed. For English language libraries
this will be correct in most instances, and is much more performant.
To test:
1 - Apply patch, update database
2 - Perform some searches, note ordering of facets
3 - Update system preference 'FacetOrder' to 'simple alphabetical'
4 - Peform some searches, note ordering of facets, should be correct unless you have unicode characters
5 - Sign off
NOTE: Benchmarks attached as a comment, you can also enable NYTProf and view results for yourself:
https://wiki.koha-community.org/wiki/Profiling_with_Devel::NYTProf#NYTProf_with_Plack
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
For SIP based payments, no branchcode is recorded for the corresponding
credit. Koha should be recording the institution id (i.e. branchcode)
for SIP payments.
Test plan:
1) Apply this patch
2) prove t/db_dependent/SIP/Transaction.t
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This fixes an error in SIP2 patron information response (64) when no currency is set, which can be the case on newly created instances.
Test plan for KTD:
a) Delete currency and related tables
- run 'koha-mysql kohadev' to start SQL client. Then run the following queries:
- delete from aqbasket;
delete from aqbooksellers;
delete from currency;
b) exit
c) run 'perl misc/sip_cli_emulator.pl -a 127.0.0.1 -p 6001 -su term1 -sp term1 -l CPL -m patron_information --patron 42'
Attempting socket connection to 127.0.0.1:6001...connected!
SEND: 9300CNterm1|COterm1|CPCPL|
READ: 941
Trying 'patron_information'
SEND: 6300120251202 115652 AOCPL|AA42|ACterm1|
READ: undef
d) recognize the last response: READ: undef
e) apply patch and restart sip2 server via 'sudo koha-sip --restart kohadev'
e) rerun 'perl misc/sip_cli_emulator.pl -a 127.0.0.1 -p 6001 -su term1 -sp term1 -l CPL -m patron_information --patron 42'
Attempting socket connection to 127.0.0.1:6001...connected!
SEND: 9300CNterm1|COterm1|CPCPL|
READ: 941
Trying 'patron_information'
SEND: 6300120251202 122545 AOCPL|AA42|ACterm1|
READ: 64 00120251202 122546000000000000000000000000AOCPL|AA42|AE koha|BLY|BV0|CC5|PCS|PIY|AFGreetings from Koha. |
f) check the last response is now a patron information response (64)
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Before this patch, we pass the previous issue date to generate the next seq - this works for the pattern as we are incrementing.
When the patter is translated ot words, we need to do this from the date of the next issue, so we must pass in the next
published date
This patch changes the call to GetNextSeq from serial-collection.pl to ensure the new issue publication date is used to
generate the numbers.
To test:
0 - Follow the plan to recreate from the initial bug description and recreate
1 - Apply patch
2 - On the serial used in previous patch go to 'Serial collection'
3 - Generate next issue
4 - Confirm the numbering is correctly generated after this patch
Signed-off-by: Wesley Owen <wesleyowen@churchofjesuschrist.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
The following C4::Auth subroutines
- checkauth()
- get_template_and_user()
were in some cases outputting html outside of C4::Output's output mechanisms.
It is important to ensure the consistency of our HTTP responses. Having
multiple output mechanisms for what is essentialy the same type of response not
only leads to unneccessary code duplication and complexity but possibly even
bugs at some point in the future.
For these reasons it is better to output only through C4::Output, when possible.
This patch does that for the two aforementioned C4::Auth cases.
To test unit tests:
1. prove t/db_dependent/Auth.t
As for checkauth(), the change only applies to unauthenticated state and failed
logins.
To test checkauth:
1. Enable system preference opacuserlogin
2. Navigate to OPAC
3. Observe OPAC loading successfully
4. Log in to OPAC with invalid credentials
5. Observe "You entered an incorrect username or password"
6. Navigate to staff client without being logged in (logout if you are in)
7. Observe staff client login screen loading successfully
8. Log in to staff client with invalid credentials
9. Observe "Invalid username or password"
As for get_template_and_user(), the change only applies to SCO/SCI user
navigating outside of the self service modules.
To test get_template_and_user:
1. Enable system preference WebBasedSelfCheck
2. Create a new test patron and note down their username & password
3. Grant them a permission self_checkout_module (under Self check modules)
4. Navigate to http://OPAC/cgi-bin/koha/sco/sco-main.pl
5. Log in with the test patron
6. Observe successful login (should display "Self-checkout system" page)
7. Navigate to http://OPAC/cgi-bin/koha/opac-main.pl
(you can get there by editing the address bar of your web browser)
8. Observe "Log in to your account" page, ie. you have been kicked out
9. Modify your test patron's permissions
10. Remove permission "self_checkout_module" and add "self_checkin_module"
11. Enable system preference SelfCheckInModule
12. Navigate to http://OPAC/cgi-bin/koha/sci/sci-main.pl
13. Log in using your test patron's credentials
14. Observe "Self check-in" page
15. Navigate to http://OPAC/cgi-bin/koha/opac-main.pl
(you can get there by editing the address bar of your web browser)
16. Observe "Log in to your account" page, ie. you have been kicked out
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Attempt to call undefined import method with arguments (StrWidth) via package C4::Creators::PDF (Perhaps you forgot to load the package?) at /kohadevbox/koha/C4/Patroncards/Patroncard.pm line 28.
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
So it is done before the other use statements.
Also uniformize the pattern, must now be:
```
use Modern::Perl;
use base 'Exporter';
BEGIN {
our @EXPORT_OK = qw(subroutine list);
}
```
Rebased-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Bug38936 exported redirect_if_opac_suppressed
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
From commit b40ceb1cf5
Bug 39367: Manual changes
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This was a bad workaround.
It was ugly and is no longer relevant now that pod_coverage.t no longer attempts to launch a SIP server.
prove xt/author/pod_coverage.t
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Because the file is now being properly checked for pod_coverage, it fails (because a lot of methods dont have pod coverage. Add here
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
By using 'unless caller' we ensure the class is not executed when Pod::Coverage->new does 'require C4/SIP/SIPServer.pm
Ensure no regressions are added:
prove t/db_dependent/SIP/*
koha-sip --restart kohadev
koha-sip --status kohadev
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
To test:
prove -v /kohadevbox/koha/xt/author/pod_coverage.t
The changes to Schema files are fixes that were overlooked originally.
The 'die' line in C4/SIP/Sip/Configuration.pm is to force the script to die if SIP configuration does not exist, which only happens when running pod_coverage.t
The original code (prior to 37893) did not consider the possibiliy of a config_file being invalid or undef, parser->XMLin(config_file) died silently, but because pod_coverage.t skips over files that die, it didn't fail. See bug 41271.
Now, with the changes from bug 37893, if a config_file doesnt exist the code falls back to whats in the cache, so it doesn't try to XMLin load an invalid config_file anymore, and fails verbosely.
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This follow-up removes the security vulnerability of storing plaintext
passwords in the sip_accounts table. The SIP2 system now relies entirely
on Koha's secure password authentication.
Changes:
- Remove plaintext password comparison from login_core() and telnet_transport()
- Remove login_password column from sip_accounts table
- Update database schema files and atomic update
- Remove password field from API definitions and Vue form
- Update all tests to remove login_password references
Authentication now works in two stages:
1. Access control: Check if userid exists in sip_accounts (allowlist)
2. Authentication: Validate password against hash in borrowers table
This maintains SIP access control while eliminating plaintext password
storage. Staff users authenticate with their Koha password (securely hashed).
Test plan:
1. Run updated tests to verify they pass
2. Verify SIP2 authentication works with staff user credentials
3. Confirm no plaintext passwords are stored
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
The classes that use Koha::SIP2::Object as base update the
config_timestamp server param whenever an object of said classes is
inserted, updated or deleted.
This patch considers that, and performs a check to see if the
config_timestamp is more recent than the one which the SIP server was
started with. If it is, config is fetched from the database for up to
date configurations.
Note: The classes currently using Koha::SIP2::Object as base are as
follows:
Koha::SIP2::Account
Koha::SIP2::Institution
Koha::SIP2::SystemPreferenceOverride
The 2 classes that are left out are:
Koha::SIP2::Listener
Koha::SIP2::ServerParam
A UI message is shown when CRUDing any of the 2 above informing that a
SIP server restart is required for the changes to take effect.
The reason these 2 are left out is because they're loaded and only
utilized when the SIP server is launched.
Co-authored-by: Mark Hofstetter <mark@hofstetter.at>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
To test:
perl ./C4/SIP/SIPServer.pm /etc/koha/sites/kohadev/SIPconfig.xml
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
In legacy code, some trailing whitespaces have been left for training purposes.
This patch removes the trailing whitespace at line 266 in C4/Barcodes.pm
Test plan:
1. View C4/Barcodes.pm and note trailing whitespace at line 266.
2. Apply patch.
3. View C4/Barcodes.pm and note trailing whitespace has been removed.
Signed-off-by: Alex Carver <alexcarver@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
To test:
1. prove t/db_dependent/Auth.t
2. Observe success
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Baptiste
Lucas Gass
Marcel de Rooy
Martin Renvoize
Arthur Suzuki
Lari Taskula
Tomás Cohen Arazi
Kyle M Hall
Jonathan Druart
Nick Clemens
Matt Blenkinsop
Matt Blenkinsop
Hammat Wele
Martin Renvoize
Jan Kissig
Pedro Amorim
Pedro Amorim
Alex Carver