Commit graph

6379 commits

Author SHA1 Message Date
Julian Maurice
c69e02c441 Bug 18782: Remove unused C4::Serials::getsupplierbyserialid
TEST PLAN
----------
git grep -i getsupplierby
-- only the code removed and the test tweaked
git bz apply 18782
sudo koha-shell -c bash kohadev
prove -v t/db_dependent/Serials.t
qa -v 2 c 1
exit
-- sign off

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
2017-07-05 13:41:47 -03:00
905572910b Bug 18651: Do no LOCK/UNLOCK the table
We cannot LOCK the old_issues table here, other tables are accessed and DBIx::Class rename it with "me":
DBD::mysql::st execute failed: Table 'me' was not locked with LOCK
TABLES [for Statement "SELECT `me`.`issue_id`, `me`.`borrowernumber`,
`me`.`itemnumber`, `me`.`date_due`, `me`.`branchcode`,
`me`.`returndate`, `me`.`lastreneweddate`, `me`.`renewals`,
`me`.`auto_renew`, `me`.`auto_renew_error`, `me`.`timestamp`,
`me`.`issuedate`, `me`.`onsite_checkout`, `me`.`note`, `me`.`notedate`
FROM `old_issues` `me` WHERE ( `me`.`issue_id` = ? )" with ParamValues:
0='2'] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1832.

Consequence: We could have a checkin refused if there is a race, but
this is the simplest and safest way to fix it.
2017-06-21 14:14:09 -03:00
ee7969d346 Bug 18651: [QA Follow-up] Fix the MAX(issue_id) calculation
Found this by inserting the same issue_id in old_issues before checkin:
The call to ->search( )->get_column is in scalar context and will
return the number of results, i.e. always 1.
If you have an issue_id 2 in old_issues, it will crash:
    DBIx::Class::Storage::DBI::_dbh_execute(): Duplicate entry '2' for key 'PRIMARY'

The fix is fairly simple: Put get_column in list context and pick the first
array entry.
NOTE: Using DBIx's get_column()->max here might look simpler here.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
2017-06-20 14:29:22 -03:00
4906d58f7c Bug 18651: [QA Follow-up] Remove unused variable
Variable $original_issue_id is not used. The id is retrieved later from
$issue when updating accountlines.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
2017-06-20 14:29:21 -03:00
26957fea0d Bug 18651: Limit the life span of the LOCK
We only need the table to be locked for the fetch, increment, store sequence

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
2017-06-20 14:29:21 -03:00
59fcad685e Bug 18651: Use a READ and WRITE LOCK
For more info, see:
  commit be156d9ad9
    Bug 15854: Use a READ and WRITE LOCK on message_queue
and
  commit b40456f7dd
    Bug 18364: Do not LOCK/UNLOCK tables from tests

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
2017-06-20 14:29:21 -03:00
79cb157b3b Bug 18651: Copy the row before modify the id
If the "max(issue_id) from old_issue + 1" already exists in issues, the
move fails.

For instance we have
1, 2, 3, 4 in issues

checkin 4
1, 2, 3 in issues (AI=5)
4 in old_issues

Restart mysql => AI is reset to MAX(issue_id) => 4

checkout a new one
1, 2, 3, 4 in issues (AI=5)
4 in old_issues

checkin 4 (will get id 5 in old_issues)
1, 2, 3 in issues (AI=5)
4, 5 in old_issues

=> This works with and without this patch

Now we have
1, 2, 3 in issues (AI=5)
4, 5 in old_issues

Restart mysql => AI is reset to MAX(issue_id) => 4

checkout 2 new ones
1, 2, 3, 4, 5 in issues (AI=7)
4, 5 in old_issues

checkin 4 (4 becomes 6 in old_issues)
1, 2, 3, 5 in issues (AI=6)
4, 5, 6 in old_issues

