Commit graph

395 commits

Author SHA1 Message Date
571a13f667 Bug 29844: Some more uncaught cases
Some more cases found using

$ git grep '\->search(' | grep -v -e '\->as_list' -e '\->get_column' \
      | grep '@'

and then manually looking at them.

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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-02-09 15:36:23 -10:00
66a01d4b47 Bug 29243: Correct call when adding items from staged records
To test:
1 - Populate system preference  NewItemsDefaultLocation
2 - Stage a file of marc records
3 - Create an acquisitions basket with 'AcqCreateItems' set to 'ordering'
4 - Attempt to add to basket from your staged file
5 - You get a 500 error, and in the logs:
    Can't use string ("") as a HASH ref while "strict refs" in use at /usr/share/koha/lib/C4/Items.pm line 1605.
6 - Apply patch
7 - Repeat #4
8 - Success!

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-18 11:28:41 +02:00
c66668b16c Bug 28769: Remove tabloop from cataloguing plugins
There is a "tabloop" variable that is passed from the add item form logic to the cataloguing plugins.
But there is confusion, sometimes it's an iterator ($i) and sometimes (batchMod.pl) an array.

Actually this tabloop variable is never used from cataloguing plugins, we should remove it.

Test plan:
Read the code and confirm the above.
You can also test a couple of plugins and confirm that they are still
working.

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>
2021-10-18 11:28:40 +02:00
08c03af2ae Bug 18984: Remove NORMARC support
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>
2021-10-07 15:36:40 +02:00
46a1342226 Bug 22690: Remove uneeded return and add no_triggers
* C4/Items.pm
  - Koha::Biblios not used

* Koha/Item.pm
  - Koha::Item->orders must return an empty set if no order attached
  - no_triggers should be passed to other update calls

* Item.t
  - No need to build a fund
  - Add new test to test Koha::Item->orders when no order attached

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-08-27 15:28:23 +02:00
Ere Maijala
2615ee1010 Bug 22690: Refactor merging of records to improve performance (Elasticsearch)
This patch allows merging of records with many items without the web server timing out.

Test plan:

Without the patch:

- Create 2 records (one with e.g. 1000 items).
- Do a cataloguing search that displays both records, select them and click "Merge selected".
- Choose the record with many items as the one to be eliminated.
- Start the merging.
- After a while the web server should give you a timeout error (the merging process may still continue)

With the patch:
- Do the same as above
- This time verify that the records are merged without timeout
- Create a new biblio with an item
- Add with the item:
  * acquisition order
  * hold (reserve)
- Merge the biblio to another one
- Verify that the item and its related data was moved
- Verify that tests pass:
  prove -v t/db_dependent/Koha/Biblio.t
  prove -v t/db_dependent/Koha/Item.t
  prove -v t/db_dependent/Koha/SearchEngine/Indexer.t

Signed-off-by: Michal Denar <black23@gmail.com>
Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-08-27 11:02:40 +02:00
421e3972c6 Bug 28776: Warns from GetItemsInfo when biblio marked as serial
GetItemsInfo sorts the items using serial fields if the biblio is
defined as a serial

The fields may not be defined. We can add a fallback to blank in this case

To test:
1 - Mark a record with items as serial in the marc (942$s)
2 - At least one of the items should have no enumchron defined
3 - Load the detail page
4 - Warns in logs:
    [WARN] Use of uninitialized value in string comparison (cmp) at /kohadevbox/koha/C4/Items.pm line 811.
5 - Apply patch and restart all
6 - Reload the details page
7 - No more warns

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-08-18 14:53:59 +02:00
914d1c97f6 Bug 28533: Fix other ocurrence
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-08-04 09:14:46 +02:00
9d6d641d1f Bug 17600: Standardize our EXPORT_OK
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>
2021-07-16 08:58:47 +02:00
0e00dc78e4 Bug 27837: (follow-up) Do not check subfield unless tag defined
This was causing extra warnings when the permanent_location field was
unmapped.

We only need to check if there is a field defined as you can define a mapping
without both a field and subfield, so the existence of one implies the other

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-05-19 15:06:42 +02:00
db382ebaff Bug 27837: Check the MARC directly, not the transformed item
If permanent location is mapped, it will exist in the MARC.
However, when we are transforming our item hash to MARC,
we are losing our permanent location mapping, at least in unit tests.

