git.koha-community.org Git

Bug 33881: Clear self-check JWT during auth kick out

This patch clears the JWT cookie during auth kick out (ie
when a web user navigates from the self-check out/in to
the rest of Koha).

Test plan:
0. Apply patch and koha-plack --reload kohadev
1. Go to http://localhost:8080/cgi-bin/koha/sco/sco-main.pl
2. Log in as the "koha" user
3. In another tab, go to http://localhost:8080/cgi-bin/koha/sco/sco-main.pl
4. Go to http://localhost:8080/cgi-bin/koha/opac-search.pl?idx=&q=a&weight_search=1
5. Note that you are prompted to "Log in to your account" via the normal Koha prompt
6. Go to http://localhost:8080/cgi-bin/koha/sco/sco-main.pl
7. Note that you are prompted to "Log in to your account" within the "Self checkout system",
and note that your self-checkout session for the "koha" user has *not* persisted like
it did before the patch was applied

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 30524: (QA follow-up) Fix tests

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

Bug 30524: (QA follow-up) Unit tests for GenerateCSRF()

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

Bug 30524: (QA follow-up) Only generate CSRF token if it will be used

This patch avoids generating CSRF tokens unless the csrf-token.inc file
is included in the template.

Passed token doesn't need HTML escaped. The docs for WWW::CSRF state:
The returned CSRF token is in a text-only form suitable for inserting into a HTML form without further escaping (assuming you did not send in strange things to the Time option).

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

Bug 34023: Prevent HTML injection in "back to results" link from search page

It is possible inject raw HTML into the "Back to search results" link by leading the user to a search with specially crafted URL.

For example, using the demo instance:

1. Visit https://koha.adminkuhn.ch/cgi-bin/koha/opac-search.pl?idx=&q=test&weight_search=1&%22%3Etest%3Ca%20foo=%22

2. Refresh the page (for some reason, "back to results" doesn't appear unless I do that at least once).

3. Click any result.

Note that the result page now contains:

<a href="opac-search.pl?idx=&q=test&weight_search=1&">test<a foo=%22" title="...

i.e. `">test<a ...` was successfully injected into the HTML.

I'm attaching a quick patch I've used to patch up our instance. It just indiscriminately URI-escapes all parameter keys. I didn't decode them back since as far as I understand all valid keys do not contain special characters.

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: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 22990: (follow-up) Fix suggestion.pl

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

Bug 22990: Fix template toolkit syntax issues in shelves_results.tt

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

Bug 22990: Fix for shelves table

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 22990: Add CSRF protection to boraccount, pay and suggestion

Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Test plan would have been nioe.
Tested by changing MAX_AGE with suggestions.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 34368: Add CSRF token to Content Management pages

This change adds a CSRF token to the Content Management pages
at additional-contents.pl.

Test plan:
0. Apply patch
1. koha-plack --restart kohadev
2. Try to add "News", "HTML customizations", and "Pages".
3. Try to delete these new content entries
4. Note that you were successful in your endeavours

JD amended patch: remove empty line removal (no need to create
unecessary conflicts)

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: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 30524: Unit tests

Test plan:
Run t/Output.t
Run t/db_dependent/Auth.t

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

Bug 30524: Core CSRF checking code

Split out from bug 22990 as requested.

Signed-off-by: David Cook <dcook@prosentient.com.au>
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>

Bug 28966: (QA follow-up) tmp_holdsqueue no longer an iterator

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

Bug 28966: DBRev 23.06.00.013

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

Bug 28966: DBIC schema

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

Bug 28966: (QA follow-up) Fix failing unit tests

Test Plan:
1) prove t/db_dependent/TestBuilder.t
2) Note tests fail
3) Apply this patch
4) Run updatedatabase.pl
5) Update the schema files ( alias 'dbic' can be used in
koha-testing-docker )
6) prove t/db_dependent/TestBuilder.t
7) Tests now pass!

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

Bug 34258: (QA follow-up) Tidy the test

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 34258: update SIP-related unit test

Test plan:

1) Run the updated SIP-related unit test *without* having applied
the other patch from this bug report -- it should fail:

$ prove t/db_dependent/SIP/ILS.t

2) Apply the patch that fixes C4/SIP/ILS/Transaction/Renew.pm

3) Re-run the unit test -- it should pass.

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 34258: pass an unblessed hash to AddIssue()

In Koha 23.05, we lost the ability to renew an item via SIP2.