=> This did not work without with patch
The update of the issue_id was made before the move (so in the issues
table), the PK did not allow it

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
2017-06-20 14:29:21 -03:00
be3d39c8b1 Bug 18651: Update accountlines.issue_id is the issue_id has been changed during the move
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
2017-06-20 14:29:21 -03:00
acb7a71748 Bug 18651: Do not charge if the checkin failed
2. If the move fails for whatever reason (see
https://lists.katipo.co.nz/pipermail/koha/2017-May/048045.html for an
example), fines can be charged. It should not

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
2017-06-20 14:29:21 -03:00
c3c9d61570 Bug 18651: Update issue_id in AddReturn
1. AddReturn returns a $issue hashref with the old issue_id value
=> At first glance it does not affect anything, but would be good to fix
it for future uses.

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
2017-06-20 14:29:21 -03:00
4e9701c36a Bug 18697: Final polishing
GetFictiveIssueNumber:
Returns undef instead of 0 for irregular frequencies. Also added to POD.
Removed unused variable $wkno.
Adding a return makes the if(unit) unneeded.
Replaced (a+b)/b by 1+a/b.

_delta_units:
Added a comment about its parameters.

GetFictiveIssueNumber.t:
Adjusted the tests for irregular frequencies accordingly.

Test plan:
[1] Run t/db_dependent/Serials/GetFictiveIssueNumber.t
[2] Run t/db_dependent/Serials/GetNextDate.t

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

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-06-19 15:35:51 -03:00
f65ca90b0f Bug 18697: Fix date calculation for dayly frequencies in Serials
The changes in _get_next_date_day are actually only cosmetic. The sub
now reads exactly the same as its counterparts for other units, but
the results are exactly the same as before.

In GetFictiveIssueNumber we now call _delta_units for each type of unit.
The two Delta_Days calls are moved to _delta_units. Note that this also
is a cosmetic change; results should be exactly the same.

Test plan:
[1] Edit a subscription. Test predication pattern for some daily freq.
[2] Run t/db_dependent/GetNextDate.t

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

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-06-19 15:35:51 -03:00
ffb1c87d29 Bug 18697: Fix date calculations for weekly frequencies in Serials
Same solution applied as in bug 18356/18607. Consistency++

The code in _get_next_date_week is again very similar to the code in
_get_next_date_month or _get_next_date_year. I will not merge them here,
but we could consider that in the future.

Code in GetFictiveIssueNo has been adjusted similarly to month and year.

Test plan:
[1] Do not apply this patch. Create a subscription for 3/week.
    When the first issue date is on a Saturday or Sunday, the
    intervals in the prediction pattern are 0,0,7,0,0,7,etc.
    Starting on Wed-Fri 1,1,5,etc. Starting on Mon-Tue 2,2,3,etc.
[2] Apply this patch. Check again.
    The interval should be always 2,2,3 now and no longer depend on the
    day_of_week of first issue date.
[3] Check another weekly frequency with multiple units per issue.
    Say 1 issue/3 weeks.

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

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-06-19 15:35:50 -03:00
adfcd8d79c Bug 18607: Fix date calculations for monthly frequencies in Serials
Similarly to the solution of bug 18356, this patch fixes the date
calculation for monthly frequencies.

The calculation in GetFictiveIssueNumber now makes use of the new
_delta_units sub introduced on bug 18356.

The calculation in _get_next_date_month is also very similar to the one
in _get_next_date_year. I do not merge them here, but this could still
be considered later on. At least consistency is achieved now between
both routines. The connection with firstacquidate has been cut thru
just like for year units.

Test plan:
[1] Without this patch, look at the prediction pattern for a
    subscription with first issue on Feb 21 and 5 per month. The first
    issues will be 21, 22, 23, 24, 25. Then jumping to 21, 23, 25, etc.
[2] Apply the patch. Look at the same prediction pattern. You will now
    see 6 day intervals and a new cycle starting on the 21st.
    So Feb 21, 27, Mar 5, 11, 17 and Mar 21, 27, etc.
[3] Edit an subscription. Try a few other monthly frequencies.
[4] The next patch adjusts related unit tests.

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

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-06-19 15:35:50 -03:00
d71fb0e17c Bug 18356: Fix date calculations for yearly frequencies in Serials
The problem as described on BZ 18356 is a combination of an error in
GetFictiveIssueNumber and GetNextDate for unit==year.

