]> git.koha-community.org Git - koha.git/log
koha.git
8 years agoBug 16272 [QA Followup] - Indexer fails on indexing temparary testing records
Kyle M Hall [Thu, 18 Aug 2016 16:33:00 +0000 (16:33 +0000)]
Bug 16272 [QA Followup] - Indexer fails on indexing temparary testing records

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 16272: Add description for tests
Jonathan Druart [Tue, 16 Aug 2016 15:59:38 +0000 (16:59 +0100)]
Bug 16272: Add description for tests

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 16272: (follow-up) Specific case when switching an on-site checkout to a regular...
Jonathan Druart [Tue, 16 Aug 2016 15:52:27 +0000 (16:52 +0100)]
Bug 16272: (follow-up) Specific case when switching an on-site checkout to a regular checkout

Same as previous patch but if another circ rule exists

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 16272: Specific case when switching an on-site checkout to a regular checkout
Jonathan Druart [Thu, 21 Apr 2016 20:42:11 +0000 (21:42 +0100)]
Bug 16272: Specific case when switching an on-site checkout to a regular checkout

In the case on-site checkouts are considered as a regular checkout in issuing
rules (i.e. ConsiderOnSiteCheckoutsAsNormalCheckouts is on):
When after the on-site checkout the maximum limit of checkouts is
reached and the patron wants to switch an on-site checkout to a regular
checkout, the C4::Circulation::TooMany subroutine will return a
TOO_MANY_CHECKOUTS error.
To avoid that, we need to allow an extra checkout in this subroutine.

Test plan:
0/ Switch ConsiderOnSiteCheckoutsAsNormalCheckouts and
SwitchOnSiteCheckouts on
1/ In the issuing rules, set the total number of checkouts (maxissueqty)
to 2 and the number of on-site checkouts to 2 (maxonsiteissueqty)
2/ Check 2 items out ticking the 'on-site checkout' checkbox
3/ Check one of these items out, to automatically switch it to a regular
checkout
=> The checkout should be allowed.

Sponsored-by: BULAC - http://www.bulac.fr/
Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 16272: Automatically switch an on-site checkout to a regular checkout when checke...
Jonathan Druart [Thu, 21 Apr 2016 10:00:20 +0000 (11:00 +0100)]
Bug 16272: Automatically switch an on-site checkout to a regular checkout when checked out

Use case: A patron checks some items out on-site and want to take it home.
To facilitate the librarian work the checkout is directly switched from
on-site to regular when checked out if the new pref SwitchOnSiteCheckouts is on.

Test plan:
0/ Let the new pref SwitchOnSiteCheckouts off
1/ Checkout one items to a patron and tick the "on-site checkout"
checkbox
2/ Check the same item out without ticking the "on-site checkout"
checkbox
=> You should get "This item can not be renewed, it's an on-site checkout"
3/ Switch the pref on
4/ Repeat 2
=> The on-site checkout should be automatically switched to a regular checkout

Sponsored-by: BULAC - http://www.bulac.fr/
Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
With small changes to apply to master.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 17222: Use jQuery selector instead
Jonathan Druart [Wed, 7 Sep 2016 10:38:43 +0000 (11:38 +0100)]
Bug 17222: Use jQuery selector instead

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 17222: Remove use of onclick from OPAC member entry page
Aleisha Amohia [Mon, 29 Aug 2016 23:22:11 +0000 (23:22 +0000)]
Bug 17222: Remove use of onclick from OPAC member entry page

There is only one instance on onclick here, and that is the clear date
function.

To test, make sure clicking 'Clear date' next to Date of Birth works the
same before and after the patch.

Sponsored-by: Catalyst IT
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 17220: Change hold confirmation string from 'Place hold' to 'Confirm hold'
Lee Jamison [Mon, 29 Aug 2016 20:24:49 +0000 (16:24 -0400)]
Bug 17220: Change hold confirmation string from 'Place hold' to 'Confirm hold'

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 16843: Help for EDIFACT messages
Aleisha [Mon, 4 Jul 2016 01:41:57 +0000 (01:41 +0000)]
Bug 16843: Help for EDIFACT messages

To test:

1) Go to Acqui -> EDIFACT messages
2) Click Help in top-right corner
3) Confirm that Help window opens with appropriate help text
4) Confirm link to manual works as expected

Sponsored-by: Catalyst IT
Signed-off-by: Marc VĂ©ron <veron@veron.ch>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 17210: Add missing return false for print-large
Jonathan Druart [Wed, 7 Sep 2016 10:19:15 +0000 (11:19 +0100)]
Bug 17210: Add missing return false for print-large

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 17210: Remove use of onclick from biblio detail sidebar in OPAC
Aleisha Amohia [Mon, 29 Aug 2016 06:07:53 +0000 (06:07 +0000)]
Bug 17210: Remove use of onclick from biblio detail sidebar in OPAC

Four instances of onclick here:
1) Print
2) Save to your lists
3) Add to your cart
4) Remove from cart

To test:
Find a biblio detail page in OPAC and confirm all four events above work
as expected before and after the patch.

Sponsored-by: Catalyst IT
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 16365: [QA Follow-up] Add some comment lines to Cache.pm
Marcel de Rooy [Thu, 28 Jul 2016 06:43:48 +0000 (08:43 +0200)]
Bug 16365: [QA Follow-up] Add some comment lines to Cache.pm

Resolve typo inifinite too.
Adds a few lines in order to stress that the thawed key of the L1
cache SHOULD ONLY be used for unsafe calls, and not be mixed with
regular (safe) calls.

Test plan:
Nothing to test, but verify the quality of the added comments.

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: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 16365: Adding additional unsafe call-candidates from Acquisition
Marcel de Rooy [Thu, 28 Jul 2016 08:29:18 +0000 (10:29 +0200)]
Bug 16365: Adding additional unsafe call-candidates from Acquisition

[1] Candidate 1 is FillWithDefaultValues.
This routine in Acquisition.pm does not autovivify the tagslib
structure but first collects the keys at tag and subfield level.
So, unsafe can be safely added here :)

[2] Candidate 2 is script acqui/neworderempty.pl.
It only (!) uses GetMarcStructure to know if there is a ACQ framework.
There should be cheaper ways to do it, but when we use the fast cache,
it does not matter that much anymore.

[3] Candidate 3 is script acqui/orderreceive.pl. Same reason as [2].

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 16365 - GetMarcStructure() "unsafe" variant in PrepareItemrecordDisplay()
Jacek Ablewicz [Wed, 27 Jul 2016 09:38:00 +0000 (11:38 +0200)]
Bug 16365 - GetMarcStructure() "unsafe" variant in PrepareItemrecordDisplay()

