Commit graph

6837 commits

Author SHA1 Message Date
Kyle M Hall
5f485e476b Bug 15524: (QA follow-up) Change Can[Book|Item]BeReserved to return hashref, pass limit to template
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-08-24 16:27:27 +00:00
Kyle M Hall
27b2b0bb95 Bug 15524: (QA follow-up) Respect ReservesControlBranch
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-08-24 16:27:26 +00:00
7e956e85b1 Bug 15524: Set limit on maximum possible holds per patron by category
It's possible to set a limit on the maximum number of holds for a particular branch/category/itemtype, but not on the total number of holds for a given patron (by branch/category).
This new rule works in conjunction with the existing branch/borrower/item rules in that Koha will use the lower of the two limits. This new rule counts all holds of all types, which prevents bib-level holds from not being counted for the purpose of these limits. This makes the most sense and was also requested by the sponsor.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Go to the circ rules editor, note the new max holds rules
   by patron category in the "Checkout limit by patron category".
   ( Should we rename this section? )
4) Create find a patron that is allowed to place a hold, count the
   number of holds that patron has. Lets make that number 'X'.
5) Set the new max holds rule to X for "All libraries"
6) Note the patron can no longer place another hold
7) Set the new max holds rule to X + 1 for the patron's home library
8) Note the patron can again place another hold
9) Set the new max holds rule to X for the patron's home library
10) Note the patron can no longer place another hold

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-08-24 16:23:27 +00:00
Baptiste Wojtkowski
f4615a2f81 Bug 15408: Remove fund's timestamp when cloning a budget
To duplicate the budget, the function CloneBudgetHierarchy create a new budget
from the old without the parameter timestamp, so that the database generates a
new one.

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

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

Added unit tests to patch

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

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

Modified to run properly the QA tests

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

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-08-22 12:51:52 +00:00
Katrin Fischer
9ccfa48f6e Bug 20631: Remove unused RemoveLostItem from C4::Accounts
RemoveLostItem was 'used' in C4::Accounts, but is not
actually used there.

Call has been removed by
  commit ed9cf81c3a
  Bug 15896 - Use Koha::Account::pay internally for makepayment

To test:
- Verify that the routine was actually not used!

Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-08-22 12:36:42 +00:00
a744a3e202 Bug 20487: AddReturn should clear items.onloan for unissued items
If an item is no longer issued but somehow still has a date in the onloan
column, checking it in should clear that date.
Adding a ModItem call in the NotIssued section.

Test plan:
[1] Run t/db_dependent/Circulation.t
[2] Bonus: Checkout item, delete issue from table, checkin. Verify that
    items.onloan has been cleared.

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

Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-08-22 12:29:10 +00:00
8eed046638 Bug 18947: LDAP - do not assume anonymous bind if no user or password
To test:
Ideally tested on a working ldap server with bind by auth and no
anonymous bind
1  - Define an LDAP config with bind by auth
2  - Don't define user/pass
3  - Define anonymous_bind = 0
4  - Attempt bind by auth
5  - Error is something like:
LDAP search failed to return object : XXXXXXXXX: LdapErr: XXXX-XXXXXX,
     comment: In order to perform this operation a successful bind must
     be completed on the connection., data 0, v2580 at
     /usr/share/koha/lib/C4/Auth_with_ldap.pm line 102.
6  - Define user/pass
7  - Now bind by auth should work
8  - remove user/pass
9  - Apply patch
10 - Attempt again
11 - Bind by auth shoudl succeed

prove -v t/db_dependent/Auth_with_ldap.t

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

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-08-20 14:40:13 +00:00
88d3865168 Bug 21203: ILS-DI - Make GetRecords handle non-existent records
Caused by
  commit cb336e633b
  Bug 18255: Koha::Biblio - Replace GetBiblioItemByBiblioNumber with Koha::Biblio->biblioitem

Can't call method "biblioitem" on an undefined value at /home/vagrant/kohaclone/C4/ILSDI/Services.pm line 212

Test plan:
- Enable ilsdi
- hit /cgi-bin/koha/ilsdi.pl?service=GetRecords&id=X
With X an existing biblionumber and a non existing one

Both requests must succeed

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-08-20 14:37:31 +00:00
Mark Tompsett
f8e0d905ff Bug 15717: Installer: Step 3 has <br /> showing
The step to install optional/mandatory things is broken with
many <br />'s instead of line breaks.

TEST PLAN
---------
1) Back up database
2) Drop database
3) Create empty database
4) Run web installer
   -- Notice that step 3 has ugly <br />'s at the last
      part of step 3.
5) Apply patch
6) Repeat steps 2-4
   -- Notice the <br />'s are now nice line breaks.
   NOTE: No promises of perfect positioning!
7) Run koha qa test tools.

Joubu: I have no idea if this is still needed. TO TEST

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

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

https://bugs.koha-community.org/show_bug.cgi?id=13618

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-08-17 15:55:08 +00:00
bce549bc4c Bug 21226: Remove xISBN services
OCLC has decided to retire all xISBN services:
https://www.oclc.org/developer/news/2018/xid-decommission.en.html

The code for related features has to be removed from Koha.

Test plan:
You need to be familiar with the different sysprefs (I am not):
- FRBRizeEditions
- SyndeticsEnabled
- SyndeticsEditions
- ThingISBN

Make sure there are no regressions introduced by this patchset.

QA Note: C4/XISBN.pm should be renammed

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-08-16 13:20:22 +00:00
889f148e4e Bug 21137: Replace USER_INFO with logged_in_user
This patch set does several things:
- it removes USER_INFO and BORROWER_INFO
These 2 variables contained logged-in patron's info. They must be
accessed from logged_in_user
- Use patron-title.inc for the breadcrumb at the OPAC, for consistencies

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-08-14 12:43:10 +00:00
e8c42c3a94 Bug 19743: Update header and footer on each item for checkin / checkout / renewal notices
When generating the checkout notice for a patron we only update the
section in between the '---' tags