[1] In GetNextDate the Add_Delta_YM calculation should be applied only to
frequencies based on years per unit.
In the case of multiple units per year we calculate the number of days to
add. And if we have reached the end of a cycle, we correct the
rounding applied in the cycle.
NOTE 1: We obsolete the idea here of rebasing dates on firstacqui. In case
of manual adjustments, we probably do not want it. And otherwise we do not
need it anymore due to the correction at the end of a cycle.
NOTE 2: The calls to Add_Delta_YM are intentionally not corrected for leap
years. Say you start at 2016-02-29. If you use 1/yr or 1/2yr, you will
switch to the Feb 28th in the following years. In 2020 there will be no
switch to Feb 29 again; if someone should need it, please use a manual
adjustment. This is probably highly exceptional.

[2] In GetFictiveIssueNumber the year should be decreased by one if you
have more units per year and you did not yet reach firstacqui day and
month. This affects calculations in GetNextDate with irregularities.
NOTE 1: I added a wrapper around Date::Calc::N_Delta_YMD in order to improve
its results; this will especially be needed when we use it later for
month units.
NOTE 2: In case of manual adjustments this calculation does not really make
sense. Another report should deal with improving irregularities.

Test plan:
[1] Verify that both GetNextDate.t as well as GetFictiveIssueNumber.t pass.
[2] Look at the prediction pattern for a few frequencies.
    For example: 1 iss/y, 1 iss/2y, 5 iss/y.

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

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-06-19 15:35:45 -03:00
8218f2b55c Bug 17975: Let C4::Letters manage today param substitution
The today parameter is properly handled from C4::Letters subroutines, we
do not need to pass it from callers.

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-06-15 15:56:00 -03:00
b28d25e9cd Bug 17975: TT syntax for notices - Prove that HOLD_SLIP is compatible
Here we need to test <<today>>.
We already pass a value, but it was wrong. We must pass a string, not a
DateTime object, otherwise the KohaDates plugin will not display the
hours part if we need it.

Test plan:
Define a HOLD_SLIP notice template to match your need.
Do not forget to use
  [% today | $KohaDates %]
or
  [% today | $KohaDates with_hours => 1 %]
To access data from the reserves table, use the 'hold' variable

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-06-15 15:56:00 -03:00
a740e773c6 Bug 17965: TT syntax for notices - Prove that DUEDGST is compatible
This notice template have the particular feature of using <<count>>.
This value is substitued during the process of the notice template.
For the TT syntax, all what we need is to send the values to substitute to the
template.

Note that items.content can also be used in these template, you can have
a look at bug 17967 to see a better alternative to this marker.

Test plan:
Generate DUEDGST and DUE notice messages.
You should be able to generate the same messages with the TT syntax.

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-06-15 15:56:00 -03:00
6906b848a7 Bug 17710 - C4::Matcher::get_matches and C4::ImportBatch::GetBestRecordMatch should use same logic
C4::ImportBatch::GetBestRecordMatch uses SQL to sort by score descending
then candidate_match_id descending. With C4::Matcher::get_matches, I
implement the same sort but use Perl code to do it, since we're sorting
search results.

It's a simple change, but it's in a big block of code, so I don't have
unit tests.

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-06-15 15:27:46 -03:00
Mark Tompsett
f97addef42 Bug 18732: Noisy t/SMS.t triggered by koha_conf.xml without sms_send_config
Upgraded systems may be lacking sms_send_config which makes t/SMS.t noisy.
This silently bypasses the problem.

Remove sms_send_config from your koha-conf.xml file
prove t/SMS.t
-- it will be noisy, but pass.
apply patch
prove t/SMS.t
-- noise gone.
run koha qa test tools.

Signed-off-by: Lee Jamison <ldjamison@marywood.edu>
Works correctly as indicated by the testing plan.
No "uninitialized" noise after patch is applied.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Slightly amended: turned the iif around.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-06-15 15:27:46 -03:00
e0444e8600 Bug 18601: OAI/Sets.t mangles data due to truncate in ModOAISetsBiblios
This patch replaces the TRUNCATE statement in ModOAISetsBiblios by a
DELETE statement. A truncate will cause an implicit commit and will
therefore commit the transaction started in the test script.

Also simplifying the module load in the test script.