This sub is a good candidate for the "unsafe" treatment too, it doesn't
modify nor autovivify anything in the marc structure.

Added a warning in the code regarding the $tagslib usage by the
custom item plugins, plus a small change to prevent possible
"Use of uninitialized value" warnings in the future.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested with neworderempty.pl and itemrecorddisplay.pl.
Amended slightly: Made warning less dramatic.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 16365: Selectively introduce GetMarcStructure() "unsafe" variant for better perfo...
Jacek Ablewicz [Wed, 27 Apr 2016 11:19:10 +0000 (13:19 +0200)]
Bug 16365: Selectively introduce GetMarcStructure() "unsafe" variant for better performance

GetMarcStructure() currently uses Koha::Cache in the "safe" mode
(returning deep copy of the result data structure by default), which
causes numerous performance issues in many Koha scripts. Switching
it to the "unsafe" mode globally (2nd patch from Bug 16140) resolves
those issues, but ensuring that it is regression-free (and that it
will stay that way in the future) is far from easy. This patch
proposes a bit more manageable solution, it introduces
a possibility to use "unsafe" variant selectively (only in those
places in the code where GetMarcStructure() is called repetitively).
That way, amount of the code that needs to be audited for possible
problems gets vastly reduced, without any performance trade-offs.

Test plan:
1) Have a look at the code and audit the parts affected by this patch
for possible regressions

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Amended the POD of GetMarcStructure, removing a TODO.

NOTE: GetAuthorisedValueDesc, as called in C4::XSLT::transformMARCXML4XSLT
and by GetISBDView, GetMarcAuthors in C4::Biblio, may autovivify some hash
entries in tagslib.
Same for Koha/Filter/MARC/ViewPolicy.pm, sub filter.

No reason however to worry; our use of this structure in Koha does not
depend on the existence of intermediate hash keys. (We seem to be safe as
long as $tagslib->{$tag}->{$subfield}->{tab} and/or hidden are not filled.)

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15839 - DBRev 16.06.00.019
Kyle M Hall [Fri, 9 Sep 2016 10:38:43 +0000 (10:38 +0000)]
Bug 15839 - DBRev 16.06.00.019

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15839: [QA Follow-up] Error checking in opac-review.pl
Marcel de Rooy [Fri, 29 Jul 2016 08:24:44 +0000 (10:24 +0200)]
Bug 15839: [QA Follow-up] Error checking in opac-review.pl

[1] Adds a check on biblionumber. (Prevents a DBIx error.)
[2] If you have a reviewid, search on that and check results.
    Add an unauthorized error in template.
[3] If you add a new review, check that there is no review yet.
    If so, edit the existing one.
    This supports the added FIXME on a unique constraint.

Note: This script could receive further attention.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested all crud ops with opac-review.pl (incl URL manipulation).

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15839: [QA Follow-up] Paging on opac-showreviews
Marcel de Rooy [Fri, 29 Jul 2016 07:27:58 +0000 (09:27 +0200)]
Bug 15839: [QA Follow-up] Paging on opac-showreviews

Paging is kind of messy here. This patch at least improves somewhat.
The page number should be rounded.
The results per page should be passed to the template too.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested a number of reviews and played with count parameter in URL.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15839: [QA Follow-up] Update existing rows in dbrev
Marcel de Rooy [Fri, 29 Jul 2016 06:16:47 +0000 (08:16 +0200)]
Bug 15839: [QA Follow-up] Update existing rows in dbrev

Adding the default is fine for new records; we also should take care
of the pending existing ones.
Trivial addition to atomic update file.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested the dbrev again with existing unapproved reviews.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15839: Koha::Reviews - Remove C4::Review residue
Jonathan Druart [Tue, 16 Feb 2016 16:32:15 +0000 (16:32 +0000)]
Bug 15839: Koha::Reviews - Remove C4::Review residue

Signed-off-by: Marc Veron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15839: Koha::Reviews - Remove savereview
Jonathan Druart [Tue, 16 Feb 2016 16:30:07 +0000 (16:30 +0000)]
Bug 15839: Koha::Reviews - Remove savereview

Signed-off-by: Marc Veron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15839: Koha::Reviews - Remove updatereview
Jonathan Druart [Tue, 16 Feb 2016 16:28:33 +0000 (16:28 +0000)]
Bug 15839: Koha::Reviews - Remove updatereview

Signed-off-by: Marc Veron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15839: Koha::Reviews - Remove getnumberofreviews
Jonathan Druart [Tue, 16 Feb 2016 16:28:09 +0000 (16:28 +0000)]
Bug 15839: Koha::Reviews - Remove getnumberofreviews

Signed-off-by: Marc Veron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15839: Koha::Reviews - Remove getreview
Jonathan Druart [Tue, 16 Feb 2016 16:15:55 +0000 (16:15 +0000)]
Bug 15839: Koha::Reviews - Remove getreview

Note that this (biblionumber, borrowernumber) pair should be defined as
a unique key at the DB level (FIXME added)

Signed-off-by: Marc Veron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15839: Koha::Reviews - Remove deletereview
Jonathan Druart [Tue, 16 Feb 2016 16:06:46 +0000 (16:06 +0000)]
Bug 15839: Koha::Reviews - Remove deletereview

Note that a future enhancement should take into account the possible
error returned by ->approve, ->unapprove and ->delete

Signed-off-by: Marc Veron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15839: Koha::Reviews - Change default value for approved to 0
Jonathan Druart [Tue, 16 Feb 2016 16:53:41 +0000 (16:53 +0000)]
Bug 15839: Koha::Reviews - Change default value for approved to 0

The reviews.approved column had a default value set to NULL.
It does not make sense, the default value should be 0, this will avoid
to have to specify the approved value when creating a new review.

Signed-off-by: Marc Veron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15839: Koha::Reviews - Remove approvereview & unapprovereview
Jonathan Druart [Tue, 16 Feb 2016 16:06:27 +0000 (16:06 +0000)]
Bug 15839: Koha::Reviews - Remove approvereview & unapprovereview

This patch adds 2 new methods to Koha::Review: approve and unapprove.

Signed-off-by: Marc Veron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15839: Koha::Reviews - Remove getallreviews
Jonathan Druart [Tue, 16 Feb 2016 15:56:03 +0000 (15:56 +0000)]
Bug 15839: Koha::Reviews - Remove getallreviews

Signed-off-by: Marc Veron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15839: Koha::Reviews - Remove getreviews
Jonathan Druart [Tue, 16 Feb 2016 15:09:01 +0000 (15:09 +0000)]
Bug 15839: Koha::Reviews - Remove getreviews