For template toolkit purposes it means we cannot affect the content
based on more than a single item. For instance, we want to add the total
cost of all items checked out.

Test Plan:
1) Add "Thank you for visiting <<branches.branchname>>." to the bottom of the checkout notice. Below the second '----'.
2) Check out an item to a patron to generate that notice
3) Note the branch name in the notice
4) Update the branch name in the branches editor, change it to something else
5) Check out a 2nd item to that patron
6) Notice the items list updated, but the branch name did not
7) Apply this patch
8) Check out a 3rd item to that patron
9) Notice the branch name updated this time

Signed-off-by: Te Rauhina Jackson <terauhina.jackson@gmail.com>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-08-14 11:55:07 +00:00
ef58eff737 Bug 20661: (QA follow-up) Add POD for output_and_exit
Resolve:
 FAIL   C4/Output.pm
   FAIL   pod
                *** ERROR: Apparent command =cut not preceded by blank line  in file C4/Output.pm
   FAIL   pod coverage
                POD is missing for output_and_exit

Test plan:
Run QA tools again.

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-08-10 12:23:03 +00:00
ff21352a4a Bug 20661: Shortcut circ scripts if a blocking error appeared
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
No test plan found ;)
But tested bookcount and request-article.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-08-10 12:23:03 +00:00
d035a140b5 Bug 21188: Convert return_date to a DateTime object
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-08-09 15:24:23 +00:00
David Bourgault
bf9527457f Bug 20660: Return date override affects debarments in AddReturn
Re-did the patch to follow best practices.

TEST PLAN:
0. Apply first patch only
1. prove t/db_dependent/Circulation.t
2. Apply second patch
3. prove t/db_dependent/Circulation.t

prove should fail the first time and pass the second time.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-08-09 11:21:43 +00:00
948566e7f3 Bug 21154: Remove unused subs from C4::Serials
Test plan:
Run these commands, grep should not return any occurences, prove should
return green:

git grep GetSerialStatusFromSerialId
git grep GetDistributedTo
git grep SetDistributedTo
git grep SetDistributedto
git grep in_array
git grep is_barcode_in_use

kshell
prove t/db_dependent/Serials.t

Signed-off-by: Pierre-Luc Lapointe <pierreluc.lapointe@inLibro.com>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-08-09 10:42:38 +00:00
c0f7aed84d Bug 21075: AutoUnsuspendHolds should unsuspend holds <= today
To recreate:
 1 - Place a hold in your system
 2 - Set that hold (or all holds) to expire today
    update reserves set suspend_until=CONCAT( CURDATE()," 00:00:00");
 3 - Run misc/cronjobs/holds/auto_unsuspend_holds.p
 4 - Note the hold is sitll suspended
 5 - Visit /cgi-bin/koha/reserve/request.pl for the book with the hold
 6 - Note the suspend date is tomorrow (and cannot be set to today]
 7 - Click update holds - the date in db is now set to tomorrow
 8 - Reset to today
 9 - Apply patch
10 - Run the cron again
11 - Visit the page and note hold is unsuspended

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-08-08 21:07:15 +00:00
15088c67d6 Bug 19436: (QA follow-up) Revert change in _handle_one_result
See Bugzilla comment 7. This change does not belong here and is
dubious on its own. Needs further attention on another report.

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

Amended: In consultation with the author the same change is applied to the
corresponding lines in Z3950SearchAuth.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-08-08 20:51:06 +00:00
Matthias Meusburger
7baa452a6a Bug 19436: Add SRU support for authorities
Test plan:
 - Apply the patch
 - Add an SRU authority server in admininistration -> Z39.50/SRU servers
   You can try with the French national library, configured as such:
   Hostname: catalogue.bnf.fr
   Port: 80
   Database: api/SRU
   Syntax: Unimarc
   Record type: authority
   Additional SRU options: version=1.2,sru=get
   SRU Search fields mapping example:
	Keyword (any): aut.anywhere
	Name (any): aut.anywhere
	Author (any): (aut.type any "pep org") and aut.accesspoint
	Author (personal): aut.type=pep and aut.accesspoint
	Author (corporate): aut.type=org and aut.accesspoint
	Author (meeting/conference): aut.type=org and aut.accesspoint
	Subject heading: (aut.type any "geo ram_nc ram_ge ram_pe ram_co") and aut.accesspoint
	Subject sub-division: aut.type=ram_pe and aut.accesspoint
	Title (any): (aut.type any "tic tut tum ram_tp ram_tu") and aut.accesspoint
	Title (uniform):(aut.type any "tut tum ram_tu") and aut.accesspoint

 - Try a search from Authorities -> New from Z39.50/SRU
 - Check that the authority is correctly displayed in "Show Marc"
 - Check that the authority is correclty added to koha in "Import"
 - prove t/db_dependent/Breeding.t

Signed-off-by: François Pichenot <fpichenot@ville-roubaix.fr>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-08-08 20:31:34 +00:00
2e6fb40ef8 Bug 21087: Hash passwords in ->update_password
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: John Doe <you@example.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2018-07-30 14:58:06 -03:00
d90ff21454 Bug 17698: Do not send pending_checkout_notes from all circ scripts
It's safer to send what we need from C4::Auth it's needed from a whole
module.
The SELECT COUNT(*) query will only be done when needed (so not made
from scripts outside of circ)

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-07-23 15:23:44 +00:00
7d7cd25f4c Bug 19166: (follow-up) Adjust table and files and QA issues
Move to aqinvoice_adjustment
Move to Koha::Acquisition::Invoice::Adjustments
Test if variable exists before count

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-07-19 17:28:41 +00:00
8a7b35de6c Bug 19166: Correct charset collate
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-07-19 17:28:40 +00:00
d6006b0b99 Bug 19166: (follow-up) Shipment cost is always added to 'spent'
Two koha professionals agreed, also it is current behvaiour so this
patch removes a change

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-07-19 17:28:37 +00:00
2d99b46715 Bug 19166: Add the ability to add adjustments to an invoice
This patchset adds the ability to add adjustments to an invoice, one can
provide a reason, an adjustment amount, select a budget, and choose
whether to encumber the funds before the invoice is closed or not