Test plan:
Do not apply this patch and observe that biblio records are added to your
database by running t/db_dependent/OAI/Sets.t.
Apply this patch, run the test again and verify that it does no longer
add records to your biblio table.

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-06-13 16:18:59 -03:00
Lee Jamison
a80ed84d07 Bug 18584: Fix trailing spaces in C4/Accounts.pm
Removed trailing spaces at line 182 of C4::Accounts.

Test plan:
1. Edit C4/Accounts.pm and verify trailing spaces
2. Apply patch
3. Verify that trailing spaces in C4/Accounts have been removed

Signed-off-by: Lee Jamison <ldjamison@marywood.edu>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-06-12 17:57:20 -03:00
88e50b6a68 Bug 18296: C4::Items - Remove GetItemInfosOf
At this point this subroutine is only used once, from reserve/request.pl.
Since we already have the items, it's easy to populate the different
hashes as the rest of the code is expecting it.

Test plan:
You need to create analytical record relationships (
EasyAnalyticalRecords needs to be set). Link an item to a biblio using
the 'Edit > Link to host item' menu from the biblio detail page.
From the staff interface place a hold on the biblio. You should see the
items from the biblio and the one you just linked

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
2017-06-07 11:30:25 -03:00
e3519f1538 Bug 8612: Remove warnings from tests
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-06-05 12:36:12 -03:00
9ca8275862 Bug 8612: [Follow-up] Fix regular expression
Fix regular expression to do what is described in the comment
Make header in CSV profile definition optional
Strip white chars from csv profile definition

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-06-05 12:02:08 -03:00
Blou
3c83e11786 Bug 8612: Use CSV profile for exporting basket
This patch allows the user to use a CSV export profile to create the fields to export the basket as CSV in a basket page.

Test plan:
1) Apply the patch
2) Go to Tools › CSV export profiles and create a profile of type "SQL for basket export in acquisition"
  example:
  biblionumber=biblio.biblionumber|auteur=biblio.author|titre=biblio.title|date=biblioitems.copyrightdate|editeur=biblioitems.publishercode|isbn=biblioitems.isbn|quantite=aqorders.quantity|prix=aqorders.rrp|panier=aqorders.basketno
3) In acquisition module, create a new basket and add an order to the basket
4) On basket detail page, there should be the split button labelled "Export to CSV"
5) Try to use the button and export CSV with your CSV profile you defined in step 2
6) Validate the CSV file.
7) Repeat 4-6 with a closed basket.
    a) close the basket
    b) View the basket
    c) validate that there is an export button
    d) test it with an export
8) prove t/db_dependent/Acquisition/GetBasketAsCSV.t t/db_dependent/Koha/CsvProfiles.t

Initial work:

Sponsored by: CCSR

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-06-05 12:02:08 -03:00
Fridolyn SOMERS
c59e395b74 Bug 11122 - publisher code and publication year not fetched in acq orders
In acquisition, several templates try to display publisher code and publication year : invoice.tt, parcel.tt, transferorder.tt.
Thoses pages use C4::Acquisition methods GetPendingOrders or GetInvoiceDetails.
The bug is that in the SQL query of those methods, biblioitems.publishercode and biblioitems.publicationyear.
In uncertainprice.pl those datas are fetch using GetBiblioData.
It whould be better to fetch them in GetPendingOrders and GetInvoiceDetails.

This patch changes SQL queries to fetch wanted datas : aqorders.*,biblio.title,biblio.author,biblioitems.isbn,biblioitems.publishercode,biblioitems.publicationyear. GetInvoiceDetails also needs : biblio.seriestitle,biblioitems.volume.
This patch also unifies the way biblio datas are displayed :
  <a href="link to catalog using biblionumber">[title]</a> <em>by</em> [author] &ndash; [isbn]
  <em>Publisher:</em> [publishercode], [publicationyear]

Test plan :
- Choose a biblio record containing a data in :
    biblio.title,
    biblio.author,
    biblioitems.isbn,
    biblioitems.publishercode,
    biblioitems.publicationyear,
    biblio.seriestitle,
    biblioitems.volume.