The C4::Reviews::getreviews subroutine retrieved the reviews for a given
record, depending on their status.
This can be achieve with a call to Koha::Reviews->search.
There were 2 calls to this subroutine. The one from opac-ISBDdetail.pl
does not look in used: the reviews are not display on this page. It
certainly comes from an old copy/paste from opac-detail.pl.
The one from opac-detail only asked for the approved reviews. So the
logged in user does not see its own review if it is pending approval.
Actually this pending approval review is only displayed when the user
submits it (because of a unecessary complex text replacement done in
JS).
With this patch, the approved reviews AND the unaproved review from the
logged in user will be displayed.
It will allow a future enhancement to add a way to delete our own
reviews.

Moreover, the reviews were retrieved even if they were not displayed (if
reviewson is off), it's now fixed.

Signed-off-by: Marc Veron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15839: Koha::Reviews - Add Koha::Review[s] classes
Jonathan Druart [Tue, 16 Feb 2016 16:41:22 +0000 (16:41 +0000)]
Bug 15839: Koha::Reviews - Add Koha::Review[s] classes

The subroutines in C4::Reviews are only doing CRUD operations.
This patch set moves them to 2 new Koha::Review[s] classes.

Test plan for the whole patch set:
0/ Set the reviewson syspref
1/ At the OPAC, create a couple of reviews for a record
Try and create a review without html tags different that br b i em big
small strong: You should get a warning.
2/ Edit a review
3/ On the Staff interface, you should see a "Comments pending approval"
link on the mainpage
4/ Approve 1 review and delete the other
5/ At the OPAC you should only see 1 review (the approved one) in the
"Comments" tab
6/ Modify the review
7/ The review should appear again in the "comments awaiting moderation"
tab.
Note that even the comment has not been changed, it will have to be
reapproved (FIXME later). This behavior already existed prior to this
patch.

Tested all patches together (with database updated)
Signed-off-by: Marc Veron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 16851 [QA Followup] - Fix Unit Tests
Kyle M Hall [Fri, 9 Sep 2016 10:21:33 +0000 (10:21 +0000)]
Bug 16851 [QA Followup] - Fix Unit Tests

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 16851: Move HasOverdues to Koha::Patron->has_overdues
Jonathan Druart [Mon, 4 Jul 2016 16:56:54 +0000 (17:56 +0100)]
Bug 16851: Move HasOverdues to Koha::Patron->has_overdues

This patch just move C4::Members::HasOverdues to
Koha::Patron->has_overdues and updated callers

Test plan:
No change in behavior is expected.
1/ If a patron is debarred and does not have overdues and
AutoRemoveOverduesRestrictions is on, the debarment will be removed on
checkin
2/ Add overdues and make sure the patron cannot renew

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 16911 [QA Followup] - Fix number of categories to look for after deleting one
Kyle M Hall [Fri, 9 Sep 2016 09:47:33 +0000 (09:47 +0000)]
Bug 16911 [QA Followup] - Fix number of categories to look for after deleting one

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 16911: (QA followup) CGI->param should not be called in list context
Tomas Cohen Arazi [Mon, 5 Sep 2016 18:18:57 +0000 (15:18 -0300)]
Bug 16911: (QA followup) CGI->param should not be called in list context

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 16911: ->get_expiry_date accepts strings
Jonathan Druart [Wed, 20 Jul 2016 18:03:53 +0000 (19:03 +0100)]
Bug 16911: ->get_expiry_date accepts strings

To make sure nothing else will be broken, it's certainly better to
handle date strings in a first phase.

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 16911: Rename extend_subscription with renew_account
Jonathan Druart [Tue, 12 Jul 2016 19:29:15 +0000 (20:29 +0100)]
Bug 16911: Rename extend_subscription with renew_account

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 16911: Koha::Patrons - Move ExtendMemberSubscriptionTo to ->extend_subscription
Jonathan Druart [Mon, 11 Jul 2016 22:28:19 +0000 (23:28 +0100)]
Bug 16911: Koha::Patrons - Move ExtendMemberSubscriptionTo to ->extend_subscription

This patch moves the code from C4::Members::ExtendMemberSubscriptionTo
to Koha::Patron->extend_subscription.

The expected behavior is:
When a new patron is created, the enrolment period defined for the
patron category is used unless an enrolment period date is defined. In
that case, this date is used.
When an account is renewed, the pref BorrowerRenewalPeriodBase is used to
determine if the subscription is renewed from today or from the day when his/her
account has expired.

Test plan:
Confirm that the behavior is correct before this patch and that it's
still the same after this patchset applied.

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 16911: Koha::Patrons - Add tests for ->extend_subscription
Jonathan Druart [Mon, 11 Jul 2016 23:18:56 +0000 (00:18 +0100)]
Bug 16911: Koha::Patrons - Add tests for ->extend_subscription

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 16911: Koha::Patron::Categories - Move GetExpiryDate to ->get_expiry_date
Jonathan Druart [Mon, 11 Jul 2016 22:45:55 +0000 (23:45 +0100)]
Bug 16911: Koha::Patron::Categories - Move GetExpiryDate to ->get_expiry_date

This patch moves the code from C4::Members::GetExpiryDate to
Koha::Patron::Categories.
Nothing much exiting, please follow test plan in next patches.

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 16911: Koha::Patron::Categories - Add tests for ->get_expiry_date
Jonathan Druart [Mon, 11 Jul 2016 23:14:56 +0000 (00:14 +0100)]
Bug 16911: Koha::Patron::Categories - Add tests for ->get_expiry_date

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 17089: [QA Follow-up] Add some test descriptions to Ratings.t
Marcel de Rooy [Fri, 12 Aug 2016 08:12:18 +0000 (10:12 +0200)]
Bug 17089: [QA Follow-up] Add some test descriptions to Ratings.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 17089: Improve errors handling
Jonathan Druart [Tue, 9 Aug 2016 16:06:56 +0000 (17:06 +0100)]
Bug 17089: Improve errors handling

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 17089: Mark out the boundaries of ratings
Jonathan Druart [Tue, 9 Aug 2016 14:08:55 +0000 (15:08 +0100)]
Bug 17089: Mark out the boundaries of ratings

