Commit graph

7633 commits

Author SHA1 Message Date
d2731c1576 Bug 24151: Fix location on return
The item's location where not passed to UpdateStat

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-07-20 15:17:42 +02:00
5ce968e0e5 Bug 24151: Copy info to the pseudonymized table when a transaction is done
This is the commit where you will find useful information about this development.

The goal of this new feature is to add a way to pseudonymize patron's
data, in a way they could not be personally identifiable.
https://en.wikipedia.org/wiki/Pseudonymization

There are different existing way to anonymize patron's information in
Koha, but we loose the ability to make useful report.
This development proposes to have 2 different tables:
  * 1 for transactions and patrons data (pseudonymized_transactions)
  * 1 for patrons' attributes (pseudonymized_borrower_attributes)
Entries to pseudonymized_transactions are added when a new transaction
(checkout, checkin, renew, on-site checkout) is done.
Also, anonymized_borrower_attributes is populated if patron's attributes are
marked as "keep for pseudonymization".

To make those informations not identifiable to a patron, we are having a
hashed_borrowernumber column in pseudonymized_transactions. This hash will be
generated (Blowfish-based crypt) using a key stored in the Koha
configuration.

To make things configurable, we are adding 3 sysprefs and 1 new DB
column:
  * syspref Pseudonymization to turn on/off the whole feature
  * syspref PseudonymizationPatronFields to list the informations of the
  patrons to sync
  * syspref PseudonymizationTransactionFields to list the informations
  of the transactions to copy
  * DB column borrower_attribute_types.keep_for_pseudonymization that is a
  boolean to enable/disable the copy of a given patron's attribute type.

Test plan:
1/ Turn on Pseudonymization
2/ Define in PseudonymizationPatronFields and
PseudonymizationTransactionFields the different fields you want to copy
3/ Go to the about page
=> You will see a warning about a missing config entry
4/ You need to generate a key and put it in the koha-conf.xml file. The
following command will generate one:
  % htpasswd -bnBC 10 "" password | tr -d ':\n' | sed 's/$2y/$2a/'
Then edit $KOHA_CONF and add it before of the end of the config section (</config)
  it should be something like:
    <key>$2a$10$PfdrEBdRcL2MZlEtKueyLegxI6zg735jD07GRnc1bt.N/ZYMvBAB2</key>
5/ Restart memcached then plack (alias restart_all)
=> Everything is setup!
6/ Create a new transaction (checkin for instance)
=> Confirm that a new entry has been added to pseudonymized_transaction with the data
you expect to be copied
7/ Edit some patron attribute types and tick "Keep for pseudonymization"
8/ Create a new transaction
=> Confirm that new entries have been added to pseudonymized_borrower_attributes
11/ Delete the patrons
=> Confirm that the entries still exist in the pseudonymized_* tables
12/ Purge the patrons (ie. use cleanup_database.pl to remove them from
the deleted_borrowers table)
=> Confirm that the entries still exist in the pseudonymized_* tables

See bug 24152 to remove data from the anonymized_* tables

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-07-20 15:17:42 +02:00
96a8710350
Bug 25566: Add option to ignore found holds and use it when checking high holds
To test:
 1 - Find or create a record with 10 items
 2 - Set sysprefs:
     decreaseLoanHighHolds - enable
     decreaseLoanHighHoldsDuration - 2
     decreaseLoanHighHoldsValue - 2
     decreaseLoanHighHoldsControl  - 'over the number of holdable items'/dynamic
 3 - Set circ rules to allow 1 hold per record on the relevant record
 4 - Place 3 holds on the record
 5 - Check one item in and confirm hold to set to waiting
 6 - Issue to the patron with the waiting hold
 7 - Get a notice that loan period is decreased
 8 - Don't confirm the checkout
 9 - Apply patch
10 - Restart all the things
11 - Repeat checkout, no decrease this time!

Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-07-16 15:32:18 +01:00
Joonas Kylmälä
082da615e6
Bug 25992: Make SIP2 logger subroutines exportable to prevent crash
If the subroutines are not exportable we get the following crash:

> Undefined subroutine &C4::SIP::SIPServer::set_logger