The relevant commit is ddc2906b77 from Bug 31735, where the
file C4/SIP/ILS/Transaction/Renew.pm was modified to no longer
pass an unblessed $patron hash to C4::Circulation::AddIssue()

This patch fixes that.

Test plan:

1) Using the SIP emulator, check out an item to a patron, then
   try to renew it. Example commands for a KTD instance:

   $ misc/sip_cli_emulator.pl -a localhost -p 6001 -l CPL -su term1 -sp term1 -m checkout --patron koha --item 3999900000001
   $ misc/sip_cli_emulator.pl -a localhost -p 6001 -l CPL -su term1 -sp term1 -m renew --patron koha --item 3999900000001

   Notice that the second command will fail!

2) Apply this patch.

3) Repeat the 2nd command -- this time the renewal should work.

4) Run the SIP-related unit tests, they should all pass:

   $ prove t/db_dependent/SIP/
   t/db_dependent/SIP/ILS.t .......... ok
   t/db_dependent/SIP/Message.t ...... ok
   t/db_dependent/SIP/Patron.t ....... ok
   t/db_dependent/SIP/SIPServer.t .... ok
   t/db_dependent/SIP/Transaction.t .. ok

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 28966: (QA follow-up) Remove superfluous joins

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

Bug 28966: (QA follow-up) Changes for Koha objects

Trivial edits.

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

Bug 28966: Prefetch patron data for holds queue viewer

Test Plan:
1) Generate the holds queue
2) Load the holds queue viewer page
3) Apply this patch
4) Restart all the things!
5) Reload the page
6) Note nothing has changed

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

Bug 28966: Add Koha::Object(s) for tmp_holdsqueue

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

Bug 32739: (follow-up) QA Cleanup

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 32739: Allow other patron identifier on pwd validation

This patch takes a step forward on the password validation endpoint, by
adding  the `identifier` parameter and making it be allowed
to be the patron's `cardnumber` or the `userid`.

The current `userid` only validation option is kept as-is.

The implementation relies on `C4::Auth::checkpw` to query for the
patron.

To test:
1. Apply this patches
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/password_validation.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 32739: Unit tests

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 34226: Add tests

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 34226: Use 'type' at the datatable config level

Lets use 'type' definitions at the datatables settings level instead

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 34226: DT wrapper - pre-processed date term when filtering

If we have filters on top of column on a table that is using the DT REST API wrapper,
we cannot filter on date using formatted dates.

This was done for "date of birth" for bug 32505.
Here we want to provide a generic approach.

Note that we cannot use what has been done on bug 22440 in some cases
(when we don't write the thead DOM directly but rely on DataTables
constructor, for instance bug 33568). The data- attributes are not
passed by DT.

Test plan:
On top of 33568, filter date columns using the full version of the
formatted date

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 33964: (QA follow-up) Remove library from cpanfile

Email::Sender::Transport::SMTP::Persistent is part of the Email::Sender
distribution, and a git diff on the repository doesn't show any
difference.

The patch author just took the number from MetaCPAN.

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

Bug 33964: (QA follow-up) Syntax error in cpanfile

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

Bug 34169: (follow-up) Fix ordering from staged files by removing superfluous form

Removes the unneded new form element as we have one big form for the whole page.

This should fix the situation where only the prices and information
of the first selected record carreid over into the order.

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

Bug 34169: Add decimal class to all relevant input fields in the acquisitions module

This is a first step towards more consistency and possibly supporting
multiple input formats as well in the future. It marks all input fields
for monetary values, such as prices, replacement prices etc. with a class
that is linked to a check for number format with the jQuery Validator plugin.

To test:

For any input field to test, try adding various false entries, like "abc" or "1,00".
It should only accept inputs with decimal dot, like: "1.00"

0) Apply patch, restart_all
1) Suggestion
  * Add a new suggestion in the staff interface
  * Test: price input field at the bottom of the form.
  * Accept the suggestion
2) Order form
  * Create a new basket
  * Create an order line from an existing record
  * Test: list price, replacement price, and actual price.
  * Check the checkbox for uncertain price before you save
3) Uncertain prices
  * Go to the uncertain prices page for this vendor
  * Test: price field
    Note: this form does its own validation, but the change should not change behaviour for now
  * Resolve the uncertain price
  * Close order
4) Receive shipment
  * Test: Shipping cost
5) Receive the order
  * Test: replacement price, actual price
  * Check checkbox for price in foreign currency
  * Test: price in foreign currency
  * Receive order line