In the doc of C4::Ratings, it was written that the rate values should be
>=0 and <=5, but it was not taken into account.
This bug should be fixed on stable releases as well.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 17089: Koha::Ratings - Remove C4::Ratings
Jonathan Druart [Tue, 9 Aug 2016 14:01:05 +0000 (15:01 +0100)]
Bug 17089: Koha::Ratings - Remove C4::Ratings

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 17089: Koha::Ratings - Remove GetRating
Jonathan Druart [Tue, 9 Aug 2016 14:01:21 +0000 (15:01 +0100)]
Bug 17089: Koha::Ratings - Remove GetRating

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 17089: Koha::Ratings - Remove DelRating
Jonathan Druart [Tue, 12 Jul 2016 10:28:57 +0000 (11:28 +0100)]
Bug 17089: Koha::Ratings - Remove DelRating

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 17089: Koha::Ratings - Remove ModRating
Jonathan Druart [Tue, 12 Jul 2016 10:06:07 +0000 (11:06 +0100)]
Bug 17089: Koha::Ratings - Remove ModRating

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 17089: Koha::Ratings - Remove AddRating
Jonathan Druart [Tue, 12 Jul 2016 10:04:41 +0000 (11:04 +0100)]
Bug 17089: Koha::Ratings - Remove AddRating

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 17089: Koha::Ratings - move tests
Jonathan Druart [Tue, 12 Jul 2016 10:28:42 +0000 (11:28 +0100)]
Bug 17089: Koha::Ratings - move tests

Rewrite and move the tests.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 17089: Koha::Ratings - Add Koha::Rating[s] classes
Jonathan Druart [Tue, 12 Jul 2016 10:28:25 +0000 (11:28 +0100)]
Bug 17089: Koha::Ratings - Add Koha::Rating[s] classes

The subroutines in C4::Ratings are only CRUD operations.
This patch set moves the entire module to Koha::Rating[s] based on
Koha::Object[s]

Test plan for the whole patch set:
0/ Set OpacStarRatings to "results and details"
1/ Log in at the OPAC
2/ Launch a search, you should see the star ratings
3/ Go on a detail page, you should see the star ratings
4/ Rate the record
5/ Log in with another patron
6/ Go on the same detail page, you should see the previous rating
7/ Rate the racord, you should see "2 votes" and the average rating
value.
8/ Cancel your rate
9/ Disable JavaScript and repeat 7

Note: You cannot cancel your rate if JavaScript is disabled (not a
regression).

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 16822: Make koha-common call koha-plack without --quiet
Tomas Cohen Arazi [Tue, 26 Jul 2016 13:23:43 +0000 (10:23 -0300)]
Bug 16822: Make koha-common call koha-plack without --quiet

This patch removes the --quiet option switch in koha-common.init so
problem information is not hidden for sysadmin users.

To test:

- Have plack enabled for a koha instance:
  $ sudo koha-plack --enable kohadev
  $ sudo koha-plack --start  kohadev
- Run:
  $ cd kohaclone
  $ debian/koha-common.init {start|stop|restart}
=> SUCCESS: No behaviour change
- Disable the 'cgi' apache module:
  $ sudo a2dismod cgi
- Run:
  $ debian/koha-common.init {start|stop|restart}
=> FAIL: No warning is shown
- Apply the patch
- Run:
  $ debian/koha-common.init {start|stop|restart}
=> SUCCESS: Warnings show up
- Sign off :-D

Signed-off-by: Liz Rea <liz@catalyst.net.nz>
Perfect thanks :)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 17267 - Document koha-create --adminuser
Magnus Enger [Thu, 8 Sep 2016 12:55:58 +0000 (14:55 +0200)]
Bug 17267 - Document koha-create --adminuser

A description of --adminuser was missing from debian/docs/koha-create.xml
This patch adds a description in the Options section, as well as under
DEFAULTSQL in the list of configuration variables.

To test:
Run these commands and look at the formatted man page:
$ xsltproc /usr/share/xml/docbook/stylesheet/docbook-xsl/manpages/docbook.xsl \
  debian/docs/koha-create.xml
$ man -l koha-create.8

Make sure this test passes:
$ prove -v xt/verify-debian-docbook.t

Signed-off-by: Liz Rea <liz@catalyst.net.nz>
Edited commit message to fix typo - koha-mysql -> koha-create as that's what we're testing. :)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 16918 (QA Followup) - Add reverse proxy and fix intranet vs opac typo
Nick Clemens [Thu, 8 Sep 2016 16:19:08 +0000 (16:19 +0000)]
Bug 16918 (QA Followup) - Add reverse proxy and fix intranet vs opac typo

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Amended the patch to adjust a typo: ndex should be index.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 16918: Serve OPAC/intranet homepage via Plack
Jesse Weaver [Thu, 21 Jul 2016 22:29:13 +0000 (16:29 -0600)]
Bug 16918: Serve OPAC/intranet homepage via Plack

Test plan:
  1) Shut down the Plack server.
  2) The OPAC and staff side homepage should still load (as it is
     mistakenly being loaded via CGI).
  3) Apply patch.
  4) Both homepages should now not load.

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 7045 - Fix DBRev 16.06.00.016
Kyle M Hall [Fri, 9 Sep 2016 09:06:51 +0000 (09:06 +0000)]
Bug 7045 - Fix DBRev 16.06.00.016

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15758: [QA Follow-up] Final touch?
Marcel de Rooy [Wed, 31 Aug 2016 12:20:34 +0000 (14:20 +0200)]
Bug 15758: [QA Follow-up] Final touch?

From qa tools:
reports/cat_issues_top.pl: pod: =over on line 39 without closing =back

Note: qa tools does also report "options_for_libraries: not found" on two
template includes. Considering this as a false alarm, since the block is
defined in another include loaded in doc-head-open.
[1] koha-tmpl/intranet-tmpl/prog/en/includes/subscriptions-search.inc
[2] koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-topissues.inc

Found a template where a USE Branches was missing:
koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-add.tt
Easy to spot since the libraries combo was empty.
Tried to find similar occurrences with few greps, but couldn't find any.

I did find duplicate USE Branches though in:
koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
koha-tmpl/intranet-tmpl/prog/en/modules/serials/checkexpiration.tt
This patch removes them.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15758: Fix variable name, it's a branchcode
Jonathan Druart [Fri, 26 Aug 2016 13:05:42 +0000 (14:05 +0100)]
Bug 15758: Fix variable name, it's a branchcode

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15758: [QA Follow-up] Corrections related to GetBranches
Marcel de Rooy [Fri, 26 Aug 2016 11:22:31 +0000 (13:22 +0200)]
Bug 15758: [QA Follow-up] Corrections related to GetBranches

[1] C4/Search
A call to Koha::Libraries is added to routine pazGetRecords, but the
results of that call are not used. So removing it again.

[2] catalogue/itemsearch.pl
Although A=>B=>C=>D works, we'd better use here A=>B, C=>D.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15758: [QA Follow-up] Some corrections related to GetBranchName
Marcel de Rooy [Fri, 26 Aug 2016 07:02:04 +0000 (09:02 +0200)]
Bug 15758: [QA Follow-up] Some corrections related to GetBranchName