- Create an order using this biblio.
- Look at this order in pages : parcel.pl, transferorder.pl, uncertainprice.pl
=> You see publisher code and publication year
- Look at this order in page : invoice.pl
=> You see publisher code, publication year, series title and volume

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

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-06-05 11:48:16 -03:00
cf5a1d5e59 Bug 18279: [QA Follow-up] Correct @EXPORT
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-06-05 11:43:26 -03:00
8e7718ee65 Bug 18279: Remove C4::Items::GetLostItems
The JOIN done by this subroutine are not always useful (depending on
item-level_itypes). They also search with LIKE when it is not needed.

Since we have now Koha::Items, we can replace this subroutine with a
call to Koha::Items->search with the correct parameters.

A change in previous behaviours can happen: If a items.itemlost contains
a value that is not defined as a LOST authorised value, the item will
not be displayed. I think it's the expected behaviour, even if it should
not happen in correctly configured installations.

Test plan:
To test with item-level_itypes set to item and biblio:
List the lost items you have on your system, using the different
filters available.
The result table should contain the correct item's info.

Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-06-05 11:43:26 -03:00
f466a856a9 Bug 18295: C4::Items - Remove get_itemnumbers_of
The code from scripts and subroutines using this subroutine was not very
elegant. Most of the time the code was unnecessarily complex.
This patch removes this subroutine and adapt the code to use
Koha::Items instead.

1. C4::Items::get_hostitemnumbers_of
I did not understand why the code was so complicated, it seems that we
only want to know if a given item has a given biblionumber
2. cataloguing/merge.pl
We want to retrieve the itemnumber for a given biblio.
We could also have done that with:
  Koha::Biblios->find( $biblionumber )->items;
3. labels/label-item-search.pl
We want to loop over the items for a given biblio, no need to use
get_itemnumbers_of and GetItemInfosOf.
We just need to use:
  Koha::Items->search({ biblionumber => $biblionumber })
4. reserve/request.pl
We want to retrieve the itemnumbers of the biblio's items
We could also have done that with:
  Koha::Biblios->find( $biblionumber )->items->get_column('itemnumber');

Test plan:
1.You need to create analytical record relationships (
EasyAnalyticalRecords needs to be set). Link an item to a biblio using
the 'Edit > Link to host item' menu from the biblio detail page.
From the staff interface place a hold on the biblio. You should see the
items from the biblio and the one you just linked
2. Merge two bibliographic records (with items), the resulting record
should contain items from both original records
3. Create a new label batch, edit it.
Add items to this batch ('Add items' button).
Fill the input with a barcode.
You should see all the items of a biblio.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-06-05 11:43:16 -03:00
e0acd9bdf8 Bug 18278: C4::Items - Remove GetItemLocation
This subroutine is no longer in used.
It was previously call from serials/serials-recieve.pl, which was not used and has been removed by
    commit 65b7ad030c
    Bug 13423: Remove unused serials-recieve

Test plan:
  git grep GetItemLocation

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-06-05 11:38:33 -03:00
Mark Tompsett
8ed599113c Bug 5395: Update C4::Acquisition::SearchOrders POD
Comparing the perldoc to the function:
- basketname
- basketgroupname
- budget_id
Were missing. This adds them.

Also, a description of ordernumber and search were
added, as they are not self-evident by their name
alone.

There are no code changes, so all tests should pass
or fail identically before and after patch.

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-05-28 22:31:21 -04:00
59b58513fe Bug 18664: Make IssueSlip returns if params are not valid
Problem raised by bug 17762: IssueSlip should return if the params are
not valid.
The tests contain 2 FIXME to highlight this problem already, it's time
to fix them.

Note that, theoretically, this change may produce software error. Indeed
the caller expecting a hashref (letter) will access the "content" key.
But that should not happen.

Test plan:
Tests must return green

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

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-05-28 22:26:23 -04:00
b37df275b8 Bug 18611: [QA Follow-up] Make SQL query more readable
Make it more explicit by adding join statements.

