Commit graph

6372 commits

Author SHA1 Message Date
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
7200710045 Bug 17257: Fix add/edit patrons under MySQL 5.7
If no guarantor is defined the patron won't be added/modified and an
error will be raised:

DBD::mysql::st execute failed: Incorrect integer value: '' for column
'guarantorid'

Test plan:
Using MySQL 5.7 (and/or sql_mode=STRICT_TRANS_TABLES)
Create a patron without guarantor

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>

Tested with MariaDB 10.0.30 (sql_mode='STRICT_TRANS_TABLES')
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-05-12 08:59:44 -04:00
Alex Arnaud
a08b309c4f Bug 12063 - Remove checking of ExpireReservesMaxPickUpDelay in CancelExpiredReserves(). Koha::Hold::set_waiting calculate expiration date from today instead of hold's waiting date.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-05-09 08:59:40 -04:00
Alex Arnaud
26634151db Bug 12063 - Fix QA failures
- Remove expiration date calculation in C4::Letter since it's done
    when setting the reserve waiting,
  - remove expiration date calculation in circ/waitingreserves.pl. Use
    the one in DB,
  - add a new atomic update that calculate expiration date for
    waiting reserves,
  - add tests for days_foward function and fix the infinite loop.

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-05-09 08:59:39 -04:00
Alex Arnaud
7cf3c12f5b Bug 12063: Change date calculation for reserve expiration to skip all holiday
This patch makes koha automatically set expiration date when reserves become
waitting. Also it adds a new syspref "ExcludeHolidaysFromMaxPickUpDelay" that allows to
take holidays into account while calculating expiration date.

Test plan:

  - Install this patch and run updatedatabase.pl script,
  - allow ExpireReservesMaxPickUpDelay in system preferences,
  - set ReservesMaxPickUpDelay to 5.

  - Place an hold on a checked out item and check in this item:
    The hold's expiration date should be today + 5.

  - Allow ExcludeHolidaysFromMaxPickUpDelay in system preferences,
  - add holiday during this pickup delay period,
  - Create a new hold and make it comes waitting:
    The hold's expiration date should be today + 5 + number of closed
    day(s).

Also:
  - Check that ExpireReservesOnHolidays syspref works again
    without ExcludeHolidaysFromMaxPickUpDelay.
  - Check that cancel fees apply again if wanted.

Signed-off-by: sonia BOUIS <sonia.bouis@univ-lyon3.fr>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-05-09 08:59:39 -04:00
a900a434d5 Bug 17190 - Marked REST API as mandatory (require = 1) in PerlDependencies.pm
For testing I went in Staff client to Home > About Koha > tab  Perl modules
and verified that both Mojolicious (6.0) and Swagger2 (0.59) are marked
as mandatory.
Signed-off-by: Marc Véron <veron@veron.ch>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-05-08 09:06:30 -04:00
e96f39ab52 Bug 7550: [QA Follow-up] Resolve param warning from sco-patron-image
Resolve this warning:
  CGI::param called in list context from package C4::Service line 212, this can lead to vulnerabilities. See the warning in "Fetching the value or values of a single named parameter" at /usr/share/perl5/CGI.pm line 436.

It comes from the require_params call in sco-patron-image.pl.

Git grepping on require_params tells me this:
  members/default_messageprefs.pl:my ($categorycode) = C4::Service->require_params('categorycode');
  opac/sco/sco-patron-image.pl:my ($borrowernumber) = C4::Service->require_params('borrowernumber');
  opac/sco/sco-patron-image.pl:my ($csrf_token) = C4::Service->require_params('csrf_token');
  svc/cataloguing/metasearch:my ( $query_string, $servers ) = C4::Service->require_params( 'q', 'servers' );

The only candidate for multi_param seems to be 'servers', but as we can see
this variable is a scalar. Additional servers returned by require_params are
lost. This should be solved on its own report.
So, we can safely add scalar to the params call, resolve the warning and
keep the same behavior.

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-05-08 09:00:26 -04:00
Stephane Delaune
bf551a0722 Bug 18152 : fix unimarc label in SetMarcUnicodeFlag
The standard UNIMARC requires than the 9th character (starting from 0) in
labels must be blank (while it may be 'a' in marc21)

the problem is that C4::Charset::SetMarcUnicodeFlag (called in particular when
we import a record) always add 'a' char in the 9th label'pos whereas it should
do it just for MARC21 and NORMARC (not for UNIMARC) :

C4::Charset::SetMarcUnicodeFlag add 'a' char in the 9th label character for
MARC21 and NORMARC (it's normal), but just before doing this it call
"$marc_record->encoding('UTF-8')" which is a MARC::Record function which, when
called with 'UTF-8' parameter, do only one thing : add 'a' char in the 9th
label character

This patch only removes this incorrect function call, so, when we import a bib
record in UNIMARC : it no longer adds erroneous character (this does not change
anything for MARC21 and NORMARC because SetMarcUnicodeFlag explicitly adds 'a'
char in the 9th label for them)

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-05-08 08:36:57 -04:00