Lari Taskula [Mon, 12 Dec 2016 14:49:44 +0000 (16:49 +0200)]
Bug 16387: Fix default shortened loan period time
When a loan period is shortened due to using decreaseLoanHighHolds* the time is
always set to the current time in X days, even if the original loan period is
given in days and not in hours.
It should default to 23:59 as is normal for loan periods given in days.
As original due date time defaults to 23:59 when given in days, this patch
modifies the hours and minutes of shortened due date to be equal to original due
date.
To test:
1. prove t/db_dependent/DecreaseLoanHighHolds.t
Signed-off-by: Grace McKenzie <grace.mcky@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Josef Moravec [Tue, 24 Jan 2017 07:16:37 +0000 (07:16 +0000)]
Bug 17929 - You can't edit indicators in the cataloging screen
Test plan:
0. Do not apply the patch
1. Edit a biblio record, note you can't edit an indicator (in fact you edit it, but can't see the value)
2. Edit an authority record, note you can't edit an indicator (in fact you edit it, but can't see the value)
3. Apply the patch, you may need clear the browser cache (in Firefox Ctrl+F5 is often enough)
4. Repeat steps 1 and 2 - but now you will be able to edit the indicator
Signed-off-by: J Schmidt <jschmidt@switchinc.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Julian Maurice [Wed, 18 Jan 2017 10:29:57 +0000 (11:29 +0100)]
Bug 17922: Use correct number of digits when replacing date placeholders
This patch also fixes a typo ("<<MM><" should be "<<MM>>")
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>
Mirko Tietgen [Mon, 30 Jan 2017 14:59:59 +0000 (15:59 +0100)]
Bug 18015 - On shelf holds allowed > "If all unavailable" ignores notforloan
If in the circ rules matrix you set "On shelf holds allowed" to "If all unavailable",
items with status "Not for loan" are considered available and break the functionality.
Test plan:
- Set "On shelf holds allowed" to "If all unavailable" for your patron and item
category (or everyone and everything)
- Have two items for a record. Check out one
- Set 7 - Not for loan: "Not For Loan" for the second item
- Try to place a hold. Does not work.
- Apply the patch
- Try to place a hold. Should work now.
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>
David Cook [Tue, 10 Jan 2017 01:36:25 +0000 (12:36 +1100)]
Bug 17871: Remove zebra::snippet to allow access to facets in YAZ 5.8.1+
This patch restores access to zebra facets (or zebra::snippet) with YAZ 5.8.1 or higher.
It was failing due to The <retrieval syntax="xml" name="zebra::*" /> entry in
retrieval-info-bib-dom.xml which IndexData said it wasn't even needed to
get that access.
Edit: I amended the commit message (tcohen)
Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
I tested on kohadevbox and found no regression or behaviour change. I
will provide a followup for the packages.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Josef Moravec [Wed, 18 Jan 2017 12:36:37 +0000 (12:36 +0000)]
Bug 18005: Re-styled pagination on search results with Bootstrap
Test plan:
0. Don't apply path
1. Make catalogue search in staff client with more then 20 results,
scroll down to see that pagination is broken
2. Apply the patch
3. Make similar search and confirm, the pagination looks OK with new
style
Signed-off-by: Claire Gravely <claire_gravely@hotmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
SQL expects lists to be comma separated. A trailing comma must also
be avoided.
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>
Bug 17775 - Add new user with LDAP not works under Plack
This patch fixes internal server error:
Undefined subroutine &C4::Auth_with_ldap::AddMember called at /srv/koha_ffzg/C4/Auth_with_ldap.pm line 213.
It occurs only under plack, and it's strange since C4::Members
does EXPORT AddMember and we are importing it into Auth_with_ldap.pm
(and it does work under CGI).
Signed-off-by: Liz Rea <liz@catalyst.net.nz> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
I did not test but trust author and signoffer. The change cannot hurt.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jonathan Druart [Thu, 12 Nov 2015 12:28:38 +0000 (12:28 +0000)]
Bug 15030: Add tests
This test will prevent regression on the lost of data when
items.itemcallnumber is linked with a plugin.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Blou [Thu, 22 Oct 2015 20:49:10 +0000 (16:49 -0400)]
Bug 15030 - continue. The enumchron value was overwritten by 'header' even when value supplied
Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Blou [Wed, 21 Oct 2015 18:35:18 +0000 (14:35 -0400)]
Bug 15030 - Fixes the serials fields associated with a plugin, to not overwrite the previously saved value
This fixes the remaining fields from serials-edit.pl that were seeing their previously entered values
be oblitarated with each new edit. The fields associated to a plugin (dateaccessioned and barcode) were
always displaying <empty> with each new edit, losing the previous effort.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Blou [Mon, 19 Oct 2015 16:16:05 +0000 (12:16 -0400)]
Bug 15030 - Certain values in serials' items are lost on next edit
When editing serials subscription, we can edit them but some values are not pulled from the DB correctly to be put in the edit box. If not noticed, the value will be overwritten on the next save.
Test:
- Create a subscription
- Edit itemcallnumber (952o?) and make sure to have a different value than the default one.
- Save.
- Edit it again
- The saved value is not there.
This is true for itemcallnumber and a few other fields.
This was caused by calls to ->field($subfield). This would always fail, of course.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 17255 - Upgrade Elastic Search code to work with version 5.1
Builds on top of commit:
Bug 17255 - Upgrade Elastic Search code to work with version 2.4+ - rebased wip
-Fix data type 'string' to 'keyword' and 'text'
-index: not_analyzed deprecated, replaced with type: keyword which is equivalent
-store: yes was deprecated, use store: true
TODO: Installer bindings to both the debian package install and the raw developer install.
A taster in Buug 17851
ZE TEST PLAN
0. Remove existing ES and reinstall ES 5.1
apt-get purge elasticsearch
Follow instruction here:
https://www.elastic.co/guide/en/elasticsearch/reference/current/deb.html
1. Reset Elasticsearch index since facets are hard coded to dynamic search_marc_mappings.
1a. perl -e 'use Koha::SearchEngine::Elasticsearch; Koha::SearchEngine::Elasticsearch->reset_elasticsearch_mappings();'
1b. If you get trouble, simply DELETE FROM [search_fields|search_marc_to_fields|search_marc_mapping];
and retry 1a.
1c. Destroy elasticsearch index
curl -XDELETE localhost:9200/koha_biblios
so it can be recreated
2. Recreate the index:
perl misc/search_tools/rebuild_elastic_search.pl
2a. Add something to index if your koha.biblio-table is empty
3. Fetch all indexed records and the facet for subject__facet
4. Run the included tests:
perl t/db_dependent/Koha_Elasticsearch_Indexer.t
perl t/db_dependent/Koha_Elasticsearch.t
perl t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t
Have fun with your new ES 5.1 cluster!
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>
Bug 17255 - Upgrade Elastic Search code to work with version 2.4+ - rebased wip
-Changed deprecated facets to aggregations
-Fixed boolean datatypes not allowing analyzers to be specified
-Fixed deprecated '_id' to 'es_id'. Now the ES-index has the correct id==biblionumber
ZE TEST PLAN
1. Reset Zebra index since facets are hard coded to dynamic search_marc_mappings.
2. perl misc/search_tools/rebuild_elastic_search.pl
3. Fetch all indexed records and the facet for subject__facet
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>
Josef Moravec [Wed, 1 Feb 2017 19:48:49 +0000 (20:48 +0100)]
Bug 18033: Remove duplicate code in paycollect.pl
Test plan:
0) apply the patch
1) try to pay individual fee, with full amount and partial amount
it should work the same as before patch
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jonathan Druart [Thu, 2 Feb 2017 08:22:09 +0000 (09:22 +0100)]
Bug 17960: Replace missing occurrences
Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jonathan Druart [Tue, 24 Jan 2017 08:14:26 +0000 (09:14 +0100)]
Bug 17960: Add opac_news.content values in tests
Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jonathan Druart [Thu, 19 Jan 2017 15:54:50 +0000 (16:54 +0100)]
Bug 17960: DBIC Schema changes for opac_news.content
Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jonathan Druart [Fri, 20 Jan 2017 01:31:41 +0000 (02:31 +0100)]
Bug 17960: Update installed files
git grep opac_news.new installer
should not return any occurrences in sql files
Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jonathan Druart [Tue, 17 Jan 2017 07:29:23 +0000 (08:29 +0100)]
Bug 17960: Rename opac_news.new with opac_news.content
The field opac_news.new is very confusing and should be renamed.
If you want to access it via Koha::NewsItem you will have trouble:
use Koha::News;
my $news_item = Koha::News->next;
say $news_item->new;
=> Attempt to bless into a reference at /home/vagrant/kohaclone/Koha/Object.pm line 78.
This patchset is going to rename this DB field to opac_news_content instead.
Since the opac_news.new can be used in notice templates, we need to warn the
user during the update DB process that some templates must be updated.
Test plan:
0/ Apply the first patch "Add a test to highlight the issue" and confirm that
the test fail
1/ Apply this second patch
2/ Execute the DB entry
3/ Confirm that you get a warning if at least one of your notice templates is
using opac_news.new
4/ Confirm that the test new pass
5/ Add/update and delete a news
6/ Confirm that the RSS new feed still works as expected
Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jonathan Druart [Sat, 21 Jan 2017 09:49:40 +0000 (10:49 +0100)]
Bug 17960: Add a test to highlight the issue
With only this patch applied, proving t/db_dependent/Koha/News.t
will return
"Attempt to bless into a reference at /home/vagrant/kohaclone/Koha/Object.pm line 78."
Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Kyle M Hall [Thu, 10 Nov 2016 19:53:30 +0000 (19:53 +0000)]
Bug 17610 - Allow the number of plack workers and max connections to be set in koha-conf.xml
It would be nice if we could control the number of workers and max
requests on a per instance basis, rather than the numbers being
hardcoded in the plack startup script.
Test Plan:
1) Build a new package of Koha with this patch applied ; )
2) Verify koha-plack still works
3) Add the following to the config section of your koha-conf.xml:
<plack_max_requests>75</plack_max_requests>
<plack_workers>4</plack_workers>
4) Stop plack
5) Start plack
6) Verify the number of works and max requests worked!
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Larry Baerveldt <larry@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Rebased against master and added a description for the new configuration
entries
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Katrin Fischer [Mon, 30 Jan 2017 15:47:23 +0000 (16:47 +0100)]
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>
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>
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>
Jonathan Druart [Thu, 19 Jan 2017 10:46:21 +0000 (11:46 +0100)]
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>
Jonathan Druart [Thu, 19 Jan 2017 09:00:40 +0000 (10:00 +0100)]
Bug 9569: Update warning message
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>
Jonathan Druart [Tue, 16 Aug 2016 13:01:40 +0000 (14:01 +0100)]
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>
Jonathan Druart [Tue, 16 Aug 2016 12:56:25 +0000 (13:56 +0100)]
Bug 9569: Remove unused occurrence of AutoLocation
`git grep ManualLocation` does not return any results
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>
Jonathan Druart [Tue, 16 Aug 2016 12:56:19 +0000 (13:56 +0100)]
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>
Jonathan Druart [Tue, 16 Aug 2016 13:02:58 +0000 (14:02 +0100)]
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>
Jonathan Druart [Fri, 13 Jan 2017 15:19:45 +0000 (16:19 +0100)]
Bug 17905: FIX CSRF in member-flags
If an attacker can get an authenticated Koha user to visit their page
with the url below, privilege escalation is possible
The exploit can be simulated triggering
/cgi-bin/koha/members/member-flags.pl?member=42&newflags=1&flag=superlibrarian
Test plan:
Trigger the url above
=> Without this patch, 42 is now superlibrarian
=> With this patch, you will get the "Wrong CSRF token" error.
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>
Jonathan Druart [Fri, 13 Jan 2017 15:46:51 +0000 (16:46 +0100)]
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>
Jonathan Druart [Fri, 13 Jan 2017 15:40:59 +0000 (16:40 +0100)]
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>
Jonathan Druart [Fri, 13 Jan 2017 16:07:34 +0000 (17:07 +0100)]
Bug 17901: Force context to scalar
See bug 15809 for more references.
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>
Jonathan Druart [Fri, 13 Jan 2017 16:03:41 +0000 (17:03 +0100)]
Bug 17901: Fix possible SQL injection in shelf editing
It has been reported that
/cgi-bin/koha/opac-shelves.pl?op=edit&referer=view&shelfnumber=146&owner=4&shelfname=testX&sortfield=titleaaaaaa\`&category=1
Could lead to SQL injection
Actually it explodes because the generated SQL query is not correctly formated.
However it would be good to limit the possible values for sortfield.
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>
Jonathan Druart [Fri, 13 Jan 2017 16:43:25 +0000 (17:43 +0100)]
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>
David Cook [Tue, 24 Jan 2017 22:58:40 +0000 (09:58 +1100)]
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>
Jonathan Druart [Tue, 27 Dec 2016 08:05:18 +0000 (09:05 +0100)]
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>
Jonathan Druart [Tue, 6 Dec 2016 08:22:11 +0000 (09:22 +0100)]
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>
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>
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>
Marcel de Rooy [Tue, 10 Jan 2017 16:11:08 +0000 (17:11 +0100)]
Bug 17501: [Follow-up] QA Requests
This patch makes the following changes, as requested by QA:
[1] UploadedFile->delete always calls SUPER::delete.
The return value normally comes from SUPER::delete; if removing the
file failed, we return false. Two warns are kept.
Since delete does no longer return the filename, a few changes were
needed in tools/upload.pl.
[2] Method getCategories is moved to UploadedFiles. Script tools/upload.pl
now only contains one call. Added a use C4::Koha.
[3] Calls UploadedFiles->delete as class method. As a result I removed
method delete_errors for now; may be reconsidered on a new report.
[4] Adjusted three ->search calls for id and public to ->find calls.
[5] If you pass no id to upload.pl when deleting, you don't get an alert.
All by all, we got rid of 15 lines !
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>
Marcel de Rooy [Thu, 24 Nov 2016 13:13:22 +0000 (14:13 +0100)]
Bug 17501: Additional polishing (POD, unit tests)
This patch adds some documentation lines.
And mainly rearrangs the tests in Upload.t. The 'basic CRUD testing' is
not needed separately any more. A new test catches the "file missing"
warn.
Test plan:
[1] Run perldoc on UploadedFile[s].pm
[2] Run t/db_dependent/Upload.t
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>
Marcel de Rooy [Thu, 24 Nov 2016 09:02:19 +0000 (10:02 +0100)]
Bug 17501: Rename Upload to Uploader
Why? Koha::Uploader now only contains the actual CGI upload. The new name
better reflects its handler status.
Pragmatically, the difference between Uploaded and Uploader makes it
easier to specifically search for them in the codebase.
Test plan:
[1] Run t/db_dependent/Upload.t.
[2] Add an upload via the interface.
[3] Check the code:
git grep "Koha::Upload;"
git grep "Koha::Upload\->"
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>
Marcel de Rooy [Thu, 24 Nov 2016 08:27:23 +0000 (09:27 +0100)]
Bug 17501: Move getCategories and httpheaders from Upload.pm
Class method getCategories has no strict binding to Upload.pm. While
Upload.pm is now restricted to the actual uploading process with CGI
hook, this routine fits better in the UploadedFile package.
Class method httpheaders can be moved as well for the same reason. Note
that it actually is an instance method. The parameter $name is dropped.
Test plan:
[1] Run t/db_dependent/Upload.t.
[2] Check the categories in the combo box of tools/upload.
[3] Check a download via tools/upload and opac-retrieve-file.
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>
Marcel de Rooy [Tue, 22 Nov 2016 15:57:21 +0000 (16:57 +0100)]
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>
Marcel de Rooy [Mon, 21 Nov 2016 13:53:27 +0000 (14:53 +0100)]
Bug 17501: Move Koha::Upload::delete to Koha::UploadedFile[s]
Since delete is not part of the upload process, we will move it now
to Koha::UploadedFile[s].
Deleting the file will be done in UploadedFile.
The (multiple) delete method in UploadedFiles refers to the single delete.
Test plan:
[1] Run t/db_dependent/Upload.t
The warning ("but file was missing") in the last subtest is fine;
the file did not exist. Will be addressed in a follow-up.
[2] Search for uploads on Tools/Upload. Clone this tab (repeat search on
a new tab in your browser).
[3] Delete an existing upload on the first tab.
[4] Try to delete it again on the second tab. Error message?
[5] Bonus points:
Add an upload. Mark the file immutable with chattr +i. Try to delete
the file. You should see a "Could not be deleted"-message.
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>
Marcel de Rooy [Mon, 21 Nov 2016 13:12:17 +0000 (14:12 +0100)]
Bug 17501: Use Koha::Object in Koha::Upload::_delete
Note: This is the last occurrence where we use DBI to perform a CRUD
operation. In this case a delete from uploaded_files.
We now call Koha::UploadedFile[s]->delete to only delete the record
from the table. A next step will be moving the additional functionality
of removing the file(s) too.
Test plan:
[1] Run t/db_dependent/Upload.t
[2] Delete an upload from tools/upload.pl
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>
Marcel de Rooy [Mon, 21 Nov 2016 07:48:02 +0000 (08:48 +0100)]
Bug 17501: Use Koha::Object in Koha::Upload::_lookup
The _lookup routine performs a few select statements on uploaded_files.
In this patch the SQL statements are replaced with Koha::Object calls.
One call of _lookup is replaced directly by Koha::UploadedFiles.
Note: _lookup can be removed in a later stage.
Test plan:
[1] Run t/db_dependent/Upload.t
[2] Upload a file in some upload category
[3] Try to upload the same file into the same category. Error?
[4] Try to upload the same file in another category. Should work.
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>
Marcel de Rooy [Sun, 20 Nov 2016 15:10:33 +0000 (16:10 +0100)]
Bug 17501: Use Koha::Object in Koha::Upload::_register
The _register routine basically inserts a new record in uploaded_files.
It should use Koha::UploadedFile now.
Test plan:
[1] Run t/db_dependent/Upload.t
[2] Upload a file via Tools/Upload.
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>
Marcel de Rooy [Sat, 19 Nov 2016 16:24:11 +0000 (17:24 +0100)]
Bug 17501: Introduce Koha::Object[s] classes for UploadedFile(s)
In the next set of patches we will start using these new classes in
Koha::Upload, and scripts using it.
This is just the starting point of that migration.
Test plan:
[1] Run t/db_dependent/Upload.t
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>
Marcel de Rooy [Wed, 15 Jun 2016 12:08:28 +0000 (14:08 +0200)]
Bug 16733: Adjust other debian scripts using PERL5LIB
This patch makes the following changes:
koha-foreach, koha-upgrade-schema (shell scripts):
[1] Read default file
[2] Include helper functions
[3] Add call to adjust_paths_dev_install
[4] Replace hardcoded path by $PERL5LIB
koha-shell (perl script):
[1] Remove hardcoded lib path
[2] Add a sub that reads PERL5LIB from default or koha-conf, just as the
shell scripts do.
koha-plack (shell script), plack.psgi:
[1] Add call to adjust_paths_dev_install
[2] Remove hardcoded lib path
[3] Add installer path to PERL5LIB, remove it from plack.pgsi
koha-sitemap (shell script):
[1] Add call to adjust_paths_dev_install
[2] Remove hardcoded lib path
[3] Add installer path to PERL5LIB
[4] Adjust path for call to sitemap cron job
koha-start-sip (shell script):
[1] Read default file
[2] Include helper functions
[3] Add call to adjust_paths_dev_install
[4] Adjust path to C4/SIP
koha-stop-sip (shell script):
[1] Remove KOHA_CONF and PERL5LIB (not needed to stop the daemon)
[2] Same for paths in daemon client options
NOTE: Script debian/scripts/koha-upgrade-to-3.4 has been left out
intentionally.
Test plan:
[1] Regular install:
Run koha-foreach echo Hi
Run koha-upgrade-schema yourinstance
Run koha-shell yourinstance
If you have plack, run koha-plack --start|--stop yourinstance
Run koha-sitemap --generate yourinstance
Run koha-start-sip yourinstance
Run koha-stop-sip yourinstance
[2] Dev install [yourinstance] with <dev_install> in koha-conf.xml:
Run koha-upgrade-schema yourinstance
Run koha-shell yourinstance
If you have plack: koha-plack --start|--stop yourinstance
Run koha-sitemap --generate yourinstance
Run koha-start-sip yourinstance
Run koha-stop-sip yourinstance
[3] Git grep on koha/lib
You should no longer see occurrences in debian/scripts except:
koha-translate: see report 16749
koha-upgrade-to-3.4: left out intentionally
[4] Git grep on koha/bin
You should only see hits for lines with koha-functions in the
debian scripts except:
koha-upgrade-to-3.4: left out intentionally
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Most scripts tested on Wheezy (although it would not matter much).
Plack script tested on Jessie.
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>
Marcel de Rooy [Wed, 15 Jun 2016 11:57:12 +0000 (13:57 +0200)]
Bug 16733: Adjust koha-rebuild-zebra
[1] Add a call to the new adjust_paths_dev_install
[2] Differentiate location of rebuild_zebra.pl
[3] Replace a hardcoded path by $PERL5LIB
Test plan:
Adjust a biblio record in package or dev install.
Run koha-rebuild-zebra -b -z for same instance.
Verify that the change has been indexed.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> 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>
Marcel de Rooy [Wed, 15 Jun 2016 11:43:41 +0000 (13:43 +0200)]
Bug 16733: Adjust koha-indexer
[1] Add a call to the new adjust_paths_dev_install
[2] Differentiate location of rebuild_zebra.pl
NOTE: The scripts assume koha-functions.sh to be in /usr/share/koha/bin.
Finding a better location for this shell library may be hard.
Test plan:
Run koha-indexer for a regular package install or a dev install.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> 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>
Marcel de Rooy [Tue, 14 Jun 2016 14:00:58 +0000 (16:00 +0200)]
Bug 16733: Add adjust_paths_dev_install to koha-functions.sh
This new function checks koha-conf.xml for a given instance and if it
contains a dev_install line, it adjusts PERL5LIB and KOHA_HOME
accordingly. Otherwise it does not touch the values of these
variables as normally read from /etc/default/koha-common.
The function will be used in various debian scripts to allow for more
flexibility with dev installs. And at the same time aiming to make better
use of PERL5LIB and KOHA_HOME.
Test plan:
[1] Add <dev_install>/not/there</dev_install> to your koha-conf.xml.
[2] Run on the command line:
PERL5LIB=test
source [path-to-your-instance]/debian/scripts/koha-functions.sh
echo $PERL5LIB
adjust_paths_dev_install [name-of-your-instance]
echo $PERL5LIB
The last echo should be: /not/there
[3] Remove the <dev_install> line and repeat step 2.
The last echo should be: test
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> 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>
Jonathan Druart [Wed, 28 Dec 2016 13:51:17 +0000 (13:51 +0000)]
Bug 7533: Add a warning to the about page if template_cache_dir is not set
We need to tell the administrators that it would be great for them to
set this config entry.
Test plan:
- Do not set template_cache_dir and confirm that you see the warning
- Set template_cache_dir and confirm that you do not see the warning
Signed-off-by: Magnus Enger <magnus@libriotech.no>
Both templates for koha-conf.xml are updated. After applying the
patach a warning was correctly displayed. After adding
template_cache_dir to koha-conf.xml and restarting memcached it
went away.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jonathan Druart [Wed, 28 Dec 2016 13:49:59 +0000 (13:49 +0000)]
Bug 7533: Add the template_cache_dir entry to koha-conf.xml
And comment it, as we don't know what are the sysop's preferences
Signed-off-by: Magnus Enger <magnus@libriotech.no> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
radiuscz [Sun, 23 Oct 2016 22:57:03 +0000 (00:57 +0200)]
Bug 17487: Styling moved from style attribute into staff-global.css
Test plan:
1) Apply patch
2) Display Z39.50 search dialogs:
- cataloguing / new from Z39.50
- authorities / new from Z39.50
- acquisition / new from an external source
3) Select all / Clear all should be placed below "Search targets" header
4) [Optionally] Set some style in IntranetUserCSS for class z3950checks
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>
radiuscz [Sun, 23 Oct 2016 00:51:53 +0000 (02:51 +0200)]
Bug 17487: Links to "select/clear all" moved below the header tag
Test plan:
1) Apply patch
2) Display Z39.50 search dialogs:
- cataloguing / new from Z39.50
- authorities / new from Z39.50
- acquisition / new from an external source
3) Select all / Clear all should be placed below "Search targets" header
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>
Fridolin Somers [Fri, 13 Jan 2017 15:10:20 +0000 (16:10 +0100)]
Bug 17899 - Show only mine does not work in newordersuggestion.pl
Bug 12775 added a link "Show only mine" in newordersuggestion.pl.
This does not work, no results.
Also corrects the fact that click must not do default action by adding e.preventDefault().
Test plan :
- You must have suggestions you have accepted
- Create a new order from suggestion : /cgi-bin/koha/acqui/newordersuggestion.pl
- Click on Show only mine
=> Without patch the table is empty showing "No matching records found"
=> With patch you see only suggestions you have accpeted
Signed-off-by: Zoe Schoeler <crazy.mental.onion@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Mark Tompsett [Tue, 17 Jan 2017 06:42:11 +0000 (06:42 +0000)]
Bug 17920: t/db_dependent/Sitemapper.t fails because of permissions
The directory it attempts to create an xml file may not be writable for
the user running the test. By changing the directory from the current
directory to a temporary one, the test runs. After all 'chmod 777
t/db_dependent' is a bad idea.
TEST PLAN
---------
1) sudo koha-shell "prove t/db_dependent/Sitemapper.t" kohadev
-- fails
2) apply patch
3) sudo koha-shell "prove t/db_dependent/Sitemapper.t" kohadev
-- succeeds
4) run koha qa test tools
Tested without qa tools Signed-off-by: Claire Gravely <claire_gravely@hotmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 17813: (QA followup) Properly check DB structure before altering it
This patch makes the atomic update use the new functions introduced by
bug 17234 for checking the structure before attempting to call ALTER
TABLE.
It checks for the column existence, and also if it is a primary key.
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>
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>
Bug 17813: Add a primary key to borrower_attributes
This patch adds 'borrower_attributes' a field (if) which
will act as a primary key.
This is needed for DBIC to be used to handle rows, and also will help
when faced with the implementation of the REST api for this resource.
To test:
- Run all patron modification / attributes and verify nothing breaks
- Sign off :-D
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>
Jonathan Druart [Fri, 23 Dec 2016 09:18:03 +0000 (09:18 +0000)]
Bug 17808: Fix behavior when editing a circ rule
The original behavior is broken, see https://stackoverflow.com/questions/21410484/jquery-selector-to-find-out-count-of-non-empty-inputs
Test plan:
Edit a circ rule
=> Without this patch you get a useless message
=> With this patch, no message
Edit a circ rule with content in inputs
=> With or without this patch you get a useful message
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Marcel de Rooy [Fri, 20 Jan 2017 12:51:29 +0000 (13:51 +0100)]
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>
Marcel de Rooy [Tue, 17 Jan 2017 10:59:53 +0000 (11:59 +0100)]
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>
Marcel de Rooy [Tue, 17 Jan 2017 10:37:55 +0000 (11:37 +0100)]
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>
Marcel de Rooy [Tue, 17 Jan 2017 09:09:40 +0000 (10:09 +0100)]
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>
Marcel de Rooy [Tue, 17 Jan 2017 08:32:44 +0000 (09:32 +0100)]
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>
Marcel de Rooy [Mon, 16 Jan 2017 13:54:37 +0000 (14:54 +0100)]
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>
mbeaulieu [Mon, 18 Aug 2014 14:04:55 +0000 (10:04 -0400)]
Bug 17913: Adjust merge test for AuthorityMergeMode
Original fix from a patch on bug 11315.
Amended by Marcel de Rooy January 2017.
Test plan:
Run t/db_dependent/Authorities/Merge.t in both loose and strict mode.
Should no longer make a difference.
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>
Frédérick [Wed, 27 Nov 2013 17:30:55 +0000 (12:30 -0500)]
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>
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>
Original patch from bug 5572, dating back to 2011!
Amended by Marcel de Rooy January 2017. Renamed the pref.
The fix on this report is based on this preference.
Depending on the pref, subfields will be deleted or kept.
Test plan:
Run the dbrev.
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>
Marcel de Rooy [Fri, 20 Jan 2017 10:29:30 +0000 (11:29 +0100)]
Bug 17909: [Follow-up] Quick fix for UNIMARC
UNIMARC inserts field 100. This interferes the field count and order
in the test.
Note: This is a quick fix. Will polish it after bug 17913.
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>
Josef Moravec [Mon, 16 Jan 2017 13:03:21 +0000 (13:03 +0000)]
Bug 17909: Followup - fix typos
Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
EDIT:
Adjusted three small typos that did not disturb the test in its current
form, but do when we are fixing bugs on bug 17913.
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>
Marcel de Rooy [Mon, 16 Jan 2017 11:15:56 +0000 (12:15 +0100)]
Bug 17909: Additional polishing
No spectacular things:
[1] Move the framework modifications to a sub. Use same style to create auth types and linked fields.
[2] Change some new Object occurrences to Object->new.
[3] Add tests for field count and field order in the first two subsets.
[4] Few whitespace changes (sorry) and comment lines.
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>
Marcel de Rooy [Wed, 4 Jan 2017 10:15:51 +0000 (11:15 +0100)]
Bug 17909: Add tests for merging with another authtype
Originally aimed for bug 9988. Adjusted in line with other subtests.
Will polish the three subtests a little more on a follow-up.
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>
mbeaulieu [Mon, 18 Aug 2014 14:04:55 +0000 (10:04 -0400)]
Bug 17909: Adding tests from bug 11315
Based on original patch from Maxime Beaulieu on bug 11315.
Amended by Marcel de Rooy on report 17909.
EDIT:
Original tests have been adjusted in view of:
[1] Test on bug 11315 heavily leaned on DBD::Mock. Since we are
using Test::DBIx::Class on such tests now, this would need attention.
Moreover, the advantage of mocking the database in this case is at
least arguable.
[2] Matching the first (somewhat older) subtest of 11700.
[3] Simplification and readability.
Look e.g. at the use of $MARCto and $MARCfrom on 11315.
This made me merge them in the db_dependent counterpart.
Also note that this subtest adds another needed test case: the merge from
auth1 to modified auth1, while 11700 tested auth1 to auth2.
Test plan:
Just 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>
Julian Maurice [Wed, 19 Feb 2014 10:31:13 +0000 (11:31 +0100)]
Bug 17909: Add unit tests for C4::AuthoritiesMarc::merge
Original patch from Julian Maurice on bug 11700.
With sign offs by: Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Amended by Marcel de Rooy on report 17909.
EDIT (January 2017):
Removed some tests not related to merge.
Put remaining tests in a subtest, made them working on current merge.
Slightly revised the mocking.
Note: I plan to move the zebra retrieval stuff outside merge in one of
the next stages, and replace it by calling Koha::SearchEngine. This will
reduce mocking complexity here.
Test plan:
Just 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>
David Cook [Thu, 12 Jan 2017 00:15:22 +0000 (11:15 +1100)]
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>
David Cook [Thu, 12 Jan 2017 00:14:34 +0000 (11:14 +1100)]
Bug 17880 - Add test to check version number comparison
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>
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>
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>
projects / koha.git / log