Commit graph

5741 commits

Author SHA1 Message Date
3cfedcf238 Bug 16011: $VERSION - Remove empty BEGIN block
perl -p -i -0 -e 's/BEGIN \{\n?\n?\}\n//' **/*.pm

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-24 17:20:41 +00:00
4c0e309677 Bug 16011: $VERSION - Remove use vars qw();
perl -p -i -e 's/use vars qw\(\s*\);\n//' **/*.pm

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-24 17:20:39 +00:00
798d38e4c7 Bug 16011: $VERSION - Remove comments
perl -p -i -e 's/^.*set the version for version checking.*\n//' **/*.pm

+ manual adjustements

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-24 17:20:29 +00:00
017699c345 Bug 16011: $VERSION - Remove the $VERSION init
Mainly a
  perl -p -i -e 's/^.*3.07.00.049.*\n//' **/*.pm
Then some adjustements

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-24 17:20:28 +00:00
3830d78d46 Bug 16011: $VERSION - remove use vars $VERSION
perl -p -i -e 's/^(use vars .*)\$VERSION\s?(.*)/$1$2/' **/*.pm

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-24 17:20:26 +00:00
92fbb1f3d0 Bug 15741: Fix rounding in total fines calculations
C4::Members::GetMemberAccountRecords wrongly casts float to integer
It's common to use sprintf in Perl to do this job.

% perl -e 'print int(1000*64.60)."\n"';
64599
% perl -e 'print sprintf("%.0f", 1000*64.60)."\n"';
64600

Test plan:
1) Create manual invoice for 64.60 (or 1.14, 1.36, ...)
2) Try to pay it using "Pay amount" or "Pay selected" buttons

Signed-off-by: Sally Healey <sally.healey@cheshiresharedservices.gov.uk>

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

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-24 16:11:09 +00:00
94dde6b48d Bug 15809: Redefine multi_param is CGI < 4.08 is used
On debian Jessie, the CGI version is >= 4.08
Since this version, the param method raise a warning
"CGI::param called in list context".
Indeed, it can cause vulnerability if called in list context

https://metacpan.org/pod/CGI#Fetching-the-value-or-values-of-a-single-named-parameter
http://blog.gerv.net/2014/10/new-class-of-vulnerability-in-perl-web-applications/

There is a long journey to get rid of these warnings.
First I suggest to redefine the multi_param method when the CGI version
 installed is < 4.08, it will allow us to move the wrong ->param calls to
 ->multi_param without waiting for everybody to upgrade.

The different ways to call these 2 methods are:

my $foo = $cgi->param('foo'); # OK

my @foo = $cgi->param('foo'); # NOK, will raise the warning
my @foo = $cgi->multi_param('foo'); #OK

$template->param( foo => $cgi->param('foo') ); # NOK, will raise the warning
                                               # and vulnerable
$template->param( foo => scalar $cgi->param('foo') ); # OK

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested a call to multi_param with CGI < 4.08.
With reference to the comments on Bugzilla, this workaround is arguable,
but provides a base to move to multi_param. If we come up with a better
solution, it should be easy to adjust.

Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
2016-03-22 23:23:39 +00:00
charles
db0ecc3cc5 Bug 15585 - Move C4::Passwordrecovery to the new namespace Koha::Patron::Password::Reset
As promised, here is the long-awaited sequel to #8753.

What has changed :

    - The Koha::Patron::Password::Reset is now used in place of C4::Passwordrecovery
    - That ugly shift-grep contraption is no more (goodbye old friend)
    - The generated unique key won't end in a dot anymore

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

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

Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
2016-03-22 23:08:21 +00:00
4a3404594f Bug 15163: Do not erase patron attributes if limited to another library
The patron attributes displayed on editing a patron are not displayed if
limited to another library.

C4::Members::Attributes::SetBorrowerAttributes will now only delete attributes
the librarian is editing.
SetBorrowerAttributes takes a new $no_branch_limit parameter. If set,
the branch limitations have not effect and all attributes are deleted
(same behavior as before this patch).

Test plan:
1/ Create 2 patron attributes, without branch limitations.
2/ Edit a patron and set a value for these attributes
3/ Limit a patron attributes to a library (one you are not logged in
with).
4/ Edit again the patron.
=> You should not see the limited attributes
5/ Edit the patron attributes and remove the branch limitation
=> Without this patch, it has been removed from the database and is not
displayed anymore.
=> With this patch, you should see it.

Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>

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

Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
2016-03-21 16:56:37 +00:00
47fbbb7cf5 Bug 11998: Add a L1 cache for sysprefs
Accessing to the cache for each call to C4::Context->preference might
have an impact on performances.
To avoid that this patch introduces a L1 cache (simple hashref). It will
be populated by accessing the L2 cache (Koha::Cache).
If a pref is retrieved 10x, the first one will get the value from the L2
cache, then the L1 cache will be check.
To do so we will need to clear the L1 cache every time a page is loaded.

Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>

Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
2016-03-15 07:08:29 +00:00
56a782c666 Bug 11998: Use t::lib::Mocks::mock_preference in tests
Unless in t/db_dependent/Context.t where we want to test the cache
behaviors.

Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>

Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
2016-03-15 07:08:28 +00:00
9820f9dfbd Bug 11998: Use Koha::Cache to cache sysprefs
At the moment, the sysprefs are only cache in the thread memory
executing the processus
When using Plack, that means we need to clear the syspref cache on each
page.
To avoid that, we can use Koha::Cache to cache the sysprefs correctly.

A big part of the authorship of this patch goes to Robin Sheat.

Test plan:
1/ Add/Update/Delete local use prefs
2/ Update pref values and confirm that the changes are correctly taken
into account

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Tested with plack with syspref cache enabled, there is some time between setting the syspref and applying it, but it takes just one reload of page, it shouldn't be problem, should it?
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>
Tested with CGI and CGI + memcache; some small issues still remain,
but it would be better to deal with them in separate bug reports
if necessary

Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
2016-03-15 07:08:28 +00:00
fb167c0e4b Bug 15111: Change X-Frame-Options with SAMEORIGIN
There are some places where frames are used, the greybox JS plugin for
instance.

We need either to allow them from Koha or replace this plugin.
The easier for now is to switch the value from DENY with SAMEORIGIN.

Test plan:
- modify a record in a batch (tools/batch_record_modification.pl)
- click on preview marc
=> With only the previous patch you will get a blank page.
=> With this patch apply, it will work as expected.

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-03-14 16:30:08 +00:00
dc03bca76c Bug 15111 - Koha is vulnerable to Cross-Frame Scripting (XFS) attacks
Web pages that can be embedded in frames are vulnerable to cross-frame
scripting attacks. Cross-frame scripting is a type of phishing attack
that involves instructions to an unsuspecting user to follow a specific
link to update confidential information in an online application.
Because the link leads to a legitimate page from the online application
that is embedded in a frame hosted by the attackers' server, the
attackers can capture all the information that the user enters.

https://www.owasp.org/index.php/Clickjacking_Defense_Cheat_Sheet
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-03-14 16:30:08 +00:00
aa73c96aed Bug 15656: Move guarantor/guarantees code - GetMemberRelatives
Note:
QA question: Does the Koha::Patron->siblings method should return undef
if there is no guarantor?
It would avoid the weird  != undef, = $borrowernumber conditions.

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

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

Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
2016-03-12 23:40:10 +00:00
bff0ca0b0b Bug 15656: Move guarantor/guarantees code - GetGuarantees
2 subroutines of C4::Members deal with guarantor/guarantees:
GetGuarantees and GetMemberRelatives.
Since we already have a Koha::Patron->guarantor method, it makes sense
to move these 2 subroutines to this module.

This first patch deals with GetGuarantees.

Test plan for the entire patch set:
1/ Create 5 patrons A (adult), B (child), C (child), D (child), E
(child), F (adult)
2/ Add relation between them: A is father of B, C and D.
E does not have a guarantor
F does not have guarantees
3/ Check some items out for all of these patrons
4/ On the "Check out" and "Details" tabs, you should not see any
differences with these patch applied : The "Relatives' checkouts" tabs
should list all of the guarantor/guarantee/siblings checkouts

Note:
$template->param('C' => 1);
I have not found any reference of this 'C' in the template.
It seems it's an old c/p from members/memberentrygen.tt

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

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

Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
2016-03-12 23:40:10 +00:00
186f630407 Bug 15653: Remove unused C4::Members::UpdateGuarantees subroutine
Looking at the code, there is some broken with the guarantees code.
It seems that the expected behavior would be to update address, fax,
B_city, mobile, city and phone info of the guarantees when a guarantor
is modified.
But the code in C4::Members::ModMember is broken:

 668         my $borrowercategory= GetBorrowercategory(
$data{'category_type'} );
 669         if ( exists  $borrowercategory->{'category_type'} &&
$borrowercategory->{'category_type'} eq ('A' || 'S') ) {
 670             # is adult check guarantees;
 671             UpdateGuarantees(%data);
 672         }

First, GetBorrowerCategory expects a categorycode, not a category_type.
Then UpdateGuarantees retrieves the param like:

 989 sub UpdateGuarantees {
 990     my %data = shift;

Which means that %data will always be something like ( a_key => undef )
And nothing more.

The updateguarantees subroutine (It has been renamed) has been introduced by

commit 56825e415f
Date:   Mon Aug 30 13:48:58 2004 +0000
    modularizing (with Members.pm) members management
    (beginning of...)

And the `%data = shift` already existed...

This code has never worked and could be removed.

See http://lists.koha-community.org/pipermail/koha-devel/2016-January/042241.html

Test plan:
Confirm the previous assertions.

Note that I have found this bug working on bug 15631, see patch "Bug
15631: Koha::Cities - remove getidcity and GetCities"

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

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

Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
2016-03-12 23:39:09 +00:00
Nicole C Engard
39f9b9ddb0 Bug 15694: Add aliases for date/time last modified
This patch will add indexes for Date/time-last-modified.

To test:

1. apply patch
2. reindex
3. search for dtlm:DATE and date-time-last-modified:DATE
4. confirm that you get results

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

Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
  I confirm Hector signing-off. A simple Zebra server restart suffice to get
  working the searches on date-time-last-modified and dtlm.

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

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-11 21:56:50 +00:00
Zeno Tajoli
69cf2d9451 Bug 15955: Tuning function 'New child record' for Unimarc 205$a -> 461$e
Now the sYstem tries to insert value of 205$a into 461$a when a child is
created from the father record.  In UNIMARC 46x tags there is not
present a subfield for ediction value (205$a in UNIMARC).

To Test:
1) Check to have EasyAnalyticalRecords on 'off'
2) Check to use UNIMARC
3) Create a record with data in 200$a (title), 205$a (ediction), 700
   (author) 215$a(Place), 215$d(date)
4) From those record create a child using 'New'->'New child record'
5) See the values in 461 tag: You can see that in 461$a there is the
   value of 205$a from father This is wrong, you need to have the value
   of 700 $a and $b from father record, and 205$a in 461$e.
6) Appy the patch
7) Redo 4-5
8) Now 461 is good

Signed-off-by: Frédéric Demians <f.demians@tamil.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
I have not checked the doc but trusting author and signoffer.

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-11 21:55:24 +00:00
0391709c28 Bug 16009: fix GetMember() search on NULL/undef values
This patch fixes a bug whereby GetMember(searchfield => undef)
(i.e., searching for patron records where 'searchfield' is NULL)
would crash.

This fixes a regression introduced by bug 15344 that in turn
exposed a long-standing bug in GetMember().

To test:

[1] Import some offline circulation transactions that include at
    least one return.
[2] Attempt to view the list of pending transactions; a crash
    will occur.
[3] Apply the patch and view the list of pending transactions again;
    this time, it should work.

Signed-off-by: Galen Charlton <gmcharlt@gmail.com>
Signed-off-by: Srdjan <srdjan@catalyst.net.nz>

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

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-11 15:23:37 +00:00
ffbb575061 Bug 14306: Follow-up for URLs in 555$u
This patch removes the code for inserting the <a> anchor tags around
URLs in GetMarcNotes (as added originally).
The URLs are placed in separate array elements; the template should take
care of further handling.
The unit test has been adjusted accordingly.

Test plan:
Run the unit test.

Signed-off-by: Marc Véron <veron@veron.ch>

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

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-07 17:58:33 +00:00
caae161a4e Bug 14306: Show URL from MARC21 field 555$u under Title Notes/Descriptions
This patch includes:
[1] Add some logic to GetMarcNotes to embed the contents of MARC21 field
    555$u in a html anchor tag.
[2] Add a unit test for GetMarcNotes in Biblio.t
[3] Remove calls to GetMarcNotes from sendbasket.pl (opac and staff).
    A closer look revealed that the data was not used; the notes in the
    mail of sendbasket are taken from GetBiblioData.

Test plan:
[1] Edit a record. Add one or two URLS in 555$u. Add something in 500$a too.
[2] Check if you can click the URLs in opac and staff detail tab Notes or
    Descriptions.
[3] Run the unit test t/db../Biblio.t
[4] Add something in the cart. Click More Details and send the cart.
    Verify that you have something in Notes (from 500$a).

Signed-off-by: Marc Veron <veron@veron.ch>
Followed test plan. Works as expected. QA tools OK.

Tested with all patches together, works as expected
Signed-off-by: Marc Véron <veron@veron.ch>

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

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-07 17:58:32 +00:00
f2dd831542 Bug 14694 - Make decreaseloanHighHolds more flexible
This patch allows for more flexibility for determining when the number
of holds a record has should trigger the reduction of the loan length
for items on that record.

This patch adds a new system preference decreaseLoanHighHoldsControl,
which defaults to 'static', the original behavior of the feature.
It also has a new behavior 'dynamic' which makes the feature only
decrease the loan length if the number of holds on the record exceeds
the number of holdable items + decreaseLoanHighHoldsValue.

It also allows items to be filtered from the list of items based
on the damaged, lost, not for loan, and withdrawn values even if
those values would have allowed holds ( i.e. values < 0 )

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Set decreaseLoanHighHolds to Enable
4) Set decreaseLoanHighHoldsControl to "over the number of items on the record"
5) Set decreaseLoanHighHoldsDuration to 1
6) Set decreaseLoanHighHoldsValue to 3
7) Create a record with 5 items
8) Please 8 or more holds on the record
9) Check out one of the items to a patron
10) Note the loan length is reduced to 1 day
11) Set decreaseLoanHighHoldsValue to 3 to 2
12) Check out one of the items to a patron
13) Note the loan length is *not* reduced
14) Enbale all the filters possible in decreaseLoanHighHoldsIgnoreStatuses
15) Set one item to be damaged
16) Note the loan length is reduced
17) Unset the damaged status
18) Repeat steps 15 - 17 for lost, not for loan, and withdrawn

Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>

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

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-07 17:48:51 +00:00
1307f26bd1 Bug 5404: Move the test to a new IsMarcStructureInternal sub
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

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

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-07 17:30:09 +00:00
2237e0f871 Bug 5404: C4::Koha - remove subfield_is_koha_internal_p
The commit b5ecefd485
Date:   Mon Feb 3 18:46:00 2003 +0000

had a funny description:
Added function to check if a MARC subfield name is "koha-internal"
(instead of checking it for 'lib' and 'tag' everywhere); temporarily
added to Koha.pm

"Temporarily", since 2003, everything is relative, isn't it? :)

The thing is that GetMarcStructure returns hash like

field_200 => {
    subfield_a => {
        %attributes_of_subfield_a
    },
    %attributes_of_field_200
}

The attributes for field_200 can be 'repeatable', 'mandatory', 'tag', 'lib'.
We don't want to loop on these values when looping on subfields.
Since there are just { k => v } with v is a scalar (string), it's easier
to test if we are processing a subfield testing the reference.

At some places, we don't need to test that, we are looping on values
from MARC::Field->subfields which are always valid subfields.

Test plan:
1/ Edit items using the batch item mod tool
2/ display and edit items via the cataloguing module.

You should not see any changes between before and after the patch
applied.

Tech notes:
We need to check what we are processing when we loop on 'subfields' from
GetMarcStructure, not from MARC::Field->subfields.

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

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

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-07 17:30:09 +00:00
617b72a54e Bug 13871: [QA Follow-up] Add $server for FID_SCREEN_MSG
When you are ready, you still see that small detail.

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

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-07 17:22:21 +00:00
b02aa7c421 Bug 13871: [QA Follow-up] Adjust Patron Info Request
Conform QA comment, Patron Info request is slightly adjusted to be
consistent with changes to Patron Status request.

If the cardnumber is ok and the password is wrong, BL=N is reported but
also add 'Invalid password' in AF.
Additionally, an invalid card number is reported in AF.

Test plan:
[1] Send patron info request for invalid card.
[2] Idem for valid card, no password.
[3] Idem for valid card, good password.
[4] Idem for valid card, wrong password.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Verified by telnetting SIP server.
And tested additionally with the new unit test of bug 15956.

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-07 17:22:21 +00:00
a32a5c4e08 Bug 13871: [QA Follow-up] Adjust Patron Status Request
Conform QA comment on Bugzilla, we do this:

[1] Attribute for overdrive mode/invalid credentials is not really needed.
    We can always pass a screen message that card or password is invalid.
[2] If the cardnumber is correct and the password is wrong, we should
    still honour the request. The bad password is recognized by BLN and
    an additional message in AF.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Verified by telnetting SIP server.
And tested additionally with the new unit test of bug 15956.

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-07 17:22:20 +00:00
8637627dc9 Bug 13871 - OverDrive message when user authentication fails
NOTE: apply this patch after the additional perltidy patch

this patch is basically a small work-around to fix some confusing login text,
when users enter incorrect auth details via Overdrive's website

with this option disabled (default) there is no change to SIP's behaviour

to test...

1/ configure your overdrive account to talk to your Koha's SIP service

2/ start Koha's SIP

3/ enter a correct username and correct password in overdrive
see overdrive display '(1) Greetings from Koha' (good)

4/ enter a correct username and *incorrect* password in overdrive
see overdrive display '(1) Greetings from Koha' (bad)

5/ enter an incorrect username in overdrive
see overdrive display '(1)' (badder)

6/ apply patch, enable 'overdrive-mode' in Koha's SIPConfig.xml

example...
---------------------
<accounts>
<login id="kohasip" password="xxxxx" delimiter="|"
error-detect="enabled" institution="YYY" overdrive-mode="1" />
</accounts>
---------------------

7/ restart SIP

8/ enter a correct username and correct password
see overdrive display '(1) Greetings from Koha'

9/ enter a correct username and *incorrect* password
see overdrive display '(1) Invalid patron or patron password'

10/ enter an incorrect username and incorrect password
see overdrive display '(1) Invalid patron or patron password'

http://bugs.koha-community.org/show_bug.cgi?id=1387

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-07 17:22:20 +00:00
42e731857d Bug 13871: [TITLE_AMENDED] Additional changes
The original perltidy patch from Mason has been amended.

The perltidy itself has been moved to a separate patch with the current
perltidyrc applied.

As noted on Bugzilla, the original perltidy patch included some extra
changes:
[1] You prefix timestamp with Sip
    This is not actually needed (it is imported), but if we should prefix it,
    we should prefix now with C4::SIP::Sip. But you only changed two
    occurrences (out of 26). So I remove these two changes.
[2] You remove the $server parameter from two calls of maybe_add:
    A closer look at the remaining code tells me that $server is always
    passed to maybe_add for FID_SCREEN_MSG. So this only left me the
    current whitespace change.
But at least we documented what we did or did not, and why..

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

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-07 17:22:19 +00:00
a5babb0eff Bug 13871: Adjusted perl tidy on MsgType.pm
Run perltidy pro=xt/perltidyrc on the file.

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

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-07 17:22:19 +00:00
9be221b14e Bug 15968: Unnecessary loop in C4::Templates
From C4::Templates::output

     # add variables set via param to $vars for processing
     for my $k ( keys %{ $self->{VARS} } ) {
         $vars->{$k} = $self->{VARS}->{$k};
     }

This loop is not necessary, we could do the same with

     $vars = { %$vars, %{ $self->{VARS} } };

After a quick benchmark, it gains 100 microseconds when we pass 170 vars
to the template.

Test plan:
Do some clicks on the interface, everything should be ok.

Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
  Perl idiosyncratic way of merging hash, clearer, if not quicker (not
  verified)

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

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-07 17:20:00 +00:00
545b64f869 Bug 15635: Koha::Patron::Images - Remove GetPatronImage
To retrieve a patron image, we can call Koha::Patron::Images->find or
Koha::Patrons->find->image
Both will return a Koha::Patron::Image object.

Test plan:
1/ From the patron/member module, open all tabs on the left (Checkouts,
detail, fines, etc.)
The image should be correctly displayed.
2/ At the OPAC, on the patron details page (opac-memberentry.pl) the
image should be displayed as well.
3/ Same on the sco module.

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-03-04 12:54:15 +00:00
85633155f4 Bug 15635: Koha::Patron::Images - Remove RmPatronImage
The C4::Members::RmPatronImage just removed a row in patronimage.
This can be accomplished using the delete method of Koha::Patron::Image.

Test plan:
From the patron defail page, try to delete the image of a patron.

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-03-04 12:53:01 +00:00
d8980b60ee Bug 15635: Koha::Patron::Images - Remove PutPatronImage
The C4::Members::PutPatronImage inserted/updated the image of a patron.
This can be done easily with ->find->set->store or ->new->store

Test plan:
1/ Modify the image of a patron from the patron detail page
2/ Add an image to a new patron
3/ Use the "Upload patron images" tools (tools/picture-upload.pl) to add
or modify the image of a patron
4/ Use the "Upload patron images" tools (tools/picture-upload.pl) to add
or modify the image of several patrons, using a zip file.
Stress the script trying to get as many errors as possible (wrong
cardnumber, wrong mimetype, file does not exist, etc.)
With this patch, if the cardnumber does not exist, you will get a
specific error "Image not imported because this patron does not exist in
the database"

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-03-04 12:53:00 +00:00
771e63f5bf Bug 15669: Alphabetize marc modification templates
This patch sorts the modification templates when getting them from the
database  This affects the pages below:

  tools/batch_record_modification.pl
  tools/marc_modification.pl
  tools/stage-marc-import.pl

To test:

1 - Add some marc modification templates in a non alpha order (Shoes,
    Hats, Cats)
2 - visit the pages above and note the templates are in order
    added
3 - apply patch
4 - visit the three pages and note correct order
5 - verify all tools continue to work as expected

Signed-off-by: Frédéric Demians <f.demians@tamil.fr>

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

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-03 22:11:44 +00:00
77e1e7c4ef Bug 15548 [QA Followup] - More new uses of Koha::Borrower
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>
2016-03-03 14:39:00 -07:00
3691bd8419 Bug 15548: Move new patron related code to Patron*
The 'borrower' should not be used anymore, especially for new code.
This patch move files and rename variables newly pushed (i.e. in the Koha
namespace).

Test plan:
1/
  git grep Koha::Borrower
should not return code in use.

2/
Prove the different modified test files

3/ Do some clicks in the member^Wpatron module to be sure there is not
an obvious error.

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Works as described. Tested with Circulation, Members/Patrons, Discharge,
Restrictions modules and the must common functionalities

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

Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>
2016-03-03 14:38:26 -07:00
774993ae2d Bug 15632 [QA Followup] - Get rid of use of uninitialized value errors for unit tests
* Set $user to "" if not passed
* Tidy sub

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-03 21:22:15 +00:00
2d74d926ce Bug 15632: Koha::Patron::Messages - Remove GetMessages
This subroutine just retrieved the messages given some parameters.
Some job should not have been done in this subroutine.
It was called only 3 times, in circ/circulation.pl and opac-user.pl.
Basically it was used to retrieved the message to displaye for a given
patron ($borrowernumber) at the OPAC (B) or Staff (L).

For the 3 calls, the 2 parameters $borrowernumber and $type
(message_type) were passed, the "%" trick at the beginning of the
subroutine was useless.
Moreover, the date formatting should be done on the TT side, not in
subroutine.
The can_delete flag was set if the branchcode given in parameter was the
same as the one of the message. This has been delegated to the template.
Indeed the can_delete was not valid, since it must depend on the
AllowAllMessageDeletion pref.
The test is now:
  IF message.branchcode == branch OR
  Koha.Preference('AllowAllMessageDeletion'')

There is not specific test plan for this patch, the changes have already
been tested in previous patches.

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-03 21:22:14 +00:00
74180472ad Bug 15632: Koha::Patron::Messages - Remove GetMessagesCount
The GetMessageCount subroutine was only used once, in opac-user.pl, to
know if some messages will be displayed.

Test plan:
1/ Create messages to display at the OPAC for a patron
2/ Logged this patron in at the OPAC, you should see the messages
displayed.

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-03 21:22:13 +00:00
e6bd28682e Bug 15632: Koha::Patron::Messages - Remove DeleteMessage
The DeleteMessage just deleted and logged if needed.
This is now be done by Koha::Patron::Message->delete.

Test plan:
1/ Go on the "check out" page of a patron
2/ Delete some messages

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-03 21:22:13 +00:00
d9c983c568 Bug 15632: Koha::Patron::Messages - Remove AddMessage
The AddMessage subroutine just
1. checked if all the parameters were passed
2. inserted the message in the DB
3. logged the action if needed

These 3 jobs are now done by the overloaded store method of
Koha::Patron::Message.

Test plan:
1/ Go on the "check out" page of a patron
2/ Add different new messages

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-03 21:22:13 +00:00
85d08edddf Bug 15084: Remove C4::Budgets::ConvertCurrency
This subroutine is unused and can be removed.
There is no trace left of currency in C4::Budgets.

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

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

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-03 20:39:01 +00:00
1538e9ecf4 Bug 15084: Replace C4::Budgets::GetCurrencies with Koha::Acquisition::Currencies->search
Most part of the code here is unnecessary complex. We should selected
the currency if it is selected, that's all :)

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

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

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-03 20:39:01 +00:00
c0d7030b02 Bug 12803 [QA Followup] - Allow holiday caching to be disabled for testing purposes
Bug 12803 [QA Followup] - Remove use of C4::Dates

C4::Dates was being included, but not used in the code!

Bug 12803 [QA Followup] - Fix koha-qa.pl errors

Bug 12803 [QA Followup] - Update unit tests due to changes in master

Bug 12803 [QA Followup] - Fix to stop failing unit tests

Bug 12803 [QA Followup] - Remove duplicate 'use' lines

Bug 12803 [QA Followup] - Remove NO_CACHE

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

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-03 20:19:00 +00:00
3c9d50d134 Bug 12803 - Add ability to skip closed libraries when generating the holds queue
The holds queue is typically generated many times a day in order to
select items to fill holds. Often these items are to be sent to a
different library. However, if the library whose item is picked to fill
a hold is closed, that hold will remain unfilled even if there are other
open libraries who own that item. It would be helpful if we could skip
closed libraries for the purpose of selecting items to fill holds.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Create a record with two items on it, one at Branch A, and one at
   Branch B
4) Place a hold for pickup at Branch C
5) Generate the holds queue
6) Note which branch's item is selected for the hold
7) Enable the new system preference HoldsQueueSkipClosed
8) Add today as a holiday for that branch noted in step 6
9) Regenerate the holds queue
10) View the holds queue, notice the item selected is not from
    the closed branch!
11) prove t/db_dependent/HoldsQueue.t

Signed-off by: Jason Robb <jrobb@sekls.org>

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

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-03 20:19:00 +00:00
9bd9476b1d Bug 12426: [QA Follow-up] Clear to_address to force update
When resending an email from the Notices tab in Patrons, we would like
to use the recent email address.

Test plan:
[1] Look up a patron with some notices sent.
[2] Adjust the patron email address (watch AutoEmailPrimaryAddress).
[3] Resend the notice. Verify if the new address was used.

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

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-03 20:16:07 +00:00
Lari Taskula
fbb7097b1d Bug 12426: Allow resend for sent messages
This patch allows to resend both sent and failed messages.

With messages in 'sent' status, we have to be careful not to accidentally send
sent messages again. With the previous patch using GET request, this was likely
to happen because of browser storing the GET parameters.

This patch changes request method from GET to POST. Instead of a simple link,
we now have a form element.

In notices.pl we redirect back to notices.pl, because with POST there is a risk
of resending the message accidentally by form resubmission at refresh.

To test, find/create a Patron that has sent or failed notices in message_queue:
1. Enable EnchancedMessagingPreferences system preference
2. Go to Patrons -> Notices
3. In the Notice column, click the title of the sent or failed message
4. Observe that there is nothing for resending the sent or failed message
5. Apply the patches.
6. Reload Notices page and repeat step 3
7. Observe that there is now a link "Resend" in the Status-column
8. Click Resend
9. Observe that the message gets into 'pending' status

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-03 20:16:07 +00:00
0813c72b1f Bug 12426: Simplify the code adding a new subroutine GetMessage
The C4::Letters module does not have a GetMessage subroutine, which
could be quite useful.
This patch adds it and simplifies the code added by the previous patch.
It also adds a few tests and fixes POD typos.

Note that ResendNotice only resends failed messages. This will avoid to
resend already sent messages (using an url from the browser history for
instance).

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-03 20:16:07 +00:00