To test:

1 - Create a new invoice with or without a shipping cost
2 - Note there are no existing adjustments
3 - Add an adjustment
4 - Submit the form withno changes, nothing happens
5 - Update the adjustment you created, ensure changes are saved but no
extra adjustment created
6 - Add another invoice prodiving only reason or amount (you can have 0
        value adjustments)
7 - Verify the adjustment total at bottom is correct
8 - Recieve some orders
9 - Verify totals are correct

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-07-19 17:28:35 +00:00
284226a496 Bug 13560: Add an 'Add' option for marc modification templates
Add/Update would update a field or create new if it existed, but didn't
allow for creating new if the field existed.

This patchset splits the options to 'Add & Update' so that 'Add' will always
add a field and 'Update' will operate as it always has

To test:
1 - Have a record with a known existing field (make a copy)
2 - Define a marc modification template that 'Add/update' on that field
3 - Define an 'Add/Update' on a field that doesn't exist
4 - Batch modify the copy of record using the above template
5 - Verify the existing field was updated
6 - Verify the non-existing field was updated
7 - Apply patch and update database
8 - Make another copy
9 - Modify the copy with the same template as above
10 - Should match initial modification
11 - Add a new rule to add a new field
12 - Modify using the updated template
13 - Ensure your new field is created
14 - Test various options in the modification tool
15 - prove t/db_dependent/MarcModificationTemplates.t

Signed-off-by: Victor Grousset <victor.grousset@biblibre.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-07-19 17:22:18 +00:00
d7fb53f6fc Bug 18322: (follow-up) Use authorized value labels
Rather than using the raw ccode's use the labels as defined in the
authorized values.

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-07-18 17:42:50 +00:00
17a354f5e8 Bug 18322: Add a facet for ccode fields to Zebra
This patch adds the index definitions for zebra faceting of ccode in
koha for marc21, normarc and unimarc.

We also add lines to the templates to expose the new facet and enable
non-zebra faceting for ccode too.

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-07-18 17:42:48 +00:00
Lari Taskula
7816d09e71 Bug 18072: Add deprecation warning to C4::Circulation Branch Transfer Limit functions
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-07-18 17:29:03 +00:00
d2a2d973ce Bug 20287: Move ModMember to Koha::Patron
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-07-18 15:49:50 +00:00
cfbc53bb22 Bug 20287: Add plain_text_password (& Remove AddMember_Opac)
But actually we could remove it if it does not make sense for other use.
Callers could deal with it since the password is not generated here

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-07-18 15:49:49 +00:00
dc6b6e030c Bug 20287: Remove AddMember_Auto
I am not sure I understood the point of this subroutine.
Did I miss something here?

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-07-18 15:49:49 +00:00
b4d9be74fc Bug 20287: Fix export issues in tests
t/db_dependent/Holds/RevertWaitingStatus.t ..
Undefined subroutine &C4::Circulation::MoveReserve called at /home/vagrant/kohaclone/C4/Circulation.pm line 1316.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-07-18 15:49:47 +00:00
ef410fd62f Bug 20287: Replace occurrences of AddMember with Koha::Patron->new->store->borrowernumber
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-07-18 15:49:47 +00:00
ae927390ff Bug 20287: Move AddMember to Koha::Patron->store
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-07-18 15:49:46 +00:00
a6059c4d2d Bug 20287: Move trim values to a method
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-07-18 15:49:45 +00:00
1b13c453e2 Bug 20287: Move fixup_cardnumber
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-07-18 15:49:44 +00:00
1563ec3813 Bug 18822: Check if we have MARC::Record and convert if not when using ES
The new_record_from_zebra subroutine assumes that when using ES we
always get MARC::Record objects when using ES, but sometimes we get them as xml via Z39 or
internally. This adds a test to new_from_zebra to confirm we have a
record object and to convert it if not

To test:
1 - Perform an advanced search from the advanced editor, make sure to
select 'Local catalog' and a remote source
2 - Error 'Internal search error [Object object]'
3 - Apply patch
4 - Repeat search, success!
5 - Select and deselect various servers, search should still work

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-07-13 12:15:32 +00:00
Mirko Tietgen
4b8bcd9be9 Bug 20949: Koha depends on Clone
Clone was not declared a dependency before. It used to be a dependency
of Hash::Merge but isn't in the recent version.

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-07-12 13:39:40 +00:00
Victor Grousset
82e0e97a00 Bug 20084: Fix Industrial2of5 and COOP2of5 patron card layouts
== Test plan ==
This is an oversimplification of a full patron card setup used in production.

1. Create a batch with 1 patron
2. Create a layout and set
     the name
     "Print card number as barcode"
     "Barcode type:" to "Code 39"
3. Create a card template without filling anything
4. Export the batch using the layout and template
5. You should have a white page with a barcode
     And no errors in the relevant log file
     This show that this setup isn't completely bogus
     (although Code 39 is the only type working...)
6. Layout: set "Barcode type:" to Industrial2of5
7. Export the batch
8. You should have a white page with no barcode
     And errors in the relevant log file
     "Invalid Characters"
     This is the bug.
9. Layout: set "Barcode type:" to COOP2of5
10. Export the batch
11. You should have a white page with no barcode
      And errors in the relevant log file
      "Invalid Characters"
      This is the bug.
