Commit graph

6195 commits

Author SHA1 Message Date
Katrin Fischer
40cb8e3b75 Bug 17902: Follow-up fixing SQL statement
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-30 13:02:57 +00:00
f42dbd67d1 Bug 17902: Fix possible SQL injection in serials editing
/cgi-bin/koha/serials/serials-edit.pl?serstatus=*/+,2,3,'2016-12-12','2016-12-12',6,'jjj7','jjj8'%20--%20-&subscriptionid=1+and+1%3d2+Union+all+select+111+/*

The SQL query is not constructed correctly, placeholders must be used.
Subscription id and status list can be provided by the user.

This vulnerability has been reported by MDSec.

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

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>
2017-01-30 13:02:56 +00:00
4dc7c32a3d Revert "Bug 17902: Fix possible SQL injection in serials editing"
This reverts commit 904716f581.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-30 12:12:08 +00:00
904716f581 Bug 17902: Fix possible SQL injection in serials editing
/cgi-bin/koha/serials/serials-edit.pl?serstatus=*/+,2,3,'2016-12-12','2016-12-12',6,'jjj7','jjj8'%20--%20-&subscriptionid=1+and+1%3d2+Union+all+select+111+/*

The SQL query is not constructed correctly, placeholders must be used.
Subscription id and status list can be provided by the user.

This vulnerability has been reported by MDSec.

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-30 12:08:31 +00:00
e2d1bafa22 Revert "Bug 17902: Fix possible SQL injection in serials editing"
This reverts commit 8924439054.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-30 11:52:56 +00:00
8924439054 Bug 17902: Fix possible SQL injection in serials editing
/cgi-bin/koha/serials/serials-edit.pl?serstatus=*/+,2,3,'2016-12-12','2016-12-12',6,'jjj7','jjj8'%20--%20-&subscriptionid=1+and+1%3d2+Union+all+select+111+/*

The SQL query is not constructed correctly, placeholders must be used.
Subscription id and status list can be provided by the user.

This vulnerability has been reported by MDSec.

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-30 11:52:38 +00:00
93cc0956a9 Bug 9569: Security patch for AutoLocation
If a patron is not allowed to access the staff interface because its IP
address in the authorised range of IPs, the cookie should not contain
the CGISESSID.
If it is, the patron is logged in and will be able to access the staff
interface if he reload the page (or hit another one).

Test plan:
Confirm the that AutoLocation feature is now working as expected.

Note: It seems that this feature has never really worked as intended.
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-30 11:25:06 +00:00
af0af36bb9 Bug 9569: Do not check the IP for login at the OPAC
At the OPAC, the AutoLocation feature should not be taken into account:
login to the OPAC from outside the IP range should work

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-30 11:25:06 +00:00
acabdc87c9 Bug 9569: AutoLocation should not depend on IndependentBranches
Those 2 prefs can be independent and it does not make sense to consider
AutoLocation only if IndependentBranches is set.

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-30 11:25:05 +00:00
a8fdac38d8 Bug 9569: Fix AutoLocation - handle .* for subnets
The example in branches.tt is:
  Can be entered as a single IP, or a subnet such as 192.168.1.*

But actually the regex in C4::Auth does not handle subnets.

Test plan:
0/ Apply all the patches
1/ Switch AutoLocation on
2/ Define a subnet (192.168.0.* if your ip is like 192.168.0.X) in the IP
range of your library
3/ Log in on the staff interface
=> Should work

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-30 11:25:05 +00:00
b0bb1b0aa6 Bug 17904: Fix possible SQL injection in late orders
To recreate:
/cgi-bin/koha/acqui/lateorders.plop=send_alert&ordernumber=1)and%20(select*from(select(sleep(20)))a)--%20&letter_code=0

Notice the delay.

The SQL query is not constructed correctly, placeholders must be used.

This vulnerability has been reported by MDSec.

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-30 11:22:33 +00:00
179ff58b09 Bug 17903: Fix possible SQL injection in serial claims
To recreate:
/cgi-bin/koha/serials/claims.pl?serialid=1)and%20(select*from(select(sleep(20)))a)--%20&letter_code=0

Notice the delay.

The SQL query is not constructed correctly, placeholders must be used.

This vulnerability has been reported by MDSec.

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-30 11:21:19 +00:00
a70980d825 Bug 17900: Fix possible SQL injection in patron cards template editing
To recreate:
/cgi-bin/koha/patroncards/edit-template.pl?op=edit&element_id=23%20and%201%3d2+union+all+select+1,user(),@@version+--%20

Look at the Profile dropdown list.

To fix this problem and to make sure it does not appears anywhere else
in the label and patroncards modules, I have refactored the way the
queries are built in C4::Creators::Lib
Now all of the subroutine takes a hashref in parameters with a 'fields'
and 'filters' parameters.
From these 2 parameters the new internal subroutine _build_query will
build the query and use placeholders.

Test plan:
1/ Make sure you do not recreate the vulnerability with this patch
applied.
2/ With decent data in the labels and patroncards modules, compare all
the different view (undef the New and Manage button groups) with and
without this patch applied.
=> You should not see any differences.

This vulnerability has been reported by MDSec.

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>
2017-01-30 11:19:55 +00:00
4ff78a9a0d Bug 17986: Perl dependency evaluation incorrect
It looks like I made a copy/paste error in a previous patch.

While the fix was working when you pass the param "module" to
version_info, it wasn't populating the version correctly
for the "all" param, which causes koha_perl_deps.pl to
think all OK modules actually need an upgrade.

TEST PLAN

0) Be on a system where you know your Koha Perl dependencies are
mostly up-to-date

1) Run ./koha_perl_deps.pl -a -c
2) Note that most modules say they need an upgrade even when
the installed version is the same as the minimum version

3) Apply patch

4) Run ./koha_perl_deps.pl -a -c
5) Note that most moduls say they're OK, especially when the
installed version is the same or greater than the minimum version

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Running koha_perl_deps.pl -u convinced me.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-30 11:18:27 +00:00
bcf9fdafab Bug 17588: ->get_issues has been replaced with ->checkouts
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 14:25:35 +00:00
90f9a3c6ac Bug 17588: get_account_lines->get_balance has been replace with account->balance
On previous bugs

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 14:25:35 +00:00
45cee0cec8 Bug 17588: Koha::Patrons - Move GetMemberIssuesAndFines
The GetMemberIssuesAndFines subroutine used to retrieve the issues,
overdues and fines for a given patron. Most of the time, only 1 or 2 of
these values were used.
This patch removes this subroutine and uses the new get_issues,
get_overdues and get_balance method from Koha::Patron and Koha::Account::Lines.

Test plan:
1/ Add overdues, issues and fines to different patrons
2/ On the checkout, checkin and patron search result and the patron
detail pages, these 3 informations, if displayed before this patch, must be
correctly displayed.
3/ Use the batch patron deletion tool and make sure that patrons with a
balance > 0 are not deleted

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 14:25:34 +00:00
42bef19fe9 Bug 17824: Remove C4::Members::GetBorrowersWhoHaveNeverBorrowed
This subroutine is no longer in used and can be removed.

Test plan:
  git grep GetBorrowersWhoHaveNeverBorrowed
must not return any results

NOTE: grep -i getborrowerswhohave `find . -type f`
      works well enough to find the cleanborrowers.pl too.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 14:24:09 +00:00
158442eb9e Bug 17501: Remove Koha::Upload::get from Koha::Upload
The get routine actually returns records from uploaded_files. It should be
possible to replace its calls by direct calls of Koha::UploadedFiles.

This patch is the crux of this patch set. It deals with all scripts that
use Koha::Upload.

In the process we do:
[1] Add a file_handle method to Koha::UploadedFile. This was previously
    arranged via the fh parameter of get.
[2] Add a full_path method to UploadedFile. Previously returned in the
    path hash key of get. (Name is replaced by filename.)
[3] Add a search_term method too (implementing get({ term => .. }).
    This logic came from _lookup.
[4] Add a keep_file parameter to delete method. Only used in test now.

Test plan:
[1] Run t/db_dependent/Upload.t
[2] Go to Tools/Upload. Add an upload, download and delete.
[3] Add another public upload , search for it.
    Use the hashvalue to download via opac with URL:
        cgi-bin/koha/opac-retrieve-file.pl?id=[hashvalue]
[4] Go to Tools/Stage MARC for import. Import a marc file.
[5] Go to Tools/Upload local cover image. Import an image file.
    Enable OPACLocalCoverImages to see result.
[6] Test uploading a offline circulation file:
    Enable AllowOfflineCirculation, and create a koc file (plain text):
    Line1: Version=1.0\tA=1\tB=2
    Line2: 2016-11-23 16:00:00 345\treturn\t[barcode]
    Note: Replace tabs and barcode. The number of tabs is essential!
    Checkout the item with your barcode.
    Go to Circulation/Offline circulation file upload.
    Upload and click Apply directly.
    Checkout again. Repeat Offline circulation file upload.
    Now click Add to offline circulation queue.
[7] Connect the upload plugin to field 856$u.
    Enable HTML5MediaEnabled.
    Upload a webm file via the plugin. Click Choose to save the URL,
    and put 'video/webm' into 856$q. Save the biblio record.
    Check if you see the media tab with player on staff detail.
    (See also: Bug 17673 about empty OPACBaseURL.)

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 14:20:05 +00:00
6ddd4b2f5f Bug 17913: [Follow-up] Fix duplicate $9s after merging in loose mode
We need to add $9 to the skip_subfields hash too. Formerly, it was
added to $exclude as well.
Thanks, Julian, for catching this one.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 13:55:13 +00:00
dd1c63230f Bug 17913: Run perltidy on the inner foreach loop
Kept the same number of lines.
You could verify with diff -w.

Test plan:
[1] Run t/db_dependent/Authorities/Merge.t
[2] As the last patch in this series, also test the interface:
    Set AuthorityMergeMode to loose. Set dontmerge to Do.
    Modify an authority record attached to multiple biblios.
    Edit a subfield, clear a subfield and add a subfield.
    Save. Wait a bit for the merge and Zebra update.
    Verify that the changes are merged properly into biblio records.
[3] Repeat step 2 with AuthorityMergeMode to strict.
    Remember that this affects the extra subfields in biblio records.

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 13:55:12 +00:00
28b74224d3 Bug 17913: We always need some housekeeping
Remove some commented warnings
Remove the commented old code at the end of sub merge
Explicitly set merge mode in the first subtest
Move the return to loose mode from the second subtest to the third

Test plan:
Run t/db_dependent/Authorities/Merge.t

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 13:55:12 +00:00
ede08d725e Bug 17913: Remove possible duplicates in strict merge mode
Since strict mode does not allow additional subfields that would make
identical fields linked to the same authority different, there is no
need to keep them while merging.

We achieve this goal by simply:
[1] Count the number of same fields linked to mergefrom in strict mode to
    eliminate duplicates.
[2] Replaces the if-statement on auth_number by a next. (Tidy follows.)

Test plan:
Run t/db_dependent/Authorities/Merge.t

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 13:55:12 +00:00
681ae8430e Bug 17913: Do not keep a cleared subfield in loose merge mode
If you modify an authority and clear a specific subfield, you expect that
merge respects your edit and clears this subfield too in the biblio
records. It does in the new strict mode, but it does not yet in the
default loose mode.

This patch fixes that by adjusting the code around $exclude so that it
uses a new hash skip_subfields, built from the reporting tags from the old
and the new authority record.

This is supported again by some changes in the unit test.

Test plan:
Run t/db_dependent/Authorities/Merge.t

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 13:55:11 +00:00
8cde85451e Bug 17913: Fix the new field tag in merge when changing type
Originally aimed for 9988, adjusted for this report.

Old behavior was: pick the first tag. This is definitely wrong.
If you (would) merge 610 to 611, you don't want to get a 111.

This patch resolves the problem by determining the new tag in a small
helper routine _merge_newtag, and corrects the position of the new field
in the MARC record with append_fields_ordered. Too bad that MARC::Record
does not have such a function; it looks like insert_fields_ordered, but
it is different in case of multiple fields with the same tag.

Note: These two small helper functions are not tested separately, since they
should not be called outside of merge. They are implicitly tested by the
adjusted tests in Merge.t.

Note: In adding tests for this fix, I chose to simplify compare_field_count
(no need for the pass parameter), and replace the pass parameter of sub
compare_field_order by an exclude parameter, a hash of fields to exclude in
counting fields.

Test plan:
Run t/db_dependent/Authorities/Merge.t

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 13:55:11 +00:00
Frédérick
79c81920ab Bug 17913: Use AuthorityMergeMode pref in sub merge
Original fix from a patch on bug 11315.
Amended by Marcel de Rooy January 2017.

Test plan:
If you set mode to loose, the test will still pass.
If you set mode to strict, one test will fail. (Fixed later.)

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 13:55:10 +00:00
ffe1f6555c Bug 17913: Use replace_with instead of insert_grouped_field
Original fix from a patch on bug 5572.
Amended by Marcel de Rooy January 2017.

Note: This does not yet resolve the field order when merging to another
auth type, but is a good start.

Test plan:
Run t/db_dependent/Authorities/Merge.t

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 13:55:10 +00:00
b732963e2f Bug 17880 - Use version.pm to parse version numbers in C4::Installer::PerlModules
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 13:47:27 +00:00
13923e7b05 Bug 17836: (ILSDI) Make GetPatronInfo fill 'charges' correctly
This trivial fix corrects a typo on C4/ILSDI/Services.pm.
It was hidden because the tests for ILSDI only cover the 'attributes'
portion of the response. I added regression tests for this.

To test:
- Have the regression test patch applied
- Run:
  $ prove t/db_dependent/ILSDI_Services.t
=> FAIL: Tests fail because 'charges' is always set to 1
- Apply the patch
- Run:
  $ prove t/db_dependent/ILSDI_Services.t
=> SUCCESS: Tests pass
- Sign off :-D

Sponsored-by: ByWater Solutions

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 13:44:56 +00:00
11dfb2e0b2 Bug 8361: Do not allow checkouts if no rules are defined
We should require a circulation rule to allow checkouts and reject them
if no rules are defined.

Test plan:
- Delete all issuing rules
- Check an item out
=> Without this patch the checkout is allowed
=> With this patch applied it is rejected

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 13:41:59 +00:00
Alex Arnaud
e331a9c0d0 Bug 17615 - Fix updating borrower attributes in checkpw_ldap
Signed-off-by: Oliver Bock <oliver.bock@aei.mpg.de>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 13:37:33 +00:00
3a19e55382 Bug 17894 - Remove and replace WriteOffFee
WriteOffFee is the last of the "payment" subroutines that need to be
merged into Koha::Account::pay ( as a writeoff is really just type of
payment ).

Test Plan:
1) Apply this patch
2) Verify the writeoff, and writeoff all buttons still work

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

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-19 11:15:26 +00:00
c718ad6375 Bug 17894 - Update pay() and use it internally for WriteOffFee
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-19 11:15:25 +00:00
c919e8bfa2 Bug 6782: Fix fixup_cardnumber call
The fixup_cardnumber subroutine takes only 1 parameter, the cardnumber.
This call is wrong and morevover makes a lot of tests fail:

t/db_dependent/Letters.t .. 1/79 Can't use an undefined value as a HASH
reference at /home/vagrant/kohaclone/C4/Members.pm line 502.

This happens because the userenv is not mocked in a lot of test files.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-18 11:47:47 +00:00
0ffe964fb7 Bug 17196: [QA Follow-up] Adjust some text on marcxml
No code changes here, just text.

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-13 13:49:30 +00:00
80fb3160d9 Bug 17196: [QA Follow-up] Correct POD in ILSDI/Services.pm
QA tools complains about:
Apparent command =cut not preceded by blank line.

Trivial fix.

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-13 13:49:29 +00:00
98874bf9e2 Bug 17196: Fix - Update metadata on update
Signed-off-by: Mason James <mtj@kohaaloha.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it>

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-13 13:49:27 +00:00
27b1acf209 Bug 17196: Fix query builder for item search
The item search needs to join on biblio_metadata to allow search on
marcxml field

Test plan:
Launch complex item searches (using marc fields).

Signed-off-by: Mason James <mtj@kohaaloha.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it>

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-13 13:49:26 +00:00
c6e488f4af Bug 17196: Move marcxml out of the biblioitems table
Two discussions on koha-devel lead to the same conclusion:
biblioitems.marcxml should be moved out this table
- biblio and biblioitems
http://lists.koha-community.org/pipermail/koha-devel/2013-April/039239.html
- biblioitems.marcxml & biblioitems.marc / HUGE performance issue !
http://lists.koha-community.org/pipermail/koha-devel/2016-July/042821.html

There are several goals to do it:
- Performance
As Paul Poulain wrote, a simple query like
  SELECT publicationyear, count(publicationyear) FROM biblioitems GROUP BY publicationyear;
takes more than 10min on a DB with more than 1M bibliographic records
but only 3sec (!) on the same DB without the biblioitems.marcxml field
Note that priori to this patch set, the biblioitems.marcxml was not
retrieved systematically, but was, at least, in
C4::Acquisition::GetOrdersByBiblionumber and C4::Acquisition::GetOrders
- Flexibility
Storing the marcxml in a specific table would allow use to store several
kind of metadata (USMARC, MARCXML, MIJ, etc.) and different formats (marcflavour)
- Clean code
It would be a first step toward Koha::MetadataRecord for bibliographic
records (not done in this patch set).

Test plan:
- Update the DBIC Schema
- Add / Edit / Delete / Import / Export bibliographic records
- Add items
- Reindex records using ES
- Confirm that the following scripts still work:
    * misc/cronjobs/delete_records_via_leader.pl
    * misc/migration_tools/build_oai_sets.pl
- Look at the reading history at the OPAC (opac-readingrecord.pl)
- At the OPAC, click on a tag, you must see the result

Note: Changes in Koha/OAI/Server/ListRecords.pm is planned on bug 15108.

Signed-off-by: Mason James <mtj@kohaaloha.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-13 13:49:26 +00:00
b664d68ca4 Bug 17629: Koha::Biblio - Remove ModBiblioframework
There is only one call to C4::Biblio::ModBiblioframework, it's called
just before C4::Biblio::ModBiblio in cataloguing/addbiblio.pl
At first glance this call does not seems useful: all the subroutines
called from ModBiblio send the frameworkcode in parameter.

I'd go to remove it, but I'd like to get confirmation by others.

No test plan here, you need a good pair of eyes and deep into the
C4::Biblio code.

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-13 12:37:08 +00:00
d51d1899e8 Bug 17486: [QA Follow-up] Changes as to set_userenv
Add shibboleth parameter to POD of set_userenv.
Removed a 12th set_userenv parameter from Borrower_Discharge.t.
Replaced set_userenv call in PatronLists.t looking like a fortunate typo.

Test plan:
Run the two corrected tests.

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-13 12:32:20 +00:00
e55b38928a Bug 17486: Remove Mozilla Persona
Persona never really took off, and although many browsers currently
support it, very few services actually implement it.

This has lead to it's founders, Mozilla, to end the project. In their
own words:

=============================================================================
Persona is no longer actively developed by Mozilla. Mozilla has
committed to operational and security support of the persona.org
services until November 30th, 2016.

On November 30th, 2016, Mozilla will shut down the persona.org services.
Persona.org and related domains will be taken offline.

If you run a website that relies on Persona, you need to implement an
alternative login solution for your users before this date.

For more information, see this guide to migrating your site away from
Persona:

https://wiki.mozilla.org/Identity/Persona_Shutdown_Guidelines_for_Reliers

=============================================================================

Given the above, and that the Persona authentication methods as a whole
are no longer being actively maintained by anyone anywhere to ensure
ongoing security, we should deprecate the option from koha.

Test plan:
Apply this patch and make sure you do not find any references of Persona
Have a look at patches from bug 9587 and confirm that everything has
been reverted

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Code looks good to me.
Also ran several tests including: Auth.t, Auth_with_shibboleth.t.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-13 12:32:19 +00:00
Olli-Antti Kivilahti
292b9df914 Bug 14187: branchtransfer needs a primary key (id) for DBIx and common sense.
* C4/Circulation.pm (GetTransfers, GetTransfersFromTo): Also return
  branchtransfer_id in return columns.
* installer/data/mysql/atomicupdate/14187.perl: New file.
* installer/data/mysql/kohastructure.sql: Modify branchtransfers structure.
* t/db_dependent/Circulation/transfers.t: Update tests to expect
  branchtransfer_id.

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

Test plan successful on all steps.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Amended patch: Remove Schema changes from this patch
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-13 12:20:11 +00:00
Meenakshi.R
141d29358f Bug 6782 - Move auto member cardnumber generation to occur when record is "Saved" (avoid collisions).
Currently the card number is generated when the user enters the patron creation form. This creates a problem of concurrency - when two or more simulataneous users are registering members, the error "card no. in use" can occur.

This change moves the card number generation to occur after the "Save" button is pressed.

Changes:
-C4/Members.pm:
Added code to fixup_cardnumber,If the cardnumber is blank and "autoMemberNum" ON.
-koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt:
Added code to display "leave blank for auto calc during registration" in cardnumber label in patron registration form only if "autoMemberNum" ON.
-members/memberentry.pl:
Added code to get weather or not "autoMemberNum" is on or off and removed fixup_cardnumber generation.

Test cases:
-If "autoMemberNum" ON:
->In blank case, must generate auto card number in simulataneous users.
->If user entered, check for unique card number.

-If "autoMemberNum" OFF:
Must work normal.

Followed test plan, works as expected.
Note: Syspref PorrowerMandatoryField must not include cardnumber, otherwise
      you can not save. Maybe that should be mentioned in the comment for
      syspref autoMemberNum.
Signed-off-by: Marc Véron <veron@veron.ch>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-13 11:47:00 +00:00
Benjamin Rokseth
5a08969a71 Bug 17766 - Patron notification does not work with multi item holds
This patch fixes notification when same biblio has multiple reserves with same borrower,
introduced in Bug 14695. C4::Reserves::ModReserveAffect uses internal method
_koha_notify_reserve but sends itemnumber and biblionumber instead of record_id.

To test:
Prerequisites:
- One biblio with two items attached
- A patron with hold_filled notification activated
- A letter for HOLD with <<reserves.reserve_id>> in it
1) Place two reservations on same biblio
2) checkin item x on pickup branch, observe patron message generated
3) checkin item y on pickup branch, observe patron message generated
4) note that reserve_id is the same on both messages, which is wrong
5) apply this patch and repeat 1-3
6) now observe notifications have correct (different) reserve_id

Signed-off-by: Hugo Agud <hagud@orex.es>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-13 11:29:28 +00:00
f59c22b72b Bug 15905 - Remove use of makepayment
Test Plan:
1) Apply these patches
2) prove t/db_dependent/Accounts.t
3) 'git grep makepayment' should return no results

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-12 13:43:24 +00:00
05fdd855c8 Bug 17234: Need to separate KEY and FOREIGN KEY checks
In the previous patch we use the constraint_exists subroutine to verify
if an index or a foreign key exists.
But the `SHOW INDEX` query does not return foreign keys (as its name
suggests!).
We need another subroutine foreign_key_exists to check the FK existence.

I have found that because t/db_dependent/TestBuilder.t fails on
oai_sets_biblios, because oai_sets_biblios_ibfk_1 has not been removed.

Test plan:
0/ Do not apply this patch
1/ Use a 3.20 DB
2/ update the DB
3/ SHOW CREATE TABLE oai_sets_biblios
will display oai_sets_biblios_ibfk_1

Apply the patch and repeat 1, 2, 3
=> Will not display oai_sets_biblios_ibfk_1
It has been removed as expected.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-12 12:43:10 +00:00
1e0becf915 Bug 15908 - Remove use of recordpayment_selectaccts
Test Plan:
1) Apply this patch
2) prove t/db_dependent/Accounts.t
3) Test fine payment via the "Pay selected" button

Signed-off-by: Laura Slavin <lslavin@hmcpl.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-11 14:41:42 +00:00
9f51f7a7ad Bug 15909 - Remove the use of makepartialpayment
Test Plan:
1) Apply this patch
2) prove t/db_dependent/Accounts.t
3) Test fine payment via the "Pay" button,
   but make the payment for less then the full amount

Signed-off-by: Laura Slavin <lslavin@hmcpl.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-11 14:41:04 +00:00
e134350813 Bug 15898 - Use Koha::Account::pay internally for makepartialpayment
This is the fourth patch in a series to unify all payment functions into
a single mathod

Test Plan:
1) Apply this patch
2) prove t/db_dependent/Accounts.t
3) Test fine payment via the "Pay" button,
   but make the payment for less then the full amount

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-11 14:06:33 +00:00