To test:
 In kohadevbox run for example:
 $ ps -aux # check that no existing sip server is running, kill the process if exists
 $ perl /kohadevbox/koha/C4/SIP/SIPServer.pm /etc/koha/sites/kohadev/SIPconfig.xml
 $ koha/misc/sip_cli_emulator.pl -su koha -sp koha -l CPL -a 127.0.0.1 -p 6001 --item 3999900000001 -m item_information

 After applying this patch the Undefined subroutine error should be gone.
 Note: when using the sip_cli_emulator.pl the credentials can be anything.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-07-16 15:26:47 +01:00
189851fb9b Bug 25964: Prevent data loss when editing items from a MARC record
Coming from:
  Bug 23463: Use new method Koha::Object->set_or_blank

We have DB fields that are not mapped with MARC fields, for instance paidfor. They are not handled correctly.

In ModItemFromMarc, we get a MARC record in parameter and update the item in DB. But we are loosing the fields that are not in the MARC record

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2020-07-13 11:41:45 -03:00
7b66b90fe7 Bug 25750: fix fallback to ecost_tax_included/ecost_tax_excluded
If 'Actual cost' has not been set then it has the value of 0.00 which
Perl evaluates to true so this patchset resets it to 0, so the fallback
to ecost_tax_included/ecost_tax_excluded happens.

Test plan:
1. Add item to acquisition basket (make sure the vendor has: tax rate: 15%, 'List prices: Include tax', 'Invoice prices: Include tax')
2. Set 'Vendor price' = 10 and do not set 'Actual cost'
3. Save order
4. Observe basket.pl shows 'Total tax exc.' has a value of 0.00 and GST
column has value of -8.70

5. Jump into the database:
select tax_value_on_ordering from aqorders where
ordernumber=<ordernumber>;
[You can get the ordernumber from clicking on the 'Modify' line the item
is listed in]
6. Observe a negative value: -8.70

7. Apply patch and restart plack
8. Add a second item to the basket
9. Set 'Vendor price' = 10 and don't set 'Actual cost'
10. Save order
11. Observe basket.pl shows 'Total tax exc' has value of 8.70 and GST
has value of 1.30
12. Repeat step 5 and observe tax_value_on_ordering = 1.30
13. Run t/Prices.t unit test:
sudo koha-shell <instancename>
prove t/Prices.t

Sponsored-by: Horowhenua District Council, NZ

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-07-09 11:50:42 +02:00
Julian Maurice
3b06780fb2 Bug 21395: Fix C4/Barcodes/ValueBuilder.pm
$DEBUG variable was always set to 0, so it was useless

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-06-29 15:43:41 +02:00
61aa3f7942 Bug 21395: Remove 'variable $DEBUG masks earlier declaration in same scope' warning
% prove t/db_dependent/Serials.t
t/db_dependent/Serials.t .. 8/49 "my" variable $DEBUG masks earlier declaration in same scope at /kohadevbox/koha/C4/Barcodes/ValueBuilder.pm line 45.
"my" variable $DEBUG masks earlier declaration in same scope at /kohadevbox/koha/C4/Barcodes/ValueBuilder.pm line 87.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-06-29 13:44:05 +02:00
b9e526a12f Bug 21395: (QA follow-up) POD fixes
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-06-29 12:37:02 +02:00
e968af4377 Bug 21395: (QA follow-up) Remove some introduced issues
This patch removes some new error cases introduced during rebase

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-06-29 12:37:02 +02:00
Julian Maurice
b168f4a2e9 Bug 21395: Make perlcritic happy
This patch adds a .perlcriticrc (copied from qa-test-tools) and fixes
almost all perlcrictic violations according to this .perlcriticrc
The remaining violations are silenced out by appending a '## no critic'
to the offending lines. They can still be seen by using the --force
option of perlcritic
This patch also modify t/00-testcritic.t to check all Perl files using
the new .perlcriticrc.
I'm not sure if this test script is still useful as it is now equivalent
to `perlcritic --quiet .` and it looks like it is much slower
(approximatively 5 times slower on my machine)

Test plan:
1. Run `perlcritic --quiet .` from the root directory. It should output
   nothing
2. Run `perlcritic --quiet --force .`. It should output 7 errors (6
   StringyEval, 1 BarewordFileHandles)