6) Invoice summary
  * Finish receiving
  * Test: shipping cost
  * Test: invoice adjustments: amount in the form for the first entry, amount in the table after adding it
7) Merging invoices
  * Receive another shipment and create and invoice
  * Go to invoices and search all
  * Check the 2 entries for merging
  * Test: shipping cost
8) Adding orders from a staged/new file
  * Export some records using the cart or list
  * Create a new basket
  * Order from new file
  * Import your file, ignore item records
  * Test: price and replacement price
  + Bonus: also test with items, test plan and file from bug 22802 are really helpful here

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 34169: Use jQuery validator plugin to validate amounts

This is a first step towards more consistency and possibly supporting
multiple input formats as well in the future. It allows us to mark all
input fields for monetary values, such as prices, replacement prices,
fees etc. with a class that is linked to a check for the 'number' format
in the jQuery Validator plugin.

This is the base patch that does nothing by itself, please see
test plan in second patch.

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 33964: (QA follow-up) Handle absence of smtp server

Resolve:
Use of uninitialized value in hash element at /usr/share/koha/C4/Letters.pm line 1472.
Use of uninitialized value in hash element at /usr/share/koha/C4/Letters.pm line 1473.

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

Bug 33964: Use Email::Sender::Transport::SMTP::Persistent for sending email

As described in bug 30013, some outgoing SMTP services ( such as Gmail ) do not like Koha's current behavior of initiating a new connection for each email sent. If we switch from Email::Sender::Transport::SMTP to Email::Sender::Transport::SMTP::Persistent and store the object for the duration of the message queue processing, this should solve that issue.

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

Bug 33978: Adding authority from automatic linker closes imported record

If you import a record, then create an authority record using the automatic linker, it closes the biblio record. The problem occures when a record is edited in a new tab.

To recreate:

1. Import the example records
   1.1. Download the example records
   1.2. Go to Cataloging > Stage records for import
   1.3. Choose the downloaded file
   1.4. Click Upload file
   1.5. Click Stage for import
   1.6. Click View batch
   1.7. Click Import this batch into the catalog
   1.8. Click View detail of the enqueued job
   1.9. Click Manage imported batch

Correct behaviour:

2. In another tab, search for one of the records (for example, Fafounet)
3. Click Edit > Edit record
4. Go to field 100
5. Click Link authorities automatically
   --> It should say 100 - No matching authority found.
6. Click the plus sign next to 100
7. Fill out the mandatory fields by clinking in the text fields (000, 003, 005, 008, 040), field 100 should already be filled
8. Click 'Save'
   --> Authority number is added in 100 and you get to stay in the record for more edits if needed

Incorrect behaviour:

9. Go back to the imported batch tab
10. Click Edit next to the second title (the one by Paventi, Eza)
11. Redo steps 4 to 8
    --> Record is closedclear :(
    The behaviour should be the same, stay in the bibliographic record until it is saved.

12. Apply the patch
13. Redo step 9, 10, 4
14. Edit field 100, Type 'Paventi Test 2'
15. Redo step 5 to 8
    --> Authority number is added in 100 and you get to stay in the record for more edits if needed

Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 34280: (QA follow-up) perltidy

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 34280: Fix warning in logs when saving patron details

If a patron has no valid email address then a warning message appears in the logs when saving:

"Use of uninitialized value $email in string ne at /kohadevbox/koha/Koha/Patron.pm line 1445."

This patch fixes that error by removing an unnescessary string ne

Test plan:
1) Create/choose a patron with no email addresses
2) On the patron record in the page section for Contact information, click edit
3) Now click save
4) The warning above should appear in the logs
5) Apply patch
6) Repeat steps 2 and 3
7) The warning should no longer appear

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 34028: Show the secret when registering from the auth screen

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

Bug 33105: Fix Auth/Permissions.t

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

Bug 34181: Fix jQuery selector to make delete checkboxes reappear

Bug 32257 changed the page structure slightly to fix a display
issue with the labels. This resulted in a broken selector in the
function for displaying the checkboxes for deleting/emptying a
certain patron field.

To test:
* Go to Tools > Batch patron modifications
* Enter some cardnumbers or borrowernumbers
* On the batch patron edit form, verify that the checkboxes
  behind each input field are missing
* Apply patch
* Verify the checkboxes reappeared
  * Verify that for mandatory fields the checkbox is locked
* Make some batch edits and verify the checkboxes work as
  intended