By checking the original MARC for the permenent location, we remove the
risk of losing it the "empty" permenent location through the
transformation process. We only need to know that it existed in the
original item marc to know we need to update it in the database.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-05-19 15:06:42 +02:00
aa1d86920b Bug 27837: Permanent location to be empty string, check if key exists
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-05-19 15:06:42 +02:00
97647da9a0 Bug 27837: Update permanent location if passed into ModItemFromMarc
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Rhonda Kuiper <rkuiper@roundrocktexas.gov>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-05-19 14:52:19 +02:00
58c275e04b Bug 23583: Handle OpacHiddenItems with yaml_preference
This patch makes all places in Koha that rely on OpacHiddenItems
actually use C4::Context->yaml_preference instead of manually calling
the YAML libraries and handling it.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Items* \
           t/db_dependent/Koha/Item* \
           t/db_dependent/Koha/Filter/EmbedItems.t
=> SUCCESS: Tests pass!
3. Try hiding things with opac-ISBDdetail.pl and opac-search.pl
=> SUCCESS: Things work the same!
4. Sign off :-D

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-21 15:25:07 +02:00
d776fa7b27 Bug 27545: Use NewItemsDefaultLocation from places where an item is created
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>
2021-03-09 17:20:47 +01:00
6ec94fb198 Bug 24446: Update ModItemTransfer for daterequested/datecancelled
C4::Items::ModItemTransfer is used throughout the codebase and currently
it will never set the daterequested or datecancelled fields.  With the
modifications to how circulation deals with transfers we need to update
this function to set those fields appropriately. Functionality has been
retained, ModItemTransfer will continue to add a transfer regardless of
limits or current transits existing.

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>
2021-03-03 15:36:13 +01:00
d9ff0b5023 Bug 8976: Change for the item forms
Sponsored-by: Orex Digital

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-03-01 15:14:22 +01:00
fafcbff015 Bug 27673: Fix encoding issues
There is a difference between YAML::Load and YAML::XS::Load
From YAML::XS pod:
"YAML::XS only deals with streams of utf8 octets"

Test plan:
We are going to test 1 occurence and QA will confirm others don't
contain typos.
0. Don't apply the patches
1. Create a new itemtype with code=❤️
2. Create a new item using this itemtype (to biblionumber=1 will work)
3. Fill OpacHiddenItems with
 itype: [❤️]
4. Search for "street shuffle" or any terms that will return the biblio
Notice that the item is there (there is an error in logs)
5. Apply the patches
6. Repeat 4 and confirm that the item is now hidden

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-02-16 14:54:50 +01:00
46f7239b08 Bug 27673: Replace YAML with YAML::XS
From tht YAML pod:

"""
This module has been released to CPAN as YAML::Old, and soon YAML.pm will be changed to just be a frontend interface module for all the various Perl YAML implementation modules, including YAML::Old.

If you want robust and fast YAML processing using the normal Dump/Load API, please consider switching to YAML::XS. It is by far the best Perl module for YAML at this time. It requires that you have a C compiler, since it is written in C.
"""

See also
https://gitlab.com/koha-community/qa-test-tools/-/merge_requests/35

Test plan:
Try some place where YAML::XS is not used and confirm that it works
correctly

QA note: This patch removes some uses of YAML that were not useful

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-02-16 14:54:50 +01:00
c8adefecb7 Bug 4037: Correctly join the itemtypes array
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-02-09 09:23:51 +01:00
Aleisha Amohia
8057a433f6 Bug 4037: Add item type filter to inventory tool
This patch adds the ability to filter inventory by item type. Multiple
item types can be selected at once.

To test:
1) Apply patch and restart services.
2) Set up (at least) one item of a specific item type.
3) Go to Tools -> Inventory.
4) Scroll down to find the item types filter. Confirm the 'select all'
and 'clear all' buttons work as expected.
5) Select a few item types, but DO NOT include the item type that you
just set for your item. Confirm that your item does not show in the
results.
6) Go back to the Inventory tool. This time submit a search that DOES
include the item type you just set for your item. Confirm that your item
does show in the results.
7) Confirm tests pass:
prove t/db_dependent/Items/GetItemsForInventory.t

Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ)

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-02-09 09:23:51 +01:00
dcd4ab94cd Bug 27509: Prevent cn_sort value to be lost when editing items
This is a bit dirty, cn_sort is not passed from the UI but built in
Koha::Item->store depending on the values of itemcallnumber and
cn_source.
It must be updated only if one of those 2 attributes are modified.
The problem is that, as it's not passed, $item->{cn_sort} does not exist,
and set_or_blank will set it to undef.
The trick here is to backup the value before set_or_blank and set it
back to the item object.
Another solution would be to force the processing of cn_sort each time
we call Koha::Item->store. I don't think that's a good idea.