3. Run `TEST_QA=1 prove t/00-testcritic.t`
4. Read the patch. Check that all changes make sense and do not
   introduce undesired behaviour

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-06-29 12:37:02 +02:00
2cc3d05d26 Bug 15400: Display date of birth and age more consistantly
Test plan:
0) Apply the patch
1) Go to all of these pages
    Patron detail
    Other patron pages - look on the left side (circ-menu)
    Patron search
    Guarantor search ( go to child patron -> edit -> in guarantor
        section click "Set to patron"
    Search through "Check out" (in the header)

2) Confirm that does show date of birth and date consistantly,
    try it on patrons with and without date of birth set to find
    possible reggressions

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-06-29 12:37:02 +02:00
7218767851 Bug 25875: Move check for module_bit and code to the JOIN
If we limit the JOIN to rows with the correct subpermission we won't
duplicate the returned patrons

To test:
 1 - Give a patron full acquisitions permissions
 2 - Also give them several subpermissions on other areas
 3 - Go to Acquisitions
 4 - Edit a fund
 5 - Add a user to the fund
 6 - Search for user above
 7 - They return multiple times in results
 8 - Apply patch
 9 - Restart all the things
10 - Repeat search
11 - Patron appears once

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-06-29 12:37:01 +02:00
Slava Shishkin
d81438e143 Bug 25491: Fix for "Use of uninitialized value" in InstallAuth.pm
This warning was thrown:
    Use of uninitialized value $info{"invalid_username_or_password"}
    in numeric eq (==) at /home/vagrant/kohaclone/C4/InstallAuth.pm
    line 387.

There is the case when hash key can be undefined in numeric comparison.

Fixed by adding additional precheck for
$info{"invalid_username_or_password"} being Perl's "true".

To test:
    1) Go to the first page of the web-installer where it asks to login.
    2) Observe the warning in the log file.
    3) Apply patch.
    4) Repeat step 1.
    7) Check that previous warning suppressed.

Mentored-by: Andrew Nugged <nugged@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-06-29 12:37:01 +02:00
2201edfe6e Bug 24156: Fix some QA failures
* Add POD to get_table_settings
* Remove USE Dumper debug statement
* Add missing "10" entry
* Fix newly created test file (and renamed)

Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-06-25 10:55:38 +02:00
0c04b397a4 Bug 24156: Make sort order and number of items to display configurable (basket page)
This patch is the main patch of this patchset, you will find the
description and the test plan.

The idea of this new enhancement is to add the ability to define the
default sort order and the default number of rows displayed on the
acquisition basket page.
The existing "columns settings" feature was replaced by a
"tables settings" feature. To prepare the ground, there were some
works that were needed:
  * rename variables and files
  * Modify the structure of the yml files
  * Create a new DB table to store the tables settings

Test plan:
0)
  a. Execute the update DB entry to create the new table
  b. Restart all (to get a new version of the yml file, that is cached by
   memcached)
  c. Create several orders for a given basket
1) Go to the basket view page
=> The default values are the same than without this patchset, the
number of entries to display is set to "20" and the table is sorted by
basket number (first column)
2) Go to the "Columns settings" page
3) Unfold the "Acquisition" tab
=> Notice the 2 dropdown lists at the bottom of the basket table
4) Select different values for "Default display length" and "Default
sort order"
5) Refresh the basket view page
=> Notice that the default settings are now effective on the table

QA note: We can decide to replace the different occurrences of "Columns settings"
by "Tables settings" if needed.

Sponsored-by: Institute of Technology Tallaght

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-06-25 10:51:59 +02:00
de54267510 Bug 24156: move ColumnsSettings to TablesSettings
We are preparing the ground with this patch. As the "Columns settings"
page will now add the ability to modify settings for the whole table, it
makes sense to rename the file and the variables.

Note that the controller script (admin/columns_settings.pl) and the yml
(admin/columns_settings.yml) files have not been moved to not break
shortcuts and abits people could have. But if QA decides, it could be
easy to do.

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-06-25 10:51:59 +02:00
692fb7e134 Bug 24159: (QA follow-up) Make terminology consistent
This patch changes the new circulation rule that's introduced from
useDaysMode to daysmode to improve consistency with other rule names.