12. Apply this patch
13. Retry with both non-working patches
    1. You should have a white page with a barcode
    2. And no errors in the relevant log file

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-07-12 12:39:34 +00:00
Alex Arnaud
a6b8fc9b2e Bug 21046: Return the correct borrowernumber when there is empty cardnumber(s)
Test plan:
  - Use a patron with an empty cardnumber to authenticate with ILSDI
    AuthenticatePatron
    (cgi-bin/koha/ilsdi.pl?service=AuthenticatePatron&username=userid&password=pass),
  - make sure you have other patron(s) with empty cardnumber and there
    borrowernumber is
    smaller than the one you authenticate with,
  - you should get a wrong borrowernumber,
  - apply this patch,
  - test again, you should get the right one

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-07-12 12:16:27 +00:00
505630ca08 Bug 21020: Fix return branch on transfer - SIP
Caused by
  commit 546379cc92
  Bug 17680: C4::Circulation - Remove GetItemIssue, simple calls

Since this commit AddReturn return a Koha::Issue object, not a hashref
with item's info.

Test plan:
1 - Enable SIP server on your test instance
   For kohadevbox:
   cp /etc/koha/SIPConfig.xml /etc/koha/sites/kohadev/
   create patron with user/pass term1/term1 and grant circulation
   privileges
2 - enable AutomaticItemReturn
3 - Checkin an item which will generate a transfer via SIP2:
   perl /home/vagrant/kohaclone/misc/sip_cli_emulator.pl -a localhost -p
   6001 -su term1 -sp term1 -l LPL --item {BARCODE} -m checkin -t
   CR
4 - Note no CT field
5 - Apply patch
6 - Repeate checkin via SIP2
7 - Transfer destination is correctly returned in CT

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-07-06 10:13:32 +00:00
f253c72bc3 Bug 20272: Changes for Breeding.pm and Record.pm
In Breeding.pm we let Z3950Search return the xslt handler error codes back
to the template. They are converted to text messages by using an new include
file (added for opac and intranet now). The generic xslt_err code is now
obsoleted.

In Record.pm the errstr call is removed. The croak is done with the new
error code in err. This seems sufficient.

Test plan:
[1] Run Breeding.t
[2] Run Record.t
[3] Add a nonexisting xslt file to one of your Z3950 targets. Search on that
    target and check if you see a error 'XSLT file not found'.

The bonus is these error messages are now translatable as they are in
the templates

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

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-07-02 12:12:49 +00:00
4ec5a67c6f Bug 20495: Remove get_saved_report
To test:
1 - prove t/db_dependent/Reports/Guided.t
2 - grep "get_saved_report" - ensure there are no occurences of the
singular form
3 - create, save, edit, and convert a report
4 - access a public report and report json from opac and staff client
5 - Ensure all function as expected

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-07-02 12:06:54 +00:00
Jérôme Charaoui
f99a1daf25 Bug 20920: Add missing dependency on CGI::Compile
Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-07-02 11:24:10 +00:00
6965c58308 Bug 20879: Fix regression in shibboleth when ldap enabled
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-06-22 20:13:13 +00:00
Kyle M Hall
ebd5f905ae Bug 20727: Add use line for File::Spec in C4::Context, sort used modules
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-06-22 16:10:11 +00:00
05812dbbba Bug 20727: (QA follow-up) Remove a few use statements again
We do no longer need "use Koha::UploadedFile" in a few places.

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-06-22 16:10:10 +00:00
Kyle M Hall
3ff7c27d98 Bug 20727: Move temporary_directory() to C4::Context
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-06-22 16:10:09 +00:00
Kyle M Hall
c2b0c92800 Bug 20727: Replace usage of File::Spec->tmpdir with Koha::UploadedFile->temporary_directory
As explained in bug 20428 use tmpdir can cause issues and it just makes sense to standardize our temp directory in a universal way.

Test Plan:
1) Apply this patch
2) Verify you can still log in and use Koha
3) Verify the web installer still works
4) Verify EDI module can still download files via FTP
5) Verify fines.pl still runs with -o option
6) prove t/db_dependent/Plugins.t
7) prove t/db_dependent/Sitemapper.t
8) prove t/db_dependent/Templates.t

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-06-22 16:10:08 +00:00
e20238ab98 Bug 19820: (QA follow-up) Fix pod in C4::Biblio
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-06-22 15:08:26 +00:00
David Gustafsson
13a5ffb7e1 Bug 19820: Add unsafe param to GetMarcSubfieldStructure
Add unsafe param to GetMarcSubfieldStructure and use this options
where it's safe to do so to increase performance

Sponsored-by: Gothenburg University Library

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-06-22 15:08:26 +00:00
Katrin Fischer
f6d15dc248 Bug 20456: Remove the C4::Serials::GetSubscriptionsFromBorrower
Bug 20400 added a routing list tab to the patron account in the
OPAC using Koha::Object. After changing the staff view too,
we can now safely remove GetSubscriptionsFromBorrower as it
is no longer used.

To test:
- Make sure that GetSubscriptionsFromBorrower is no longer used.

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-06-22 15:02:46 +00:00
8480491cce Bug 20702: Bind results of GetHostItemsInfo to the EasyAnalyticalRecords pref
Triggered by the finding on bug 20697.
The three calls of GetHostItemsInfo should be controlled by the pref. This
patch makes the sub return an empty list when the pref is disabled.

The patch simplifies the sub by merging the two identical foreach loops
depending on the field number in MARC21/UNIMARC.

Will add a unit test on a follow-up patch.

Test plan:
See next patch.

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

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-06-22 13:37:15 +00:00
Kyle M Hall
c967e010c0 Bug 20285: Lost item refund won't always pay down lost item fee first
Due to a simple typo, the accountline passed to Koha::Account::pay from _FixAccountForLostAndReturned is not used. That means that the credit for the lost item fee may be applied to other fees before it is used on the lost fee itself.

Test Plan:
1) Find a patron with existing fines
2) Ensure your settings will charge patrons for lost items
3) Check out an item to a patron with existing fees that need paid
4) Mark the item lost, charging the lost item fee
5) Return the item
6) Note the fee was refunded, but it paid down earlier fines first,
   and not the lost item fee first