Test plan:
- Create a new item with a cn_source value and an itemcallnumber value
- write a quick report to see the cn_sort value: SELECT cn_sort FROM items WHERE itemnumber=your itemnumber, see your item has a cn_sort value
- edit your item and save it without changing either the cn_source of the itemcallnumber
- run your report again, cn_sort is not modified
- edit your item, changing either the cn_source or itemcallnumber
- run report again, cn_sort is modified as expected

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-01-22 13:46:38 +01:00
a96c73eb76 Bug 25306: Remove framework paramter from ModBiblioMarc
Tested with :

For a framework (not the default) :

Creation of a biblio record
Edition of this biblio record
Creation of an item of this record
Creation of an item of this record

./misc/batchRepairMissingBiblionumbers.pl OK

prove t/db_dependent/Biblio/ModBiblioMarc.t OK

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-01-12 16:13:50 +01:00
Matthias Meusburger
cb4df3b29a Bug 20888: (follow-up) 'is not' operator behaves as 'not like'
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-11-04 12:59:33 +01:00
07cab93d5d Bug 26032: (follow-up) Replace IFNULL() mysqlism with COALESCE()
Signed-off-by: Emmi Takkinen <emmi.takkinen@outlook.com>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-11-04 12:59:32 +01:00
8a874de3f4 Bug 26032: Add 'is new' filter in items search
In items search, add a filter on value of items.new_status with a Ignore/Yes/No.
Yes meaning != 0, No meaning = 0, with considering that NULL value is like 0.

This patch adds 'isnull' option to filter, this may be reused on other
fields.

Test plan :
1) Define an item subfield on items.new_status with authorised values YES_NO (0 or 1)
2) Edit an item A to set new_status=1
3) Edit an item B to set new_status=0
4) Perform an item search with 'is new' = 'ignore' => you get all items
5) Perform an item search with 'is new' = 'yes' => you get item A
6) Perform an item search with 'is new' = 'no' => you get all items but A

Signed-off-by: Emmi Takkinen <emmi.takkinen@outlook.com>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-11-04 12:59:32 +01:00
8a5cda8b7a Bug 26581: Only reindex records once per checkin
This patch simply passes skip_record_index calls to Koha:Item:store
for all the changes done in AddReturn.

Testing is really verifiying that items are still correctly indexed at the end

For both search engines
To test:
1 - Find or create a record with multipel items
2 - Populate both:
     UpdateItemLocationOnCheckin
     UpdateNotForLoanStatusOnCheckin
3 - Confirm that checking in an item correctly updates the item status in search results
4 - Test with items issued, and items not issued
5 - Test when generating a transfer (checkin at different branch)
6 - Test when item was marked lost
7 - Test when filling transfer

Signed-off-by: Séverine Queune <severine.queune@bulac.fr>
Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-10-22 10:23:51 +02:00
205d909a56 Bug 25265: Rename skip_modzebra_update to skip_record_index
Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-09-28 10:10:08 +02:00
88cb7f223d Bug 25265: Prevent double reindex of the same item in batchmod
When batch editing, 2 reindex calls are sent to ES/Zebra.
We can easily avoid that reusing the skip_modzebra_update (renamed skip_record_index)

Additionally we should only send one request for biblio, and we should
only do it if we succeed

As the whole batch mod is in a transaction it is possible to fail in which case
Zebra queue is reset, but ES indexes have already been set

In addition to the skip param this patchset moves Zebra and Elasticsearch calls to
Indexer modules and introduces a generic Koha::SearchEngine::Indexer so that we don't
need to check the engine when calling for index

The new index_records routine takes an array so that we can reduce the calls to
the ES server.

The index_records routine for Zebra loops over ModZebra to avoid affecting current behaviour

Test plan:

General tests, under both search engines:
  1 - Add a biblio and confirm it is searchable
  2 - Edit the biblio and confirm changes are searchable
  3 - Add an item, confirm it is searchable
  4 - Delete an item, confirm it is not searchable
  5 - Delete a biblio, confirm it is not searchable
  6 - Add an authority and confirm it is searchable
  7 - Delete an authority and confirm it is not searchable