We also update the accessors and code using them to reflect the new
term.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-06-25 10:51:59 +02:00
71e235751f Bug 24159: Set days_mode according to circ rules in 3 other places
There are 3 other occurrences where the new circ rule can be used:
 * C4::Circulation::checkHighHolds
 * Koha::Hold->set_waiting
 * misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl

Test plan:
* checkHighHolds
Enable decreaseLoanHighHolds and fill decreaseLoanHighHoldsDuration
Setup things to hit a "high demand" alert with a shortened due date
Check an item out
=> The due date must be recalculated depending on the circ rule useDaysMode.

* set_waiting
Set ExcludeHolidaysFromMaxPickUpDelay to "1" (note that there is currently
a bug in the description of the syspref, see bug 22381 comment 19)
Mark a hold waiting
The expiration date should have been set depending on the value of the
circ rule.

* TalkingTech cronjob
Cannot test this

Signed-off-by: Simon Perry <simon.perry@itcarlow.ie>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-06-25 10:51:59 +02:00
f1e248fad8 Bug 24159: Move useDaysMode pref to circulation rules
Moving the useDaysMode system preference to a circulation rule will add
much more flexibility in the calculation of the due date.

The initial request was to make hourly loan returned on closed when
(when checked out on the same close day).
To do so we do not want to take into account the calendar.
However the calendar need to be taken into account for other loan item types.

Other scenarios are possible, for instance depending on the branch.

This patchset will add a new "Days mode" column (next to "Loan period")
to the circulation rules page, with the different values of the
"useDaysMode" system preference + a "default" value, to default to the
system preference value.

Test plan:
- Define a long loan item type (like 10 days) that will use the calendar
(or default to the pref value, if the pref is not set to "ignore the
calendar")
- and a hourly loan (like 2 hours) that will ignore the calendar
- Create items with those item types
- Mark today as a closed day
- Check the items out
=> The hourly loan is due the same day
=> The other loan is due on an open day

QA note:
There is the need to force the "days_mode" option when Koha::Calendar is
initiated for the due date calculation. To make sure devs will not
forget it, the methods that need have it defined will throw an
exception.

Sponsored-by: Institute of Technology Carlow
Signed-off-by: Simon Perry <simon.perry@itcarlow.ie>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-06-25 10:51:59 +02:00
326c0159a0 Bug 25232: Add ability to specify multiple notforloan values to skip
Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-06-25 10:50:02 +02:00
465e5562fc Bug 25232: Add ability to skip trapping items with a given notforloan value
This is a companion/alternative to bug 25184, in that it allows an
explicit workflow for placing returned books into temporary storage for
a few days for decontamination purposes.

The idea here is to create a specific notforloan value for "In
Decontamination" or something along along those lines. This notforloan
value would never be trappable. At the end of decon,
UpdateNotForLoanStatusOnCheckin  could be used to remove the
notforloan status and allow checkins to be trapped to fill holds.

Test Plan:
1) Apply this patch
2) Restart all the things!
3) Give an item a negative notforloan value
4) Place a hold on the item
5) Check the item in
6) Note the item is trapped for hold
7) Set SkipHoldTrapOnNotForLoanValue to the same notforloan value
   you used in step 3
8) Check the item in again
9) Note Koha did not ask you to trap the item for hold!

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-06-25 10:50:02 +02:00
017b67e6c5 Bug 25805: Return empty strings instead of undef in C4::SIP::ILS::Item::hold_patron_name
This bug is basically the same as bug 24966, but for hold_patron_name instead of hold_patron_bcode.
The subroutine hold_patron_bcode should always return an empty string, not undef.

Test Plan:
1) Using the SIP cli emulator, checkin an item that is not checked out
2) Note the DA field contains someting like "C4::SIP::SIPServer=HASH(0x88175c8)"
   The hex number will almost certainly be different from this example
3) Apply this patch
4) Restart the SIP server
5) Run the SIP checkin again
6) Note the DA field is no longer present!

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>
2020-06-25 10:39:37 +02:00
Katrin Fischer
f090c1d2cb Bug 11994: OpenSearch plugins needs to be delivered with Content-Type application/opensearchdescription+xml
https://developer.mozilla.org/en-US/docs/Web/OpenSearch
Your server should serve OpenSearch plugins using
Content-Type: application/opensearchdescription+xml.