Test plan:
See next patch for adding a unit test.

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-05-28 22:23:34 -04:00
092f02d227 Bug 18611 - Followup, remove tabs to make qa tools happy
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-05-28 22:23:34 -04:00
7249351f12 Bug 18611 - Create labels action fails in manage-marc-import.pl if an item has been deleted from the import batch.
To test:
1 - Import a batch with some items
2 - Delete one of the imported items
3 - Browse to Tools->Staged MARC record management
4 - Click (Create label batch) for the batch you imported
5 - Recieve an error
6 - Apply patch
7 - Click (Create label batch)
8 - Batch is created with remaning items from the import

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

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-05-28 22:23:34 -04:00
8f726ae06d Bug 18478 - QA Followup
Make sure to build necessary letters
Fix awkward construction

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-05-28 22:17:13 -04:00
4fa3df9462 Bug 18478 - Some notices sent via SMS gateway fail
It seems that for HOLD and DUE (and maybe more) notices we rely on
C4::Letters::SendQueuedMessages
to populate the correct address.

This patch adjust that subroutine to correctly populate the field and/or
fail messages if no SMS provider available

To test:
 1 - Define a messaging prefs for a patron to recieve hold notices via
 SMS
 2 - Ensure you have defined an SMS message for 'HOLD' letter
 3 - Set an SMS alert number for patron
 4 - Set the SMS::Send driver to 'Email'
 5 - Fill a hold for the patron
 6 - Check the db and note the address is null
 7 - run process_message_queue.pl
 8 - Check db - address is null and message pending
 9 - Apply patch
10 - run process_message_queue
11 - Message to_address should be populated and message sent

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-05-28 22:17:13 -04:00
f22d2e7200 Bug 17898: Automagically convert SQL reports
Bug 17196 move the marcxml out of the biblioitems table.
That will break SQL reports using it.
It would be handy to propose an automagically way to convert the SQL
reports.

We do not want to update the reports automatically without user inputs,
it will be too hasardous.
However we can lead the user to convert them.

In this patchset I suggest to warn the user if a report is subject to be
updated.

TODO: Add a way to mark this job done (using a pref?) to remove the
check and not to display false positives.

Test plan:
- Create some SQL reports (see https://wiki.koha-community.org/wiki/SQL_Reports_Library)
- Go on the report list page (/reports/guided_reports.pl?phase=Use saved)
- For the reports using biblioitems.marcxml you will see a new column
warning you that it is obsolete
- Click on update link
=> that will open a modal with the converted SQL query
- Click on the update button
=> you will be informed that the query has been updated

If all the reports are updated, the new column "Update" will no longer
be displayed.

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-05-19 18:48:26 +00:00
c9de665c29 Bug 18578: Use subdirectory in /tmp for session storage during installation
Apply the change from bug 15553 to InstallAuth.pm too.

Test plan:
[1] Remove all cgisess_* files from your /tmp directory.
[2] Remove directory /tmp/cgisess_koha_[your instance], if there.
[3] Run the webinstaller
    /cgi-bin/koha/installer/install.pl?step=1&op=updatestructure
[4] Check if you have cgisess_ files in /tmp/cgisess_koha_[your instance].

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

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-05-19 10:46:37 -04:00
37de20b236 Bug 17665 - SIP2 Item Information Response returns incorrect circulation status of '08' ( waiting on hold shelf ) if record has any holds
If a record has any holds on it, the SIP2 item information response will
return a value of 08 "waiting on hold shelf" even if the item is not
actually a waiting hold. This is clearly a bug.

Test Plan:
1) Find an item that is not a waiting hold, but whose record has one or
   more holds.
2) Issue a SIP2 item information request
3) Note in the response, the circulation status field is '08'
4) Apply this patch
5) Repeat the item informationr request
6) Note the code is now '03' ( available )
7) Check the item in to fill the hold
8) Repeat the item information request
9) Verify the circulation status is now '08'

Followed test plan, works as expected
Signed-off-by: Marc Véron <veron@veron.ch>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-05-19 10:30:45 -04:00
Oliver Bock
4c631a0824 Bug 14625 - LDAP: skip extended patron attributes in 'borrowers' attribute update
* Any extended patron attributes will cause the update to fail as those attributes don't exist in the 'borrowers' table
* The update of the extended patron attributes is already dealt with in checkpw_ldap()
* Ergo: just skip those attributes here

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
I did not test this patch but code looks good

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-05-19 09:52:05 -04:00
a5e84d45c0 Bug 18314: Fix reset number of login attempts on login success
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-05-12 10:59:00 -04:00
cfc484b173 Bug 18314: Account lockout
To prevent brute force attacks on Koha accounts, staff and opac, we need to
implement an account lockout process to Koha.