7) Apply this patch set
8) Repeat steps 1 - 5
9) Note the lost item fee is the first fee to be paid off by the lost
   item fee refund

Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>
Followed the test plan and it works as expected.

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-06-15 10:38:12 +00:00
Katrin Fischer
870913b3c3 Bug 2426: Remove deprecated management permission
After review of the code it turns out that the management permission
that has been marked as deprecated a long time ago, does not have
any function.

The patch removes all remaining code related to it.

To test:
- Make sure you have a patron with the management permission
- Apply patch
- Run database update
- Check everything still works as expected

Bonus:
borrowers.flags is recalculated for patrons with management
permission.

To check:
- Create some 'permission twins' with and without management
  permission
- Note the value in borrowers.flags
- Apply patch, don't run database update
- Save permissions from GUI for one of the twins
- Note the newly calculated value
- Run database update
- Now both twins should have the same borrowers.flags value

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-06-08 12:17:25 +00:00
421746dd63 Bug 20877: Do not consider DB user has permissions
Test plan:
0/ Do not apply the patch
1/ Confirm the new test fails
2/ Apply the patch
3/ Confirm the new test passes
4/ Test the installation process

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-06-08 10:43:37 +00:00
3d68ab447e Bug 18821: (QA follow-up) Last tweaks for performance
[1] passing unsafe has no use since it is a scalar, removed it to unconfuse
[2] remove caching when pref is disabled
[3] caching userid removes the need for calling Patron->find each time
[4] subsequent changes in unit test
[5] cosmetic renames to move from session to daily basis (changed dev angle)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
First call going thru Koha::Patron takes about 0.0150 sec.
Subsequent calls only use caching and take about 0.0006 sec.

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-06-08 10:30:17 +00:00
495512f6ea Bug 18821: Convert to using cache with date checking
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-06-08 10:30:16 +00:00
6491ba0233 Bug 18821: TrackLastPatronActivity is a performance killer
Test Plan:
1) Apply this patch
2) Start a new session ( a private browser window works well )
3) Note the lastseen column in the borrowers table is updated
4) Browse a few pages, not the lastseen column is not updated again
5) Close the browser window and repeat steps 2-4
6) prove t/db_dependent/Auth.t

Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-06-08 10:30:16 +00:00
6049836023 Bug 20889: Prevent not for loan items to be checked out
1. Item type defined at item level (item-level_itypes=1)
2. Mark an item type not for loan (itemtypes.notforloan=1)
3. Checkout an item using this item type (items.itype="BK" for instance)
=> Checkout is not blocked!

I suspect
  commit 3953fdb921
  Bug 19943: Remove itemtype vs itype confusion in CanBookBeIssued
to be the root of this issue.
One occurrence of $item->{itemtype} has not been replaced.
In this case it refers to the biblioitem->{itemtype} value whereas we want to use
$item->{itype}. So this issue does not happen if items.itype==biblioitem.itemtype
(just in case you are not reproducing the problem).

Test plan:
Make sure not for loan items cannot be checked out

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-06-08 10:19:41 +00:00
Kyle M Hall
777b24bbb4 Bug 17561: (QA follow-up) Simplify new ReserveSlip code
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-06-04 13:40:37 -04:00
Benjamin Rokseth
2c1f7d2281 Bug 17561: ReserveSlip needs itemnumber for item level holds on same biblio
This patch fixes a regression after bug 14695.
This patch adds itemnumber and barcode as optional params in ReserveSlip used
by hold-transfer-slip.pl to generate HOLD_SLIP. This is for ReserveSlip to be
able to generate correct slips when items in multi-item holds are checked in.

Test plan:

1) activate a circulation rule with multi-item holds
2) Place two holds on same biblio for patron
3) for debugging, either use browser console to observe POST request and responses
   or use info from reserves, e.g. reserve_id in the HOLD_SLIP
4) checkin two items from same biblio on pickup branch
5) note that both holds are effectuated, but reserve_id is the same on both slips
6) also note that there is no itemnumber or barcode in the requests from returns.pl
7) Apply this patch
8) repeat 2-4
9) note that reserve_id is now different on the two slips

and/or:
Run tests:
  t/db_dependent/Reserves/ReserveSlip.t

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Maksim Sen <maksim.sen@inlibro.com>

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-06-04 13:40:36 -04:00
69f74104d5 Bug 20696: Fix a few ugly "eq undef" comparisons in Search.pm
Comparisons like $a eq undef should normally raise a warning like:
    Use of uninitialized value in string eq at ...
But unfortunately we still suppress warnings here and there.

