Nick Clemens [Fri, 21 Apr 2017 14:08:27 +0000 (10:08 -0400)]
Bug 18469: Suspend all holds when specifying a date to resume hold does not keep date
Name of field had 'datepicker' embedded, this caused variable issue
To test:
1 - Place several holds for a patron
2 - Go to holds tab in circulation
3 - Select a date for suspend all holds until
4 - Suspend all holds
5 - Note date is not used, suspended indefinitely
6 - Apply patch
7 - Resume all suspended holds
8 - Select a date for suspend all holds until
9 - Suspend all holds
10 - Note date is used
Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Colin Campbell [Tue, 8 Aug 2017 10:47:40 +0000 (11:47 +0100)]
Bug 19061: Avoid SQL Injection vulnerability
Embedding values in the SQL statement allows the passing of values
that would normally be rejected resulting in mysql errors
variables should always be passed via placeholders and
the execute call
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Jonathan Druart [Wed, 9 Aug 2017 16:01:37 +0000 (13:01 -0300)]
Bug 18292: Tests do not need to return 1 - xt
Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Test plan:
git grep '^1;$' t/**/*.t
should not return any results
NOTE: does not fix C4/SIP/t, nor xt tests.
Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Bug 18292 - t/db_dependent/Circulation.t doesn't need to return 1;
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Marcel de Rooy [Fri, 11 Aug 2017 06:34:45 +0000 (08:34 +0200)]
Bug 19049: [QA Follow-up] Mock config, default format
As requested by QA:
[1] Mock_config enable_plugins in the test.
[2] Fallback to MARC when format is empty. Remove die statement.
Added:
[3] Remove $marc. This variable got obsolete during development.
[4] Add test on $input_file and $plugin_class. Test $text before calling
Handler or processing $text. No need to split undef if somehow Handler
returned undef, etc. If the routine returns an empty arrayref,
stage-marc-import will do fine.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Marcel de Rooy [Thu, 10 Aug 2017 11:02:23 +0000 (13:02 +0200)]
Bug 19049: [Follow-up] Make sure that GetPlugins finds the plugin
We have to mock_config the pluginsdir before Plugins is loaded, and
we should pass an absolute path (not a relative one).
If you did not install the test to_marc plugin, this test would fail.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Verified patch is compatible with original KitchenSink to_marc plugin
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Marcel de Rooy [Tue, 8 Aug 2017 12:51:41 +0000 (14:51 +0200)]
Bug 19049: Testing RecordsFromMarcPlugin with a to_marc plugin
This patch adds a simple to_marc plugin in t/Koha/Plugin that is used
in the added subtest in ImportBatch.t.
Test plan:
[1] Run t/db_dependent/ImportBatch.t
[2] Copy the to_marc test plugin from t to your plugin directory.
Under Debian packages, you should do something like:
mkdir -p /var/lib/koha/master/plugins/Koha/Plugin/
cp [yourclone]/t/Koha/Plugin/MarcFieldValues.pm /var/lib/koha/master/plugins/Koha/Plugin/
[3] Check if you see this plugin on plugins/plugins-home.pl
[4] Create a text file with some fields like:
(Note: The plugin needs an empty line between both "records".)
100,a = Test Author 1
245,a = Title One
100,a = Author 2
245,a = Title Two
[5] Go to stage-marc-import.pl. Upload the created file. Select the plugin
in the format combo and proceed. Did you create two records ?
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Marcel de Rooy [Mon, 7 Aug 2017 19:24:48 +0000 (21:24 +0200)]
Bug 19049: Fix regression on stage-marc-import with to_marc plugin
Bug 12412 added the use of to_marc plugins allowing arbitrary file formats
in stage-marc-import (as long as the plugins can handle them). The feature
was not very visible in the code, and when bug 10407 added the marcxml
format, it made some changes that broke the use of to_marc.
This patch restores the functionality by:
[1] Adding a sub RecordsFromMarcPlugin to ImportBatch.pm, specifically
addressing the conversion from arbitrary formats to MARC::Record.
The original to_marc interface is used: pass it the file contents,
and it returns a string consisting of a number of MARC blobs separated
by \x1D.
Consequently, the call of to_marc is removed from routine
BatchStageMarcRecords where it did not belong. The to_marc_plugin
parameter is removed and two calls are adjusted accordingly.
[2] Instead of a separate combo with plugins, the format combo contains
MARC, MARCXML and optionally some plugin formats.
[3] The code in stage-marc-import.pl now clearly shows the three main
format types: MARC, MARCXML or plugin based.
Note: This patch restores more or less the situation after bug 12412, but
I would actually recommend to have the to_marc plugins return MARC::Record
objects instead of large text strings. In the second example I added a
to_marc plugin that actually converts MARC record objects to string format,
while RecordsFromMarcPlugin reconverts them to MARC::Records.
Test plan:
See second patch.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Josef Moravec [Tue, 8 Aug 2017 08:23:57 +0000 (08:23 +0000)]
Bug 18331: Followup - fix path to orders_by_budget csv header
Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Lee Jamison <ldjamison@marywood.edu> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Marc Véron [Thu, 3 Aug 2017 15:50:30 +0000 (17:50 +0200)]
Bug 18331: Force tt to insert newline to empty_line.inc
See end of comment #4 and IRC:
http://irc.koha-community.org/koha/2017-08-03#i_1958062
Signed-off-by: Lee Jamison <ldjamison@marywood.edu> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Jonathan Druart [Mon, 12 Jun 2017 20:04:47 +0000 (17:04 -0300)]
Bug 18331: Fix CSV export (once and for all!)
I am going to try and explain the problem here in order to make it a
have-to-read for next changes.
There are several things to test when and most of the time we break
something when we try to fix something else.
You have to test different CSV export, not only one. There are not all
processed the same way. For instance acqui/csv/basket.tt does not
contain any strings to translate and the translate script will copy the
original file as it.
But reports/csv/orders_by_budget.tt will not! Indeed it contains "TOTAL"
which will be translated. The generated tt file for the translated
language will remove all the carriage returns! That's why we cannot rely
on TT for newlines (contrary to bug 16914 assumption).
There are two possible methods:
1/ Fix the translate script
=> Hum... nope
2/ Remove all carriage returns and make them explicit by including an
file that only contains 1 carriage return
This second method is implemented in this patch.
How it works: Use the PRE_CHOMP "[%-" and POST_CHOMP "-%]"
(http://www.template-toolkit.org/docs/manual/Config.html#section_PRE_CHOMP_POST_CHOMP)
to remove all the newlines that could be added by TT in the original
(en) files. Then include the new_line.inc to add a new line.
That way original and translated files will behave the same way.
The BLOCK in the csv_headers avoid to have the newlines added, in any
cases. For instance: by default we will have an empty line at the end of
the headers, but the translated headers will not have it.
Test plan:
At least 2 signoffs will be needed, please test carefully!
You will need to generate CSV with at least 2 entries!
Test with the non-translated interface (en) and with the language you
want. To test with a translated language you will *have to*:
- cd misc/translator
- perl translate update LANG
- manually edit the po file, remove the fuzzy tags, correctly translate
the string (do not forget the additional %s in the headers)
- perl translate install LANG
1/ Export basket and basketgroup as CSV
2/ Export late orders as CSV
3/ Export items (from the item search) as CSV
4/ Export the 2 reports "Orders by fund" and "Cash register" as CSV
Signed-off-by: Lee Jamison <ldjamison@marywood.edu> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Nick Clemens [Fri, 7 Jul 2017 17:55:50 +0000 (17:55 +0000)]
Bug 16976 - Unit tests
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
I dug into this a little bit, it seems the issue is that we were double
processing the query because of an extra call to
C4::AuthoritiesMarc::SearchAuthorities
I think we can simply remove this call
To test:
1 - Perform an authority search for a value that returns results
2 - Wrap it in quotes and search
3 - 'Internal server error'
4 - Apply patch
5 - Search with quotes succeeds
Signed-off-by: Lee Jamison <ldjamison@marywood.edu> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Jonathan Druart [Mon, 7 Aug 2017 16:50:32 +0000 (13:50 -0300)]
Bug 19053: Keep auto_renew flag if a confirmation is needed
If an item is checked out with the auto renewal flag and a confirmation
is needed (holds exist, past due date, etc.) the auto renewal flag will
get lost.
Test plan:
- Check an item out and specify a past due date
- Confirm the checkout on the confirmation screen
=> Without this patch the checkout is not marked as auto renewal
=> With this patch applied the auto renewal flag will be set to the
checkout
Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This test makes it explicit that the only string producing _all as index on build_authorities_query_compat is 'all'.
To test:
- Apply this patch
- Run:
$ sudo koha-shell kohadev
k$ cd kohaclone
k$ prove t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t
=> FAIL: Test fails because the list of valid values is wrong in Koha.
Note: this list has to be in sync with the templates passing the same values. A followup will be added
to fix a discrepancy found between OPAC and Intranet.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Marc Véron [Wed, 2 Aug 2017 16:01:06 +0000 (18:01 +0200)]
Bug 19027 - Circulation rules: Better wording for standard rules for all libraries
In Home > Administration > Circulation and fine rules, the standard value for
"Select a library: All libraries" is confusing and leads to support cases.
Change wording to "Standard rules for all libraries".
To test:
- Apply patch
- Go to Home > Administration > Circulation and fine rules
- Verfiy that text in drop down 'Select a library' makes sense.
Followed test plan which works as intended. I agree with the wording it
is significantly clearer than previously Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Owen Leonard [Tue, 1 Aug 2017 17:07:33 +0000 (17:07 +0000)]
Bug 18985 - SQL reports 'Last edit' and 'Last run' columns sort alphabetically, not chronologically
This patch alters the DataTables configuration of the SQL reports list
so that date columns are identified via class rather than position.
To test, apply the patch and go to Reports -> Use saved.
Test the re-ordering of the creation date, last edit, and last run
columns. Each should sort correctly based on the date.
Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Nick Clemens [Fri, 14 Jul 2017 11:25:42 +0000 (11:25 +0000)]
Bug 18941 - C4::Budgets GetBudgetByCode should return active budgets over inactive budgets
To test:
1 - Create an active budget
2 - Create an inactive budget
3 - Ensure they each have a fund with the same code
4 - Set MarcFieldsToOrder to get the budget_code from a marc field
5 - Stage a file using the duplicated code
6 - Add to a basket from the staged file
7 - Add the items
8 - Note funds are encumbered from the inactive budget
9 - Apply patch
10 - Repeat 5-8 with a new basket
11 - Note the active budget is now used
Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Nick Clemens [Fri, 14 Jul 2017 11:06:16 +0000 (11:06 +0000)]
Bug 18941 - Unit tests
Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Kyle M Hall [Wed, 19 Jul 2017 18:57:26 +0000 (14:57 -0400)]
Bug 13012 - suggestion.suggesteddate should be set to NOW if not defined
Test Plan:
1) Apply this patch
2) prove t/db_dependent/Koha/Suggestions.t
Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Marcel de Rooy [Thu, 10 Aug 2017 07:24:08 +0000 (09:24 +0200)]
Bug 19071: Fix Members/IssueSlip.t
Resolve:
DBD::mysql::db do failed: Cannot delete or update a parent row: a foreign key constraint fails (`koha_master`.`clubs`, CONSTRAINT `clubs_ibfk_2` FOREIGN KEY (`branchcode`) REFERENCES `branches` (`branchcode`)) [for Statement "DELETE FROM branches"] at t/db_dependent/Members/IssueSlip.t line 44.
We do not need to delete all branches here.
Note: The test still needs attention for noisy userenv warns, but it should
pass now.
Test plan:
Run t/db_dependent/Members/IssueSlip.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Marcel de Rooy [Thu, 10 Aug 2017 07:13:44 +0000 (09:13 +0200)]
Bug 19071: Fix Circulation/issue.t
Resolve:
DBD::mysql::db do failed: Cannot delete or update a parent row: a foreign key constraint fails (`koha_master`.`clubs`, CONSTRAINT `clubs_ibfk_2` FOREIGN KEY (`branchcode`) REFERENCES `branches` (`branchcode`)) [for Statement "DELETE FROM branches"] at t/db_dependent/Circulation/issue.t line 65.
Cause:
See also bug 19070.
We do not need to delete all branches here.
Test plan:
Run t/db_dependent/Circulation/issue.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Marcel de Rooy [Thu, 10 Aug 2017 07:04:16 +0000 (09:04 +0200)]
Bug 19070: Fix Circulation/Branch.t
Resolve:
DBD::mysql::db do failed: Cannot delete or update a parent row: a foreign key constraint fails (`koha_master`.`clubs`, CONSTRAINT `clubs_ibfk_2` FOREIGN KEY (`branchcode`) REFERENCES `branches` (`branchcode`)) [for Statement "DELETE FROM branches"] at t/db_dependent/Circulation/Branch.t line 49.
Resolve:
not ok 14 - AddReturn respects branch item return policy - noreturn
Failed test 'AddReturn respects branch item return policy - noreturn'
at t/db_dependent/Circulation/Branch.t line 279.
got: 'yqiKrIkX'
expected: undef
Cause:
There is a record in clubs. The constraint in clubs on branchcode does not
include a cascaded delete. The test deletes all branches.
Test 14 depends on item-level_itypes==1. When you set it to Biblio, it fails.
Test plan:
Run t/db_dependent/Circulation/Branch.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Bug 19073: (bug 15758 follow-up) Dereference correct value from Koha::Object.
* tools/modborrowers.pl: Dereference 'branchcode' rather than
'value'.
Test Plan:
- Carry out "Batch patron modification" with any number of patrons.
+ Change the library.
+ Hit Save.
- The library will not have been updated.
- Apply patch.
- Run same modification.
- The library will have been updated
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Jonathan Druart [Thu, 10 Aug 2017 18:20:43 +0000 (15:20 -0300)]
Bug 19074: Do not crash if cardnumber does not exist
If a cardnumber does not exist, $borrower will be undef and the ->category call will explode
Can't call method "category" on an undefined value at
/home/vagrant/kohaclone/tools/modborrowers.pl line 370.
This patch makes sure the patron exists before calling any methods.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Bug 19074: Fix category display in Batch patron modification.
* tools/modborrowers.pl (GetBorrowerInfos): Fix setting of patron
category description.
* koha-tmpl/intranet-tmpl/prog/en/modules/tools/modborrowers.tt: Use
it.
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Marc Véron [Mon, 8 May 2017 14:18:41 +0000 (16:18 +0200)]
Bug 18555: Create patron list from patron import
Create a patron list from imported patrons to be used for printing patron
cards and other patron list related actions.
To test:
- Apply patch
- Go to Home > Tools > Import patrons
- Select a file to import and check new checkbox 'Create patron list'
- Import file
- Verify that in import results you see the time stamped name of the
patronlist containig the imported patrons (if any patrons were imported)
- Go to Home > Tools > Patron lists
- Verify that the list shows up and contains the imported patrons
Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Marcel de Rooy [Mon, 7 Aug 2017 06:44:43 +0000 (08:44 +0200)]
Bug 19047: Fix AddBiblio call in Reserves.t
AddBiblio does not return a title; the biblioitemnumber is stored in the
title variable.
The variables for biblioitemnumber are not used and can be removed.
Test plan:
Run t/db_dependent/Reserves.t
Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
TEST PLAN
---------
prove t/db_dependent/Letters.t
-- there will be a message: "C4::Context->userenv not defined!"
apply this patch
prove t/db_dependent/Letters.t
-- there will no longer be that message.
run qa test tools
Signed-off-by: Lee Jamison <ldjamison@marywood.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Bug 18471 added number formating in order receive.
I found a bug generated by this.
Koha::Number::Price defines methods to (un)format price depending on CurrencyFormat syspref.
The bug is that for CurrencyFormat US, some params are not defined in this module so there are retrieved from system locale. If system locale is not en_US, the params will be wrong. For example fr_FR will define ',' as decimal and so saved unit price is divided by 1000.
To recreate:
- On a server with locale 'fr_FR'
- With Plack mode
- Set CurrencyFormat on 'US'
- Open a basket
- Place an order for an item
- Close basket
- Receive order with changing 'Actual Cost' to '1,234'
- Note the 'Actual Cost' is now '1.23'
- Run unit test t/Number/Price.t without and with patch
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Bug 18900: (QA followup) Make tests conditional to locale existence
It is important to highlight that tests have been skipped instead of having them pass
on the absence of the required fr_FR.UTF-8 locale installed on the system.
This patch does that check and effectively skips them.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch adds unit tests to t/Number/Price.t in order to valid the problem.
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Owen Leonard [Fri, 9 Jun 2017 12:29:58 +0000 (12:29 +0000)]
Bug 5471 - Quotes in tags fail
This patch makes changes to the tag moderation template and JavaScript
to fix handling of tags with double or single quotes. This patch also
moves the tags moderation JavaScript out of the template and into a
separate JS file.
To test you should have multiple tags awaiting moderation, including
tags which contain double and single quotes.
- Go to Tools -> Tags.
- In the list of tags pending approval, test approving and rejecting
tags, including those containing single or double quotes.
- The state of the "Approve" or "Reject" buttons should correctly
change according to the action you chose.
- The label in the status column should update correctly.
- In the "Check lists" form, submitting approved, rejected, and
unclassified terms should result in the correct message.
Signed-off-by: Lee Jamison <ldjamison@marywood.edu>
Works correctly based on test plan. Tested using single- and
double-quoted tags. Passes QA Tools.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Marcel de Rooy [Thu, 22 Jun 2017 08:14:29 +0000 (10:14 +0200)]
Bug 18980: Show distinction between shared and private lists in staff
Just as we show this distinction in OPAC, this patch adds a type column
in the Your lists tab that displays Private or Shared. It always contains
Public in the other tab.
Test plan:
[1] Check if you see Shared for a private lists with shares in staff.
[2] Run t/db_dependent/Utils/Datatables_Virtualshelves.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Lee Jamison <ldjamison@marywood.edu> Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Marcel de Rooy [Wed, 21 Jun 2017 14:53:56 +0000 (16:53 +0200)]
Bug 18980: Remove js function Check from opac-shelves
This function only serves to check if the shelf name is not empty. It
does not even work since it incorrectly refers to f.addshelf (while it
should test #shelfname).
We can solve this and improve consistency by doing the same as in the
staff template with html5 required.
Test plan:
[1] Without this patch it was possible to add a list without a name in
OPAC (with js error: TypeError: f.addshelf is undefined).
[2] With this patch, verify that you cannot do this anymore.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Lee Jamison <ldjamison@marywood.edu> Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Marcel de Rooy [Wed, 21 Jun 2017 14:24:57 +0000 (16:24 +0200)]
Bug 18980: Add js to dynamically show/hide the Anyone remark
If we we move from Private to Public or vice versa, this should reflect
on the Anyone remark from the first patch.
If we go back to a private list without shares, we should show it.
Handled in a js sub AdjustRemark, triggered by onchange of the category
combo or the permission combo.
Test plan:
[1] Edit a private list without shares in OPAC. Toggle category and/or
permissions. Is the remark shown or hidden accordingly?
Note: Include a test with OpacAllowPublicListCreation enabled.
[2] Edit a shared list or public list in staff. Toggle category/permissions.
You should never see the remark.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Marcel de Rooy [Wed, 21 Jun 2017 11:13:17 +0000 (13:13 +0200)]
Bug 18980: Add remark next to "Anyone seeing this list" permission
This is a follow-up of report 18228 that simplifies the permissions for
lists. It reapplies the idea behind bug 10865 by adding a remark next to
the Anyone permission if it is not actually effective. A next patch will
make it behave dynamically.
Note: The Anyone permission is not relevant if it is a private list that
has no shares. Note that looking at the prefs OpacAllowPublicListCreation
and/or OpacAllowSharingPrivateLists is not decisive. You should look at
list permissions and shares in the database; turning off the Sharing pref
does not automatically remove all shares in the system. It only blocks
creating new shares.
We only need changes in opac-shelves.tt and virtualshelves/shelves.tt.
Test plan:
[1] Verify that you see the remark now in OPAC and staff.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Liz Rea [Mon, 26 Jun 2017 23:42:44 +0000 (11:42 +1200)]
Bug 18857 - Add buttons to left hand side of circ rules table
To test:
* Apply this patch
* Define some rules in the interface using the left hand buttons
* Define some rules in the interface using the right hand buttons (scroll to see)
* Delete some rules in the interface using the left hand buttons
* Delete some rules in the interface using the right hand buttons (scroll to see)
* Choose a rule to edit using the left hand buttons
* Edit the rule, saving using the left hand button
* Edit a rule, but clear the values using the left hand buttons
* Choose a rule to edit using the right hand buttons (scroll to see)
* Edit the rule, saving using the right hand buttons (scroll to see)
* Edit a rule, but clear the values using the right hand buttons (scroll to see)
Signed-off-by: Guillaume <guillaume_lair@ville-limoges.fr> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Jonathan Druart [Wed, 9 Aug 2017 15:59:20 +0000 (12:59 -0300)]
Bug 19067: Test Makefile.PL mapping
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Bug 19067: Map clubs/ into INTRANET_CGI_DIR in Makefile.PL
This patch adds the required entry in Makefile.PL so the clubs/ dir is correctly installed
during the make install step.
To test:
- Run:
$ cd kohaclone
$ perl Makefile.PL
- Follow any install method and options
- Run
$ make
$ ls blib/INTRANET_CGI_DIR/ | grep clubs
=> FAIL: No clubs dir
- Run:
$ make clean
- Apply this patch
- Run:
$ perl Makefile.PL
- Follow any install method and options
- Run
$ make
$ ls blib/INTRANET_CGI_DIR/ | grep clubs
=> SUCCESS: The clubs/ dir is correctly picked for inclussion in the intranet cgi dir
- Sign off :-D
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Fridolin Somers [Wed, 2 Aug 2017 09:21:58 +0000 (11:21 +0200)]
Bug 19023 - inventory tool performance
Inventory tool performance seems to be worst since 16.11.
I think it is because of authorized values computing changed by Bug 17249.
For each subfield of each item, we try to get the authorized value description with Koha::AuthorisedValues->search_by_marc_field.
But this method does not use cache like Koha::AuthorisedValues->get_description_by_koha_field.
I propose to use Koha::AuthorisedValues->get_description_by_koha_field and also to look for authorized value description only for item fields used in TT : location, notforloan, itemlost, damaged, withdrawn.
I have experimented inventory time on 100 items from 5s to 1s.
Test plan :
- Without patch
- Perform inventory with barcode file
- Check results and mesure execution time
- Apply patch
- Reperform inventory with same barcode file
- Check results is the same
- Compare execution time
- Run prove t/db_dependent/Items/GetItemsForInventory.t
Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
David Cook [Thu, 27 Jul 2017 01:58:28 +0000 (11:58 +1000)]
Bug 18898 - Some permissions for Reports can be bypassed
If you manually visit the following links when you only have
permission to run reports, you'll still be able to access the ability
to create and edit reports:
This patch ties these 2 unaccounted for phases to the create_reports
permission.
With patch, issue no longer can be reproduced. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Jonathan Druart [Thu, 6 Jul 2017 21:10:10 +0000 (18:10 -0300)]
Bug 18906: Display all funds the logged in user can use
There are complex conditions to know if a librarian can use a fund, we
cannot do that with a SQL query, we need to use C4::Budgets::CanUserUseBudget
When this patch is applied then all funds are visible to both
superlibrarian patrons and patrons with all acq permissions
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Bug 19003: Add a TestBuilder default for borrowers.login_attempts
Bug 18314 causes t/db/SIP/Message.t to fail (quite often) since
TestBuilder fills login_attempts with a random number. (Note: Only
when FailedLoginAttempts is non-zero.)
Trivial fix: TestBuilder should have a zero default for login_attempts.
Test plan:
Do not yet apply this patch.
Set pref FailedLoginAttempts to say 3.
Run t/db_dependent/SIP/Message.t. Might fail on the password test (CQ).
Apply this patch.
Run t/db_dependent/SIP/Message.t again. Does not fail anymore.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Jonathan Druart [Mon, 31 Jul 2017 14:30:00 +0000 (11:30 -0300)]
Bug 17699: Add test descriptions
Test plan:
prove -v t/db_dependent/Koha/Patrons.t
Subtest: renew_account
1..30
ok 1 - 2016-02-29T00:00:00 + 12 months must be 2017-02-28T00:00:00
ok 2 - 2016-02-29T00:00:00 + 12 months must be 2017-02-28T00:00:00
ok 3 - With BorrowerLogs, Koha::Patron->renew_account should have logged
ok 4 - today + 12 months must be 2017-03-31T00:00:00
ok 5 - today + 12 months must be 2017-03-31T00:00:00
ok 6 - Without BorrowerLogs, Koha::Patron->renew_account should not have logged
ok 7 - today + 12 months must be 2017-03-31T00:00:00
ok 8 - today + 12 months must be 2017-03-31T00:00:00
ok 9 - 2016-04-30T00:00:00 + 12 months must be 2017-04-30T00:00:00
ok 10 - 2016-04-30T00:00:00 + 12 months must be 2017-04-30T00:00:00
ok 11 - 2016-10-30T00:00:00 + 12 months must be 2017-10-30T00:00:00
ok 12 - 2016-10-30T00:00:00 + 12 months must be 2017-10-30T00:00:00
ok 13 - With BorrowerLogs, Koha::Patron->renew_account should have logged
ok 14 - today + 12 months must be 2017-11-30T00:00:00
ok 15 - today + 12 months must be 2017-11-30T00:00:00
ok 16 - Without BorrowerLogs, Koha::Patron->renew_account should not have logged
ok 17 - today + 12 months must be 2017-11-30T00:00:00
ok 18 - today + 12 months must be 2017-11-30T00:00:00
ok 19 - 2016-12-30T00:00:00 + 12 months must be 2017-12-30T00:00:00
ok 20 - 2016-12-30T00:00:00 + 12 months must be 2017-12-30T00:00:00
ok 21 - 2017-06-30T00:00:00 + 12 months must be 2018-06-30T00:00:00
ok 22 - 2017-06-30T00:00:00 + 12 months must be 2018-06-30T00:00:00
ok 23 - With BorrowerLogs, Koha::Patron->renew_account should have logged
ok 24 - today + 12 months must be 2018-07-31T00:00:00
ok 25 - today + 12 months must be 2018-07-31T00:00:00
ok 26 - Without BorrowerLogs, Koha::Patron->renew_account should not have logged
ok 27 - today + 12 months must be 2018-07-31T00:00:00
ok 28 - today + 12 months must be 2018-07-31T00:00:00
ok 29 - 2017-08-31T00:00:00 + 12 months must be 2018-08-31T00:00:00
ok 30 - 2017-08-31T00:00:00 + 12 months must be 2018-08-31T00:00:00
Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Jonathan Druart [Mon, 31 Jul 2017 14:03:37 +0000 (11:03 -0300)]
Bug 17699: Use limit as end_of_month
From DateTime::Duration pod:
""
For positive durations, the "end_of_month" parameter defaults to wrap.
For negative durations, the default is "limit". This should match how
most people "intuitively" expect datetime math to work.
""""
We need end_of_month => limit for positive durations as well.
Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Jonathan Druart [Mon, 31 Jul 2017 14:02:42 +0000 (11:02 -0300)]
Bug 17699: Add more tests to highlight the problem
Add problematic cases to highlight the problem.
Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Jonathan Druart [Mon, 31 Jul 2017 14:18:23 +0000 (11:18 -0300)]
Bug 19009: Fix random failures from Circulation.t
From jenkins output:
Subtest: CanBookBeIssued + Koha::Patron->is_debarred<7c>has_overdues
1..8
not ok 1
[SKIP]
I executed it several times and display the different $error, $alerts and question keys.
GNA and RESTRICTED were sometimes set, which block the issue.
Reading the code it seems that some patron's attributes must be removed to avoid the checkin rejection.
Test plan:
Execute the tests several times and notice that it fails randomly
With this patch they should always pass.
Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Jonathan Druart [Wed, 14 Jun 2017 14:32:30 +0000 (11:32 -0300)]
Bug 18802: Fix Circulation.t if finesMode ne 'production'
If finesMode is not set to production, only 1 fine will be created (the renewal
one will not). This is what assumes the tests.
If set to 'production', the tests will fail because the fines will not
be deleted (because of the DBIx::Class) warning.
Now we mock the value before charging.
prove t/db_dependent/Circulation.t
t/db_dependent/Circulation.t .. 16/95 DBIx::Class::Storage::DBI::select_single(): Query returned more than one row. SQL that returns multiple rows is DEPRECATED for ->find and ->single at t/db_dependent/Circulation.t line 491
t/db_dependent/Circulation.t .. 56/95
# Failed test 'Can auto renew, OPACFineNoRenewals=10, patron has 10'
# at t/db_dependent/Circulation.t line 670.
# got: 'auto_too_much_oweing'
# expected: 'auto_renew'
# Looks like you failed 1 test of 6.
Test plan:
prove t/db_dependent/Circulation.t
should return green whatever the value of finesMode
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Jonathan Druart [Wed, 5 Jul 2017 20:38:41 +0000 (17:38 -0300)]
Bug 18851: Tests must be skipped in cardnumber.t if Test::DBIx::Class is not installed
Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Jonathan Druart [Wed, 5 Jul 2017 20:38:22 +0000 (17:38 -0300)]
Bug 18851: Test::DBIx::Class is not needed in 00-load.t
Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Jonathan Druart [Wed, 5 Jul 2017 20:37:41 +0000 (17:37 -0300)]
Bug 18851: Database handler must not be created at runtime
Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Jonathan Druart [Mon, 7 Aug 2017 12:54:27 +0000 (09:54 -0300)]
Bug 19048: (bug 17829 follow-up) Fix regression in self checkout
$patronid is not necessarily set or does not match a valid cardnumber.
These cases must be taken into account to avoid the script to crash and
raise the following error:
Can't call method "unblessed" on an undefined value at
(...)/koha/opac/sco/sco-main.pl line 117
Test plan:
Hit sco/sco-main.pl and confirm that the error is gone with this patch
applied
Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
There is an action_logs entry via logaction() without transaction to be rolled
back in t/db_dependent/Log.t. This leaves an entry in action_logs after
the test is over.
To replicate:
1. prove t/db_dependent/Log.t
2. Observe a new entry in action_logs table
To test:
1. Apply patch
2. prove t/db_dependent/Log.t
3. Observe there are no new entries in action_logs
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>
Alex Buckley [Fri, 21 Jul 2017 08:47:48 +0000 (08:47 +0000)]
Bug 18987 - Added Koha::Account package pragma to Patron.pm
Test plan:
1. On an up to date branch go to Patrons and try to browse for a patron
by last name
2. Notice that the page processes indefinately
3. Apply patch
4. Repeat step 1
5. Notice the patron results are displayed
Signed-off-by: David Cook <dcook@prosentient.com.au>
Works as described.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This is a trivial fix, but finding the cause for a failing checkin in
SIPServer surely was not.
Without this simple change, I could not checkin an item while the SIP server
just closed the connection and syslog contained nothing useful.
The error can be easily reproduced without this patch by:
use Modern::Perl;
use C4::SIP::ILS::Item;
my $item = C4::SIP::ILS::Item->new('some_barcode');
where some_barcode should refer to an item checked out.
You should trigger the error (with your due date):
2017-07-28 23:59:00 is not an object at line 3.
Why the ILS/Item.pm module still compiles without a warning, is not clear
though.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Marcel de Rooy [Mon, 1 May 2017 11:04:07 +0000 (13:04 +0200)]
Bug 15140: Display $i when ind2==8
See MARC specification.
Note that we have a preceding text before all 776s too.
Test plan:
[1] Set field 776 ind2 to 8. And add text in $i.
[2] Look for this text on OPAC detail and staff detail.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Felix Hemme <felix.hemme@thulb.uni-jena.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Marcel de Rooy [Thu, 16 Mar 2017 11:37:12 +0000 (12:37 +0100)]
Bug 15140: Add MARC21 776 to OPAC and staff display
Adds code to opac and staff detail XSLT in order to show 776$t (or $a) and
a link based on 776$w (record control number). $w should be in the form:
(MARCOrgCode)Record-number
For instance: (NL-AmRIJ)12345
Note: You should have enabled default opac/intranet XSLT view.
Test plan:
[1] Add two 776s (one with $t and $w and one with $t only)
[2] Verify that you see "Additional physical formats" on opac-detail. Does
the link for $w work?
[3] Check intranet detail too.
[4] Empty 776$t and put text in $a. Check detail page again.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Felix Hemme <felix.hemme@thulb.uni-jena.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
To replicate:
1. Check the row count of borrowers, branches, categories, sessions, sms_providers
tables
2. prove t/db_dependent/Search/History.t
3. Repeat step 1
4. Observe borrowers the following tables have increased in row count:
- borrowers
- branches
- categories
- sessions
- sms_providers
To test:
1. Before applying the patch, go through steps at "To replicate" plan
2. Apply patch
3. Go through steps at "To replicate" plan
4. Observe step 4 no longer applies and those tables have the same number of
rows as before executing the test.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Test t/db_dependent/Auth.t seems to have an ineffective test data cleanup.
Data generated by TestBuilder is left in borrowers, branches, categories,
sms_providers and sessions tables after the test.
To replicate:
1. Check the row count of borrowers, branches and categories tables
2. prove t/db_dependent/Auth.t
3. Repeat step 1
4. Observe borrowers the following tables have increased in row count:
- borrowers
- branches
- categories
- sessions
- sms_providers
To test:
1. Before applying the patch, go through steps at "To replicate" plan
2. Apply patch
3. Go through steps at "To replicate" plan
4. Observe step 4 no longer applies and those tables have the same number of
rows as before executing the test.
This issue has been happening in REST tests as well, and this solution is
directly copy-pasted from t/db_dependent/api/v1/cities.t
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Bug 18977: Rollback branch in t/db_dependent/SIP/Message.t
To test:
1. Before applying patch, check the amount of branches in database
2. prove t/db_dependent/SIP/Message.t
3. See that a new branch is stored
4. Apply patch
5. prove t/db_dependent/SIP/Message.t
6. See that a new branch is no longer stored
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Jonathan Druart [Thu, 20 Jul 2017 16:39:43 +0000 (13:39 -0300)]
Bug 18966: Do not deal with duplicate issue_id on checkin
Koha suffers of big bugs due to its history: When data are deleted, they
are moved to another tables.
For instance issues and old_issues: when a checkin is done, it is moved
to the old_issues table.
That leads to a main problem that is described on
https://wiki.koha-community.org/wiki/DBMS_auto_increment_fix
However we tried first to fix the problem (for issues/old_issues) at
code level on bug 18242.
The goal was to prevent data lost.
Data lost may happens in this case:
Check an item out (issue_id = 1)
Check an item in (issue_id = 1)
Restart MySQL (reset auto increment for issue_id to 1)
Check an item out (issue_id = 1)
Check an item in => BOOM, the issue_id is a PK in old_issues and the
move fails.
Before bug 18242 the data were lost, we inserted the value into
old_issues, which fails silently (because of RaiseError set to 0 in
Koha::Database), then delete the row from issues.
That has been fixed using a transaction.
This patch introduced a regression we tried to fix on bug 18651 comment
0, the patron was charged even if the checkin was rejected.
A good way to fix that would have been to LOCK the tables:
1- Start a transaction
2- LOCK the table to make sure nobody will read id and avoid race
conditions
3- Move the content from one table to the other, dealing with ids
4- UNLOCK the table
5- Commit the transaction
But there were problems using LOCK and DBIx::Class (See commit 905572910b3a - Do no LOCK/UNLOCK the table).
Finally the solution implemented is not acceptable for several reasons:
- batch checkins may fail
- issue_id will always stay out of sync (between issues and old_issues)
See 18651 comment 66.
Since the next stable releases are very soon, and we absolutely need to
fix this problem, I am suggesting to:
1- Execute the move in a transaction to avoid data lost and reject the
checkin if we face IDs dup
=> It will only reject 1 checkin (max is 1 * MySQL restart), no need to
deal with race conditions,
2- Display a warning on the checkin page and link to a
solution/explanation
3- Communicate as much as we can on the proper fix: Update auto
increment values when the DBMS is restarted -
https://wiki.koha-community.org/wiki/DBMS_auto_increment_fix
4- Display a warning on the about page for corrupted data (see bug
18931)
5- Write and make available a maintenance script to fix corrupted data
(TODO LATER)
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Jonathan Druart [Mon, 24 Jul 2017 20:46:27 +0000 (17:46 -0300)]
Bug 18931: Add items/deleteditems tables
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Jonathan Druart [Wed, 12 Jul 2017 18:52:19 +0000 (15:52 -0300)]
Bug 18931: Add a "data corrupted" section on the about page - MySQL AI
See the following wiki page for more information
https://wiki.koha-community.org/wiki/DBMS_auto_increment_fix
Test plan:
Create (at least) a patron, a checkout, a biblio and a hold
Then fill the old_* or deleted* tables with:
INSERT INTO deletedborrowers SELECT * from borrowers WHERE borrowernumber=XXX;
INSERT INTO deletedbiblio SELECT * from biblio WHERE biblionumber=XXX;
INSERT INTO old_issues SELECT * from issues WHERE issue_id=XXX;
INSERT INTO old_reserves SELECT * from reserves WHERE reserve_id=XXX;
Go to the about page, 'System information' tab. You should see a new
"Data problems" section with the list of the ids that are wrong
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Jonathan Druart [Tue, 25 Jul 2017 19:49:38 +0000 (16:49 -0300)]
Bug 18982: Create a circ rule for the data we created
No need to have a default circ cule, we create one for the categorycode
and itemtype we are going to use.
The 3 checkouts will not be rejected (5 are allowed)
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
projects / koha.git / log