After a number of failed login attempts a users account would become locked.
The user would then need to use the reset password functionality to send a reset
token to their email account. After a successful password reset the lockout flag
would be removed.

The number of failed login attempts before lockout is configurable using a new
system preference 'FailedLoginAttempts'.

How does it work?
When a patron enter an invalid password, the borrowers.login_attempts value
for this patron is incremented. When this value reach the value of the
pref FailedLoginAttempts, the password comparison is not done and the
authentication is rejected.
This login_attempts field is reset when a patron correctly logs in. When
the account is locked the patron has to reset his/her password using
the OpacResetPassword feature or ask a staff member to generate a new
password.
If the pref is not set (0, or '') the feature is considered as disabled,
but the failed login attempts are stored anyway.

Test plan:
0/ Apply patch and execute the update DB entry
1/ Switch on the feature by setting FailedLoginAttempts to 3
2/ Use an invalid password to login at the staff or OPAC interface
3/ After the third consecutive failures, you will be asked to reset your
password if OpacResetPassword is set, or contact a staff member
4/ Switch on OpacResetPassword and reset your password
5/ Confirm that you are able to login
6/ Play with the different combinations

QA details: The trick happens in C4::Auth::checkpw, to make things clear
I had to create a return value (note the awesome name: @return) and
replace the 3 successives if statements with elsif. Indeed if one of
the condition is reached, it will return inside the given block.

Signed-off-by: Jonathan Field <jonathan.field@ptfs-europe.com>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-05-12 10:58:44 -04:00
d08a0bc685 Bug 15582: Ability to block auto renewals if OPACFineNoRenewals is reached
If a patron owes more than the OPACFineNoRenewals value, the issue won't
be auto renewed anymore (driven by the new pref OPACFineNoRenewalsBlockAutoRenew).

Test plan:
Note: You will have to manually change data in your DB, make sure you
have access to the sql cli.
1/ Set the OPACFineNoRenewals to 5 (for instance)
2/ Set OPACFineNoRenewalsBlockAutoRenew to block
3/ Check an item out to a patron and mark is as an auto renewal
4/ Make sure the patron does not have any fees or charges.
5/ Execute the automatic renewals cronjob script (misc/cronjobs/automatic_renewals.pl)
Confirm that the issue has been renewed
6/ Create an invoice for this patron with a amount > OPACFineNoRenewals (6
for instance)
7/ Execute the automatic renewals cronjob script (misc/cronjobs/automatic_renewals.pl)
Confirm that the issue has not been renewed.
8/ Set OPACFineNoRenewalsBlockAutoRenew to allow
9/ Execute the automatic renewals cronjob script (misc/cronjobs/automatic_renewals.pl)
Confirm that the issue has been renewed

Sponsored-by: University of the Arts London
Signed-off-by: Jonathan Field <jonathan.field@ptfs-europe.com>
Signed-off-by: Janet McGowan <janet.mcgowan@ptfs-europe.com>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-05-09 21:05:29 +00:00
c33b3b1342 Bug 17762 (QA Followup) Use default lang if pref disabled
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-05-09 20:56:43 +00:00
b4254a7ce3 Bug 17762: Fix test for NewsChannels.t
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-05-09 20:56:42 +00:00
c49bdc3a8d Bug 17762: Add the lang parameter to C4::Letters::getletter
Sponsored-by: Orex Digital

Signed-off-by: Hugo Agud <hagud@orex.es>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-05-09 20:56:42 +00:00
f7b11f38e8 Bug 17762: Send lang to GetPreparedLetter
This patch set the lang parameter when C4::Letters::GetPreparedLetter is
called to generate the notice.
Note that we do not need to pass it if want_librarian is set.

TODO: I do not know what to do with TransferSlip

Sponsored-by: Orex Digital

Signed-off-by: Hugo Agud <hagud@orex.es>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-05-09 20:56:41 +00:00
c99fc9d7c2 Bug 17762: Update the letter form interface
If the pref is on, the notice template will be translatable in different
languages

Sponsored-by: Orex Digital

Signed-off-by: Hugo Agud <hagud@orex.es>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-05-09 20:56:41 +00:00