Test plan:
[1] Just read this patch and confirm the small changes.
[2] Git grep on "eq undef" and do not find other occurrences.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Passes test plan and QA tools. Searching works correctly.

Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-05-29 14:12:52 +00:00
96070c9ea4 Bug 20825: (bug 19943 follow-up) call notforloan on itemtype instead of biblioitem
On bug 19943:
 -        elsif ($biblioitem->{'notforloan'} == 1){
 +        elsif ($biblioitem->notforloan == 1){

The biblioitems table does not contain a notforloan column, this comes
from the item type.

This bug only appears when item type is defined at biblio level
(item-level_itypes=0)

Test plan:
Set item-level_itypes = biblio
Check an item out

Without this patch it explodes with
"The method notforloan is not covered by tests!"

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
I reproduced the error condition and verified the tests failed without
this patch. After this patch is applied, tests pass and checkout
succeeds.

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-05-29 11:37:32 +00:00
1798d22e76 Bug 20724: Move the ReservesNeedReturns logic to AddReserve
Signed-off-by: Victor Grousset <victor.grousset@biblibre.com>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-05-16 10:53:13 -03:00
Kyle M Hall
c585f11497 Bug 20724: Correctly mark an hold as waiting - ReservesNeedReturns
We need to call Koha::Hold->set_waiting to correctly
calculate the expiration date.

It fixes a regression introduced by
  commit 26634151db
  Bug 12063 - Fix QA failures

The expiration date has to be set for waiting holds

== Test plan (time to execute: less than 4min) ==
1. Set ReservesNeedReturns to "Don't automatically"
2. Place a hold on a specific item
3. Check it in and confirm hold
4. The hold should have an expiration date
     record page → Hold → "Expiration" column
5. It should be listed in staff:/cgi-bin/koha/circ/waitingreserves.pl
6. Set ReservesNeedReturns to "Automatically"
7. Place a hold on a specific item
    (which should also behave like we check the item in to
    keep it for the patron)
8. The hold should have an *empty* expiration date
     record page → Hold → "Expiration" column
9. Holds awaiting pickup page should crash
     staff:/cgi-bin/koha/circ/waitingreserves.pl
10. Cancel the hold to remove the corrupted data
     record page → Hold → the red X
11. Apply this patch
12. Place a hold on a specific item
13. The hold should have an expiration date (not empty)
14. It should be listed in staff:/cgi-bin/koha/circ/waitingreserves.pl
15. Celebrate!

Signed-off-by: Victor Grousset <victor.grousset@biblibre.com>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-05-16 10:53:13 -03:00
Mirko Tietgen
f9e06c3849 Bug 20693: Plack fails, because 'libcgi-emulate-psgi-perl' package is not installed
Add CGI::Emulate::PSGI to PerlDependencies.pm

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-05-15 14:59:37 -03:00
d8a3fae361 Bug 20737: Use https for baker and taylor cover images
Easy change, should be able to verify with code review or testing with
dummy values

To test:
1 - Put some values in baker and taylor prefs (don't need to be valid)
2 - Do a search on the opac (and have some items with isbns)
3 - Inspect the cover images links to ensure they are formed correctly
4 - prove -v t/External/BakerTaylor.t

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-05-11 11:36:23 -03:00
935af3eab5 Bug 19792: Few minor fixes
- No string extrapolation needed on SQL queries
- '== undef' must be written 'if not defined'

Signed-off-by: Jon Knight <J.P.Knight@lboro.ac.uk>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-05-11 10:52:47 -03:00
8f3f232175 Bug 19792: (QA follow-up) Move anonymous sub out of GetHierarchy
Test plan:
Run t/db_dependent/Budgets.t.
Without both patches, running the test is slower than with only the first
or both patches.

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

Signed-off-by: Jon Knight <J.P.Knight@lboro.ac.uk>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-05-11 10:52:46 -03:00
David Bourgault
1edd7f8265 Bug 19792: Performance boost for GetBudgetHierarchy
This patches reduces the number of SQL calls by combining multiple calls to the recursive functions GetBudgetSpent(), GetBudgetOrdered(), etc. into 4 big queries.
It also removes duplicate function calls from acqui-home.pl

Test plan:
0) Visit Acquisition home
0) Apply patch
1) Refresh page. It shoud look identical.
2) prove t/db_dependent/budgets.t

Signed-off-by: Jon Knight <J.P.Knight@lboro.ac.uk>

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

Signed-off-by: Jon Knight <J.P.Knight@lboro.ac.uk>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-05-11 10:52:46 -03:00
17e8acad70 Bug 20624: Net::OAuth2::AuthorizationServer is not a hard dependency
While we get packaging sorted, Net::OAuth2::AuthorizationServer is not a
hard dependency for Koha and the feature requiring it is disabled by
default.

This patch:
- Makes the dependency optional
- Makes the unit tests for the OAuth2 client credentials flow skip if
  the dependency is not met.

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-05-09 12:56:03 -03:00
Julian Maurice
5c7d12e4e0 Bug 20402: Remove dependency on Mojo::Plugin::OAuth2::Server
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-05-08 15:55:42 -03:00
Julian Maurice
43a4b3c22c Bug 20402: Implement OAuth2 authentication for REST API
It implements only the "client credentials" flow with no scopes
support. API clients are tied to an existing patron and have the same
permissions as the patron they are tied to.
API Clients are defined in $KOHA_CONF.

Test plan:
0. Install Net::OAuth2::AuthorizationServer 0.16
1. In $KOHA_CONF, add an <api_client> element under <config>:
     <api_client>
       <client_id>$CLIENT_ID</client_id>
       <client_secret>$CLIENT_SECRET</client_secret>
       <patron_id>X</patron_id> <!-- X is an existing borrowernumber -->
     </api_client>
2. Apply patch, run updatedatabase.pl and reload starman
3. Install Firefox extension RESTer [1]
4. In RESTer, go to "Authorization" tab and create a new OAuth2
   configuration:
   - OAuth flow: Client credentials
   - Access Token Request Method: POST
   - Access Token Request Endpoint: http://$KOHA_URL/api/v1/oauth/token
   - Access Token Request Client Authentication: Credentials in request
     body
   - Client ID: $CLIENT_ID
   - Client Secret: $CLIENT_SECRET
5. Click on the newly created configuration to generate a new token
   (which will be valid only for an hour)
6. In RESTer, set HTTP method to GET and url to
   http://$KOHA_URL/api/v1/patrons then click on SEND
   If patron X has permission 'borrowers', it should return 200 OK
   with the list of patrons
   Otherwise it should return 403 with the list of required permissions
   (Please test both cases)
7. Wait an hour (or run the following SQL query:
   UPDATE oauth_access_tokens SET expires = 0) and repeat step 6.
   You should have a 403 Forbidden status, and the token must have been
   removed from the database.
8. Create a bunch of tokens using RESTer, make some of them expires
   using the previous SQL query, and run the following command:
     misc/cronjobs/cleanup_database.pl --oauth-tokens
   Verify that expired tokens were removed, and that the others are
   still there
9. prove t/db_dependent/api/v1/oauth.t