To test:
- Verify the Content-Type the file is delivered with
  is correct, for example using wget
  http://127.0.0.1:8080/cgi-bin/koha/opac-search.pl?format=opensearchdescription

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-06-24 14:31:37 +02:00
980419ed15 Bug 25189: Don't create authority if results found
Automatic authority creation assumes that if we don't match we need a new authority.

Using the Default linker, however, we don't match if there exists more than one match.
This leads to repeatedly generating authorities once there is a duplicate in the system

We shoudl instead only create a new authority if there are no results

To test:
1 - Set Linker Module to 'Default'
2 - Enable  AutoCreateAuthorities  and  BiblioAddsAuthorities and  CatalogModuleRelink and LinkerRelink
3 - Add two copies of a single authority via Z39
4 - Add a heading for that authority to a bib record
5 - Save the record and note a new authority is generated
6 - Repeat and see another is generated
7 - Apply patch
8 - Restart all the things
9 - Save the record again, no new authority created

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-06-24 14:31:37 +02:00
b17a04dd07 Bug 25786: Holds Queue building may target the wrong item for item level requests that match holds queue priority
Bug 23934 removed the limitation that prevented item level holds from
getting local holds priority. The problem is the code has never checked
if the item level hold matches the given item! This means the wrong item
may be requested to fill an item level hold.

Test Plan:
1) Create 3 items on a record
2) Place a hold for the 2nd item you created
4) Ensure that hold would be picked up by local holds priority
5) Build the holds queue
6) Note the holds queue is asking for the wrong item!
7) Apply this patch
8) Rebuild the holds queue
9) Holds queue should now be asking for the correct item!

Signed-off-by: Kim Peine <kim@williston.lib.vt.us>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-06-18 18:51:58 +02:00
db235d33a4 Bug 25783: Holds Queue treating item-level holds as bib-level
The holds queue builder does not honor
the new item_level_hold flag. Instead, it only item_level_request if
in the loop dealing with item level holds. This is incorrect. Item level
holds may be trapped in the local holds priority loop as well. It's
trivial to just pass though the correct item/biblio level hold flag.

I do not know how to write a reproducable test plan for these issues.

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

Signed-off-by: Kim Peine <kim@williston.lib.vt.us>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-06-18 14:55:51 +02:00
2d7e08bc0e Bug 20783: Use iframe to embed Youtube videos
WWW::YouTube::Download is broken and not reliable.
Other alternative was to use HTML::Video::Embed but not updated since
years.

The best alternative seems to follow youtube advise and use an iframe
https://developers.google.com/youtube/iframe_api_reference

Test plan:
Put youtube video in 856$u (using different url formats, youtu.be,
youtube.com/embed, etc.)
Enable HTML5MediaEnabled and HTML5MediaYouTube and confirm that the
youtube videos are correctly embeded.

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-06-15 10:32:43 +02:00
3ce4024fcb Bug 25266: Remove C4::Bookseller
This was the only occurrence of GetBooksellersWithLateOrders and it was
the only subroutine of C4::Bookseller

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-06-15 10:32:20 +02:00
35bb41dfc1 Bug 25701: Remove sort on removed field
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-06-15 10:29:53 +02:00
d669694426 Bug 25701: (bug 14419 follow-up) Always display facet groups in the same order
It seems that this regression comes from bug 14419, but I have not found
a logic reason behind that.
This patch restores the behaviour we always had: facet groups must be
displayed in a given order: the Zebra index (au, ccode, holdingbranch,
etc.)

Test plan:
Apply this patch, restart all and confirm that the facets are not moving
up and down when you refresh your search result page.

QA note:
I think the following line must be removed
@facets_loop = sort {defined $a->{expand} && defined $b->{expand} && $a->{expand} cmp $b->{expand}} @facets_loop;

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-06-15 10:29:53 +02:00
051b2d0142 Bug 25444: More minor improvements to simplified loop
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-05-28 17:01:55 +02:00
6b1a53da6d Bug 25444: Simplify the code using a loop
In order to prevent typos or further regressions it is better (I think)
to have this code into a loop

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-05-28 17:01:55 +02:00
8423070649 Bug 25444: Backup/restore course items fields correctly
This patch makes the code set the *_storage fields when adding new
fields to an existing course item. And reverts those fields correctly
when removing the item from the course.