[1] Acquisition.pm
The lines filling $row in GetBasketGroupAsCSV may have side-effects when
the library name is not found. This change restores former behavior. Just
theoretically more safe.
Note that it also contained a typo: $row->{deliveryplace} should have been
$row->{$place}.

[2] Auth.pm
checkauth: $branchname = Koha::Libraries->find($branchcode)->branchname;
Should normally be fine, but I rather have an empty string here than
crashing on "Can't call method branchname on undefined value".
Same for sub check_api_auth.
Note that this holds for a larger number of calls, but I am adding a check
here because it is checkauth.
Also removed a duplicate use Koha::Libraries-statement.

[3] Search.pm
Also removed a duplicate use statement for Libraries.

[4] svc/holds
Added an (explicit) use statement for Koha::Libraries.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15758: [QA Follow-up] Remove onlymine from Plugin/Branches.pm
Marcel de Rooy [Fri, 26 Aug 2016 06:01:07 +0000 (08:01 +0200)]
Bug 15758: [QA Follow-up] Remove onlymine from Plugin/Branches.pm

Hard to miss that one:
Template process failed: plugin error - Bareword "C4::Branch::onlymine" not allowed while "strict subs" in use at Koha/Template/Plugin/Branches.pm line 59.
Compilation failed in require at /usr/lib/perl5/Template/Plugins.pm line 206.

Removed OnlyMine from Plugin/Branches. Replaced by a template var in
cleanborrowers; it was used only once.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15758: Koha::Libraries - Do not select an option if selected is defined
Jonathan Druart [Mon, 8 Feb 2016 17:17:31 +0000 (17:17 +0000)]
Bug 15758: Koha::Libraries - Do not select an option if selected is defined

This will certainly need a few more tweaks.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15758: Koha::Libraries - Add tests for the Branches TT plugin
Jonathan Druart [Mon, 8 Feb 2016 15:20:18 +0000 (15:20 +0000)]
Bug 15758: Koha::Libraries - Add tests for the Branches TT plugin

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15758: Koha::Libraries - Ultimate duel for C4::Branch
Jonathan Druart [Fri, 5 Feb 2016 12:34:08 +0000 (12:34 +0000)]
Bug 15758: Koha::Libraries - Ultimate duel for C4::Branch

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15758: Koha::Libraries - Move onlymine to C4::Context::only_my_library
Jonathan Druart [Fri, 5 Feb 2016 12:30:29 +0000 (12:30 +0000)]
Bug 15758: Koha::Libraries - Move onlymine to C4::Context::only_my_library

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15758: Koha::Libraries - Remove GetBranches
Jonathan Druart [Thu, 4 Feb 2016 15:24:23 +0000 (15:24 +0000)]
Bug 15758: Koha::Libraries - Remove GetBranches

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15758: Koha::Libraries - Move mybranch to C4::Context
Jonathan Druart [Thu, 4 Feb 2016 15:24:12 +0000 (15:24 +0000)]
Bug 15758: Koha::Libraries - Move mybranch to C4::Context

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15758: Koha::Libraries - Remove GetBranchesLoop
Jonathan Druart [Wed, 3 Feb 2016 15:58:08 +0000 (15:58 +0000)]
Bug 15758: Koha::Libraries - Remove GetBranchesLoop

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15758: Koha::Libraries - Remove GetBranchName
Jonathan Druart [Tue, 2 Feb 2016 17:28:30 +0000 (17:28 +0000)]
Bug 15758: Koha::Libraries - Remove GetBranchName

This is the fourth and last patch set to remove C4::Branch.
The real purpose of this patch is to standardise and refactor some code
which is related to the libraries selection/display.
Its unconfessed purpose is to remove the C4::Branch package.

Before this patch set, only 6 subroutines still existed in the C4::Branch
package:
- GetBranchName
- GetBranchesLoop
- mybranch
- onlymine
- GetBranches
- GetBranch

GetBranchName basically returns the branchname for a given branchcode.
The branchname is only used for a display purpose and we don't need to
retrieve it in package or pl scripts (unless for a few exceptions).
We have a `Branches` template plugin with a `GetName` method which does
exactly this job.
To achieve this removal, we will use this template plugin and delete the
GetBranchName from pl and pm files.
The `Branches.all()` will now select the library of the logged in user
if no `selected` parameter has been passed.
This new behavior could cause regressions, for instance there are some
places where we do not want an option preselected (batch item
modification for instance), keep that in mind when testing.

GetBranchesLoop took 3 parameters: $branch and $onlymine.
The first one was used to set a "selected" flag, for a display purpose:
select an option in the libraries dropdown lists.
The second one was useless: If not passed or set to 0, the
`C4::Branch::onlymine` subroutine was called.
This onlymine flag was use to know if the logged in user was able to see
other libraries infos.
A patron can see the infos from other libraries if IndependentBranches
is not set OR if he has the superlibrarian permission.
Prior to this patch set, the "onlymine test" was done on different
places (neworderempty.pl, additem.pl, holidays.pl, etc.), including the
Branches TT plugin. In this patch set, this test is only done on one
place (C4::Context::only_my_library, code moved from
C4::Branch::onlymine).
To accomplish the same job as this subroutine, we just need to call the
`Branches.all()` method from the `Branches` TT plugin. It already
accepts a `selected` parameter to set a flag on the option to select.
To avoid the repetitive
  [% IF selected %]<option selected="selected">[% ELSE %]<option>[% END %]
pattern, a new `html_helpers` TT include file has been created, it
defines an `options_for_libraries` block, which takes a `selected`
parameter. We could imagine to use this include file for other
selects.