[1] https://addons.mozilla.org/en-US/firefox/addon/rester/

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-05-08 15:55:42 -03:00
046f1f3401 Bug 20284: (QA follow-up) superlibrarian has ill permissions
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-05-07 11:55:01 -03:00
Mark Tompsett
be35e4883a Bug 20663: Add some POD, so koha test tools don't complain
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-05-04 09:19:26 -03:00
Mark Tompsett
32a2528d53 Bug 20663: Remove dead code related to compound reports
TEST PLAN
---------
git grep "Create Compound Report"
-- only one file
git grep compound | grep 1
-- this is the only setting of the compound tt variable
less koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt
-- There is an TT IF statement for compound.
-- In that statement it would trigger 'Save Compound'
git grep save_compound
-- only the template and the guided report perl
git grep create_compound
-- only triggered by the save code in the guided report perl
-- in the export for the C4/Reports/Guided.pm
git grep run_compound
-- left over in export

apply the patch
look around and see the pieces are cleaned up.

run koha qa test tools

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-05-04 09:19:26 -03:00
Mark Tompsett
6d59eee2ba Bug 20103: Readonly::XS is completely bogus post perl 5.8
A ha! Lie about who we are, and the module will return a version.

./koha_perl_deps.pl -a
-- before patch Readonly::XS is 0
-- after patch it has a version number.
run koha qa test tools

Signed-off-by: Victor Grousset <victor.grousset@biblibre.com>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-05-03 13:26:49 -03:00
b7787d1022 Bug 20444: Remove sub GetAttributes
0) Apply this patch
1) git grep GetAttributes should return no occurencies

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-05-03 13:26:49 -03:00
d6f3e1fbe6 Bug 18342: Mark Cache::Memcached as mandatory
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-05-03 12:47:07 -03:00
8d3eba6a29 Bug 19204: Make the debarment date calculation depends on finesCalendar
This patchs adds the ability to calculate the end of the suspension date
(debarment date) using the finesCalendar syspref.
Prior to this patch it was never calculating without taking into account
the calendar.

calculated without taking holidays into account.
This was a problem because the restriction could end in the middle of a
period the library is closed.

Test plan:
- Set finescalendar to 'not including days the library is closed'
- Set a circulation condition with no fine/maxfine, but fine days and
max fine days instead
- Check out an item with a due date in the past
- Check the item in and verify the restriction date
- Clean the restriction

- Add holidays to your calendar on the calculated restriction date
- Check the item out again with the same due date in the past
- Check in the item again
- Verify the calculated restriction end date has changed, it's set to the day
after the holiday.

Fines in days restriction calculation is correctly taking calendar
into account.

Sponsored-by: Goethe-Institut

Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-04-25 10:52:20 -03:00
5c7ff786d5 Bug 19855: Move getalert, addalert and delalert to Koha::Subscription
This patch removes 3 subroutines from C4::Letters:
- getalert
- addalert
- delalert

And add 3 methods to Koha::Subscription:
- subscribers
- add_subscriber
- remove_subscriber

It makes the code cleaner for future cleanup.
TODO - we should remove alert.alertid and alert.type, and rename
alert.externalid with alert.subscriptionid
That way alert will be renamed borrowers_subscriptions (or similar) and
will become a simple join table between borrowers and subscriptions.
We will need to deal with FK that could not be satisfied.
Let's do that after this patch is pushed.

Test plan:
Subscribe and unsubscribe to email notifications sent when a new issues
is available.
Make sure everything works as before and you receive the emails.

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-04-23 14:22:15 -03:00
0bd1f30c8c Bug 19855: Remove $type from the alerts
It looks like this feature has never been finished. It has been
developed with more flexibility in mind, but only 'issue' is used for
this parameter. Apparently it could have been 'virtual', for virtual shelves.

Let remove this parameter and clean the code a bit.
TODO: Remove the DB column

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-04-23 14:22:15 -03:00
8a81a4adab Bug 19855: Remove C4::Letters::findrelatedto
This subroutine is called only once. It only concat firstname and
surname for subscribers.
It can be easily replaced with Koha::Patron

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-04-23 14:22:15 -03:00
Katrin Fischer
9c0d403586 Bug 20400: (follow-up) Several fixes from RM review
- "your routing lists" tab is now highlighted when active
- get_routinglists was renamed to get_routing_lists
- Koha::Patron->get_routing_lists returns the ->search result
  directly
- Koha::Subscription::RoutingList->subscription uses DBIC
  relationship
- Undo changes to C4/Auth.pm

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-04-20 17:59:06 -03:00
Katrin Fischer
9af0b9de7d Bug 20400: Add routing list tab in OPAC
This patch adds the base for the new feature:
Show a list of the serial titles a patron is on routing
lists for in the OPAC.

Test plan applies to the complete patch set:

To test:
- Apply all patches
- Make sure RoutingSerials is not activated
- Check patron account in OPAC - no tab should appear
- Activate RoutingSerials
- Create subscriptions and different routing lists, test with:
  - Patron with no routing list entries = no tab
  - Patron with one or more routing list entries = tab appears

Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Bug 20400: Rewrite using Koha::Objects

Adds
- Koha::Subscription::Routinglist
- Koha::Subscription::Routinglists

Adds 2 methods
- Koha::Patron::get_routinglists
- Koha::Routinglist::subscription

Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>

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

Bug 20400: Add unit tests

prove t/db_dependent/Koha/Subscription/Routinglists.t
prove t/db_dependent/Koha/Patrons.t

Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>

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

Bug 20400: Display new tab in OPAC only for patrons with routing lists

The visibility of the routing list tab in the OPAC depends
on the system preference RoutingSerials and the existence
of routing list entries for the patron.

Some libraries only offer routing lists to certain user groups and
would not want it generally visible. As there are currently no
actions you can perform from the list, this appears to be a
reasonable behaviour.

See test plan in first patch.

Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>

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

Bug 20400: (follow-up) Use Asset TT plugin on opac-routing-lists.tt

Patch applies and functions as described.
Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>

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