If a new field is enabled on an existing course reserve, the storage
field is not given a value, so when the item goes off reserve, the
item field will always be updated to NULL.

To test:
1. Apply the regression tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/CourseReserves/CourseItems.t
=> FAIL: Tests fail, data is not reverted correctly
3. Apply this patch and repeat 2
=> SUCCESS: Tests pass! Data is correctly reverted
4. Sign off :-D

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-05-28 17:01:55 +02:00
10af741387
Bug 24413: Do not remove the restrictions from AddReturn
As we are now removing them from MarkIssueReturned they should not be
removed from AddReturn as well.
Also I think this will fix a regression, if $doreturn is not set (in
case the item is withdrawn and BlockReturnOfWithdrawnItems or the item
is lost and BlockReturnOfLostItems, and other specific cases).

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-05-22 09:33:17 +01:00
70ae2eaf9c
Bug 24413: Apply AutoRemoveOverduesRestrictions for lost items
It's quite hard to know where this need to be fixed.
it can be either MarkIssueReturned or LostItem, depending on the
different cases we want to handle.

This patch picked MarkIssueReturned, but maybe the similar code in
AddReturn needs to be removed then.

== Test plan ==
1. Set MarkLostItemsAsReturned to 'from items tab of the catalog module'
2. Set AutoRemoveOverduesRestrictions to 'Do'
3. Set up an overdues restriction in the notice triggers
4. Check out an item and let the overdues process restrict the account
5. Navigate to the moredetail.pl page (items tab) for the overdue item
6. Mark the item lost
7. Return to the account in question - notice the item has been returned, but the restriction remains
8. Clean state: remove restriction + remove item lost status
9. Apply patch
10. Redo the test but this time in addition to the item being returned,
    the restriction will be lifted.

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-05-22 09:33:17 +01:00
aa1f0fd73f
Bug 24612: Use the reserve_id to identify a reserve when building a notice
Now that we have the reserve_id PK on the reserves table we should use
it (instead of the couple borrowernumber, biblionumber)

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-05-22 09:33:17 +01:00
d864c5640a
Bug 24612: Make hold-transfer-slip take reserve_id
To make sure we are going to display the correct hold's info we need to
pass the reserve_id.

== Test plan ==
1. Add some content to HOLD_SLIP notice, e.g.

  <h2>[% branch.branchname %]</h2>
  <div>[% biblio.author %]<br>[% biblio.title %]<br>[% item.barcode %]
  <ul><li> Reserve ID:  [% hold.reserve_id %]</li>
  <li>Expiration date: [% hold.expirationdate %]</li></ul>