The 'mybranch` and `onlymine` subroutines of the C4::Branch package have
been moved to C4::Context. onlymine has been renamed with
only_my_library. There are only 4 occurrences of it, against 11 before
this patch set.
There 2 subroutines are Context-centric and it makes sense to put them
in `C4::Context` (at least it's the least worst place!)

GetBranches is the tricky part of this patch set: It retrieves all the
libraries, independently of the value of IndependentBranches.
To keep the same way as the existing calls of `Branches.all()`, I have
added a `unfiltered` parameter. If set, the `Branches.all()` will call
a usual Koha::Libraries->search method, otherwise
Koha::Libraries->search_filtered will be called. This new method will
check if the logged in user is allowed to see other libraries or only
its library.
Note that this `GetBranches` subroutine also created a `category` key:
it allowed to get the list of groups (of libraries) where this library
existed. Thanks to a previous patch set (bug 15295), this value was
not used anymore (I may have missed something!).

Note that the only use of `GetBranch` was buggy (see bug 15746).

Test plan (for the whole patch set):
The best way to test this whole patch set is to test with 2 instances: 1
with the patch set applied, 1 using master, to be sure there is no
regression.
It would be good to test the same with `IndependentBranches` and the
without `IndependentBranches`.
No difference should be found.
The tester must focus on the library dropdowns on as many forms as
possible.
You will notice changes in the order of the options: the libraries will
now be ordered by branchname (instead of branchcode in some places).
A special attention will be given to the following page:
- acqui/neworderempty.pl
- catalogue/search.pl
- members/members-home.pl (header?)
- opac/opac-topissues.pl
- tools/holidays.pl
- admin/branch_transfer_limits.pl
- admin/item_circulation_alerts.pl
- rotating_collections/transferCollection.pl
- suggestion/suggestion.pl
- tools/export.pl

Notes for QA:
- There are 2 FIXMEs in the patch set, I have kept the existing behavior,
but I am not sure it's the good one. Feel free to open a bug report and
I will fill a patch if you think it's not correct. Otherwise, remove the
FIXME lines in a follow-up patch.
- The whole patch set is huge and makes a lot of changes.
But it finally will tremendously reduce the number of lines:
716 insertions for 1910 deletions

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15407: Uniformise calls in patron-attr-types
Jonathan Druart [Fri, 12 Aug 2016 11:56:22 +0000 (12:56 +0100)]
Bug 15407: Uniformise calls in patron-attr-types

Does not make sense to call first in list context and later in scalar
context.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15407: [QA Follow-up] Confusing cathash removed
Marcel de Rooy [Fri, 12 Aug 2016 11:26:46 +0000 (13:26 +0200)]
Bug 15407: [QA Follow-up] Confusing cathash removed

One of the patches removed %cathash. Some kind of silly remainder
was left in a string. Just used as a text field to explain the
filters used on borrowers_stats.pl. The filter name cathash is not
really easy and also was confusing in relation to the variable.

So just a string patch, replacing it with Patron category.

NOTE: The output of borrowers_stats.pl with the filters listed, is
not one of the most beautiful Koha screens.
Furthermore, translatibility of these filters is still an issue to
be handled on another report.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15407: (follow-up) Koha::Patron::Categories - replace C4::Category->al
Jonathan Druart [Fri, 12 Aug 2016 10:05:46 +0000 (11:05 +0100)]
Bug 15407: (follow-up) Koha::Patron::Categories - replace C4::Category->al

Categories.GetName should not have been removed in the previous patch
(merge conflict issue).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15407: [QA Follow-up] Joubu, add your tests
Marcel de Rooy [Fri, 12 Aug 2016 09:20:11 +0000 (11:20 +0200)]
Bug 15407: [QA Follow-up] Joubu, add your tests

Trivial fix (yes again).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15407: [QA Follow-up] Fix POD in reserves_stats.pl
Marcel de Rooy [Fri, 12 Aug 2016 08:49:32 +0000 (10:49 +0200)]
Bug 15407: [QA Follow-up] Fix POD in reserves_stats.pl

Trivial fix.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15407: Koha::Patron::Categories - replace GetborCatFromCatType
Jonathan Druart [Tue, 22 Dec 2015 10:12:48 +0000 (10:12 +0000)]
Bug 15407: Koha::Patron::Categories - replace GetborCatFromCatType

This unnecessary complicated subroutine returned an arrayref and an
hashref of the patron categories available for the logged in user, for a given
category_type, ordered by categorycode.
This can now be done with the search_limited method.

Test plan:
- Same prerequisite as before
For the following pages, you should not see patron categories limited to other
libraries. They should be ordered as before this patch, by categorycode.
- Add/edit a patron, change his/her patron category value.
- On the 3 following reports:
    reports/bor_issues_top.pl
    reports/borrowers_out.pl
    reports/cat_issues_top.pl
The display for these 3 reports are different than the 2 from the first
patch (borrowers_stats.pl issues_avg_stats.pl): they are ordered by
categorycode and the ones limited to other libraries are not displayed
(should certainly be fixed).

Note that the big part of this patch has already been tested before
(update child related: CATCODE_MULTI).

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15407: Koha::Patron::Categories - replace GetBorrowercategoryList
Jonathan Druart [Mon, 21 Dec 2015 17:27:08 +0000 (17:27 +0000)]
Bug 15407: Koha::Patron::Categories - replace GetBorrowercategoryList

C4::Members::GetBorrowercategoryList returned all the patron categories,
ordered by description.
This can be done using the search method of the Koha::Patron::Categories
module.

Test plan:
- Same prerequisite as before
For the following pages, you should not see patron categories limited to other
libraries. They should be ordered as before this patch, by description.
- On the "patron attribute types" admin page (admin/patron-attr-types.pl).
- On the "Batch patron deletion/anonymization"
  (tools/cleanborrowers.pl), the dropdown list should be filled
  correctly.
- On the import patron tool
- On the following report:
    reports/reserves_stats.pl
Confirm that the patron categories are displayed and that the report
still works as before.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15407: Koha::Patron::Categories - replace GetBorrowercategory
Jonathan Druart [Mon, 21 Dec 2015 17:13:12 +0000 (17:13 +0000)]
Bug 15407: Koha::Patron::Categories - replace GetBorrowercategory

The purpose of C4::Members::GetBorrowercategory was to return the patron
category for a given categorycode. This can be done easily with the
Koha::Patron::Categories->find method.

Test plan:
- Same prerequisite as before
- Edit a guarantor and confirm the information will be updated for his/her
  guarantee(s).
- Update a child to adult patron (Note: Should not we hide the patron
  categories limited to others libraries? If yes, it must be done on another
  bug report).
  There is a special behavior if there is only 1 adult category, the
  user does not need to select a category. So the same as before with
  only 1 adult patron category.
- Import a patron with a category_code which does not exist. You should
  be warned.
- Modify some patron using the batch patron modification tool. The
  patron category descriptions should correctly be displayed.

Note that the overduerules page has already been tested in a previous
patch.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15407: Koha::Patron::Categories - replace C4::Category->all
Jonathan Druart [Mon, 21 Dec 2015 17:04:30 +0000 (17:04 +0000)]
Bug 15407: Koha::Patron::Categories - replace C4::Category->all

The C4::Category module contained only 1 method to return the patron
categories available for the logged in user.
The new method Koha::Patron::Categories->search_limited does exactly the
same thing (see tests) and must be used in place of it.

Test plan:
- Same prerequisite as before
For the following pages, you should not see patron categories limited to
other libraries.
- On the 'Item circulation alerts' admin page
  (admin/item_circulation_alerts.pl), modify the settings for check-in
  and checkout (NOTE: Should not we display all patron categories on
  this page? If yes, it must be done in another bug report to ease
  backporting it).
- Search for patrons in the admin (budget) and acquisition (order) module.
- On the patron home page (search form in the header)

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 15407: Koha::Patron::Categories - remove sql queries in some pl and pm
Jonathan Druart [Mon, 21 Dec 2015 15:04:45 +0000 (15:04 +0000)]
Bug 15407: Koha::Patron::Categories - remove sql queries in some pl and pm

This patch replaces sql queries done in some pl script and in
C4::Reports::Guided.
Since we have now a Koha::Patron::Categories module, we should use it
where it is possible.

Test plan:
- Prerequisite: Be sure you have several patron categories created, with
  different option enabled, and limit some to certain libraries.
- On the 'Circulation and fine rules' admin page (admin/smart-rules.pl),
  all the patron categories should be displayed (even the ones limited to
  another library), ordered by description. Try to add/update existing rules.
- On the overdue rules page (tools/overduerules.pl), all the patron
  categories with overduenoticerequired set should be displayed.
  Try to add/update existing rules.
- On the following reports:
    reports/borrowers_stats.pl
    reports/issues_avg_stats.pl
The patron categories should be displayed. Note that there is an
inconsistency with these 2 reports: the patron categories limited to
other libraries are displayed on them, when they are not on the other
reports. This should certainly be fixed (on another bug report).

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 14695 - DBRev 16.06.00.018
Kyle M Hall [Thu, 8 Sep 2016 12:47:25 +0000 (12:47 +0000)]
Bug 14695 - DBRev 16.06.00.018

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 14695 [QA Followup]
Kyle M Hall [Thu, 8 Sep 2016 09:45:27 +0000 (09:45 +0000)]
Bug 14695 [QA Followup]

* Add tests for CanItemBeReserved returns 'itemAlreadyOnHold' to t/db_dependent/Holds.t
* Don't let GetReserveId explode
* Use search instead of map
* Remove instances of resbarcode
* Fix badly formed li closing tag

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
8 years agoBug 14695: Fix GetHardDueDate.t tests
Jonathan Druart [Wed, 7 Sep 2016 13:20:00 +0000 (14:20 +0100)]
Bug 14695: Fix GetHardDueDate.t tests

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 17189 [QA Followup] - Missing $ on $key var for unsafe variant
Kyle M Hall [Thu, 8 Sep 2016 12:19:30 +0000 (12:19 +0000)]
Bug 17189 [QA Followup] - Missing $ on $key var for unsafe variant

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 16035: MARC default bibliographic framework Export fix
Aleisha Amohia [Wed, 24 Aug 2016 21:24:45 +0000 (21:24 +0000)]
Bug 16035: MARC default bibliographic framework Export fix

This patch makes sure that, if there is no framework code,
     $frameworkcode is set to 'default'. This fixes the format issue.
     Then, if $frameworkcode = default, an empty string (rather than
     null) is passed to the ExportFramework method, as the framework
     code in the DB for default codes is "".

To test:
1) Go to Admin -> MARC Bibliographic framework
2) Try to export default framework in all formats, notice the file is
only exported as .ods and is an empty file. Notice warns in intranet
error log.
3) Apply patch and refresh page
4) Confirm exporting default framework works in all formats and
spreadsheet has data
5) Notice warns in error log are gone
6) Confirm export still works for other frameworks

