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>
typo responsability
typo defautl in authorities.pref
typo reveived in t/db_dependent/Acquisition.t
typo ;; in advance_notices.pl
typo Stopping in restart_indexer (koha-indexer)
typo instutitional in moremember.pl
typo Corretly (Biblio.t)
typo periodicy in help serials
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
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>
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>
Test Plan:
1) Apply this patch
2) prove t/db_dependent/Clubs.t
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>
MARC::Record::append_fields takes a list of MARC::Field (not an arrayref)
Use $record->subfield() instead of $record->field()->subfield() to avoid errors
when field doesn't exist
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch makes sure the tests have the biblio.biblionumber mapping mocked
so we test the case where the mapping is to a control field instead of just
regular data fields (in the case of UNIMARC).
To test:
- Apply the patch
- Run:
$ prove t/db_dependent/Koha/Filter/EmbedItemsAvailability.t
=> FAIL: Tests fail due to an attemp to access a subfield on a control field.
Sponsored-by: ByWater Solutions
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Removing dbh from one script, changing rollback in the other.
Schema is leading now.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
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>
Signed-off-by: Andreas Hedström Mace <andreas.hedstrom.mace@sub.su.se>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
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>
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>
TEST PLAN
---------
1) prove -v t/db_dependent/Letters.t
-- fails at test 5 or so.
2) apply patch
3) run step 1
-- success
4) run koha qa test tools
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Fixed OAI-PMH Server tests to delete any existing issues before deleting biblios,
to delete oai_sets to avoid sets in the responses and to work with UNIMARC and
NORMARC as well as MARC21.
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
- Fixed date handling to use UTC as specs require.
- Added support for second precision in time stamps.
- Added support for marc21 metadata prefix as recommended in the
guidelines (synonym for marcxml).
- Improved performance of database queries especially for large
collections.
- Unified functionality of ListRecords and ListIdentifiers to a common
base class.
- If items are included in the records, their timestamps are taken into
account everywhere so that whichever is the most recent (timestamp of
biblioitem or any of its items) is considered the record's timestamp.
- Fixed OAI.xslt to show correct record range.
- Incorporated extended tests from Bug 17493 and their tweaks from Bug 15108.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Removes a warning.
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
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>
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>
Return the current checkout for a given item.
Note it may not exist.
Test plan:
prove t/db_dependent/Koha/Items.t
should return green
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>
Return the patron related to a given checkout
Test plan:
prove t/db_dependent/Koha/Checkouts.t
should return green
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>
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>
Reading https://perlmaven.com/how-to-return-undef-from-a-function
this sound like the more correct behaviour.
Considering:
$template->param(
stuff => Koha::Stuffs->find( $id ),
foo => 1,
);
without this patch, if the $id does not represent any rows in the DB,
stuff will be assigned to 'foo' and $foo will be undef in the template.
That can lead to very bad side-effects.
With this patch we make sure that it will never happen again.
Test plan:
prove t/db_dependent/Koha/Objects.t
should return green
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>
This patch makes the tests fail if extended attributes handling fails due to
wrong UTF-8 data handling.
To test:
- Run:
$ sudo koha-shell kohadev
k$ cd kohaclone
k$ prove t/db_dependent/Koha/Patron/Modifications.t
=> FAIL: Tests explode
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
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>
As noted on bug report 17669, the return values of delete (both singular
and plural), delete_missing and delete_temporary should be consistent.
Removed the if-construction around full_path. We do not need it; in the
very exceptional case that full_path would be empty, we should delete
the record since the file is missing.
Adjusted POD and unit tests accordingly.
Test plan:
Run t/db_dependent/Upload.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
If you deleted files from the upload directories manually, or you rebooted
with files in the temporary upload folder, or for some other reason have
records without a file, you may want to cleanup your records in the
uploaded_files table.
This patch adds the method delete_missing to Koha::UploadedFiles. It also
supports a keep_record parameter to do a dry run (count the missing files
first).
Also, we add an option --uploads-missing to cleanup_database. If you add
the flag 1 after this option, you will delete missing files. If you add the
flag 0 or only use the option, you will count missing files.
A subtest is added to Upload.t for delete_missing tests.
Test plan:
[1] Run t/db_dependent/Upload.t
[2] Upload a file and delete the file manually.
[3] Run cleanup_database.pl --uploads-missing
It should report at least one missing file now.
Check that the record has not been deleted.
[4] Run cleanup_database.pl --uploads-missing 1
It should report that it removed at least one file.
Check that the record is gone.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Added test description on line 387.
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>
Since we here only use one rollback and this test does not care, it is
more consistent to keep that pattern.
Additionally, promoting two globals to our.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch adds a new method to t::lib::TestBuilder so it can return
Koha::Object-derived objects. The new method is called ->build_object
and requires the plural of the target class to be passed.
'class' is a mandatory param, and a warning is raised and undef is
returned if absent.
It accepts 'value' as the original ->build() method, and that is passed as-is
to ->build().
To test:
- Apply the patches
- Run:
$ sudo koha-shell kohadev
k$ cd kohaclone
k$ prove t/db_dependent/TestBuilder.t
=> SUCCESS: Tests pass!
- Sign off :-D
Sponsored-by: ByWater Solutions
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Test plan:
prove t/db_dependent/Koha/Items.t
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>
Tests in db_dependent may expect a Koha database, but should not rely on
hardcoded categories, currencies, branch codes, etc.
This patch fixes a bunch of those. But this is a continuous project. We also
need QA to closely watch new edits.
Accounts.t: hardcoded category PT replaced
Acquisition/OrderFromSubscription.t: hardcoded USD
Acquisition/StandingOrders.t: same
ArticleRequests.t: create itemtype, branch and category for testing
AuthorisedValues.t: remove $dbh, add two test branches
AuthoritiesMarc.t: add hardcoded GEOGR_NAME authtype
Bookseller.t: add test currency
Koha.t: add test itemtype instead of hardcoded BK
UsageStats.t: add test branch and category
Test plan:
Run the adjusted tests.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
All tests successful (see comment #9)
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>
See Bugzilla comment36 (QA request).
Koha::UploadedFile->delete
Returns 1, 0E0 or -1 (unknown).
Koha::UploadedFiles->delete and delete_temporary
Returns number of deleted records, 0E0 or -1.
POD lines are corrected accordingly. Unit tests adjusted too.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Added a note that Koha::Object->delete itself is not completely consistent
with Koha::Objects->delete (DBIx). The singular may return 1 when it
gets 0E0 from DBIx, while the plural one may return 0E0. But should be
handled somewhere else.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
As requested by QA on comment33.
If the pref is 0 or the overriding command line parameter is 0, all
temporary files will be deleted. But if the pref is NULL or empty string,
we will not delete files.
Also adjusted the description of the preference in this regard.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Requested by QA on comment25.
Result of:
git grep -l Upload_PurgeTemporaryFiles_Days | xargs sed -i -e "s/Upload_PurgeTemporaryFiles_Days/UploadPurgeTemporaryFilesDays/g"
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Test plan:
Run t/db_dependent/Upload.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Typo: have been checkin => checked.
And five test descriptions ;)
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>
From the CHECKIN and CHECKOUT templates you should be able to access the
following information: item, biblio, biblioitem, patron and library
Test plan:
Define CHECKIN and CHECKOUT notice templates.
You should be able to define them using the TT syntax to produce the
same generated notice messages as with 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>
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>
We are going out of scope here, but these tests need a branch/item.
Test plan:
Run the adjusted tests.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Test plan:
prove all these tests, they must all pass
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>
No need to do it that way, let's use TestBuilder.
Test plan:
prove t/db_dependent/Suggestions.t
should still return green
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>
No need to create Staff users here.
Test plan:
prove t/db_dependent/Members.t
should return green, even if no categories.categorycode 'S' exists
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>
No need to create Staff users here.
Test plan:
prove t/db_dependent/Passwordrecovery.t
should return green, even if no categories.categorycode 'S' exists
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>
No need to create Staff users here.
Test plan:
prove t/db_dependent/Budgets.t
should return green, even if no categories.categorycode 'S' exists
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>
This patch makes the test create an itemtype, and use it for the created item so there's no warning.
To test:
- Run:
$ prove t/db_dependent/Serials.t
=> FAIL: item-level_itypes set but no itemtype set... warning raised
- Apply the patch
- Run:
$ prove t/db_dependent/Serials.t
=> SUCCESS: Tests pass and no warning is raised
- Sign off :-D
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>
Test plan:
Run the adjusted t/DateUtils.t
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
NOTE: This patch is amended in QA. The exceptions are moved from a separate
module to the general section of Exceptions.pm.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch makes the following changes:
[1] In Koha/DateUtils.pm, sub output_pref:
Add a test if $dt is really a DateTime before calling method ymd.
Preventing a crash like:
Can't locate object method "ymd" via package "dateonly".
See also BZ 17502/15822.
[2] Adds a few unit tests in t/DateUtils.t.
Test plan:
[1] Run the adjusted unit test t/DateUtils.t
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>
Clean a bit existing tests by adding default values and add a test to
highlight the issue.
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>
That way if prefs contain other languages, the test will still pass.
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>
The previous query was wrong. If an item type did not contain the
translation in the interface's language, the ->search_with_localization
did not return it at all.
What we need is definitely to add a second condition on the join.
For reference:
http://search.cpan.org/dist/DBIx-Class/lib/DBIx/Class/Relationship/Base.pm#conditionhttps://blog.afoolishmanifesto.com/posts/dbix-class-parameterized-relationships/
That sounds hacky but seems to be the DBIx::Class path to follow.
Bug 17835: follow-up
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>
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>
To properly move C4::Koha::GetItemTypes to Koha::ItemTypes we need
DBIx::Class to make a join on the localization table to retrieve the
possible translated description of the item types.
To do so there are 2 possibilities. The first one would have been to
rename the localization table to something like itemtype_localization.
That way we could have had a relationship between
itemtype_localization.code and itemtypes.itemtype
That would have meant to create one table per "entity" (here an entity
is itemtype) we allow the translability. There are pros and cons for
this choice, so I opt for another solution.
The other solution is to create a view on top of this localization
table. With this new view we can define the missing relationship.
That sounds like a quite clean solution and easy to implement.
Once we have this relationship, the
Koha::ItemTypes->search_with_localization will join on this view an
return the same result as GetItemTypes( style => 'array' ).
To replace
GetItemtypes( style => 'hash' )
which is the default behavior of this subroutine, we can do something like:
my $itemtypes = Koha::ItemTypes->search_with_localization;
my %itemtypes = map { $_->{itemtype} => $_ } @{ $itemtypes->unblessed };
This patchset must not introduce big changes but it changes certain
behaviors (which were wrong) in some scripts. Indeed sometimes the
descriptions of the item types were not the translated ones. Moreover it
happens that the item types displayed in a dropdown list were not
ordered by translated description, but by description of code
(itemtypes.itemtype value). These 2 behaviors are what we expect.
Test plan:
Bugs will be hard to catch since these patches change a lot of file, it
will be easier to read the diff and catch possible typos or logic
errors.
However signoffers can focus on modified files and the item types
values.
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>
[1] See comment102. Moved sub reporting_tag_xml to MergeRequest.pm.
Adjusted t/db_dependent/Koha/Authorities.t accordingly.
This resolves the C3 inconsistent hierarchy errors.
[2] Removed empty POD section Instance Methods from MergeRequests.
This resolves the POD error in comment102 point 2.
[3] Include a tag 100 for UNIMARC in reporting_tag_xml to resolve an error
on encoding in MARC::File::XML. Subtest for oldmarc and subtest for
reporting_tag_xml adjusted accordingly.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
The cron job is moved from migration tools to cronjobs. And renamed to
a plural form. The script is now based on Koha objects. It does no longer
include the code to merge one record. This can be done via the interface,
and will be added to a maintenance script on bug 18071. Should not be part
of this cron job.
Adding a cron_cleanup method to MergeRequests; this method is called from
the cron script to reset older entries still marked in progress and to
also remove old processed entries. Tested in a separate unit test.
Test plan:
[1] Run t/db_dependent/Authorities/MergeRequests.t
[2] Set AuthorityMergeLimit to 0. (All merges are postponed.)
[3] Modify an authority linked to a few records.
[4] Delete an authority linked to a few records with batch delete tool.
[5] And select two auth records with linked records.
Merge these two records with authority/merge.pl.
Note: Do not select Default. See also bug 17380.
[6] Check the need_merge_authorities table for inserted records.
[7] Run misc/cronjobs/merge_authorities.pl -b and inspect the linked
records and the record status in need_merge_authorities.
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>
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>
This subtest shows that we need a few little tweaks to make merge
work in some specific postponed merge situations.
Test plan:
[1] Run t/db_dependent/Authorities/Merge.t.
The last subtest should fail.
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>
When replacing the Zebra code in sub merge, we actually need CountUsage
as well as the results it gets from SearchEngine.
In order to get count and/or results, we now create:
[1] instance methods get_usage_count and linked_biblionumbers in
Koha::Authority,
[2] class methods of the same name in Koha::Authorities.
The instance method calls the class method. The class method can be used
for deleted authority records, and in 'legacy calls' before refactoring.
Note: On BZ 18149 we will replace all CountUsage calls.
Test plan:
Run t/db_dependent/Koha/Authorities.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>
When we will replace the Zebra code in sub merge, we will call SearchEngine
to pass records and we need a routine to extract a biblionumber from
a search result record. A record from Zebra still must be converted to
MARC::Record. This is no longer needed for a ElasticSearch record.
Test plan:
Run t/db_dependent/Koha/SearchEngine/Search.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 patch adds two Koha objects: MergeRequest(s).
MergeRequest has a new method and an oldmarc method.
A class method reporting_tag_xml is added to MergeRequests.pm.
All new routines are tested in Authorities.t.
Removes a few unneeded modules from Koha::Authority.
Test plan:
Run t/db_dependent/Koha/Authorities.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 patch adds a new filter for MARC records to be used with
Koha::RecordProcessor. It's purpose is to inject the information about
items not-onloan in a fixed subfield, 999$x.
To test:
- Apply the patch
- Run:
$ prove t/db_dependent/Koha/Filter/EmbedItemsAvailability.t
=> SUCCESS: Tests pass!
- Sign off :-D
Sponsored-by: ByWater Solutions
Followed test plan, test passes OK
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>
On the same way of Koha::Biblio->get_holds,
Koha::Biblio->get_holds_placed_before_today and Koha::Patron->get_holds,
this new subroutin will permit to retrieve the holds placed on a
specific item.
Note that at the moment we do not need a Koha::Item->get_holds method:
we do not want to display future holds placed in the future.
Test plan:
I would suggest to test this patch with patches from bug 17736 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: Kyle M Hall <kyle@bywatersolutions.com>
SendAlert now needs a userenv to find branch email and the fallback
of KohaAdminEmailAddress if the branch mail is not filled.
Test plan:
Run t/db_dependent/Letters.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Test plan:
prove t/db_dependent/Koha/Objects.t
Should return green
Followed test plan, result 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>
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>
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>
Unit tests for an overloaded Koha::Patron::Attribute->store that
checks attribute type's uniqueness property and raises an exception
conveniently.
It also tests for repeatable attribute type's property.
Test plan on the implementing patch.
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>
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>
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>
Since Koha::Objects has a update method, we should allow it too in
Koha::Object. Note that it is just redirecting to DBIx immediately.
Changed the exception when the method generates an error. The previous
code suggested that the method was not found, but this is not the case.
Test plan:
Run t/db_dependent/Koha/Object.t
Followed test plan, tests passes OK.
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>
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>
- Remove useless comments
- Use Koha::Objects::find instead of Koha::Objects::search
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Test plan:
* run without the patch, the test will fail
* run with the patch, the test will pass
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
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>
For an unknown reason, the use_ok('Circulation') does not work as
intended (see 3660c451a3).
With the new use of C4::Log, the trick does no longer work.
It does not make sense to add the use_ok('C4::Log') in Circulation.t,
removing it.
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>
