Resolve:
ok 5 - Engine returns error on undefined text
XSLT::Base: Empty String at /usr/share/koha/Koha/XSLT/Base.pm line 214.
ok 6 - Engine returns error on empty string
Test plan:
Run perl Base.t and verify that this warning is no longer printed.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Moving the separate small attachments into the test script.
Creating them now as temporary files or code fragments.
Deleting original files.
Since Breeding.t also used one of these files, a similar change was made in
that script.
Test plan:
Run t/db_dependent/Koha/XSLT/Base.t. Should pass now.
Run t/db_dependent/Breeding.t again.
Git grep on XSLT_Handler. You should only find release notes.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Should be XSLT::Base now.
Removes old XSLT_Handler stub too (from bug 23290).
Result of a git grep | sed statement.
Test plan:
Run qa tools (so modules compile).
Run t/db_dependent/Breeding.t
Run t/db_dependent/Koha/XSLT/Base.t (This test fails when only this patch
has been applied; see subsequent patch.)
Enable XSLT use on results and details display. Check search results and
detail view on OPAC and staff.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
The IDreamBooks service has not seen updates in a long time, so
we should remove the service from Koha as it's no longer operational.
To test:
- Apply patch and run the database update
- Verify that the IDreamBooks related system preferences are gone
- Verify that opac detail pages and result lists still work
as expected
- Run t/db_dependent/UsageStats.t
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Test plan:
Run t/db_dependent/Biblio/TransformHtmlToMarc.t without the module change
patch. Verify that the test fails on the order of fields.
Go to test plan of the second patch.
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
In the advanced search form, you can enable several limits using syspref
AdvancedSearchTypes (namely itemtypes, shelving locations, collections)
When used, the resulting query parts end up being joined with OR, even
if the field is different. That means that if you pick "Book" under
itemtypes tab, and "Fiction" under collection tab, it will search
"itype:BOOK OR ccode:FIC". It should be AND.
For instance, if you select:
Itemtypes:
✓ Book
✓ DVD
Location:
✓ Child
✓ Adult
it should search:
itype:(Book OR DVD) AND location:(Child OR Adult)
Test plan:
0. Do not apply the patch yet
1. Enable elasticsearch
2. Set syspref AdvancedSearchTypes = 'itemtypes|loc|ccode'
3. Create a new itemtype and a new authorised value for categories LOC
and CCODE
4. Create a biblio with the new itemtype, another biblio with the new
location, another biblio with the new collection, and again another
biblio with the new itemtype, location and collection
5. Verify that you can find these new biblio records using only the
"advanced search types" in the advanced search form
6. In the advanced search form, pick all 3 limits (itemtype, location,
collection) and verify that it returns the 4 records.
7. Apply the patch
8. Repeat step 6, it should now return only the biblio that satisfies
all criteria
9. Verify that if you select more than one
{itemtype|location|collection} it still returns results that
satisfies any selected criteria
10. prove t/Koha/SearchEngine/ElasticSearch/QueryBuilder.t
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
merge_with were returning Koha::Patron::Attribute for existing attribute
and hashref for new attribute.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
There is much more to do here, but this patch has the same behavior than
before: a warn is displayed in the log, the UI is not aware of it
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
->get_patrons has been removed but not replaced!
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Either we display everything, or only the info from the attribute itself
(i.e. not its type)
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
We do no longer need this package, we can use
Koha::Patron::Attribute::Types directly instead.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
We can then now start to move methods from C4::Members::AttributeTypes
as well.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
There is already a method in Koha::Patron::Attribute to check the
uniqueness constraint, let us it to replace CheckUniqueness
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This patch replace Koha::Patron->get_extended_attributes with
->extended_attributes
It's now a getter a setter method.
It permits to replace UpdateBorrowerAttribute and use
create_related from DBIx::Class
Notes:
* We face the same variable names difference than in a previous patch
(value vs attribute)
Bug 20443: Remove SetBorrowerAttributes
squash + RM get_extended_attributes
RM get_extended_attributes
SQUASH Bug 20443: Remove UpdateBorrowerAttribute and SetBorrowerAttribute
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Koha::Patron::Attributes->search mimicks what is done in
Koha::AuthorisedValues->search.
But actually it should be more explicit when the caller use it.
For instance filter_by_branch_limitation (see discussion on bug 11983).
This will be useful for the following patches as we will need a way to
replace the $no_branch_limit flag.
When the $no_branch_limit flag is called, a simple ->search call should
be done.
When we want to limit on a specific library we can pass the branchcode
in paramter of filter_by_branch_limitation (this is not used yet).
If not passed the logged-in user library will be used by default.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
The GetBorrowerAttributes subroutine return the attributes for a given
patron.
Using get_extended_attributes we can acchieve it easily. The problematic
here is to restore the method's name (value vs attribute,
value_description vs description of the authorised value, as well as
display_checkout that should not be a method of Attribute, but
Attribute::Type instead)
value_description was used when the attribute types were attached to an
authorised value category. To avoid the necessary test in template and
controller there is now a $attribute->description method that will
display either the attribute's value OR the value of the authorised
value when needed. We should certainly use this one from few other
places.
Notes:
* This patch rename Koha::Patron->attributes with Koha::Patron->get_extended_attributes.
It will be renamed with Koha::Patron->extended_attributes in ones of the next
patches when it will become a setter as well.
* GetBorrowerAttributes did not care about the library limits, we still
do not
* The opac_only flag was not used outside of test, we drop it off.
* To maintain the existing behavior we add a default order-by clause to
the search method [code, attribute]
* From C4::Letters::_parseletter we always display the staff description
of the AV, There is now a FIXME to warn about it
* FIXMEs are not regressions, existing behaviors must be kept
* TODO add a new check to bug 21010 to search for inconsistencies in
patron's attributes attached to non-existent authorised values
* One test has been updated in Modifications.t, order_by is now
by default set to ['code', 'attribute']
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
We want to retrieve a specific patron's attribute for a given patron.
We then add a new method to Koha::Patron.
This patch add a getter method ->get_extended_attribute_value
to use the DBIx::Class relation
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Same as previously for methods that have been added by bug 17792.
It's better to be explicite and tell we are fetch the related attribute's type
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
These methods have been added to Koha::Patron::Attribute but are wrong, see bug 18339
We should use ->type->$method instead
Moreover the tests exist in another subtest, we do not need them.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
There is something wrong, and a regression has been caught by those
tests:
If an invalid date is passed from the add item form, the app now
crashes.
Before:
* if the date was completely invalid, the field was blanked
silently
* DateTime::Format::MySQL was used to convert dates, and it's not
strict at all. For instance, what happened in the selenium tests for
dateaccessionned: %Y-%m-%d was prefilled by the framework plugin, then
the biblionumber was added, we ended with something like (eg for today)
2020-03-234242 (with biblionumber=4242). DateTime::Format::MySQL
converts that to 2020-03-23
We must deal with invalid dates, but I do not think it is good to add it
back to Koha::Item->store, we will prefer to raise the error to the end
user, saying that something went wrong (and more specifically the
dates).
The (ugly) trick was in C4::Items::_mod_item_dates
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This patch fixes an issue when editing items.
* The issue
Cannot blank a subfield when editing an item.
If you have an item with itemcallnumber=42, then edit it, blank it and
save. The itemcallnumber is still 42
* Why? (line numbers from https://gitlab.com/joubu/Koha/-/tree/bug_23463)
additem (and other item's edition forms) receives a list of tags,
subfields and values, generates a MARC::Record::XML then calls
ModItemFromMarc:
717 my $itemtosave=MARC::Record::new_from_xml($xml, 'UTF-8');
727 my $newitem = ModItemFromMarc($itemtosave, $biblionumber,
$itemnumber);
And ModItemFromMarc:
282 my $item = TransformMarcToKoha( $localitemmarc,
$frameworkcode, 'items' );
283 $item->{cn_source} = delete $item->{'items.cn_source'}; #
Because of C4::Biblio::_disambiguate
284 $item_object->set($item);
ModItemFromMarc never knows that the field has been blank.
Prior to bug 23463 we had a map of default values, and ModItemFromMarc
was doing:
426 my $item = TransformMarcToKoha( $localitemmarc,
$frameworkcode, 'items' );
427 my $default_values = _build_default_values_for_mod_marc();
428 foreach my $item_field ( keys %$default_values ) {
429 $item->{$item_field} = $default_values->{$item_field}
430 unless exists $item->{$item_field};
431 }
I do not want to reinsert that list of default values.
Here I wrote a generic method in Koha::Object to set the value passed in parameter,
or "blank" if not passed.
It's nulled if can be set to null in DB, or the default value is retrieved from
the schema info.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
From commit bf49eecdd87e2b29760226281ab1afc0a185c7f0
Bug 23463: Replace AddItem calls with Koha::Item->store
in build_sample_item:
- my $itype = delete $args->{itype}
- || $self->build_object( { class => 'Koha::ItemTypes' } )->itemtype;
+ # If itype is not passed it will be picked from the biblio (see Koha::Item->store)
So before we generated a new itemtype if not passed, now we pick the one from biblioitem->itemtype.
For this specific test we need to make sure they are different.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
It seems that some of those tests could be removed as safe_delete is
widely tested in Koha/Item.t
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>
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>
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>
Few occurrences have been added since this patchset has been originaly
written
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 change to Koha::Item->store seems correct here, but tests from /db_dependent/Items.t is failing now.
Adjusting them to make them pass.
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>
So, we get less warnings. I do not really understand why...
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>
And adding a FIXME about discard_changes
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>
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>
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>
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>
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>
Starting to replace the ModItem calls with Koha::Item->store
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>
We are done with AddItem, only need to log and index.
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>
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>
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>
The subroutine SearchItemsByField is only called once and can be
replaced easily with SearchItems
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>
Test plan
1/ Run test and verify it passes
2/ Signoff
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Magnus Enger <magnus@libriotech.no>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
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>
Test plan:
0/ Read the commit and confirm the new tests make sence
1/ Run the tests and verify they pass
2/ Signoff
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