Sponsored-by: Catalyst IT
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 17231 - (QA Followup)
Nick Clemens [Wed, 7 Sep 2016 13:03:43 +0000 (13:03 +0000)]
Bug 17231 - (QA Followup)

Nicer grep

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 17231 - HTML5MediaYouTube should recognize youtu.be links from youtube as well...
Nick Clemens [Wed, 31 Aug 2016 17:37:56 +0000 (17:37 +0000)]
Bug 17231 - HTML5MediaYouTube should recognize youtu.be links from youtube as well at the full links

To test:
Enable HTML5MediaYouTube and ensure WWW::YouTube::Download is installed
Add an 856$u to a record like: https://www.youtube.com/watch?v=tu0qtEwb9gE
Verify you can see the embedded player
Use the youtube shortened like (from the share button) like:
https://youtu.be/tu0qtEwb9gE
Verify you can see the embedded player

Signed-off-by: Claire Gravely <claire_gravely@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 14060: Improve regex to limit year to 4 integers
Jonathan Druart [Tue, 6 Sep 2016 11:58:31 +0000 (12:58 +0100)]
Bug 14060: Improve regex to limit year to 4 integers

01/01/01984 should not be considered as a valid date.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 14060: Consider an empty string as a valid date
Jonathan Druart [Mon, 5 Sep 2016 08:16:02 +0000 (09:16 +0100)]
Bug 14060: Consider an empty string as a valid date

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 14060: Add the datepicker class to dateofbirth
Jonathan Druart [Tue, 30 Aug 2016 08:56:46 +0000 (09:56 +0100)]
Bug 14060: Add the datepicker class to dateofbirth

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 14060: Force leading zeros on date
Jonathan Druart [Tue, 30 Aug 2016 08:54:55 +0000 (09:54 +0100)]
Bug 14060: Force leading zeros on date

If a date does not contain leading zeros for day and month, the date
will be refused.
parseDate function considers 1/1/1990 as a valid date, but our perl code
does not. We must refuse it.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 14060: Display date format pattern instead of date format name
Jonathan Druart [Fri, 26 Aug 2016 07:44:06 +0000 (08:44 +0100)]
Bug 14060: Display date format pattern instead of date format name

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 14060: Add support for dmydot
Jonathan Druart [Mon, 4 Jan 2016 11:03:43 +0000 (11:03 +0000)]
Bug 14060: Add support for dmydot

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 14060: Force the input to contain a valid date
Jonathan Druart [Tue, 1 Dec 2015 17:33:15 +0000 (17:33 +0000)]
Bug 14060: Force the input to contain a valid date

Before this patch, the user was still allowed to enter a valid date.
Now, if the date is not valid, the input is emptied.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 14060: Display the date format on the warning popup
Jonathan Druart [Mon, 27 Apr 2015 08:42:51 +0000 (10:42 +0200)]
Bug 14060: Display the date format on the warning popup

The date format is now displayed on the warning popup.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 14060: Add OnClose event on missing datepicker
Jonathan Druart [Mon, 27 Apr 2015 08:39:14 +0000 (10:39 +0200)]
Bug 14060: Add OnClose event on missing datepicker

The "suspend until" inputs (hold list on circ/circulation.pl and
members/moremember.pl) were not set in the same way.
The minDate option should be set for both (bug 10703 removed it for
circ/circulation.pl).
This patch reintroduces the option and add the OnClose event on both.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 14060: Remove readonly attributes on date inputs
Jonathan Druart [Fri, 24 Apr 2015 13:36:34 +0000 (15:36 +0200)]
Bug 14060: Remove readonly attributes on date inputs