Signed-off-by: Lisette Scheer <lisette.scheer@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 32711: Fix biblio embeds

We need to drop the embed part of the args we pass to biblioitem else we end
up with some very strange behaviours on the acquisitions endpoint.

Signed-off-by: Silvia Meakins <smeakins@eso.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 32711: Add 'public_read_list' appropriately

We're exposing biblio (and thus biblioitem) to the public API now and as
such we need to explisitely list which fields to include in the response
when called on the public api.

Signed-off-by: Silvia Meakins <smeakins@eso.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 32711: Add biblio details to modal

This patch adds biblio details to the self-checkout modal. As part of
this we add 'biblio' to the x-koha-embed option on the public items
endpoint, clone the js-biblio-format include from the staff client
adapting it slightly for OPAC use along the way and also import
escape_str and escapeHtml from staff to opac too.

Signed-off-by: Silvia Meakins <smeakins@eso.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 34213: import_patrons.pl - Fix short version of matchpoint option in POD

44 'c|confirm' => \$confirm,

46 'm|matchpoint=s' => \$matchpoint,

The POD is wrong.

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

Bug 34332: Removed extra parenthesis that was causing an error

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 34334: Item(s) in MARCdetail untranslatable

This patch adds a <span> around the text "Item(s)" in the template for
the MARC detail page in the staff interface. Without the span the
translation tool can't detect the string.

To test, apply the patch and update a translation, e.g. fr-FR:

  > cd misc/translator
  > perl translate update fr-FR

- Open the corresponding .po file, in this case
  misc/translator/po/fr-FR-staff-prog.po
- Confirm that the string is now in the .po file for translation. You
  should find these lines:

koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/MARCdetail.tt:146
c-format
msgid "Item(s)"
msgstr "Exemplaire(s)"

I found that the translation was already populated. Install the updated
po file:

> perl translate install fr-FR

Test the MARC detail page in your translated language to confirm that
the string is correct.

Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 33286: Update 'catalog' to 'bibliographic' in preferences

Correct the terminology to make it clear catalog concerns only concerns
bibliographic records at the moment, not authority records too.

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

Bug 34320: Hold reordering arrows look broken after Font Awesome upgrade

This patch makes minor CSS corrections to the arrows on the place hold
page in the staff interface. The corrections are necessary because of
changes in the display of icons in Font Awesome 6.

To test, apply the patch and locate a bibliographic record in the staff
interface to place a hold on.

- Place at least one hold on that title.
- After placing the hold, you should see a table of all the existing
  holds on that record.
- The arrow control links should look correct, with the "move to top"
  and "move to bottom" arrows having an extra border to differentiate
  them from the others.

Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 33556: Avoid relying on $c->validation

Talking to the OpenAPI plugin maintainer, he mentioned the use of $c->validation->output should be avoided as the plugin is not designed to have a stable behavior there, and he even thought of just removing the method.

That method returns an internal data structure the plugin uses to validate things, and then updates the request itself.

Take the following example:

GET /patrons/123
x-koha-embed: checkouts,library

without the OpenAPI plugin, requesting the header like this:

$c->req->headers->header('x-koha-embed')

would return a scalar, the string 'checkouts,library'.

When using the plugin, and with `x-koha-embed` being defined as collectionFormat: csv, that header is entirely replaced by an arrayref.

That's how the plugin works and how it is expected to be used. So we need to replace the uses of $c->validation format, with normal Mojo usage to avoid future headaches.

This patch changes:
* $c->validation->param => $c->param
* $c->validation->param('body') => $c->req->json