Batch mod tests, under both search engines
  1 - Have a bib with several items, none marked 'not for loan'
  2 - Do a staff search that returns this biblio
  3 - Items show as available
  4 - Click on title to go to details page
  5 - Edit->Item in a batch
  6 - Set the not for loan status for all items
  7 - Repeat your search
  8 - Items show as not for loan
  9 - Test batch deleting items
    a - Test with a list of items, not deleting bibs
    b - Test with a list of items, deleting bibs if no items remain where all items are only item on a biblio:
     SELECT MAX(barcode) FROM items GROUP BY biblionumber HAVING COUNT(barcode) IN (1)
    c - Test with a list of items, deleting bibs if no items remain where some items are the only item on a biblio:
     SELECT MAX(barcode) FROM items GROUP BY biblionumber HAVING COUNT(barcode) IN (1,2)
 10 - Confirm records are update/deleted as appropriate

Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-09-28 10:10:08 +02:00
0629d4082f Bug 24134: Add placeholder for 2 digit years to allow autogeneration of dates in 008
This patch modifies the code for parsing MARC framework configurations
so that the placeholder "<<YY>>" translates to a two-year date.

To test, apply the patch and modify a framework to include placeholders.
For example, in MARC21:

 - Administration -> MARC structure -> 952 -> Edit subfields -> d
 - Expand "Advanced constraints" and add a default value:
   <<YYYY>> <<YY>> <<MM>> <<DD>> <<USER>>
 - Save your configuration and go to Cataloging.
 - Go to the add/edit items page for a record which uses the framework
   you edited.
 - In the "Add item" form, in the "Date aquired" field, you should see
   the correct values. For example: '2019 19 12 03 Leonard'

Perform the same test when adding an item during the Acquisitions
process (depending on your AcqCreateItem setting).

Note that acqui/neworderempty.pl has been modified because the
placeholder-replacement code is repeated there, but I couldn't discover
how to test it (if it is used at all?)

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
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>
2020-08-13 07:55:44 +02:00
189851fb9b Bug 25964: Prevent data loss when editing items from a MARC record
Coming from:
  Bug 23463: Use new method Koha::Object->set_or_blank

We have DB fields that are not mapped with MARC fields, for instance paidfor. They are not handled correctly.

In ModItemFromMarc, we get a MARC record in parameter and update the item in DB. But we are loosing the fields that are not in the MARC record

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2020-07-13 11:41:45 -03:00
Julian Maurice
b168f4a2e9 Bug 21395: Make perlcritic happy
This patch adds a .perlcriticrc (copied from qa-test-tools) and fixes
almost all perlcrictic violations according to this .perlcriticrc
The remaining violations are silenced out by appending a '## no critic'
to the offending lines. They can still be seen by using the --force
option of perlcritic
This patch also modify t/00-testcritic.t to check all Perl files using
the new .perlcriticrc.
I'm not sure if this test script is still useful as it is now equivalent
to `perlcritic --quiet .` and it looks like it is much slower
(approximatively 5 times slower on my machine)

Test plan:
1. Run `perlcritic --quiet .` from the root directory. It should output
   nothing
2. Run `perlcritic --quiet --force .`. It should output 7 errors (6
   StringyEval, 1 BarewordFileHandles)
3. Run `TEST_QA=1 prove t/00-testcritic.t`
4. Read the patch. Check that all changes make sense and do not
   introduce undesired behaviour

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-06-29 12:37:02 +02:00
abc00eb1cf
Bug 23081: Revert "Bug 24443: Consider NULL as 0 for issues in items search"
This reverts commit 80f1374f26.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-04-27 11:32:56 +01:00
Katrin Fischer
6f3c1d5af8
Bug 21927: Acq - Add blank values in pull downs of mandatory item subfields
This is the same fix as on bug 14662, which fixed the behaviour in
cataloguing, but for the item form in acquisitions.

The code assumes that if a subfield is marked as mandatory, there
should be no empty entry in the pull downs.

This assumption is not correct, as it leads to the first entry of the
pull down being preselected if there is no default set. As the field
can never be 'unset', there will never be a 'required' warning.
Furthermore, it might be counterproductive to use mandatory fields,
as it might be easily forgotten to change the preselected value and
those mistakes will be hard to find.

Correct behaviour would be to preselect the empty value when there is
no default. This means on saving the item an error message is triggered
and the cataloger is forced to set the value.

To test:
- This is best tested with an ACQ framework, but default can be used
  when no ACQ framework was created.
- In your MARC bibliographic framework:
  - In 952 make itemtype, classification source and some other pull downs
    like location or collection mandatory and set them to visibel if needed
- Create a new basket with 'items created while ordering'
- Add a new order, an existing record with 942$c set will work best
- Add items for your order line
  - Verify that the first value of each pull down is preselected,
    there is no way to trigger the 'required' error