In order to remove accessibility issues due to the readonly attributes
on date inputs, this patch will remove them and introduce a javascript
validation on them.

This patch is not perfect for some reason:
I didn't manage to force the user to select a valid date. One solution
would be to reopen the datepicker plugin until a valid date is inserted.
But it could be annoying for users (and for me: I did not manage to
implement this solution).
You will note that input is emptied if the date is not valid. This is a
quick and efficient solution to prevent submitting invalid date and make
Koha explodes. A proper solution would be to implement the check server
side send a friendly message to the user.

Test plan:
For all inputs, try an invalid and a valid date.
 1/ Debar a patron
 2/ On the checkout tables (circulation and moremember), add a renewal
due date (at the bottom of the tables)
 3/ On the checkout page, specify a due date
 4/ On the return page, specify a return date
 5/ On the invoice page (acquisition module), enter a shipment and
billing date
 6/ On the invoice search page (invoices.pl) use filters shipment and
billing dates
 7/ On the offline circ page, specify a due date
 8/ On the edit patron page (memberentry), add a debarment
 9/ On the reserve page (reserve/request.pl), use the date inputs to
suspend until a defined date
10/ Edit patrons in a batch (tools/modborrowers.pl) and use the
registration and expiry date inputs

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 17152: Do not copy value when duplicating a subfield
Hector Castro [Wed, 31 Aug 2016 19:33:53 +0000 (13:33 -0600)]
Bug 17152: Do not copy value when duplicating a subfield

When cataloguing, if you want to duplicate a subfield that is not
empty, the new subfield is created with a copy of data in it.
This is not the case when you duplicate an whole field. The new one is
created with subfields but without data in it.

Test plan:
Add or edit a bibliographic record
Fill a subfield
Duplicate the subfield
=> Without this patch the value of the input will be copied
=> With this patch the new input will be emptied

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 17072: 006 not filling in with existing values
Jacek Ablewicz [Thu, 1 Sep 2016 16:13:50 +0000 (18:13 +0200)]
Bug 17072: 006 not filling in with existing values

When you use the 006 builder to view the existing values in the 006
tag, Koha is not pulling these existing values into the builder
window. Instead, Koha appears to be using defaults.

To reproduce:

1. Open a record in the editor
2. Switch from BKS to another material type.
3. Change as many settings as possible.
4. Close plugin.
5. Reopen plugin - notice your selections are not shown.

Trivial patch, heavilly inspired by Bug 9093.

To test:

1. Apply patch
2. Ensure that issue described above is no longer reproductible

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Works as advertised

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 11592: (QA followup) Add missing framework code to ViewPolicy filter calls
Tomas Cohen Arazi [Tue, 30 Aug 2016 15:44:14 +0000 (12:44 -0300)]
Bug 11592: (QA followup) Add missing framework code to ViewPolicy filter calls

This patch adds the frameworkcode option param, using each record's frameworkcode
as expected by the filter. Otherwise the ViewPolicy filter falls back to the
default framework.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 11592: (QA followup) Simplify code
Tomas Cohen Arazi [Mon, 22 Aug 2016 14:00:41 +0000 (11:00 -0300)]
Bug 11592: (QA followup) Simplify code

Koha::RecordProcessor and the defined filters are supposed to bring us
joy and happiness. Let's keep the code compact, simple and clean.

This patch removes record cloning all over the place.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
8 years agoBug 11592: MARCView and ISBD followup
Mark Tompsett [Thu, 21 Apr 2016 01:49:41 +0000 (21:49 -0400)]
Bug 11592: MARCView and ISBD followup

There are still some leaks, but it is not as a result
of the filter, but rather a result of poorly written
template files.

Bug fixing template files is beyond the scope of this
set of patches.

TEST PLAN
---------
 1) Backup your DB
 2) run the following SQL on your DB.
    > UPDATE marc_subfield_structure set hidden=-8;
    -- this should set EVERYTHING to hidden across the board.
 3) In staff client, set OPACXSLTDetailsDisplay to blank
 4) In OPAC, view any detail.
    -- Normal view may mostly leak values still.
    -- MARC view may leak values.
    -- ISBD view may leak values.
 5) In staff client, set OPACXSLTDetailsDisplay to default
 6) In OPAC, view any detail.
    -- same issues as step 4
    -- 'View Plain' may leak too.
 7) 'Save record' -> 'Dublin Core'
 8) Apply this patch
 9) run koha qa test tools
    -- should be fine
10) prove -v t/db_dependent/Filter_MARC_ViewPolicy.t
    -- should pass
    -- this proves Koha/Filter/MARC/ViewPolicy.pm tweaks too
11) In OPAC, view any detail.
    -- Normal view:
       -- Material type comes from the LEADER field.
       -- Lists this is on will still display
       -- 'Tags from this library' will still display
       -- Item information in table will still display
          (THIS IS BEYOND SCOPE)
    -- MARC view:
       -- Record number is leaked
          (THIS IS BEYOND SCOPE)
       -- 'View plain' leaks LEADER field.
    -- ISBD view may leak field headings, but not values.
       (THIS IS BEYOND SCOPE)
12) In staff client, set OPACXSLTDetailsDisplay to blank
13) In OPAC, view any detail.
    -- same kind of output as step 10
14) 'Save record' -> BIBTEXT
    -- Should be next to nothing leaked.
15) 'Save record' -> Dublin Core
    -- Should be the same or less leaked between the two versions.
    -- (XML FILTERING IS BEYOND SCOPE)
16) In the staff client, go view the same record.
    -- it should be mostly hidden in ISBD View.
17) run the following SQL on your DB.
    > UPDATE marc_subfield_structure set hidden=1;
    -- this should set EVERYTHING to hidden in OPAC, but not
       the STAFF across the board.
18) Refresh the staff ISBD page
    -- values should reappear.
19) View the ISBD details in the OPAC
    -- values should still be hidden.
20) Check out the OPAC Cart and List
    -- while the intermediate pages may still leak
       the download links should leak very minimally.
    -- (CARTS AND LISTS ARE BEYOND SCOPE, THOUGH
        THE INTRANET ISBD AND SOME CART/LIST STUFF
        WERE FIXED BECAUSE OF THE GetISBDView REFACTOR)

Expectations:
Before Patch - all the OPAC Detail pages will display things
After Patch - all the OPAC Detail pages will display much less,
              and hopefully nothing (though there are known limits).
              the ISBD detail page in the Staff client will be
              filtered as well based on STAFF settings.
              The saving/exporting should generate nearly empty
              files.

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