See the Bugzilla report. I have been asked to restore the former
tests although I definitely think that they are wrong. Will address
that on bug 34308 separately.
This currently has the side-effect of negative lost values being
interpreted as 'found'. (Do not use negative lost values!)
The added subtest still reflects that now. Added a few TODOs.
Test plan:
[1] Prove t/db_dependent/Koha/Items.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Some libraries would like to have the text version of the serials
"published on" field auto-generated from a template. This template
should be definable at the subscription level.
Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Restart all the things!
4) Create or edit a new subscription
5) Edit the "Publication date template", create a template toolkit
template.
Keys available are the Koha::Subscription object as 'subscription'
and the following serial table columns as keys:
serialseq
serialseq_x
serialseq_y
serialseq_z
subscriptionid
biblionumber
status
planneddate
publisheddate
publisheddateext
notes
routingnotes
So your example template could be "[% subscription.subscriptionid %] [% biblionumber %]"
6) Generate the next serial
7) Note the next issue has a "Date published (text)" field based on the
template you set!
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
No need to do a walk through on all the results.
Correcting two test descriptions on the way.
Test plan:
Run t/db_dependent/Koha/Item.t
Run t/db_dependent/Koha/Items.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
There's been a behavior change in recent MariaDB that made some cases
deal with truncated data in DOUBLE context instead of DECIMAL.
Probably this:
https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-27380
This made the affected test fail consistently on those versions.
This trick for checking OpacHiddenItems is correct (as introduced on bug
28125 doesn't really make use of the warning, just prints it in
about.pl) so one option was to just check for a warning.
I decided to keep the test, but add the optional DECIMAL|DOUBLE check on
the qr. This way other eventual changes will make it fail and serve as a
warning in case something more relevant changes.
To test:
1. Apply this patch
2. Run:
$ kshell
k$ t/db_dependent/Koha/Items.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Note this patch also fixes wrong text in test line 1295
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
As was done in the code already.
Note that we are ignoring effective itemtype now.
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>
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>
And fix a bug!
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>
This patch moves the Koha::Biblio->adopt_items_from_biblio method to the
Koha::Items set class and updates all calls from
Biblio2->adopt_items_from_biblio(Biblio1) to Biblio->items->move_to_biblio(Biblio2)
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch adds test test cases for both a Voided Payment and Voided
Writeoff. These cases need special handling in the refund process.
Test plan
0/ Don't apply
1/ Run t/db_dependent/Koha/Items.t
2/ Prior to this commit the test should pass
3/ Apply this commit but not the dependency(bug 22435)
3/ The test should fail
4/ Bug 22435 should make the test pass again.
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch updates the unit tests to reflect the changes we have made to
offset types. We do two things:
1) Clean up calls to 'apply' to drop passing offset_type
2) Update test code to look for APPLY and CREATE instead of specific
offset types.
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
On bug 17591 we discovered that there was something weird going on with
the way we export and use subroutines/modules.
This patch tries to standardize our EXPORT to use EXPORT_OK only.
That way we will need to explicitely define the subroutine we want to
use from a module.
This patch is a squashed version of:
Bug 17600: After export.pl
Bug 17600: After perlimport
Bug 17600: Manual changes
Bug 17600: Other manual changes after second perlimports run
Bug 17600: Fix tests
And a lot of other manual changes.
export.pl is a dirty script that can be found on bug 17600.
"perlimport" is:
git clone https://github.com/oalders/App-perlimports.git
cd App-perlimports/
cpanm --installdeps .
export PERL5LIB="$PERL5LIB:/kohadevbox/koha/App-perlimports/lib"
find . \( -name "*.pl" -o -name "*.pm" \) -exec perl App-perlimports/script/perlimports --inplace-edit --no-preserve-unused --filename {} \;
The ideas of this patch are to:
* use EXPORT_OK instead of EXPORT
* perltidy the EXPORT_OK list
* remove '&' before the subroutine names
* remove some uneeded use statements
* explicitely import the subroutines we need within the controllers or
modules
Note that the private subroutines (starting with _) should not be
exported (and not used from outside of the module except from tests).
EXPORT vs EXPORT_OK (from
https://www.thegeekstuff.com/2010/06/perl-exporter-examples/)
"""
Export allows to export the functions and variables of modules to user’s namespace using the standard import method. This way, we don’t need to create the objects for the modules to access it’s members.
@EXPORT and @EXPORT_OK are the two main variables used during export operation.
@EXPORT contains list of symbols (subroutines and variables) of the module to be exported into the caller namespace.
@EXPORT_OK does export of symbols on demand basis.
"""
If this patch caused a conflict with a patch you wrote prior to its
push:
* Make sure you are not reintroducing a "use" statement that has been
removed
* "$subroutine" is not exported by the C4::$MODULE module
means that you need to add the subroutine to the @EXPORT_OK list
* Bareword "$subroutine" not allowed while "strict subs"
means that you didn't imported the subroutine from the module:
- use $MODULE qw( $subroutine list );
You can also use the fully qualified namespace: C4::$MODULE::$subroutine
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch adds "filter_by_for_hold" method in "Items.pm" and
uses it in "cat-toolbar.inc" instead of "filter_by_for_load".
Also this patch removes "filter_by_for_loan" method.
To reproduce the bug:
1) go to /cgi-bin/koha/catalogue/detail.pl?biblionumber=XXX that has
item with notforloan value set as "Ordered" (-1)
2) see that button "Place hold" is not present
3) apply the patch
4) refresh the page and ensure that "Place hold" button appears even if
item is "Ordered"
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
If OPACHiddenItems is containing badly construct it will lead to
inconsistent behaviours. For instance the holds view will not display
any items, when the detail page will display all of them (because we are
not using the same code, we are in transition to merge it).
This patch suggests to add a warning to the about page in case the query
generated by Koha::Items->filter_by_visible_in_opac produces a warning
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Bug 28125: (QA follow-up) Fix missing filter
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Change in behaviour here, but expect. We must keep the location value if
passed to store.
Same for the permanent_location
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The syspref NewItemsDefaultLocation is used to set a default value for item's location.
But it seems that there are some weirdness in the behaviour:
1. It's only used from additem. It seems that it should be used from acq and serial modules as well. And maybe for the items import too.
2. It set the location even if another one has been picked from the UI
=> We UI must preselect the syspref's value, but the controller must pick what has been selected on the UI
This patch is adding the default to the UI and extend the use of the
pref to other areas.
Test plan:
Set a value to NewItemsDefaultLocation
Catalogue a new item and confirm that the syspref's value is picked to
selected the default value on the add item form
Same behaviour should apply to the acquisition and serial modules
When importing items, the default location must be used if the imported
items did not have a location defined.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch adds the 'cancel' method to Koha::Item::Transfer which sets
the transfer as cancelled by updating the datecancelled field. We also
update Koha::Item->get_transfer here to accomodate for the new
resolution available for a transfer.
Test plan:
1/ Run the included unit tests additions (t/db_dependent/Koha/Items.t,
t/db_dependent/Koha/Item/Transfer.t
Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
We updated Koha::Item->get_transfer here to account for the new
daterequested field availability. It is now possible to set requested
without setting datesent simultaneiously.. as such we need to update
get_transfer to ensure we always return the right transfer (either the
one 'in transit' or the oldest request first.
This patch update the unit tests to test for this.
Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This test makes all the generated sample items have undef copynumber,
and all rules are added copynumber => [ 2 ] on the tests as
suggested by Andrew on his failing example.
The tests are expected to pass. i.e. copynumber is clearly not equal to
2, so those items shouldn't be hidden.
To test:
1. Apply this patch
2. Run:
$ kshell
k$ prove t/db_dependent/Koha/Items.t
=> FAIL: Tests fail! undef is considered in [ 2 ]
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
We decided to inline the opachiddenitems filter as we don't believe it
will be used exclusively.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch introduces two new methods for stacking filters on
Koha::Items:
- filter_out_lost
_ filter_out_opachiddenitems
This two filters are what actually happened inside the
filter_by_visible_in_opac. Everything is covered by tests.
In the process I added a Koha::Patron param to the method that is
internally used to decide if the OPACHiddenItems syspref needs to be
honoured or not. This *could* be better done with a fallback to
C4::Context->userenv if no param is passed.
I decided to leave that part for later, if we really find it would help
(e.g. if bug 10589 gets some action and we really need something here to
handle that).
To test:
1. Apply this patch
2. Run:
$ kshell
k$ prove t/db_dependent/Koha/Items.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
After discussing the 'rules' parameter usefulness we decided it was not
the best idea, and the gains in terms of 'performance' would me
meaningless (in-memory caching of sysprefs). This patch makes a really
minor tweak to the tests so they mock the C4::Context->yaml_preference
method, but keeping the same original rules to highlight no behaviour
change takes place.
Then the rules parameter is removed from the calls, and the tests should
keep passing.
A minor change to make $rules = undef is made to highlight the // {}
behaviour when reading the syspref..
To test:
1. Apply this patch
2. Run:
$ kshell
k$ prove t/db_dependent/Koha/Items.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
On C4::Search and C4::Circulation the uses of the items.itemlost field
highlight the fact that the comparisson itemlost <= 0 was wrong, as it
is evaluated as a Perl boolean.
The column can only be an int and NOT NULL, so we need to check if it is
0 to ponder if not hidden.
This patch changes the tests to reflect this, and adjust the
Koha::Items->filter_by_visible_in_opac implementation to adapt to this.
To test:
1. Apply this patch
2. Run:
$ kshell
k$ prove t/db_dependent/Koha/Items.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Otherwise we don't have its itemnumber yet!
Test plan:
Add and modify an item and confirm that the action logs are created
correctly
Signed-off-by: Nick Clemens <nick@bywatersolutions.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: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
.pm must not have -x
.t must have -x
.pl must have -x
Test plan:
Apply only the first patch, run the tests and confirm that the failures
make sense
Apply this patch and confirm that the test now returns green
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
There is a "norequest" boolean passed to the include cat-toolbar.inc, to
display or not the "Pace hold" button.
This flag was not calculated in some place (ISBDdetail and moredetail
for instance)
Here we centralize the code to make it more robust and less regression
prone.
Note that the same problem appears at the OPAC (opac-MARCdetail is
always display the button). That will have to be fixed separately
Test plan:
Create biblio A with 0 item, B and C with 1 item and D with 2 items
item for B is not for loan, C is for loan and D has 1 of each
Go to the bibliographic detail page of each record and confirm that the
"Place hold" button appears appropriately.
Test the different tabs of the catalogue module
QA note: This patch is only centralizing the existing code, but it is
still too naive. To manage the visibility of this button we certainly
want to copy what is done in C4::Search::searchResults:
# can place a hold on a item if
# not lost nor withdrawn
# not damaged unless AllowHoldsOnDamagedItems is true
# item is either for loan or on order (notforloan < 0)
$can_place_holds = 1
if (
!$item->{itemlost}
&& !$item->{withdrawn}
&& ( !$item->{damaged} || C4::Context->preference('AllowHoldsOnDamagedItems') )
&& ( !$item->{notforloan} || $item->{notforloan} < 0 )
);
Signed-off-by: Didier Gautheron <didier.gautheron@biblibre.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch adds a test to ensure that the 'Koha::Item->store' method
will function when the userenv is not set
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch moves the _FixAccountForLostAndFound tests from Circulation.t
to Koha/Items.t and updates then to call an item store as aposed to the
now removed _FixAccountForLostAndFound method.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch adds a test for the *_on trigger functionality to ensure
matching behaviour before and after the triggers change.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
To test:
1 - Go to Circulation->Transfer
2 - Note your signed in branch
3 - Find an item from your branch and create a transfer to branch B
4 - Confirm the item is marked as held at your current branch and is being transferred to B
5 - Find an item from a third branch, branch C
6 - Transfer that item to brnach B
7 - Confirm the item is held at your current branch and is being transferred to B
8 - prove -v t/db_dependent/Circulation.t
9 - prove -v t/db_dependent/Koha/Items.t
10 - prove -v t/db_dependent/RotatingCollections.t
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This patch makes the following methods return array references of
Koha::Library objects instead or unblessed objects;
- Koha::Item->pickup_locations
- Koha::Biblio->pickup_locations
- Koha::Libraries->pickup_locations
Bonus:
- The template plugin is adjusted to unbless things to keep behavior
- Tests are moved to the right .t file.
- Tests for the new behavior are added.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
* Bug 22284: (follow-up) Remove commented warn and address test failures
* Bug 22284: (follow-up) fix test count after merge
* Bug 22284: (follow-up) fixes after 15496
* Bug 22284: (follow-up) fixes after 18936
* Bug 22284: (follow-up) Remove HomeOrHolding from reserves
Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This patch modifies Koha::Libraries->pickup_location and moves most of the logic to
Koha::Item and Koha::Biblio in preparation for api endpoints in the future.
There where 2 methods added
1) Koha::Item->pickup_locations that given a patron, returns all pickup locations of
this item, considering hold fulfillment rules, and hold allowed rules.
2) Koha::Biblio->pickup_locations that given a patron, returns a distinct list of
libraries returned by each of this biblio items pickup location.
Koha::Libraries->pickup_location analyzes input param and calls Koha::Item->pickup_locations
or Koha::Biblio->pickup_locations as needed.
Also in opac-reserve.tt the way options where obtained to fill the pickup location select
was modified to pass the patron as a parameter.
To test:
1) opac: try to place hold on a item and check that all libraries are shown in the
pickup location select.
2) intranet: in Library groups, add 2 root groups marked as local hold group and
add different libraries to each.
3) opac: login as a user of a library belonging to one hold group, and search try to
place a hold on an item belongin to the other hold group.
4) intranet: in Circulation and fines rules, play with 'Hold policy' and 'Hold pickup
library match' rules.
5) opac: On each modification of the rules reload the page.
SUCCESS => Every time you reload the page, the number of pickup locations showed in
select varies.
6) prove t/db_dependent/Koha/Biblios.t t/db_dependent/Koha/Items.t
SUCCESS => Result: PASS
7) Sign off
Sponsored-by: VOKAL
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Before these changes the tests fail if run under Elasticsearch
This makes sure they pass either way
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Marcel de Rooy
David Gustafsson
Kyle Hall
Aleisha Amohia
Tomas Cohen Arazi
Nick Clemens
Jonathan Druart
Fridolin Somers
Martin Renvoize
Petro Vashchuk
Agustin Moyano