To test:
1. Run:
$ ktd --shell
k$ prove t/db_dependent/api/v1/*.t
=> SUCCESS: Tests pass!
2. Apply this patches
3. Repeat 1
=> SUCCESS: Tests still pass!
4. Sign off :-D

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

Bug 33556: Path parameters are handled explicitly in the controllers

In the case of $c->objects->search_rs, the variable is just not used.
In the case of /acq/orders, it's a leftover from when we removed in the
helper. Check there are tests with path params everywhere (including
orders) and it has no effect.

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

Bug 34337: (bug 33581 follow-up) Sample patrons not inserted

The "select all" link for the optional sample data is not selecting
holidays and patrons.

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

Bug 33379: DBRev 23.06.00.012

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

Bug 33379: DBIC schema

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

Bug 33379: Remove virtualshelfcontents.flags

This field is unused. A historic leftover from 2000-2010.

Test plan:
Run dbrev.
Run prove t/db_dependent/Koha/Virtualshel*

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 33105: (follow-up) DB changes

boooh!

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

Bug 34316: Add_credit should rethrow an exception correctly

Found while running Items.t on top of 33608.
Another exception was thrown but not rethrown.

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

Bug 34321: Tidy skeleton.pl

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

Bug 33105: DBRev 23.06.00.011

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

Bug 33105: Avoid using $c->validation

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

Bug 33105: Add missing POD

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

Bug 33105: Tidy files

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

Bug 33105: Redirect to 404 if no vendor exists

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

Bug 33105: Fix vendor api spec

vendor ID must be integer and date attributes should specify the format
accordingly.

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

Bug 33105: (QA follow-up) REST API

Includes removing q_header.
Inconsistent use of quotes for response codes.

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

Bug 33105: (QA follow-up) Changes for Koha objects

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

Bug 33105: (QA follow-up) Small changes in dbrev

Similar edit in kohastructure added.

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

Bug 33105: DBIC specific

Signed-off-by: Jonathan Field <jonathan.fieeld@ptfs-europe.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 33105: [DBIx] Schema changes

Former patches did not apply anymore. Refreshed them.

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

Bug 33105: Add tests

Signed-off-by: Jonathan Field <jonathan.fieeld@ptfs-europe.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 33105: REST API specs

Signed-off-by: Jonathan Field <jonathan.fieeld@ptfs-europe.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 33105: Add vendor issues

This patch is the main patch of this patch set, it contains the
controller acqui/vendor_issues.pl, its corresponding template, and some
links to this script.

It adds:
* A new DB table aqbookseller_issues linked with the aqbooksellers table
* A new subpermission acquisition.issue_manage
* A new authorised value category VENDOR_ISSUE_TYPE and two examples
MAINTENANCE and OUTAGE
* A new controller couple acqui/vendor_issues.[pl,tt]

Test plan:
0. Apply the patches, run updatedatabase and restart_all
1. Go to the acquisition module, create a new vendor or use an existing
one
2. Create a couple of issues for this vendor
3. Edit/Delete and search for those issues

This is the basics for tracking issues with vendors.
Suggestions welcome, on follow-up bug reports.

Signed-off-by: Jonathan Field <jonathan.fieeld@ptfs-europe.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 33105: Add Koha object classes

Signed-off-by: Jonathan Field <jonathan.fieeld@ptfs-europe.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 33105: DB changes

Signed-off-by: Jonathan Field <jonathan.fieeld@ptfs-europe.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 34178: (QA follow-up) Tidy

Tidy the relevant lines to pass the new QA rules

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 34178: Adjust tests

We need to clear the cache when changing rules/statuses

To test:
1 - prove -v t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t

Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 34178: Cache ItemsAnyAvailableAndNotRestricted in memory and don't precalculate

There are several places in the code where we precalculate ItemsAnyAvailableAndNotRestricted to avoid
looping on this routine when calling IsAvailableForItemLevelRequest on a list of items form a biblio

The value of ItemsAnyAvailableAndNotRestricted is only used when there is a circulation rule for
'onshelfholds' with a value of '2' (If all unavailable)

Rather than calculate a value that may never be used, let's cache this value per request when we do
calculate it - and reuse the cached value

To test:
1 - Apply patch
2 - Set circulation rule 'On shelf holds allowed' as 'If all unavailable'
make sure the rule applies to all of the items/patrons you test with
3 - Find a record with two items that are available
4 - Try to place a hold for a patron - not allowed
5 - Check out one item to another patron
6 - Attempt hold - still not allowed
7 - Check out second item to another patron
8 - Attempt hold - allowed!
9 - Apply patch
10 - Cancel and replace hold - it is allowed!
11 - Check in one item, and cancel hold
12 - Place hold - not allowed!
13 - Check in second item
14 - Place hold - not allowed!
15 - prove -v t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t

Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 28493: Make koha-passwd display the username

to test...

1/ run command
$ sudo koha-passwd dev1
Password for dev1: CraZyPa$$WoRD!!
Press enter to clear the screen...
^C

2/ apply patch

3/ run command again, note 'username' section
$ sudo koha-passwd dev1
Username for dev1: koha_dev1 <<<<<<<<<<<<
Password for dev1: CraZyPa$$WoRD!!
Press enter to clear the screen...
^C

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 34279: Don't enforce overduefinescap unless it is greater than 0

When creating a circ rule, we can set overduefinescap to blank or 0 and no cap is enforced. If we edit that rule, the blank/0 is converted to "0.00" which perl considers true, thus zero-ing out any calculated fine.

Considering we've always ignored an overdue fines cap of 0, we should also ignore 0.00. However, perl is evaluating it as a string which makes it true instead of false as 0 is.

Test Plan:
1) Apply the first patch ( unit tests )
2) prove t/db_dependent/Circulation/CalcFine.t
3) Note the test fails
4) Apply the second patch as well
5) prove t/db_dependent/Circulation/CalcFine.t
6) Note the test passes

Test Plan 2:
1) Create an all/all/all rule with an overduefinescap of 0.00, with a
daily fine. Enable CalculateFinesOnReturn
2) Backdate a checkout so it is overdue
3) Return this item, note the lack of a fine
4) Apply this patch set
5) Backdate a checkout and return it again
6) Note the fine is generated!

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 34279: Unit tests

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 32271: (QA follow-up) Restore $Price filtering for valid values

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 32271: (QA follow-up) Correction to 'can_be_blank' logic

The logic introducing the can_be_blank check into the monetary decimal
format check was flawed and meant we were no longer checking decimal
formatting in the majority of cases. This patch updates that so we pass
the unit tests and correctly check any value passed that's not an empty
string.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 32271: Unit test

Add a unit test for the allow blank option added to the monetary check
in CirculationRules.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 32271: (QA follow-up) Convert all positive numbers to monetary float and convert all 0 equivilents to blank value

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 32271: Fix overdue fines cap (amount) set to 0.00 when editing rule.

Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 33444: (QA follow-up) Add forgotten skipfinecalc in POD

No test plan needed.

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

Bug 33444: (QA follow-up) Tidy block in automatic_renewals.pl

Resolves:
The file is less tidy than before (bad/messy lines before: 94, now: 101)

Test plan:
Run qa tools.

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

Bug 33444: Update AddRenewal to take a hashref of params

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
[EDIT] Removed skip_record_index => 1 from automatic_renewals.pl. See BZ.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 34133: Set default sort on first column (ID) desc

Test plan:

Before applying patch:
* Open incognito at /cgi-bin/koha/ill/ill-requests.pl,
* Verify order is by ASC (lowest ID first)
* Close incognito

Apply patch, then:
* Open incognito at /cgi-bin/koha/ill/ill-requests.pl,
* Verify order is by DESC (highest ID first)

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Followed the test plan from Bugzilla, and added it above.
Works as advertised.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 33992: Only consider the date when auto-expiring problematic recalls

This patch carries this fix into the misc/cronjobs/recalls/expire_recalls.pl cronjob so that recalls are automatically expired when they have been waiting a problematic number of days, not considering hours, as expected.

To test, follow the test plan from the first patch. This will set you up with a waiting problematic recall.

Run the cronjob manually

perl misc/cronjobs/recalls/expire_recalls.pl

Refresh your 'Recalls awaiting pickup' page. Your problematic recall should be gone/expired.

Expiration dates will apply when expiring any 'unfulfilled' recall i.e. newly requested, overdue to be returned, and awaiting pickup.

Sponsored-by: Auckland University of Technology
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 33028: (follow-up) Lower the two digits requirement

This patch makes the code not require two decimal digits, as the main
intention here is to forbid (locale) formatted strings to reach the DB.

The number of digits we support needs to be discussed on its own bug,
and a centralized check implemented.

This patch fixes tests:

prove t/db_dependent/Circulation.t
t/db_dependent/Circulation.t .. 1/67 Exception 'Koha::Exceptions::CirculationRule::NotDecimal' thrown 'The circulation rule expected a decimal value' with name => fine, value => 0.1
t/db_dependent/Circulation.t .. Dubious, test returned 11 (wstat 2816, 0xb00)
Failed 53/67 subtests

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

Bug 33028: (follow-up) Add POD to new Exception

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 33028: Unit tests

Add unit tests for is_monetary functionality introduced in the
CirculationRules module.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 33028: Make exception less generic

While testing this bug I found Circulation.t was failing, but the
exception doesn't actually display anything useful in terms of helping
debug what's going on.

This patch makes it add the rule_name and rule_value to the message.

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

Bug 33028: (QA follow-up) Tidy introduced code

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

Bug 33028: DBRev 23.06.00.010

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

Bug 33028: Perltidy database update script

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 33028: Throw exception if not passed a decimal number

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 33028: (follow-up) Apply unformat_price to decimal fields

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>