2. Add 2 holds for 1 patron to a single record
3. Check the reserve IDs in the reserves table - on a clean sandbox, they will be 1 and 2
4. Check in one of the items from the record and print the slip
5. Note that the reserve ID on the slip is 2 and the expiration date is blank
6. Repeated check ins do not change this
7. Check in a second item from the record
8. Note that the reserve ID for this hold is also 2, but this time the expiration date is filled in
9. Check in the first item again - the reserve ID stays as 2, but this time the expiration date is filled in
10. Apply patch
11. cancel the holds to come back to a clean state
    (and maybe ensure items aren't in transit)
12. redo the test and see the following differences
13. 1st checkin:
    1. expiration date ok
    2. the reserve ID is the one of the first hold
14. 2nd checkin:
    1. expiration date ok
    2. the reserve ID is the one of the second hold

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-05-22 09:33:16 +01:00
f8f890621e
Bug 25531: Debar patron if needed when checkin is backdated
If think this case does not apply to real-life, but the logic needs to
be fixed.
If an item is due now, and AddReturn is called now with a return date in
the future, the issue is overdue and the patron must be debarred.
However it is not as we compare with now and not the return date

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-05-19 16:49:25 +01:00
48e3e6aafb
Bug 25527: Initialize the logger when required
In an OO package, the logger initialization should happen in the
constructor. This is not an OO package and the initialization is
happening on loading it. This is a wrong behaviour and certainly breaks
in environments where initialization cannot happen (package building,
for example). There could be several options to solve this, as it is
used in a single sub on this package, I opted for initializing on that
sub.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-05-18 15:24:47 +01:00
9ed284c0a4
Bug 23403: Catch other cases of ->{patron}
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-05-15 09:48:35 +01:00
29f797b980
Bug 23403: List borrowernumber in the ILS::Patron fields
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-05-15 09:48:32 +01:00
b720285bdb
Bug 23403: Fix whitespace
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-05-15 09:48:27 +01:00
23dd6651f8
Bug 23403: Remove cardnumber from SIP
== Test plan ==
1 - Have two patrons with userids and no cardnumber
2 - Note which of these has the higher borrower number
3 - Use the SIP cli emulator to connect and checkout a book to the patron with higher borrowernumber
      See example after
4 - Note the book may checkout to the wrong patron!
5 - Apply patch
6 - Checkout to both patrons via sip
7 - The patrons get the correct checkouts

=== SIP CLI emulator ===
./misc/sip_cli_emulator.pl -a 127.0.0.1 -p 6001 -su term1 -sp term1 \
-l CPL --patron 23529001000463 -m checkout --item 39999000001259

translation: via the koha user term1, checkout item 39999000001259 to
patron 23529001000463

Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-05-15 09:48:20 +01:00
d69ab6ee91
Bug 24458: Use the ViewPolicy filter on search results
This patch implements the use of the ViewPolicy record processor filter
inside C4::Search::searchResults. The idea is that the $record_processor
is instantiated once and reused inside the loop. This leaves options for
further optimizations I will do on a follow-up bug.

The filter is applied to the MARC data before it is passed to the XSLT
processor.

To test:
1. Apply the regression tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Search.t
=> FAIL: This is not implemented, tests fail
3. Apply this patch
4. Repeat 2
5. Feel the joy in your body from a long standing bug being solved
6. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-05-15 09:33:22 +01:00
2a216e206c
Bug 25416: Let OPAC XSLTs know if the context is an anonymous session
This patch makes use of the 'variables' parameter in XSLTParse4Display
method in the different places that it is used in the OPAC. It does by
passing this parameter with

    anonymous_session => 1|0

The value will depend on the output from get_template_and_user (i.e. if
there's a returned borrowernumber).

A special case takes place in search results, as the call to
XSLTParse4Display happens in C4::Search::searchResults. So a new
parameter 'xslt_variables' is added to it.

To test:
1. Apply the [DO NOT PUSH] patch
2. Open the OPAC in your browser
3. Try detail pages, search results, tags and lists/shelves pages with
   or without an active session
=> FAIL: It always says (somewhere) 'Anonymous session: Yes'
4. Apply this patch, restart_all
5. Repeat 3
=> SUCCESS: It will tell the Yes/No correctly regarding anonymous
sessions!
6. Sign off :-D

Sponsored-by: Universidad ORT Uruguay
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-05-15 09:33:22 +01:00
7b84e00f7e
Bug 25417: (QA follow-up) Restore 'fixing' of fines
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-05-12 13:46:48 +01:00
53349e8000
Bug 25417: Prevent negative debit amount on backdating returns
Koha::Account->add_credit is expecting a positive amount.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Didier Gautheron <didier.gautheron@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-05-12 13:46:38 +01:00
61718c5c57
Bug 15253: Remove specific logging output
* We should not call Log::Log4perl directly
* Not sure it is correct as I get from (comment 77):
 % koha-sip --restart

 [2020/04/23 11:23:27] [ERROR] [undef]@[undef]: Argument "0.33_01" isn't
 numeric in numeric lt (<) at /usr/share/perl5/Net/Server/Log/Sys/Syslog.pm
 line 42.
  C4::SIP::Trapper::PRINT /kohadevbox/koha/C4/SIP/Trapper.pm (24)

 Why "ERROR" when it's a warning?
 The [undef]@[undef] seems wrong here.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-05-12 11:47:07 +01:00
8cb408cf15
Bug 15253: Add POD to C4/SIP/Logger.pm and C4/SIP/Trapper.pm
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-05-12 11:47:04 +01:00