Bug 20400: (QA follow-up) Redirect to 404 if routing is disabled

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-04-20 13:34:41 -03:00
Julian Maurice
268ba1d5ec Bug 19873: (QA follow-up) Prevent warnings
"Use of uninitialized value $operands[X] in string ne"

Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-04-20 13:34:41 -03:00
Alex Arnaud
bc41a490a1 Bug 19873: Ability to search on 0 value
Test plan:
  - create a st-numeric index in zebra conf related to
    a numeric field
    i.e:
    yourindex 1=yourindex 4=109
  - fill a field with 0 in a biblio,
  - reindex your biblios,
  - search yourindex=0,
  - should not work,
  - apply this patch,
  - test again,
  - should work

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-04-20 13:34:41 -03:00
4de8790482 Bug 20603: Remove ModNote, getcredits and getrefunds from C4::Accounts
Test plan:
0) Do not apply the patch
1) Run these 3 commands
    git grep ModNote
    git grep getcredits
    git grep getrefunds
all should return occurances only in C4/Accounts.pm - the sub definition
and in t/db_dependent/Accounts.t
2) Apply the patch
3) Run commands from 1) again, now there should be no occurence
4) prove t/db_dependent/Accounts.t
    should return green

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-04-20 12:24:00 -03:00
d68fe07bf8 Bug 20489: Prevent DB user login
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-04-20 12:24:00 -03:00
d32791b5db Bug 19974: Make MarkLostItemsAsReturned multiple
Given the confusion regarding this behaviour it sounds better to make it
configurable.
This pref will take 4 different values, 1 per place an item can be
marked as lost.

Test plan:
Mark items as lost and confirm the item is returned or not, depending on
the value of the system preference.

- from the longoverdue cronjob (--mark-returned takes precedence if set)
- from the batch item modification tool
- when cataloguing an item
- from the items tab of the catalog module

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-04-20 11:57:31 -03:00
74e0dfb437 Bug 20590: Improve Koha::Exceptions in string context
I was working on bug 20580 and noticed that the exceptions are not
stringified and so not really useful when displayed in string context.

This patch is for discussion and not ready for inclusion

Still need to fix:
Koha/REST/V1/Patrons.pm (The error is "Given xxx does not exist")
Koha/Object.pm: in ->store, Koha::Exceptions::Object::FKConstraint is
raised but we cannot set 'value'

Another note: In Koha::Exceptions::Exception I do not have access to
$self->line_number and $self->filename

See also
http://search.cpan.org/~drolsky/Exception-Class-1.44/lib/Exception/Class/Base.pm#OVERRIDING_THE_as_string_METHOD

Test plan:
Apply patch from bug 20580 and execute create_superlibrarian.pl with an
invalid categorycode

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-04-19 12:46:04 -03:00
3d1f04b269 Bug 20562: Pass the Koha::Checkout object to AddIssuingCharge
We do not need to pass all those parameters, just the checkout object is
enough.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-04-18 17:20:54 -03:00
dc5d3b9abd Bug 20562: issue_id is not stored in accountlines for rental fees
Test Plan:
1) Apply this patch
2) Assign a charge to an item type
3) Checkout an item of that type to a patron
4) View the accountlines table for that patron
SELECT * FROM accountlines WHERE accounttype='Rent' and borrowernumber=##;
5) Note there is an issue_id

Or

1) Apply this patch
2) prove t/db_dependent/Circulation/issue.t

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-04-18 17:20:54 -03:00
38132cabbb Bug 18725: (QA follow-up) Use make_column_dirty instead of status change
Moving the status to the invalid 'processing' might well have unwanted
side-effects. (The status column will be set to empty string and we have
a problem if it is not processed.)

This patch allows pass-through of DBIX's make_column_dirty in
Koha::Object (simple tests included) and uses it to force an update.
If the update does not return true, it still exits.

Test plan:
[1] Read the changes.
[2] Run t/db_dependent/Koha/Object.t

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-04-16 13:07:22 -03:00
8ad4547432 Bug 18725: Prevent process_message_queue to send duplicate emails if message_queue is not writable.
Last week, we had a database server whose disk filled, causing database writes to fail.

This meant that messages in message_queue marked 'pending' were not marked as 'sent' when they were added to the postfix mail queue; messages were sent every 15 minutes (as specified in the cron job) until the disk space issues were cleared.

I would suggest adding a token write to the start of process_message_queue.pl as a 'canary in the coal mine'. If the database write fails, process_message_queue should stop, because it's not safe to proceed sending emails that may not be marked 'sent'.

Test Plan:
1) Apply this patch
2) Make the message_queue table unwriteable somehow
3) Run process_message_queue.pl
4) Script should die with error

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-04-16 13:07:22 -03:00
Julian Maurice
ed7543287b Bug 20538: Remove the need of writing [% KOHA_VERSION %] everywhere
Having to write [% KOHA_VERSION %] for each url is bad because:
- It's easily forgettable when adding new <script> or <link>
- It prevents grep'ing for the full filename
- It violates the DRY principle
- If at some point we want to change the "force js and css reload"
  mechanism, it will be tedious

This patch:
- adds a Template::Toolkit plugin that generates <script> and
  <link> tags for JS and CSS files, and inserts automatically the Koha
  version in the filename
- use the new plugin to remove all occurences of [% KOHA_VERSION %]
- remove the code that was adding KOHA_VERSION as a template variable

Test plan:
1. Apply patch
2. Go to several different pages in Koha (opac and intranet) while
   checking your browser's dev tools (there should be no 404 for JS and
   CSS files, and the Koha version should appear in filenames) and the
   server logs (there should be no "File not found")
3. `git grep KOHA_VERSION` should return nothing
4. prove t/db_dependent/Koha/Template/Plugin/Asset.t

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-04-13 11:49:44 -03:00
4fe8b62f84 Bug 20499: just check if GetItem returns something is enough
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: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-04-13 11:49:13 -03:00