- Apply patch
  - Add a new order line
    - Verify that classification source is preselected according to the
      DefaultClassificationSource system preference (try unsetting it later)
    - Verify all mandatory fields can be set to empty
    - Verify that you can't save before correctly setting them
  - Change your frameworks and set a default for itemtype (Ex: BK) and
    another mandatory and non-mandatory field of your choice
  - Add a new order line and item and verify the defaults are selected

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>
2020-04-27 11:14:58 +01:00
a1437dae8d
Bug 23463: Remove double store call
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-04-17 10:27:28 +01:00
6fa71e82f8
Bug 24027: (QA follow-up) Fix POD warning
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-04-14 08:15:13 +01:00
Andrew Nugged
3eec846d7f
Bug 24027: Call ModZebra once after all items added/deleted in a batch
Issue description:
- call to ModZebra was unconditional inside 'store' method for Koha::Item,
  so it was after each item added, or deleted.
- ModZebra called with param biblionumber, so it is the same parameter
  across calls for each items with same biblionumber, especially when we
  adding/removing in a batch.
- with ElasticSearch enabled this makes even more significant load
  and it is also progressively grows when more items already in DB

Solution:
- to add extra parameter 'skip_modzebra_update' and propagate it down to
 'store' method call to prevent call of ModZebra,
- but to call ModZebra once after the whole batch loop in the upper layer

Test plan / how to replicate:
- make sure that you have in the admin settings "SearchEngine" set to
  "Elasticsearch" and your ES is configured and working
  ( /cgi-bin/koha/admin/preferences.pl?op=search&searchfield=SearchEngine )
- select one of biblioitems without items
  ( /cgi-bin/koha/cataloguing/additem.pl?biblionumber=XXX )
- press button "add multiple copies of this item",
- enter 200 items, start measuring time and submit the page/form...

On my test machine when adding 200 items 3 times in a row (so 600 in
total, but to show that time grows with every next batch gradually):

WHEN ElasticSearch DISABLED (only Zebra queue):
- 9s, 12s, 13s
WHEN ElasticSearch ENABLED:
- 1.3m, 3.2m, 4.8m
WITH PATCH WHEN ElasticSearch ENABLED:
- 10s, 13s, 15s

Same slowness (because also same call to ModZebra) happens when you try
to delete all items ("op=delallitems"). And same fix.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-04-14 08:14:42 +01:00
01219b09ba
Bug 21800: Check the correct framework for the repeatable tag
This depends on the framework parameter. Which should be added back to
the call in C4::Items.

Test plan:
[1] Mark in Default framework one subfield A repeatable and B not repeatable.
Go to item editor. (Work on a biblio in Default framework.)
Check saving and reopening these subfields with VAL1 | VAL2.
Subfield A should be cloned, B should be glued as entered.

[2] Mark in another Framework A not repeatable and B repeatable.
Change framework for this biblio.
Go to item editor again. Reopen item. Behavior subfields in reverse?

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Ere Maijala <ere.maijala@helsinki.fi>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-03-24 10:54:46 +00:00
243a5395de
Bug 23463: Fix wrong variable name
Fix for t/db_dependent/Labels/t_Batch.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-03-23 10:38:30 +00:00
e6d255b5e5
Bug 23463: Fix items.cn_sort vs cn_sort
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-03-23 09:26:35 +00:00
a881984413
Bug 23463: Use new method Koha::Object->set_or_blank
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>
2020-03-23 09:26:34 +00:00
befc3f19f7
Bug 23463: Fix items.cn_source vs cn_source
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>
2020-03-23 09:26:32 +00:00
2088b6857e
Bug 23463: Fix CancelReceipt.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>
2020-03-23 09:26:31 +00:00
2677da8f17
Bug 23463: Remove no longer needed subs related to default values
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>
2020-03-23 09:26:31 +00:00
a3687711b0
Bug 23463: Remove DelItemCheck and ItemSafeToDelete
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>
2020-03-23 09:26:31 +00:00
b844b19dfc
Bug 23463: Remove DelItem
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>
2020-03-23 09:26:31 +00:00
fc09b0d491
Bug 23463: Remove ModItem
There should not be any calls left

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>
2020-03-23 09:26:31 +00:00
6ae8919e92
Bug 23463: Remove _mod_item_dates
We did not do anything useful here.

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>
2020-03-23 09:26:31 +00:00
516e76d1b5
Bug 23463: _do_column_fixes_for_mod dealt with integer location and timestamp moved
That's done in Koha::Item->store, and Koha::Object->store for integers.

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>
2020-03-23 09:26:30 +00:00
7ec06a1ae8
Bug 23463: _set_derived_columns_for_mod was only used for cn_sort
Same as _set_defaults_for_add

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>
2020-03-23 09:26:30 +00:00