In testing, the messaging preferences are set in the form when EnhancedMessagingPreferences is enabled.
Switching categories silently updates the preferences unless you have made changes, in which case the
user is warned and can confirm if they want to reset or not. The code only applies to patron creatoin, and
does not seem to be taken into account during OPAC self-registration.
It seems the silent defaulting when saving can be removed in favor of the current updates/warns in the actual
form.
To test:
1 - Set some messaging preferences for at least two categories
2 - Enter a new patron
3 - Confirm switching categories correctly defaults the messaging preferences
4 - Edit the preferences and switch to another category with defaults
5 - Confirm the warning appears, and preferences are set if confirmed and ignored if not
6 - Set to a category with defaults, uncheck all, and save
7 - Ooof, patron is defaulted
8 - Apply patch
9 - Repeat 1-5 and confirm nothing has changed
10 - Set to a category with defaults, uncheck all, save
11 - Confirm patron is saved with no preferences
Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 70fde44d4f)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch fixes the overly extreme message that was designed to appeared
when someone tried to check out a booked item to the wrong user.
However, the message never appeared due to a logic error within
CanBookBeIssued which this patch also resolved.
Test plan
1) Make an item bookable
2) Add a booking for the item to patron A from tomorrow
3) In the database, update the booking start_date to today (or wait a
day before the next step)
4) Attempt to check the book out to patron B.
Note that you are displayed with the new message 'The item is booked
for another patron' and you are unable to check the item out.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 1ebd742e08)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This prevents the User/Group Not Defined warns in syslog.
Test plan:
Restart sip and check your syslog.
KTD Test Plan:
1) Stop your SIP server
2) Run "perl C4/SIP/SIPServer.pm /etc/koha/sites/kohadev/koha-conf.xml"
We need valid XML without any valid SIP settings to the output goes
to the command line
3) Note the warnings
4) Apply this patch
5) Repeat step 2
6) No warnings!
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 2973a4e81a)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch wraps the call for record in an eval, and catches any invalid metadata
exceptions, letting the warning show, but allowing the script to continue
To test:
1 - In default KTD record 369 has problems, otherwise you need to break a record
2 - Run : misc/cronjobs/update_totalissues.pl --use-stats --commit=1000 -v
3 - It dies at record 369 (or the one you broke)
4 - Apply patch
5 - Run again
6 - It succeeds, but skips the bad record
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit f3d2f34d7a)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
To test:
1 - Have something on the SCO page that won't load
Add to SCOUserJS:
</script>
<img src="http://localhost:8080/silk/famfamfam.png"/>
<script>
2 - Enable the SCO and SCI system preferences:
WebBasedSelfCheck: Enable
SelfCheckInModuke: Enable
AutoSelfCheckAllowed: Allow
AutoSelfCheckID / AutoSelfCheckPass to a valid user with permissions
3 - Add the self_checkin_module and self_checkout_module permissions to the AutoSelfCheck user
4 - Access the selfcheckout module https://yoursite.org/cgi-bin/koha/sco/sco-main.pl
5 - Sign in using a patron
6 - Enter an unkown barcode, I used "DONTMATTER"
7 - If not logged out, click to 'Return to account summary'
8 - You are asked to sign in again
9 - Repeat with the SCI module
10 - Apply patch
11 - Repeat, but this time you can return to account
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 5bab87f76b)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
If an SMS::Send driver succeeds, it returns a value that evaluates to true. Every driver I've inspected uses croak when it encounters a failure state.
When an SMS message fails to send, code hard codes the failure code to NO_NOTES (No notes from SMS driver).
We should store the real error in `failure_code` and display that if the failure code doesn't match a known failure code.
Test Plan:
1) Apply this patch
2) Set SMSSendDriver to any value
3) Generate a pending sms message
4) Run the following query:
update message_queue set status = 'failed', failure_code = "This is a test";
5) View the patron's messages, note the delivery note contains the
contents of the failure code
6) Run the following query:
update message_queue set status = 'failed', failure_code = "SMS_SEND_DRIVER_MISSING";
7) Reload the patron's messages, not the delivery note is now "The SMS
driver could not be loaded".
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 8acdc1e8e9)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Replace all instances in files with correct capitalization for the
sysprefs:
SerialsDefaultEMailAddress
AcquisitionsDefaultEMailAddress
Test plan:
1. Apply patch
2. Run database update
3. Go to Administration > System Preferences > Acquisitions
-> Confirm correct capitalization for AcquisitionsDefaultEmailAddress
(should be "Email", not "EMail")
4. Go to Administration > System Preferences > Serials
-> Confirm correct capitalization for SerialsDefaultEmailAddress
(should be "Email", not "EMail")
5. Use git grep to confirm no remaining instances of "EMail" in the code
(other than the new dbrev, original dbrev and original release notes)
e.g. git grep --files-with-matches "EMail"
6. Confirm the test plan for Bug 20755 still works
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit c1a1374100)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
GetOtherReserves attempts to set the waiting/transit status for the next
hold on the list when applicable, but in practice it either leaves the
hold state unchanged, or sets the itemnumber without setting the found
status (erroneously converting bib-level holds to item-level holds).
The latter situation only occurs when the user has been prompted to
confirm, cancel, or revert the hold, and is able to ignore the prompt.
In those situations, the hold's state should not change.
GetOtherReserves does not need to change the hold state, and it does not
do so correctly. Besides that, it does not do much other than call
CheckReserves, and is only used in 3 places.
This patch removes GetOtherReserves, and refactors returns.pl and
C4::Reserves::ModReserveCancelAll to call CheckReserves directly instead.
To test:
1. Place 2 bib-level holds for 2 different patrons (Patron A and Patron
B) on the same record, both for pickup at the logged-in library
2. Check in an item from that record to fill Patron A's hold
3. Set the hold's expiration date to yesterday by accessing the database
in the command line:
- In a ktd shell prompt, open the db client with koha-mysql kohadev
- UPDATE reserves
SET expirationdate = DATE_SUB(CURDATE(), INTERVAL 1 DAY)
WHERE borrowernumber = <Patron A's borrowernumber>
4. Go to Circulation > Holds Awaiting Pickup, and find the hold in the
"holds waiting past their expiration date" tab
5. Click the "Cancel hold" button in the Actions column next to the hold
(do not check in the book)
6. Return to the bib record and look at Patron B's hold
--> Note that Patron B's hold is now an item-level hold and does not
have a waiting status
7. Cancel Patron B's hold
8. Place 2 new holds on the record: one for Patron A at the logged-in
library, and one for Patron B at a different library
9. Check in an item to fill Patron A's hold
10. Repeat steps 3-5 to expire and cancel Patron A's hold
11. Return to the Holds tab of the bib record and look at Patron B's hold
--> Note that Patron B's hold is now an item-level hold, and there is no
"Revert transit status" button
12. Place 2 bib-level holds for 2 different patrons (Patron A and Patron
B) on the same record, both for pickup at the logged-in library
13. Check in an item from that record to fill Patron A's hold
14. Check in the same item again. A modal will pop up, saying that the
hold is already waiting
15. In the modal, choose a cancellation reason and click "Cancel hold"
--> A new modal will pop up to fill Patron B's hold
16. Click "Ignore" on the modal for Patron B's hold
17. Return to the bib record and look at Patron B's hold
--> Note that Patron B's hold is now an item-level hold and does not
have a waiting status
18. Apply patch
19. Repeat steps 1-6
--> Note that Patron B's hold is still a bib-level/"next available" hold
20. Repeat steps 7-11
--> Note that Patron B's hold is still a bib-level/"next available" hold
21. Repeat steps 12-17
--> Note that Patron B's hold is still a bib-level/"next available" hold
Make sure correct behavior is unchanged:
22. Cancel Patron B's hold
23. Place 2 new holds on the record: one for Patron A at the logged-in
library, and one for Patron B at a different library
24. Check in an item from that record to fill Patron A's hold
25. Check in the same item again. A modal will pop up, saying that the
hold is already waiting
26. In the modal, choose a cancellation reason and click "Cancel hold"
--> A new modal will pop up to fill Patron B's hold
27. Click "Print slip, transfer, and confirm" on the modal for Patron B's hold
--> Confirm that the information on the slip is correct
--> Confirm that the hold is correctly put in transit
22. Set HoldsAutoFill and HoldsAutoFillPrintSlip to "Do"
23. Place a bib-level hold for the logged-in library
24. Check in an item from that bib
--> Confirm the information on the slip is correct
--> Confirm the hold is correctly assigned and set to waiting
25. Place a bib-level hold for a different library
26. Check in an item from that bib
--> Confirm the information on the slip is correct
--> Confirm the hold is correctly put in transit
27. Change the logged-in branch to match the hold pickup location
28. Check the item in
--> Confirm the information on the slip is correct
--> Confirm the hold is correctly assigned and set to waiting
29. Repeat steps 22-26
--> Confirm a correct hold slip pops up for Patron B's hold
--> Confirm that Patron B's hold is correctly put in transit
30. Cancel Patron B's hold
31. Place 2 bib-level holds for 2 different patrons (Patron A and Patron
B) on the same record, both for pickup at the logged-in library
33. Repeat steps 24-26
--> Confirm a correct hold slip pops up for Patron B's hold
--> Confirm Patron B's hold is correctly set to Waiting
34. Prove t/db_dependent/Circulation.t
35. Prove t/db_dependent/Koha/Holds.t
--> Tests pass
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit dc00e55a32)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
On (at least) git installations of Koha checkouts and checkins fail on
error 500. Logs have following error:
Undefined subroutine &C4::Circulation::CheckReserves called...
Error happens also when one tries to open patrons checkouts from detail page.
Koha doesn't die but table just keeps loading. Solution is to add C4::Reserves
before CheckReserves when it's called from Circulation.pm.
To test:
1. Apply this patch.
2. Try to check out and check in item.
=> Confirm both operations are succesfull.
3. Attempt to open patrons checkouts from patron detail and checkout page.
=> Table should load
Also prove t/db_dependent/Circulation.t.
Sponsored-by: Koha-Suomi Oy
Signed-off-by: BabaJaga <babajagawgoglach@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 80beaf875b)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
The userenv (logged in user's info) are stored in
$C4::Context->context->{activeuser}, which persists in plack worker's
memory.
It's really bad in theory as we are not cleaning it before or after the
HTTP request, but only when set_userenv is called (what we are doing
commonly in C4::Auth::get_template_and_user).
If C4::Context->userenv is called before set_userenv we should get undef,
not the userenv from the previous request!
In practice this should not be a problem, but well... who really knows?
This patch suggests to have a middleware to deal with removing the
userenv at the beginning of each request (maybe it should be after, right? - FIXME).
To test:
1 - Edit /etc/koha/sites/kohadev/koha-conf.xml to set <plack_workers>1</plack_workers>
2 - Edit about.pl and add a line after: CGI->new:
warn Data::Dumper::Dumper( C4::Cointext->userenv() );
3 - tail -f /var/log/koha/kohadev/*.log
4 - View about.pl in staff interface, should get a "somethign's wrong" warning
5 - Reload, you get current user info
6 - Open an incognito tab, sign in as a different user and click some stuff
7 - Reload about.pl in other window
8 - You get the opac user info
9 - Apply patch
10 - Edit /etc/koha/sites/kohadev/plack.psgi and add the middleware after "RealIP":
enable "+Koha::Middleware::UserEnv";
11 - Restart all
12 - Reload about.pl - you get a "Something's wrong" warning
13 - Click things in opac on incognito window
14 - Reload about.pl - only "Something's wrong" - you no longer see any user info
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 576e7e09fdca703f76c0d10ae55eebf12ee1fdf4)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Also tidy
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
(cherry picked from commit 42d388c80fcdd98c2594ad7b111b8e40c991388a)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
(cherry picked from commit 7a626d8d870039330889d6e48c3ae5ba848d85e9)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
(cherry picked from commit ca64e4f6f30b172d86184c61134f5f29713863d2)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
When changing the fetch of holds, the check for non-priority was lost - added a loop to pull those out
so the totals and checks are correct
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Tidied (tcohen)
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit b7ad3364cb)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Before this patch we get all holds on a record and see if we can fill them with available items.
This means we check to fill holds that the item in questoion may not be able to fill, especially
in the case where no holds are allowed on the item type, this is wrong
To test:
1 - Find or create a biblio with two items of different item types
2 - Make sure one item type allows holds, and the other has:
"Default holds policy by item type"
Set to "No holds allowed"
3 - Set system preference "AllowRenewalIfOtherItemsAvailable" to "Don't allow"
4 - Check out the unholdable item to a patron
5 - Set a hold for a different patron on the next available item
6 - Confirm the checked out item can be renewed (don't renew, just view the checkouts page)
7 - Checkout the other item to a third patron
8 - Confirm the first item can still be renewed
9 - Set system preference "AllowRenewalIfOtherItemsAvailable" to "Allow"
10 - Confirm the item cannot be renewed now
11 - Apply patch, restart all
12 - Confirm the item can be renewed
13 - Set the item type to a type that allows holds
14 - Confirm the item can no longer be renewed
15 - Restore the item type
16 - Set system preference "AllowRenewalIfOtherItemsAvailable" to "Don't allow"
17 - Confirm the item can be renewed
18 - Check in the item from the third patron
19 - Confirm the item can still be renewed
20 - prove -v t/db_dependent/Circulation.t - test still pass
Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 9cc622be1f)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch adds a replacement for 'booked' to the SIP renewal handling
so we can display 'Item is booked for another borrower'.
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit c5522bc81e)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch adds a bookings check to CanBookBeRenewed
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 59afaf533b)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
To avoid injection of template toolkit code
from database fields that are controlled by
untrusted sources.
Test plan:
* review subtest 'Template toolkit syntax in
parameters' in t/db_dependent/Letters.t
* Run the unit test:
prove t/db_dependent/Letters.t
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 07ac3b0b9450f812bb48cfecf7bf3f47f63279b5)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Whilst writing the test, I found a minor flaw in the logic and fixed
that in CanBookBeIssued at the same time.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit df39d76f30)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
If autolinking is enabled, there are scenarios where the authorities merge process
updates the record, then sends it to ModBiblio which then AutoLinks the record, undoing
the changes in the record (as the 'from' authority is not removed until the process is complete)
This patch simply disables the autolinking process when merging authorities
To test:
Set system preferences
AutoCreateAuthorities = don't generate
RequireChoosingExistingAuthority = don't require
AutoLinkBiblios = Do
CatalogModuleRelink = Do
LinkerKeepStale = Don't
LinkerModule = first match
LinkerRelink = Do
1. Create an authority record
1.1. Go to Authorities
1.2. Click on "New authority" and choose an authority type (I chose Corporate name)*
1.3. Fill in the mandatory fields (000, 003, 005, 008, 040)
1.4. In tab 1, click on the label of the main heading (110 for Corporate name)
1.5. Fill in subfield
1.6. Copy content of subfield
1.7. Click on "Save"
1.8. Note the authority record number
2. Duplicate the authority record
2.1. Click on "Edit" and choose "Edit as new (duplicate)"
2.2. Click on "Save"
2.3. Click on "No: Save as new authority"
2.4. Note the authority record number (should be the number from step 1.8 +1)
3. Link one of the authority records to a bibliographic record
3.1 In another tab, do a catalog search
3.2. Click on "Edit record" under one of the search results
3.3. Go to the relevant tab (for corporate name, I used field 710 in tab 7)
3.3.a. If there are no empty fields for that tag, click the "Repeat this tag" button (two rectangles)
3.4. Paste the text previously copied in subfield
3.5. Click on "Link authorities automatically" at the top of the page
3.6. Note the authority record number in subfield
--> The linked authority record should be the first authority record (step 1.8)
3.7. Click on "Save"
4. Merge the two authority records, keep the one that is not linked to the bibliographic record
4.1. Go back to the tab with the authorities
4.1. Go to Authorities
4.2. Search for the aurhorities previous created
4.2. Click on "Actions" and choose "Merge" for both records
4.3. Select the record number that is NOT linked to the bibliographic record
4.4. Click on "Next"
4.5. Click on "Merge"
5. Check the bibliographic record
5.1. Go back to the tab with the bibliographic record and refresh the page (Ctrl+F5)
5.2. Hover the mouse cursor over the name or term, without clicking
5.3. Note the record number that appears at the bottom of the screen
--> The record number was not changed
6. Apply the patch
6.1. Run prove t/AuthoritiesMarc_MARC21.t
6.2. Repeat Previous steps 1, 2, 3, 4, 5
--> The record number is change
Signed-off-by: Barbara Petritsch <barbara.petritsch@wienmuseum.at>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 1b9371beb3)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
There should be no change in beahavior. Following the test plan from Bug 35840.
To test:
1. APPLY PATCH, restart_all
2. Turn on RecordLocalUseOnReturn
3. Create a Statistical patron.
4. Check an item out to a regular patron.
5. Check the item out to a Statistical patron.
6. This should trigger a return and you will see 2 entries in the statistics table, one for localuse and one for a return.
7. Try checking out an item to the Stats patron that is NOT checked out.
8. You should only see 1 entry, localuse, in the statistics table.
Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 834bff6317)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
To test:
1. APPLY PATCH, restart_all
2. Turn on RecordLocalUseOnReturn
3. Create a Statistical patron.
4. Check an item out to a regular patron.
5. Check the item out to a Statistical patron.
6. This should trigger a return and you will see 2 entries in the statistics table, one for localuse and one for a return.
7. Try checking out an item to the Stats patron that is NOT checked out.
8. You should only see 1 entry, localuse, in the statistics table.
Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit ffc722554e)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Test Plan:
1) Place a hold on an item
2) Build the holds queue
3) Check out the item to a different patron than the one
targeted in the holds queue
4) Verify the holds queue viewer still shows that item and patron
5) Apply this patch
6) Repeat stepts 1 through 3
7) Verify the holds queue viewer no longer shows that patron and item!
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 0eced80866)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Working on bug 31791, I found myself wondering if our current recursive
code in C4::Auth::haspermission() would allow checking AND on
subpermissions.
As it is not documented in the POD or tested, I decided to write some
unit tests for it.
It turned out it was well supported, so I decided to submit the tests,
and a small tweak in the POD to reflect that.
To test:
1. Apply this patch
2. Run:
$ ktd --shell
k$ prove t/db_dependent/Auth/haspermission.t
=> SUCCESS: Tests pass! The code supports AND on subpermissions!
3. Sign off :-D
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit aa1049fdd3)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Test plan:
Add an item to your database that has no barcode.
Run t/db_dependent/Circulation.t
It will fail without this patch, pass with this patch.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 8413b37679)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
In automatic item modifications by age missing,
conditions can be on columns of tables items or biblioitems.
Table biblio is missing.
Test plan :
1) Create an automatic item modifications by age with a condition on a column of biblio table
2) Create a record and item matching the rule
3) Run misc/cronjobs/automatic_item_modification_by_age.pl -c -v
=> Check only matching items are impacted
Signed-off-by: Philip Orr <philip.orr@lmscloud.de>
Bug 32029: (follow-up) unit test
Signed-off-by: matthias le gac <matthias.le-gac@inlibro.com>
Bug 32029: (follow-up) tidy
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 4e480d48f6)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch changes CanBookBeRenewed so that automatic renewal
errors pop up before other renewal errors. This means that a book
will be considered "auto_too_soon" before things like "too_many" or
"restricted". (Otherwise, you'll get an email saying you can't renew
a book the day after using your last auto renewal, even though the
earliest renewal isn't available until later.)
Test plan:
0. Apply patch
1. prove t/db_dependent/Circulation.t
2. prove t/db_dependent/Holds.t
3. prove t/db_dependent/Koha/Account/Line.t
4. prove t/db_dependent/Koha/Account.t
Additional tests:
5. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=RestrictionBlockRenewing
6. Change to "block"
7. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=AutoRenewalNotices
8. Change to "according to patron messaging preferences"
9. Go to http://localhost:8081/cgi-bin/koha/admin/smart-rules.pl
10. Set "Automatic renewal" to "Yes" and "No renewal before" to 4
11. Go to http://localhost:8081/cgi-bin/koha/circ/circulation.pl?borrowernumber=51
12. Checkout 39999000001310 with a due date 4 days in the future
13. Add a manual restriction
14. Run `perl ./misc/cronjobs/automatic_renewals.pl`
15. Note that it says something like the following:
Issue id: 1237 for borrower: 51 and item: 73 would not be renewed. (auto_too_soon)
Instead of something like the following:
Issue id: 1237 for borrower: 51 and item: 73 would not be renewed. (restriction)
Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 5d7a9395fb)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch adds a Koha::Session module that makes it easier
to work with Koha sessions without needing the full C4::Auth module.
Test plan:
0. Apply the patch
1. Run the following unit tests:
prove ./t/db_dependent/Auth.t
prove ./t/db_dependent/Auth_with_cas.t
prove ./t/db_dependent/Koha/Session.t
2. Observe that they all pass
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit 0e6537d199)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
It seems safer to pass the logged in user and session info at the end of
the sub.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
(cherry picked from commit 199b47e51220a22110436a2357481dc89d498537)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
If we hit the auth page we were not passing sessionID to the template
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
(cherry picked from commit 95098d23e0c0bfc5291464a625ff6422b8288888)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This code is a bit weird, its purpose it to auto select the library depending on the IP.
A problem appears if the same IP is used, then the user's choice will
might be overwritten randomly by another library.
To recreate the problem:
Turn on AutoLocation
Use koha/koha @CPL for test
And the following config:
*************************** 1. row ***************************
branchcode: CPL
branchname: Centerville
branchip: 172.18.0.1
*************************** 2. row ***************************
branchcode: FFL
branchname: Fairfield
branchip: 172.18.0.1
*************************** 3. row ***************************
branchcode: FPL
branchname: Fairview
branchip: 172.18.0.4
Connect and select CPL. Randomly FFL will be picked instead.
Signed-off-by: Magnus Enger <magnus@libriotech.no>
Tested this on top of 35890 and 35904 because git bz said they were required dependencies.
Figured out the IP Koha was seeing me as coming from in /var/log/koha/kohadev/plack.log.
Added that IP to the branchip for Centerville, Fairfield and Fairview. Set AutoLocation = Yes.
After this I could recreate the problem: If i left the "Library" field in the login screen
at "My Library" I got logged into a random library selected from the three i had set
branchip for. Applying the patches fixed this, as expected.
Tests pass, with AutoLocation off.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 4efe74fe12075298680965db3605f717f1da10d0)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 94e570d6af38c0061aeaad2ea25ab26bed2186f5)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
It is never used and add confusion
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 0ce8cc4c05bc96503172018775ba574e41b40ecb)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch suggests to add a new flag do_not_print to
C4::Auth::checkauth to not print the headers and allow to test this
subroutine more easily.
We do no longer need to mock safe_exit and redirect STDOUT to test its
return values.
There are still 3 left:
1.
733 # checkauth will redirect and safe_exit if not authenticated and not authorized
=> Better to keep this one, not trivial to replace
2.
806 # This will fail on permissions
This should be replaced but testing $template->{VARS}->{nopermission}
fails, I dont' think the comment is better.
3.
828 # Patron does not have the borrowers permission
Same as 2.
2. and 3. should be investigated a bit more.
This patch also move duplicated code to set patron's password to a
subroutine set_weak_password.
Test plan:
Read the code and confirm that everything makes sense.
QA: Do you have a better way for this? Yes it's dirty!
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
(cherry picked from commit db173d3276455a43939dca68ccc6502839fa2a55)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Adapt code to the change of return value type of checkpw
introduced in bug 34893
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
(cherry picked from commit 5f9e9e5df2)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch adds further delivery details to the notices tab in patron
details in the staff client.
Once a message is sent, we display the 'from:', 'to:' and 'cc:'
addresses in the 'Delivery note' column when they exist.
Test plan
1. Enable KTD to send email [1] (without email configured the
delivery note displayed "Unhandled email failure, check the logs for
further details").
2. Add email addresses to two patrons and to KohaAdminEmailAddress,
and run misc/cronjobs/process_message_queue.pl after generating
notices.
3. For the two patrons with email addresses, make one a guarantor.
4. Sent Welcome messages (Patron account > More > Send welcome email) -
nothing in delivery note column.
5. Checkout out an item to the guarantee (item checkout email enabled) -
nothing in delivery note column.
6. Send the notices by running misc/cronjobs/process_message_queue.pl
again.
7. Now the 'Delivery note' columns should contain from:, to: and cc:
address details.
[1] Option 1 - smpt-sink (aka the sandboxes way)
- Install the postfix package inside ktd (sudo apt install postfix)
When asked in the wizard, I named mine 'local'
- Start smpt-sink with
`nohup smtp-sink -u root -D mail 127.0.0.1:25 100 </dev/null >/dev/null 2>&1 &`
Option 2 - To test sending emails using a Google account:
- Set up an App password for your Google Account
- Edit /etc/koha/sites/kohadev/koha-conf.xml file and add this
configuration near the end (where <user_name> = your Google email
address; <password> = your APP password, not your Google account
password):
<smtp_server>
<host>smtp.gmail.com</host>
<port>587</port>
<timeout>5</timeout>
<ssl_mode>STARTTLS</ssl_mode>
<user_name>GOOGLEACCOUNTUSER</user_name>
<password>GOOGLEAPPPASSWORD</password>
<debug>1</debug>
</smtp_server>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 137376ab4b)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Test plan:
Compile module, run qa tools.
Search for the use of C4::Items in C4/Biblio.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 3385142d7c)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Resolve:
[2024/01/18 11:31:24] [WARN] Use of uninitialized value $hits in numeric eq (==) at /usr/share/koha/opac/opac-search.pl line 612.
[2024/01/18 11:31:24] [WARN] Use of uninitialized value $times in subtraction (-) at /usr/share/koha/C4/Search.pm line 1715.
Test plan:
Look for a few search expressions without results.
Check log without and with patch.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit cb7921d8d0)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
[2024/01/18 11:52:40] [WARN] Argument " " isn't numeric in numeric eq (==) at /usr/share/koha/C4/Koha.pm line 600.
[2024/01/18 11:52:40] [WARN] Argument " " isn't numeric in numeric eq (==) at /usr/share/koha/C4/Koha.pm line 659.
Note: According to the code in MARC::Field the indicator returned cannot be undef. So we do not return new
uninitialized warnings.
Test plan:
Check your log before and after patch.
Possibly you may need to create a record with spaces as field indicators.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 857d8f4d82)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
"Renew All" -function aka. 65-66-messages work like this: 66-return messages has renewed items in BM-fields and non-renewables go into BN-fields.
If all items were renewable, then the reply message looks like:
Reply:
13.07.2022 10:06:17:530 OUTESTI1 6610005000020220713 100616AOOUPK|BM564N15977171|BM564N09203960|BM564N23261720|BM564N25161958|BM564N23455012|C4::SIP::SIPServer=HASHC4::SIP::SIPServer=HASHAY6AZCEFE
If items include renewables and non-renewables, the response includes the renewed items first, then a HASH mixed into the first BN-field, then rest of the renewed ones like they should show in BN-fields and after that, anohter HASH:
13.07.2022 14:41:32:610 OUTESTI1 6610003000420220713 144130AOOUPK|BM564N23455012|BM564N23261720|BM564N25161958|C4::SIP::SIPServer=HASHBN564N09203960|BN564N04191259|BN564N15977171|BN564N13788332|C4::SIP::SIPServer=HASHAY8AZC7C5
If all loans were unrenewable (this test included just one item), you first get a HASH in a BN-field, and another at the end.
13.07.2022 14:49:00:037 OUTESTI1 6610000000120220713 144858AOOUPK|C4::SIP::SIPServer=HASHBN564N19874533|C4::SIP::SIPServer=HASHAY9AZDD3B
Our self checkout/checkin machine can handle/parse these reply messages correctly, but this might not be the case with all self checkout/checkin devices.
Test Plan:
1) Unit test patch
2) prove t/db_dependent/SIP/Message.t
3) Note failures
4) Apply this patch
5) prove t/db_dependent/SIP/Message.t
6) Tests pass!
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: lmstrand <lmstrand@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit bb7fc5025b)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
In the sub the userId was given, but the cardnumber was expected, therefore at every login the cardnumber changed to the userId.
To test:
1. Login to the OPAC with an LDAP account that has a cardnumber which is different to the userId
2. Check the patron record's cardnumber.
3. The cardnumber should equal to the userId
4. Apply the patch.
5. Login to the OPAC with an LDAP account that has a cardnumber which is different to the userId
6. Check the patron record's cardnumber.
7. The cardnumber should be the same as before
Signed-off-by: Alexander Wagner <alexander.wagner@desy.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Extra test plan:
1. Apply the regression tests
2. Run:
$ ktd --shell
k$ prove t/db_dependent/Auth_with_ldap.t
=> FAIL: Tests fail! 'cardnumber' incorrectly overwritten
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests now pass!
5. Sign off :-D
Signed-off-by: Alexander Wagner <alexander.wagner@desy.de>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 6341d520ac)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
C4/Utils/DataTables/VirtualShelves.pm: "@order_by" is declared but not used at line 86, column 9. Unused variables clutter code and make it harder to read. (Severity: 3)
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 0c96e57b0c)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
And remove C4::Utils::DataTables, which should no longer be reused
anyway.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
(cherry picked from commit 4a65c417f8bfeefe9299981b415f3fbfca5cb5a4)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch introduces some tests on the current (and new) behavior for
the `checkpw` function.
I needed it to better understand if an edge case was actually possible
(it wasn't).
Found a really minor annoyance for the internal check with expired
password not returning the $patron object for consistency with the other
use cases.
I think this method deserves (at least) changing the return value to a
sane data structure. But that's not target for backporting to stable
releases. So a separate bug.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
(cherry picked from commit 5476b18e7ea34e08d9dd163e2c446d5b223cf032)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Nick Clemens
Martin Renvoize
Marcel de Rooy
Kyle M Hall
Emily Lamancusa
Emmi Takkinen
Jonathan Druart
Andreas Jonsson
Lucas Gass
Tomas Cohen Arazi
Fridolin Somers
David Cook
Victor Grousset/tuxayo
Janik Hilser
