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>
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.
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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] – [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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
* 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>
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>
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>
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>
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>
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>
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>
- 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>
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>
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>
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>
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>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Test plan:
1. Drop and recreate your db
2. Clear memcached
3. Go through the installer (to speed up this test plan install all
sample data so you dont have to create libraries, patron categories etc. later)
4. On the installer page login as the database user and notice that it
does not work on the first attempt ( you get 'Error: You do not have
permission to access this page')
5. Try logging in as database user for a second time and notice you are
logged in successfully this time
4. In staff interface create a patron account with superlibrarian permissions
5. Logout of the staff interface
6. Login as database user
7. Notice you cant log in. You get the 'Error:: You do not have permission to access this
page' error
8. Try a second attempt and notice you get the same error
9. Open the URL in a new tab and notice the staff interface appears
showing that you are logged in
10. log out and log back in as the superlibrarian user you created and
notice it works on first login attempt
11. Apply patch
12. Log out and try logging back in as database user and notice that you
can login successfully on first attempt
13. Repeat steps 1,2,3 and login as database user and notice the login
works on first attempt
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch adds a new circulation rule (no_auto_renewal_after_hard_limit) to block/allow
auto renewals after a given date.
The idea is to stop renewals at a given date. That way the library will have
time to send overdues and get the books back before the students do on holiday.
Test plan:
0/ Execute the update DB entry
1/ Define a rule with no_auto_renewal_after_hard_limit set to tomorrow
2/ Modify the issues.issuedate, to simulate a checkout in the past:
UPDATE issues
SET issuedate = "yyyy-mm-dd hh:mm:ss"
WHERE itemnumber = YOUR_ITEMNUMBER;
with issuedate = 2 days before for instance
3/ Execute the automatic renewals cronjob script (misc/cronjobs/automatic_renewals.pl)
Confirm that the issue has been renewed
4/ Modify the no_auto_renewal_after_hard_limit and set it to yesterday
5/ Execute the automatic renewals cronjob script (misc/cronjobs/automatic_renewals.pl)
Confirm that the issue has not been renewed
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>
To reproduce:
Go to Home > Tools > Patron card creator
Click on 'Manage profiles'
Result:
Can't use string ("1") as an ARRAY ref while "strict refs" in use at /usr/share/kohaclone/C4/Creators/Lib.pm line 564.
Reason:
Select statment to get field 'template code' from table 'club_template_enrollment_fields' (!) instead of 'table creator_templates'.
To test:
Apply patch
Try to reproduce issue.
Amended because of typo (_ instead of -)
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This features would add the ability to create clubs which patrons may be
enrolled in. It would be particularly useful for tracking summer reading
programs, book clubs and other such clubs.
Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Ensure your staff user has the new 'Patron clubs' permissions
4) Under the tools menu, click the "Patron clubs" link
5) Create a new club template
* Here you can add fields that can be filled out at the time
a new club is created based on the template, or a new enrollment
is created for a given club based on the template.
6) Create a new club based on that template
7) Attempt to enroll a patron in that club
8) Create a club with email required set
9) Attempt to enroll a patron without an email address in that club
10) Create a club that is enrollable from the OPAC
11) Attempt to enroll a patron in that club
12) Attempt to cancel a club enrollment from the OPAC
13) Attempt to cancel a club enrollment from the staff interface
Followed test plan, works as expected.
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>
As per comment #7, this patch affects AddMember and ModMember.
The test plan should be the same as comment #6.
Secondary patch with tests still to come.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
If SMS via Email is enabled, and a patron has opted for SMS messages, but has not selected a service provider, the cronjob will die with the error
Can't call method "domain" on an undefined value at /usr/share/koha/lib/C4/Letters.pm line 1055.
This will cause all messages that come after the error to not be sent!
Test Plan:
1) Enable SMS via Email
2) Enable SMS for a patron, but don't set a provider
3) Perform an action that will trigger an sms message to go into
the holds queue ( item due, item checkout, etc )
4) Run process_message_queue.pl, note the error
5) Apply the patch
4) Run process_message_queue.pl, no error this time!
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
We certainly always want to retrieve the last row of a given search.
If it not sufficient we will need to rethink this code.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Using the KohaDates plugin will allow us to format dates as we want,
using the same filters as the ones defined in the KohaDates TT plugin.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
If it's a CHECKIN, C4::Circulation::SendCirculationAlert set a
"old_issues" key instead of "issues".
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Resolves:
Use of uninitialized value $payment_type_writeoff in string eq at C4/SIP/Sip/MsgType.pm line 1072, <STDIN> line 2.
Would be raised if this parameter is not found in SIPconfig.xml.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Many SIP2 services such as those by Comprise Technologies are able to or
require that an ILS be able to accept writeoffs via SIP2. The SIP2
protocol specifies that payment type be a two digit number, but does not
specify a code for writeoffs. To this end we should allow the write-off
code to be specified in the SIP2 config on a per-account basis so that
if different vendors use different fixed codes for write-offs we can
handle that gracefully.
Test Plan:
1) Apply this patch
2) Modify your SIP2 config to include
payment_type_writeoff="06"
in the login portion of the account you will be using for the test.
3) Restart your SIP2 server
4) Create a fee for a patron
5) Send a SIP2 fee paid message specifying the payment type code we
defined earlier, with a payment amount that is *not* equal to the
amount outstanding for the fee.
6) Note the fee paid response indicates the payment failed
7) Repeat step 5, but this time send the amount outstanding as the
payment amount
8) Note that the fee paid response indicates a successful payment
9) Note in Koha that the fee has been written off!
Signed-off-by: Rhonda Kuiper <kuiper@roundrocktexas.gov>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
When logging out from OPAC, plack-error.log log and/or opac-error.log
complain about 'uninitialized value $user' in C4/Auth.pm line 187. The
warning is not necessary, this patch removes it.
To test:
- try to trigger warning
- apply patch
- verify that warning no longer occurs
- prove t/db_dependent/Auth.t
- verifiy that SCO still behaves like before (especially if
you break out from sco path)
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>
Updating to use they/them and skipping the ones changed to it
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>
Comments throughout the Koha codebase assume that
all librarians or borrowers are male by using the
pronoun 'he' universally. This patch changes to
'he or she' / 'him or hers'.
Testing plan:
- ensuring modifying tests still pass:
+ C4/SIP/t/06patron_enable.t
+ t/db_dependent/Circulation.t
+ t/db_dependent/Koha/Patrons.t
+ t/db_dependent/Reserves.t
Sponsored-By: California College of the Arts
No code changes detected.
Signed-off-by: Marc Véron <veron@veron.ch>
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>
This small patch corrects the SIP2 checkin response if branch transfer is needed.
It depends on bug 7981 and also removes an obsolete alert check in SIP2.
Test plan:
- apply bug 7981
- make sure syspref AutomaticItemReturn is false
- check 'Return policy' in 'Default checkout, hold and return policy'
- make a SIP2 checkout and checkin from a branch different than item's home branch.
- verify that SIP2 response gives alert CV04 (=send to different branch)
- also verify that response field CT gives correct branch according to 'Return policy'
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
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>
The test in SendCirculationAlert is extended by adding an env var
called KOHA_NO_TABLE_LOCKS. If this var is set to a true value,
the table locking is skipped too.
This is useful when running a test without prove. The variable could be
set in a shell profile.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
From the MySQL doc:
"LOCK TABLES is not transaction-safe and implicitly commits any active transaction before attempting to lock the tables."
If the LOCK/UNLOCK statements are executed from tests, the current transaction will be committed.
To avoid that we need to guess if this code is execute from testsa or not (yes it is a bit hacky)
Better ideas are welcome!
Another fix would have been to revert
commit be156d9ad9
Bug 15854: Use a READ and WRITE LOCK on message_queue
but theorically a race is still possible.
Existing tests seem to be safe, to test this patch you will need new
tests from bug 17964.
Test plan:
prove t/db_dependent/Letters/TemplateToolkit.t
twice, and notice that changes have been comitted.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
If LocalHoldsPriority is enabled, and a record level request has more
than one item that could fill that hold, there is a possibility that a
holds queue row will be generated for the request for each of the
available items!
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Joel Sasse <jsasse@plumcreeklibrary.net>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Silly error here, the $switch_onsite_checkout is not correctly
calculated, it should not only be set to the value of the syspref, but
meet all the conditions.
Test plan:
Set a circ rule with number of on-site checkout = 2 and number of
checkouts = 2.
Switch on both ConsiderOnSiteCheckoutsAsNormalCheckouts and SwitchOnSiteCheckouts
Do a regular checkout
Do another regular checkout
At this point you have reach the maximum number of checkouts allowed.
Not try another one.
=> Without this patch you will not get the warning
=> With this patch you will see it
Followed test plan, works as expected.
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>
At this point the subroutine is no longer in used.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Intranet search results displays due date from item onloan.
This should use the TT date plugin.
Test plan :
- set syspref dateformat not on yyyy-mm-dd, for example dd/mm/yyyy
- checkout an item
- at intranet, perform a search where you see the item
=> You must see : "date due : dd/mm/yyyy"
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Implemented invalid_username_or_password template
variable being handed to Auth.tt. Removed indentation
changes and modification to return value of checkpw
Test plan:
See comment #2
Alternative test plan:
- Clear session cookies or close / open your browser
- Go to [MY SERVER]/cgi-bin/koha/installer/install.pl
- Verify that message appears if wrong username/pw provided
- Verify that you can log in with databas administrator account
Tested with alternative test plan, works as expected. Title and
description in commit message adapted.
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>
Bug 18028 removed the install_misc directory but
install_misc/UpgradeBackup.pm was still used by the 'upgrade' rule of
make.
Other files from install_misc were useless to it may be better not to
reintroduce this directory with only 1 file.
Test plan:
`make`
`sudo make install`
`make upgrade`
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>
To test:
1) Apply patch and update database
2) Run 'git grep "OverdueNoticeBcc"' and confirm there are no other
instances
Sponsored-by: Catalyst IT
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>
The fails in the previous test showed that we need the first three
changes here. Some final polishing in points 4 to 6.
[1] Sub merge: Refine the condition for initializing $tags_new.
A postponed 'modify'-merge (A to B) makes that $authtypefrom is not
defined when running merge later. When crossing the type boundary, we
need a new field too.
[2] Sub merge: Add condition for an empty @record_to array.
This indicates also that a field should be removed, since we should
otherwise only add a $9 subfield.
[3] Sub merge: Adjust initializing @record_from.
This change is tested by verifying a cleared subfield in the test.
[4] DelAuthority: Adding a skipmerge parameter to allow the call from
authorities/merge.pl to skip an unneeded merge.
This also prevents that the 'delete-merge' would precede the
'modify-merge' under a hypothetical race condition.
[5] DelAuthority: There is actually no need to call GetAuthority.
The subfields of the old record are not relevant in this case.
[6] Added a few POD lines to merge.
[7] Removed a trailing space in a comment line in merge.
Test plan:
[1] Run t/db_dependent/Authorities/Merge.t.
The last subtest should no longer fail now.
[2] See test plan of next patch.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
[1] The preference was sent to HEA. We can now send both AuthorityMergeMode
as well as AuthorityMergeLimit.
[2] A comment in authorities/merge.pl is removed. Note that a subsequent
patch will modify and test the cron job.
[3] Script misc/batchRebuildItemsTables.pl temporarily enabled dontmerge.
This is equivalent to setting the mergelimit to zero.
The function defnonull is no longer needed. (If the pref was NULL,
we restore that value. Sub merge won't mind.)
Test plan:
[1] Run t/db_dependent/UsageStats.t
[2] Run misc/batchRebuildItemsTables.pl -t
This just ensures you it still compiles; the changes speak for itself.
[3] Now git grep on dontmerge.
You should only find hits in atomicupdate and misc/translator/po.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
At this point, we are replacing dontmerge functionality by the new
AuthorityMergeLimit logic. Instead of doing this check before calling
merge, we just call merge and check it there. In order to let the cron
job do the larger (postponed) merges, we add a parameter override_limit.
A subtest is added in Merge.t to test the 'postponed merge' feature. Since
merge now also calls get_usage_count, an additional mock is added. All
references to dontmerge are removed.
In merge two lines, initializing $dbh and $counteditbiblios, are moved.
The dontmerge test in DelAuthority and ModAuthority is removed. Since this
did not leave much in ModAuthority, I fixed the whitespace on the remaining
lines rightaway (yes, I know).
A minimal set of changes is applied to the cron script; it will get further
attention on a next patch.
Test plan:
[1] Run t/db_dependent/Authorities/Merge.t
[2] Set AuthorityMergeLimit to 2. Modify an authority with two linked
biblios. Check that the merge was done immediately.
[3] Now modify an authority with more than 2 linked records.
Verify that the merge was postponed; a record must be inserted in
the need_merge_authorities table.
[4] Testing of the merge cron job is *postponed* to a next patch.
Note: I tested a modification, but the script just needs more
attention.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Since we can now call linked_biblionumbers, we can now remove all Zebra
related code from merge. We also add a parameter biblionumbers; we use it
in the test now, but it may be handy too later in the maintenance script
when we want to trigger a merge for specific biblionumber(s). See bug
report 18071.
All mocks for ZOOM, Context::Zconn, Search::new_record_for_zebra in the
merge test can now be replaced by one mock for linked_biblionumbers. Note
that we test the biblionumbers parameter in the last subtest without that
mock.
Remove unused vars $countunmodifiedbiblio, $counterrors from merge.
Renamed zebrarecords to linkedrecords in the Merge test.
Test plan:
[1] Run t/db_dependent/Authorities/Merge.t
[2] Modify an authority record. Check the linked biblio records.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
We will need a few additional parameters for merge later on. This patch
puts the original parameters in a parameter hash.
For the same reason DelAuthority gets a parameter hash here.
Note: We remove the second parameter from the DelAuthority call in
authorities/authorities-home.pl here. It was not used and could have
presented problems in the future.
Test plan:
[1] Run t/db_dependent/AuthoritiesMarc.t.
[2] Run t/db_dependent/Authorities/Merge.t.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>
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.
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 GetItemStatus
should not return any used calls (only 1 occurrence in release notes)
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>
At this point, there should not be any occurrences of
GetReservesFromItemnumber anymore.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
In aqbudgetperiods.pl a commented line is removed that contains a
wrong userenv hash key. Should be branch instead of branchcode.
In aqbudgets GetBudgetHierarchy is called with the same wrong userenv
hash key. Should be userenv->{branch}.
This made another bug visible: if you call GetBudgetHierarchy with a
branch and without owner, the where clause should take into account
that the branchcode can be empty (not null).
Test plan:
[1] Run Budgets.t
[2] Run aqbudgets.pl from Administration
[3] Add the show_mine=1 parameter in the URL
[4] Change owner of one of the funds and repeat step 2 and 3.
t/Budgets.t returns green. Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
ref($sub) eq 'ARRAY' should always be true, what we want is to call
can_edit_subscription if there is at least 1 serial for this
subscription.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Currently we have subscriptions with 300+ serials received.
It takes a lot of time to show serials-collection.pl (~20s).
This is especially troublesome when receiving serials, since after receival we get redirected to that page.
We no longer can receive daily serials in the allotted timeframe.
This quick and dirty fix prevents checking the subscription editing for each serial, but instead checks it for the first serial only.
This reduced page load time by ~18s
TEST PLAN:
1. Receive ~300 serials (or just a bunch :) )
2. Observe the gradual slowing of the receival action.
AFTER THIS PATCH:
1. Receive ~300 serials more (or just a bunch)
2. Observe a significant performance improvement.
Signed-off-by: Paul POULAIN <paul.poulain@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
The AddRenewal subroutine currently uses the circulation rules for the branch
stored in the Issues table (which is the holding branch) when calculating the new due date.
This patch replaces using the branch from the Issues table with the branch
specified by the HomeOrHoldingBranch syspref.
To test:
1. Set up 2 branches, Branch1 and Branch2
2. Set up a loan rule in Branch1 for DVDs with a 21 day loan period and a
21 day renewal period.
3. Set up a loan rule in Branch2 for DVDs with a 14 day loan period and a
14 day renewal period.
4. Checkout a DVD belonging to Branch1 while logged into Branch2. It will
receive the correct 21 day loan period.
5. Renewing the same DVD while logged into either Branch1 or Branch2
will give a 14 day due date, rather than 21 days.
6. Checkout a DVD belonging to Branch2 while logged into Branch1. It will
receive the correct 14 day loan period.
7. Renewing the same DVD while logged into either Branch1 or Branch2
will give a 21 day due date, rather than 14 days.
8. Apply the patch and repeat steps 4-7. The correct due date should be given
when the item is renewed, regardless of where it is checked out or renewed.
This update removes reassignment of $branch variable.
Signed-off-by: Cédric Vita <cedric.vita@dracenie.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
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>
Test::Exception is available on Jessie and already installed when
koha-common gets installed
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>
Still refers to a removed parameter of _autocreate.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
[1] Correct xml error in POD
Copy-pasting this example from Auth_with_shibboleth.pm into koha-conf.xml
did not work. We need a closing tag ;)
[2] If AddMember_Opac calls AddMember_Auto, there is no need to call
fixup_cardnumber. Adding trivial test for AddMember_Auto in Members.t.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
When running tests I got two errors. Do these changes make sense?
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Updated to work with match point selection
- The base bug that this patch enhances has added the ability to choose
your match point in the configuration file. This patch adds code to
take that change into account.
Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
- Adds the option to map shibboleth attributes to koha user columns
and allows mapping to auto create non-existant users
Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Trims the URL in order prevent prefixing a space with http://
Normally you won't have a preceding space here, but I saw it happening
one day and it does not cost much to resolve it.
Bonus: Adding few simple tests in t/db_dependent/Biblio.t.
Test plan:
[1] Run t/db_dependent/Biblio.t
[2] Add a 856$u with preceding space (MARC21)
[3] Check opac-detail, Online access with OPACXSLTDetailsDisplay empty.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Followed test plan, works as expected
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
The SIP protocol allows for a Fee Paid message to specify a fee
identifier in the CG field. While this field is read and passed to
C4::SIP::ILS::pay_fee, it is not passed along to
C4::SIP::ILS::Transaction::FeePayment::pay and is thus not used. We
should enable this functionality in Koha now that accountlines each have
a unique identifier that can be passed along to the payment requestor
via the AV field line items added by bug 14512.
Test Plan:
1) Create a fee in Koha
2) Ensure your SIP2 server is running and configured correctly
3) Send a 37 Fee Paid message to Koha's SIP server with the
accountlines_id for that fee in the CG fee identifier field,
along with the other required fields.
4) Note that specific fee was paid in Koha
Signed-off-by: Rhonda Kuiper <kuiper@roundrocktexas.gov>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Cab Vinton <director@plaistowlibrary.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>
This patch allows you to use the "qualifier,qualifier" syntax
in the Record Matching Rules "Search Index" when using the QueryParser.
While QueryParser doesn't support this syntax, it will now fallback
correctly to non-QueryParser functionality. Without the patch, your search
will just fail silently.
This patch also adds a "skip_normalize" option to C4::Search::SimpleSearch(),
and uses the option during C4::Matcher::get_matches. This prevents
the s/:/=/g and s/=/:/g normalization. This normalization is heavy-handed,
and while it is necessary sometimes to generate a valid CCL query or
QueryParser query, C4::Matcher::get_matches() already produces a valid
CCL query, so we don't need to do this normalization.
Additionally, this normalization causes problems when you use a
Zebra register which isn't normalized: namely the "u" register.
Strings are stored "as is", so http://localhost/koha/resource/1 is
stored as is during indexing. When you search, you need to pass
the exact same thing through the query to get a match. Using
http=//localhost/koha/resource/1 in your query will yield zero results.
_TEST PLAN_
0) Apply patch
1) Create a Record Matching Rule in Koha with the following details:
Matching rule code: TEST
Description: Test
Match threshold: 100
Record type: Bibliographic
Match point 1:
Search index: id-other,st-urx
Score: 100
Tag: 024
Subfields: a
Normalization rule: None
2) Create a record in Koha with an indexable URI
2a) Default framework
2b) 024 $a http://koha-community.org/test $2 uri
2c) 040 $c test
2d) 245 $a This is a test record
2e) 942 $c Books
2f) Save (save again if cautioned about missing fields as these should autofill)
3) Do a full re-index of Zebra
4) Download your record from Koha as a .mrc file (ie isomarc, binary marc, etc)
5) Go to "Stage MARC records for import"
5a) Upload your .marc file.
5b) Change your "Record matching rule" to "Test"
5c) Click Stage for import
9) It should say "1 records with at least one match in catalog per matching rule "Test".
NOTE: For completeness, you can go through this process on a clean master branch,
and note that it will say '0 records with at least one match in catalog per matching rule "TEST"'
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Currently, the From field uses the emailaddress of the staff client member
from the user context. This is exceptional in Koha. And might very well be,
as in our case, the cause of fraud detection warnings.
We should use branch email address or fallback to KohaAdminEmailAddress,
as we do (almost) everywhere else.
Test plan:
Go to subscription detail in Koha.
Go to Claims.
Select a missing issue and click Send notification.
Verify the sender address in the generated notification. (Make sure that
you receive this mail.)
Signed-off-by: Grace McKenzie <grace.mcky@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Without this patch, there is an inconsistency in subscriptions:
* a new subscription has no history end date set (as expected)
* if you renew it, a date is set.
This patch removes the setting of the history end date during renewal
Test plan:
* create a subscription, go to subscription-detail.pl page, tab "Summary". No History end date is set
* renew the subscription => a date has been set
* apply the patch
* create another subscription
* renew it => no history end date has been set
* sign-off
Signed-off-by: beroud <isabelle.beroud@univ-lyon3.fr>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Fields with underscore like B_address do not print on patron cards.
To reproduce:
- Create patron card layout using fields with underscore in their name
(e.g. <surname><B_address> )
- Print (export) patron card
- Verify that fields without underscore are replaced by their value,
but fields with underscore do not replace but show the field name
To test:
- Apply patch
- Try to reproduce and verify that fields with underscore are replace
as expected
Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
The combobox on the left side of late orders is filled by sub
GetBooksellersWithLateOrders. The same change as in the first patch
must be made here to include suppliers with late orders without a
price.
Bonus: Sort the list.
Test plan:
[1] Run t/db_dependent/Bookseller.t.
[2] Go to late orders. Use the filter on suppliers.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch removes the filter of unitpricesupplier <> 0 and
unitpricelib <> 0 from GetLateOrders as used in lateorders.pl.
This allows you to claim late orders without a price entered.
Test plan:
[1] Run t/db_dependent/Acquisition.t
[2] Check if you see an order with no price in lateorders.pl. (If needed,
remove prices from a few orders.)
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Sounds more appropriate and consistent with existing action logs.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
20/02/17 : added the syspref RenewalLog
24/20/17 : added a test for the syspref Renewal Log
test plan
1 - Chose a Borrower and have him renewing an item
2 - Check the renew logs : they should be empty
3 - Apply patch and set the syspref RenewalLog to 1
4 - Have the Borrower renewing a new item
5 - Check the renew logs : there should be your renew
I called the function logaction, which is in charge of modifying the
logs, within the function which adds a new renewal at the list.
Signed-off-by: Julien Comte <julien.comte@u-psud.fr>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch updates SIP checkin to use reserve_id as param, to fix regression
introduced by Bug 14695.
To test:
Given 1 patron and 1 biblio with two items attached:
1) create two record level holds on one biblio on same user
Using SIP client or telnet:
2) checkin item x on pickup branch, observe item is waiting and hold 1 is filled
3) checkin item y on pickup branch, observe item y now is waiting and x is available
4) apply this patch and repeat 1-3, now expected behaviour should be
that both holds are filled and both items waiting
Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
I removed several lines of code in the checkpw_ldap subroutine where
LDAP authentication takes place, in the "else" part of the conditional
that checks for the auth_by_bind config parameter. I added several lines
to check whether the user can log in to LDAP using their DN and the
password supplied in the login form. If they are able to bind, login
contiues as normal and the LDAP attributes can be harvested as normal if
the update options are turned on. The routine that was in place was
failing because it was trying to check against a non-existent LDAP entry
attribute called 'userpassword'. Instead of checking against a
'userpassword' attribute, the routine really should be checking to make
sure the user can actually bind to LDAP. That's what I set up, and it is
a safer way to test authentication against LDAP.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch is a QA follow-up to fix several issues:
- 1 call to GetReserveFee was wrong in ModReserveFill
- Update DB entry was wrong and insufficient
- Add robustness to the tests in sco-main
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Currently, Koha charges all patrons a hold fee in all circumstances, if
a hold fee is applicable to their patron category.
This is immediately applied at point of request.
However, it would be useful to let patrons make requests without a
charge
being incurred until they physically have the item in their hands and
checked out to their cards.
The hold fee will only be added to the account as soon as the item is
checked out to the requesting patron.
With this scenario, we will be certain that patrons have the correct
item, and they are happy with what has been supplied.
It also means that patrons can place holds via the OPAC without reaching
the usage limit that has been selected.
Test plan:
0/ All the following steps must be done with a patron using a patron category with a hold fee
1/ Make sure that the existing options for HoldFeeMode work as before
2/ Select the third option "any time a hold is collected"
3/ Place a hold on an item
4/ Note that the patron has not been charged
5/ Check this item from the staff interface
6/ Note that the patron has been charged
7/ Place another hold
8/ Use the self checkout feature at the OPAC for the checkin
9/ Note that the patron has been charged and a message is displayed to
inform about the fee.
Sponsored-by: Cheshire Libraries
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>
This patch updates the current code to make it works with the new
option's name of the syspref.
It also refactor the tests to make them more reusable and robust.
Sponsored-by: Cheshire Libraries
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>
The solution of Jonathan can be applied in two other cases, effectively
making GetItemHolds obsolete.
Test plan:
[1] Git grep on GetItemHolds
[2] Add an order, place a hold, delete order.
[3] Add an order, receive, place hold, delete order.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
It is not about when the hold was 'placed' but if the hold pertains to
the future or not.
Test plan:
[1] Git grep on holds_placed_before_today.
[2] Run t/db_dependent/Koha/Biblios.t
[3] Run t/db_dependent/Reserves.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
At this point, there should not be any occurrences of
GetReservesFromBiblionumber left in the codebase
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
The C4::Reserve::GetReservesFromBiblionumber took 3 parameters, the
biblionumber, an optional itemnumber and a "all_dates" flag.
If set, the subroutine returned all the holds placed on a given bibliographic
record, even the ones placed in the future. Almost all of the calls had this
flag set, they will be replaced with a call to Koha::Biblio->holds.
But 5 did not have it:
- C4::Biblio::DelBiblio
-tools/batch_delete_records.pl
=> These 2 were wrong, we want to retrieve the holds to cancel them
before deleting the record. We need to get all the holds, even the ones
placed in the future /!\ CHANGE IN THE BEHAVIOR
- acqui/parcel.pl
=> 1 call per item were made to this subroutine. They have been replaced
with only 1 call to the new method Koha::Biblios->holds_placed_before_today
Then we filter on the itemnumbers.
I think this is wrong: we need the number of holds to know if the record
can be deleted, so even if future holds exist, the deletion should not
be possible.
- serials/routing-preview.pl
- C4::ILSDI::Services::GetRecords
- C4::SIP::ILS::Item->new
=> Seems ok, we just one to display holds placed before today
Test plan:
I would suggest to test this patch with patches from bug 17737 and bug 17738,
to place different kind of holds (biblio and item level, future and
past).
Then do a whole workflow to detect bug, view a record, delete record,
order, place a hold on an item which has been ordered, etc.
The hold's informations should always be the same without or without
these patches.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
For instance an issue is not fetch from its fk but using the fk
itemnumber.
We need to support them.
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>
On of the awesome things we will be able to do with the TT syntax is the support of plurals.
For instance we will be able to send a list of items, checkouts, etc. to the notice template.
That way we will get rid of our custom syntax like <<items.content>> or <item></item> for instance.
The existing code already has the playground for that but it is not used.
Basically the idea is to add a "loops" key which can contain a list of
object to retrieve from the DB and send to the template.
For instance:
loops => { overdues => [ $itemnumber_1, .., $itemnumber_N ] }
will send a variable "overdues" to the template. It will contain the
Koha::Checkout objects relative to the id passed.
There is one quite big inconvenient to this approach so far: since we
are still supporting the historical syntax, the objects can be fetch by
a script, then the script will send the id to GetPreparedLetter which
will refetch them.
This must be improved, but I suggest to do that later.
Test plan:
prove t/db_dependent/Letters/TemplateToolkit.t
should return green
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>
From C4::Letters::GetPreparedLetter:
my $tables = $params{tables};
my $substitute = $params{substitute};
$tables || $substitute || $repeat
or carp( "ERROR: nothing to substitute - both 'tables' and 'substitute' are empty" ),
return;
So if the parameter tables or substitute is passed but does not contain anything, it will not warn as expected.
Test plan:
1/ Apply the patch with tests
2/ Confirm that they do not pass
3/ Apply this patch
4/ Confirm that the tests now pass
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Nothing new here since bug 17962, the AR_* notice messages are quite
simple. They send the article_request, patron, biblio, biblioitem, item and
library linked to the article request.
All the fields from these 6 tables should still be accessible using the
TT syntax.
Test plan:
Define TT notice templates for AR_PENDING, AR_PROCESSING, AR_COMPLETED
or AR_CANCELED.
You should manage to create a template to generate the same result as
the historical syntax.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
The 3 subroutines GetFieldMapping, SetFieldMapping and
DeleteFieldMapping from the C4::Biblio module were only used from the
field mappings admin page.
They can easily replaced with new packages Koha::FieldMappings based on
Koha::Object[s]
Test plan:
Add and delete field mappings (admin/fieldmapping.pl, Home ›
Administration › Keyword to MARC mapping).
Add an existing mapping > Nothing should be added
Note that this page has not been rewritten and you will not get any
feedbacks, but it's not the goal of this page to improve it.
Followed test plan, works as expected.
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>
The code was a bit weird and this patch cleans it a bit by renaming
variables and adding a variable.
Sponsored-by: Orex Digital
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
At the moment the default behaviour is not to cumulate the restriction
periods but to apply the longest one.
This patch set creates a new syspref CumulativeRestrictionPeriods. If
on, the behaviour changes and the restriction periods are cumulated: the
days of the second restriction are added to the actual restriction
period.
We could add a new circulation rule instead of a syspref, but I am not
sure it's very useful to have such granularity for this behaviour (can
be changed if needed).
How it works:
Let's take 2 items, A and B.
A is returned with Na days late, and B Nb days late
The grace period is Ng and there is 1 day of suspension charge per day
of overdue
The suspension period is until day D = Na - Ng + Nb - Ng
I would have expected D = Na + Nb - Ng but it's how it worked before
this patch.
Test plan:
Create several overdue for a given patron
Do the checkins and confirm that the period are added if the pref is on.
If the pref is off, you should not get any changes in the existing behaviour.
Sponsored-by: Orex Digital
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.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>
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>
$record is never used later, the call is superfluous.
Test plan:
Quick glance at the code should be enough
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
By restoring some pieces of logic, with the name changed from $persona
to $emailaddress, the openid will work again for OPAC logins.
See https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10988#c68
for an excellent test plan.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Did not test it, but trust in author and signoffer
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Note: this is a squashed version of the original patchset, because it was needed
This patch adds an opac_editable property of borrower attribute types
that can be set in the interface. I'm removing work on OPAC and will
refactor it, keeping the author attribution.
Test plan:
1. Repeat the following with a new and existing borrower attribute
type:
2. Verify that "Editable in OPAC" can only be checked if "Display in
OPAC" is checked.
3. Verify that this new property is correctly saved.
Signed-off-by: Aleisha <aleishaamohia@hotmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
To make sure we will not never get a race conditions for these kinds of
notices, we need to add a LOCK on the message_queue table.
This does not smell the best way to do that, but I faced deadlock issues
when I tried to use "UPDATE FOR"
https://dev.mysql.com/doc/refman/5.7/en/innodb-locking-reads.htmlhttps://dev.mysql.com/doc/refman/5.7/en/lock-tables.htmlhttps://dev.mysql.com/doc/refman/5.7/en/commit.html
To test this patch, or another solution, you need to apply manually this
change:
my $message = C4::Message->find_last_message($borrower, $type, $mtt);
unless ( $message ) {
+ sleep(1);
C4::Message->enqueue($letter, $borrower, $mtt);
} else {
And repeat the test plan from first patch.
Do not forget to truncate the message_queue table.
Followed test plans, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
There is an obvious race condition when CHECKIN and RENEWAL are
generated from circulation.pl calling svc/renew or svc/checkin in AJAX.
The 2 first queries will try to get the id of the last message
(find_last_message) and if it does not exist, they will insert it.
Theorically that could be lead to have several "digest" messages for a
given patron.
I did not recreate more than 2 messages, from the third one at least one
of the two firsts existed in the DB already.
This patch just simplifies the code to make the SELECT and INSERT or
UPDATE closer and limit the race condition possibilities.
Test plan:
0. Set RenewalSendNotice and circ rules to have a lot of renewals available
1. Use batch checkouts (or one by one) to check out several items to a
patron
2. Empty message_queue (at least of this patron)
3. Renew them all at once ("select all" link, "renew or check in"
button)
4. Check the message_queue
Without this patch you have lot of chances to faced a race condition and
get at least 2 messages for the same patron. This is not expected, we
expect 1 digest with all the messages.
With this patch apply you have lot of chances not to face it, but it's
not 100% safe as we do not use a mechanism to lock the table at the DBMS
level.
Tested both patches together, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
To test:
1 - Add reserves.reservenotes to HOLDPLACED message
2 - Enable emailLibrarianWhenHoldIsPlaced OpacHoldNotes sysprefs
3 - Place a hold via OPAC with a note
4 - view the messagequeue and note the reservenotes is blank
5 - Apply patch
6 - Place a hold with a note
7 - view the messagequeue and note the reservenotes is populated
Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
A similar bad template check from C4::Auth::get_template_and_user
should be applied in C4::Templates::gettemplate.
Before this patch it would be possible to expose files like:
my $template = C4::Templates::gettemplate(
'/etc/passwd', 'intranet', CGI::new, 1
);
print $template->output;
Note that the is_plugin flag in the above call is the culprit. This patch
provides a quick security fix without touching get_template_and_user, and
can be backported to stable branches.
I will provide an enhanced and centralized check on report 17989, also
removing the is_plugin flag.
Note: We allow .pref here too for use in admin/preferences.pl.
Test plan:
[1] Run t/db_dependent/Auth.t (triggering get_template_and_user and
gettemplate).
[2] Run t/db_dependent/Templates.t again (see first test plan).
The tests should no longer fail.
[3] Open a page on opac or intranet.
[4] Open a systempreferences tab.
[5] Add a book to the cart and send it ([opac-]sendbasket uses gettemplate).
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
This patch is the Koha part of the Hea v2 project.
You can find the (testing) code for the server at
hea-ws - https://github.com/joubu/hea-ws/commits/v2
hea-app - https://github.com/joubu/hea-app/commits/v2
They contain the different pull requests made over the last 6 months.
More information on Hea at https://wiki.koha-community.org/wiki/KohaUsageStat_RFC
The goal of this commit message is to provide an overview of what could
be a new version of Hea.
Prior to these changes, the Hea database was filled with 1 line per Koha
installation. System preferences were filled by the libraries and a
cronjob (share_usage_with_koha_community.pl) collected these values to send
them to a webservice (hea-ws/upload.pl).
With the need to collect more data we would want to collect data at the library
level (branch) and not at the installation level.
For instance the geolocation, the url or the country can be different from one
library to another, even if managed from the same Koha installation.
The Hea DB has been upgraded to reflect that change (see hea-app/sql/schema.sql).
The hidden goal of this patch is to make Hea sexier and explain
better to libraries how it can be useful to share their information
with the Koha community. I guess the main problem is the lack of
communication and explanations about what we are doing we these data.
To fill this gap I'd like to (TODO)
1. Communicate on the ML about this new version of Hea (once it got
pushed and backported)
2. Link the Privacy_Policy.md from the Hea interface
3. Get help from a native English speaker to add
popup/help/info/whatever on "Home › Administration › Usage statistics",
to clearly explain what happens (and what will not happen!) when an option or
another is set.
You can find screenshot of this whole enhancement on bug 18066, comment 2.
What this patch does:
- Create a new branches.geolocation DB field
- Add 3 new sysprefs:
* UsageStatsGeolocation
* UsageStatsLibrariesInfo
* UsageStatsPublicID
- Integrate the Leaflet JS library to get a fancy map to pick
geolocations
How does it works:
On the new administration page where statistics to share are configured,
there are several new things. It is now possible to share information either
per Koha installation or libraries. If UsageStatsLibrariesInfo is set,
the info at library level (url, name, country, geolocation) will be
sent to the Hea webservice. If it is not set, you can decide to fill
UsageStatsLibraryUrl, UsageStatsLibraryName, UsageStatsCountry,
UsageStatsGeolocation to share these information. Note that even if the
data are retrieved at installation level, it's better to fill the prefs
as well: On the Hea website the different libraries defined for a given
Koha installation could be displayed on the same page.
This page is a public page which will be attributed to every
installation (with the pref UsageStatsPublicID). On this page all the
info available publicly will be displayed.
TODO later:
- Add a button on the administration page to delete the info shared
publicly. It will be easy to show that the info are no longer displayed
on the public page.
- Add an icon per Koha installation to get a better "public page"
- Any suggestions?
Test plan:
We will need to test hea-ws, hea-app and the Koha-side code to test the
whole enhancement.
1/ To start, clone the hea-ws and hea-app project and checkout the
'master' branch (*not* 'v2')
2/ Create the hea database and user
CREATE DATABASE hea
CREATE USER 'hea'@'localhost' IDENTIFIED BY 'hea';
GRANT ALL PRIVILEGES ON hea.* TO 'hea'@'localhost';
FLUSH PRIVILEGES;
3/ Fill the DB with some data
mysql hea < hea-app/sql/schema.sql
mysql hea < hea-app/sql/sql/mock-data.sql
4/ Checkout the 'v2' branch for both hea-ws and hea-app
5/ Execute the upgrade DB script
% cd hea-app
% perl -p -i -e 's/REPLACE_ME/hea/' sql/upgrade.pl # Fill the DB info
% perl sql/upgrade.pl
Now the DB is using the v2 structure. That means we have 1 installation
row per library previously defined. 1 library row has also been created.
5/ Configure hea-ws
% echo '192.168.50.1 hea.koha-community.org' >> /etc/hosts
<VirtualHost *:80>
DocumentRoot "/path/to/hea-ws"
ServerName "hea.koha-community.org"
<Directory "/">
Options +ExecCGI
Require all granted
AddHandler cgi-script .pl
</Directory>
</VirtualHost>
And enable it with a2ensite, then restart apache.
The copy the database.yml.sample to database.yml and edit it to fill the
DB info.
6/ Launch the hea-app
% cd hea-app
% edit README.md # to install the missing modules
% cp environments/config.yml environments/development.yml
% edit environments/development.yml # to fill the DB info
% perl bin/app.pl
Then hit localhost:3000
You should see a local version of Hea with sample data
7/ Back to Koha side
A. We will test that the webservice still works with previous version of Koha (without v2)
a. Do not configure Hea
% perl misc/cronjobs/share_usage_with_koha_community.pl -f -v
Then hit localhost:3000
=> Nothing added
b. Configure Hea on admin/usage_statistics.pl
perl misc/cronjobs/share_usage_with_koha_community.pl -f -v
=> New library added
c. Modify the Hea configuration
perl misc/cronjobs/share_usage_with_koha_community.pl -f -v
=> Info are modified
B. Not we will test that it works with the new version (much more fun ;))
% git checkout hea-v2 # koha
a. Configure Hea using /admin/usage_statistics.pl
perl misc/cronjobs/share_usage_with_koha_community.pl -f -v
=> Check the result on localhost:3000
b. Share libraries's info
perl misc/cronjobs/share_usage_with_koha_community.pl -f -v
c. Continue to play a bit and share the info.
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Test plan:
- Add a new patron attrbute and mark it searchable
- Populate a new patron with 'potato' in that field
- Add/edit another patron to have email potato@invalidemail.com'
- Perform a patron search with query 'potato' (in standard fields)
=> Both patrons are returned
- Perform a patron search with filters 'Email' and query 'potato'
=> Only 1 patron is returned and you are redirected to the patron detail page.
Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Resolve warning:
Use of uninitialized value in string eq at C4/Installer.pm line 79.
Test plan:
Add a trivial .sql file in atomicupdate (comment is sufficient).
Run updatedatabase.pl
Without the patch you have the warning.
Apply the patch and repeat (no warning).
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
