https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Component|Cataloging |Searching - Elasticsearch QA Contact|testopia@bugs.koha-communit | |y.org | CC| |ere.maijala@helsinki.fi -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #2 from Vitor Fernandes <vitorfernandes87@gmail.com> --- (In reply to axel Amghar from comment #1)

hello, when i try to merge records, i have an issue : Following required fields are missing: -801

https://snag.gy/76Fh9R.jpg this is a mandatory tag but i can't add it Do someone know why?

Axel that happens because the framework chosen for merging has 801 as mandatory field (that's my guess). -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #4 from axel Amghar <axel.amghar@biblibre.com> --- Created attachment 90061 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=90061&action=edit Bug 22690 - Merging records with many items (ElasticSearch) This patch allow us to merged many items without timeout Test plan : Without the patch :

- Create 2 records (one with 1000 items for example) - Add the records to one list and start the merging process of those records. - 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 continues)

With the patch : - make the same - this time verify that all items have been merged without time out -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #5 from axel Amghar <axel.amghar@biblibre.com> --- Created attachment 90115 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=90115&action=edit Bug 22690: Merging records with many items (ElasticSearch) This patch allow us to merged many items without timeout Test plan : Without the patch :

- Create 2 records (one with 1000 items for example) - Add the records to one list and start the merging process of those records. - 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 continues)

With the patch : - make the same - this time verify that all items have been merged without time out -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 axel Amghar <axel.amghar@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #90115|0 |1 is obsolete| | --- Comment #6 from axel Amghar <axel.amghar@biblibre.com> --- Created attachment 90116 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=90116&action=edit Bug 22690: Merging records with many items (ElasticSearch) This patch allow us to merged many items without timeout Test plan : Without the patch :

- Create 2 records (one with 1000 items for example) - Add the records to one list and start the merging process of those records. - 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 continues)

With the patch : - make the same - this time verify that all items have been merged without time out -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 axel Amghar <axel.amghar@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Alex Arnaud <alex.arnaud@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |alex.arnaud@biblibre.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #8 from Ere Maijala <ere.maijala@helsinki.fi> --- I don't get the logic of the $items_number parameter. As far as I can see the first UPDATE moves all items regardless of the parameter, but the subsequent processing of acquisitions etc. only goes through items in the $items_number parameter. If MoveItemsFromBiblio is supposed to move all items, I'd remove the $items_number parameter and do everything necessary inside MoveItemsFromBiblio. Also, I believe the 801 issue should be handled in a separate bug since it's not related to Elasticsearch. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|axel.amghar@biblibre.com |ere.maijala@helsinki.fi Status|Failed QA |ASSIGNED --- Comment #9 from Ere Maijala <ere.maijala@helsinki.fi> --- Taking this since I need this for bug 20447. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #92828|0 |1 is obsolete| | --- Comment #11 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 94505 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94505&action=edit 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/Item.t prove -v t/db_dependent/Items/MoveItemFromBiblio.t -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #14 from Ere Maijala <ere.maijala@helsinki.fi> --- Rebased. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #16 from Ere Maijala <ere.maijala@helsinki.fi> --- Mike, which test is failing? I'm seeing some errors from an unrelated test that tries to clean up the database in wrong order (remove patrons before their checkouts). -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #18 from Ere Maijala <ere.maijala@helsinki.fi> --- Mike, that seems unrelated to any of the changes here. I tried with both MARC 21 and UNIMARC default mappings and couldn't reproduce, so I'd say it's an issue with your mappings, but I'm a bit at loss if they're at defaults. Do you see the issue with plain master version of Koha? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Michal Denar <black23@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #23 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- (In reply to Joonas Kylmälä from comment #22)

I would just reverse the original idea: move_items_to_biblio. Then it should be obvious we are talking about the current object's items being moved. Not sure if perl OO style supports overloading, but in that case it could be just move_items, and the parameters defined whether we move them to another biblio or (yet to be included to Koha) to a holdings record.

Or we could do this even in the Item and Items objects for maximum code reusability, like $biblio->items->move_to_biblio? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #25 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- (In reply to Katrin Fischer from comment #24)

...

Or we could do this even in the Item and Items objects for maximum code reusability, like $biblio->items->move_to_biblio?

Maybe just move in this case? They can't go anywhere else than another biblio.

This bug is a dependency for Bug 20447 which introduces holdings records so similar moving of items to another holdings record is needed there in addition to moving to another biblio. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #98143|0 |1 is obsolete| | --- Comment #27 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 98210 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=98210&action=edit 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/Item.t prove -v t/db_dependent/Items/MoveItemFromBiblio.t Signed-off-by: Michal Denar <black23@gmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #29 from Ere Maijala <ere.maijala@helsinki.fi> --- Rename done. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #98211|0 |1 is obsolete| | --- Comment #31 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 99376 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=99376&action=edit 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/Item.t prove -v t/db_dependent/Items/MoveItemFromBiblio.t Signed-off-by: Michal Denar <black23@gmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #99376|0 |1 is obsolete| | --- Comment #33 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 99379 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=99379&action=edit 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/Item.t prove -v t/db_dependent/Items/MoveItemFromBiblio.t -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #35 from Ere Maijala <ere.maijala@helsinki.fi> --- Mike, I can't reproduce. Can you provide more insight on which tests failed? And is it an empty database or does it have existing data? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Michal Denar <black23@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #100316|0 |1 is obsolete| | --- Comment #37 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 104796 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104796&action=edit 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/Item.t prove -v t/db_dependent/Items/MoveItemFromBiblio.t Signed-off-by: Michal Denar <black23@gmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #104796|0 |1 is obsolete| | --- Comment #39 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 104797 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104797&action=edit 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/Item.t prove -v t/db_dependent/Items/MoveItemFromBiblio.t Signed-off-by: Michal Denar <black23@gmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact| |josef.moravec@gmail.com Status|Signed Off |Failed QA -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #104797|0 |1 is obsolete| | --- Comment #41 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 104871 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104871&action=edit 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/Item.t prove -v t/db_dependent/Items/MoveItemFromBiblio.t Signed-off-by: Michal Denar <black23@gmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net --- Comment #43 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 105445 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=105445&action=edit error

With the patch: - Do the same as above - This time verify that the records are merged without timeout

-- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #44 from Victor Grousset/tuxayo <victor@tuxayo.net> --- BTW, librarian question: is it expected to for the merge to complain about the 003 field being missing? (and blocking the merge). So in my test data (koha-testing-docker and I supose KohaDevBox), I have to edit the books framework to make 003 visible so it will be populated when I create the two records. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #46 from Victor Grousset/tuxayo <victor@tuxayo.net> --- The above tests where done on Debian with MariaDB 10.3 with ElasticSearch 6.8.8 == == I retried the above simplified test plan (second half of the full one) with Debian 9, MySQL 5.5 and still ES 6.8.8 Still the same issue. Though Koha is stuck on additem.pl whereas in the previous test it's merge.pl that I was seeing the process list to. (when checking that it was still running in the background despite the timeout) == == I retried what I did on comment 43 on Debian 9& MySQL 5.5 and it didn't happen (no error, stuck on additem.pl). So maybe it's linked to the OS & DB or I didn't something else that messed it up. == == I think that's all for now from me. I'll be happy to do some more tests with a new patch or precises instructions to help troubleshoot the issue in case it only happens to me. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #48 from Victor Grousset/tuxayo <victor@tuxayo.net> --- One last thing, it also happens with ES5 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #50 from Ere Maijala <ere.maijala@helsinki.fi> --- Ok, I can see it's not fast enough, but it doesn't seem to be related to Elasticsearch but the fact that Koha::Objects must be used. I'll see what I can do to improve it. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #104871|0 |1 is obsolete| | --- Comment #51 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 106144 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106144&action=edit 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/Item.t prove -v t/db_dependent/Items/MoveItemFromBiblio.t Signed-off-by: Michal Denar <black23@gmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Andrew Nugged <nugged@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nugged@gmail.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #106144|0 |1 is obsolete| | --- Comment #54 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 108777 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=108777&action=edit 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/Item.t prove -v t/db_dependent/Items/MoveItemFromBiblio.t Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #109694|0 |1 is obsolete| | --- Comment #56 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 110991 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=110991&action=edit 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/Item.t prove -v t/db_dependent/Items/MoveItemFromBiblio.t Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #58 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Rebased to solve conflict caused by "Bug 25265: Prevent double reindex of the same item in batchmod" / 88cb7f223d. Replaced ModZebra calls with Koha::SearchEngine::Indexer. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #59 from David Nind <david@davidnind.com> --- Is there an easy way to add 1,000 items for a record? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #61 from David Nind <david@davidnind.com> --- (In reply to Michal Denar from comment #60)

Hi David, yes, you can use New item/Add multiple copies of this item.

Thanks Michal! I can see it now! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #63 from Ere Maijala <ere.maijala@helsinki.fi> --- David, thanks for testing. It's very valuable! I'm seeing the same as you, and it's not good. I'll investigate why that is. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #65 from Ere Maijala <ere.maijala@helsinki.fi> --- There was a parameter name change in master that caused the biblio to be reindexed separately for every item. Now fixed, and you should see greatly improved performance. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #67 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Hi, (In reply to David Nind from comment #66)

However, the the tests for prove -v t/db_dependent/Koha/Item.t run a lot faster as well, but still fail - see attachment Bug 22690 - Output from tests for Item.t.

please run the test after running "reset_all" and it should work. The Koha unit tests tend to fail if you change system preferences (which seems to be the case here with the searchengine syspref). -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #111207|0 |1 is obsolete| | --- Comment #68 from David Nind <david@davidnind.com> --- Created attachment 111237 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=111237&action=edit 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/Item.t prove -v t/db_dependent/Items/MoveItemFromBiblio.t Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com --- Comment #70 from Nick Clemens <nick@bywatersolutions.com> --- Hi Ere, Thank you! Great refactoring! A few things: 1 - Generally we assume that we should index unless passed 'skip_record_index', rather than hardcoding not to 2 - If you change 'move_to_biblio' to take that param and allow the store to index otherwise then we can can remove MoveItemFromBiblio and simply use move_to_biblio in moveitem.pl 3 - please add a test to t/db_dependent/Koha/SearchEngine/Indexer.t to cover when indexing happens (or doesn't) for these subroutines (to prove the adopt method only calls once) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #111237|0 |1 is obsolete| | --- Comment #71 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 112100 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=112100&action=edit 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/Item.t prove -v t/db_dependent/Koha/SearchEngine/Indexer.t Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #112100|0 |1 is obsolete| | --- Comment #73 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 112184 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=112184&action=edit 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/Item.t prove -v t/db_dependent/Koha/SearchEngine/Indexer.t Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply --- Comment #75 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Patch doesn't apply anymore. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Failed QA --- Comment #77 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Rebased. However the patch has two QA failures reported by the QA tool: FAIL C4/Items.pm OK critic OK forbidden patterns OK git manipulation OK pod FAIL pod coverage OK spelling OK valid FAIL cataloguing/moveitem.pl OK critic OK forbidden patterns OK git manipulation OK pod OK spelling FAIL valid These need to be fixed. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #112567|0 |1 is obsolete| | --- Comment #78 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 112569 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=112569&action=edit 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/Item.t prove -v t/db_dependent/Koha/SearchEngine/Indexer.t Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #80 from David Nind <david@davidnind.com> --- I'm getting test failures with prove -v t/db_dependent/Koha/Item.t after the patch is applied. I ran this after applying the patches, but before adding the 1000 items and merging. They passed before the patch was applied. Note: I couldn't get the tests to complete after adding the 1000 items and merging. Apart from this, everything else in the test plan works. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #82 from David Nind <david@davidnind.com> --- Created attachment 112754 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=112754&action=edit Test results - Bug 22690 Attached are the test results I got using koha-testing-docker: - Before the patch is applied: the tests pass - After changing the search engine system preference to elasticsearch and reindexing: tests fail - After patch applied: tests fail - After 1,000 records added and merging: tests fail -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com Status|Needs Signoff |Failed QA --- Comment #84 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I think the test failure is a red herring.. However.. I believe we need to add unit tests for the new Koha::Biblio->adopt_items_from_biblio method and Koha::Item->item_orders relation accessor before we proceed. These should be fairly trivial tests to add, one to prove that the loop happens and the final indexing takes place for the set and the second to prove the relation is setup correctly. Failing QA -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #112569|0 |1 is obsolete| | --- Comment #85 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 113308 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113308&action=edit 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/Item.t prove -v t/db_dependent/Koha/SearchEngine/Indexer.t Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #87 from Ere Maijala <ere.maijala@helsinki.fi> --- (In reply to Martin Renvoize from comment #84)

However.. I believe we need to add unit tests for the new Koha::Biblio->adopt_items_from_biblio method and Koha::Item->item_orders relation accessor before we proceed.

Absolutely. Tests now added. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply --- Comment #89 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Doesn't apply anymore -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #113308|0 |1 is obsolete| | Attachment #113309|0 |1 is obsolete| | --- Comment #90 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 113941 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113941&action=edit 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/Item.t prove -v t/db_dependent/Koha/SearchEngine/Indexer.t Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Michal Denar <black23@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #93 from Michal Denar <black23@gmail.com> --- Created attachment 113968 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113968&action=edit Bug 22690: Add more tests - Tests for adopt_items_from_biblio - Tests for the relationship between items and acquisition orders - Tests for indexer calls in adopt_items_from_biblio Signed-off-by: Michal Denar <black23@gmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #95 from Ere Maijala <ere.maijala@helsinki.fi> --- (In reply to Jonathan Druart from comment #94)

So the problem is actually that the reindex is not async. Bug 27344 is trying to provide a solution for that.

That's part of it, but I believe the refactoring improves the flow and readability of the code as well. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #97 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- (In reply to Ere Maijala from comment #95)

(In reply to Jonathan Druart from comment #94)

...

So the problem is actually that the reindex is not async. Bug 27344 is trying to provide a solution for that.

That's part of it, but I believe the refactoring improves the flow and readability of the code as well.

I think I agree, the code flow is clearer with this work. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #113968|0 |1 is obsolete| | --- Comment #104 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 118227 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=118227&action=edit Bug 22690: Add more tests - Tests for adopt_items_from_biblio - Tests for the relationship between items and acquisition orders - Tests for indexer calls in adopt_items_from_biblio Signed-off-by: Michal Denar <black23@gmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #118227|0 |1 is obsolete| | --- Comment #106 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 118231 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=118231&action=edit Bug 22690: Add more tests - Tests for adopt_items_from_biblio - Tests for the relationship between items and acquisition orders - Tests for indexer calls in adopt_items_from_biblio Signed-off-by: Michal Denar <black23@gmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #108 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- The "Bug 22690: Add more tests" patch removes accidentally

$schema->storage->txn_rollback;

lines from the subtest above. Also I rebased bug 20447 bug and now the adopt_items_from_biblio() test fails there, it could be probably due to some problem in bug 20447 patches but maybe also in these patches – will investigate. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #118231|0 |1 is obsolete| | --- Comment #109 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 118391 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=118391&action=edit Bug 22690: Add more tests - Tests for adopt_items_from_biblio - Tests for the relationship between items and acquisition orders - Tests for indexer calls in adopt_items_from_biblio Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Michal Denar <black23@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Michal Denar <black23@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #118391|0 |1 is obsolete| | --- Comment #112 from Michal Denar <black23@gmail.com> --- Created attachment 119434 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=119434&action=edit Bug 22690: Add more tests - Tests for adopt_items_from_biblio - Tests for the relationship between items and acquisition orders - Tests for indexer calls in adopt_items_from_biblio Signed-off-by: Michal Denar <black23@gmail.com> Rebased-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Michal Denar <black23@gmail.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #119433|0 |1 is obsolete| | Attachment #119434|0 |1 is obsolete| | --- Comment #114 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 120820 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=120820&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #116 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Rebased. There was only small conflicts in tests. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #120821|0 |1 is obsolete| | --- Comment #118 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 120853 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=120853&action=edit Bug 22690: Add more tests - Tests for adopt_items_from_biblio - Tests for the relationship between items and acquisition orders - Tests for indexer calls in adopt_items_from_biblio 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> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #120 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Issue found, more warnings in the tests with these patches: ============ Without patches =================== kohadev-koha@kohadevbox:/kohadevbox/koha$ prove t/db_dependent/Koha/Biblio.t t/db_dependent/Koha/Item.t t/db_dependent/Koha/SearchEngine/Indexer.t t/db_dependent/Koha/Biblio.t ................ 4/14 Use of uninitialized value $sub6 in pattern match (m//) at /kohadevbox/koha/Koha/SearchEngine/Elasticsearch.pm line 596. t/db_dependent/Koha/Biblio.t ................ ok t/db_dependent/Koha/Item.t .................. ok t/db_dependent/Koha/SearchEngine/Indexer.t .. ok All tests successful. ============ With patches =================== kohadev-koha@kohadevbox:/kohadevbox/koha$ prove t/db_dependent/Koha/Biblio.t t/db_dependent/Koha/Item.t t/db_dependent/Koha/SearchEngine/Indexer.t t/db_dependent/Koha/Biblio.t ................ 4/15 Use of uninitialized value $sub6 in pattern match (m//) at /kohadevbox/koha/Koha/SearchEngine/Elasticsearch.pm line 596. t/db_dependent/Koha/Biblio.t ................ ok t/db_dependent/Koha/Item.t .................. 10/11 DBIx::Class::Storage::DBI::select_single(): Query returned more than one row. SQL that returns multiple rows is DEPRECATED for ->find and ->single at /kohadevbox/koha/t/lib/TestBuilder.pm line 235 DBIx::Class::Storage::DBI::select_single(): Query returned more than one row. SQL that returns multiple rows is DEPRECATED for ->find and ->single at /kohadevbox/koha/t/lib/TestBuilder.pm line 235 DBIx::Class::Storage::DBI::select_single(): Query returned more than one row. SQL that returns multiple rows is DEPRECATED for ->find and ->single at /kohadevbox/koha/t/lib/TestBuilder.pm line 235 t/db_dependent/Koha/Item.t .................. ok t/db_dependent/Koha/SearchEngine/Indexer.t .. 1/2 Zebra at t/db_dependent/Koha/SearchEngine/Indexer.t line 91. Koha::Item at t/db_dependent/Koha/SearchEngine/Indexer.t line 93. Zebra at t/db_dependent/Koha/SearchEngine/Indexer.t line 91. Koha::Item at t/db_dependent/Koha/SearchEngine/Indexer.t line 93. Zebra at t/db_dependent/Koha/SearchEngine/Indexer.t line 91. Koha::Item at t/db_dependent/Koha/SearchEngine/Indexer.t line 93. Zebra at t/db_dependent/Koha/SearchEngine/Indexer.t line 91. Koha::Item at t/db_dependent/Koha/SearchEngine/Indexer.t line 93. Elasticsearch at t/db_dependent/Koha/SearchEngine/Indexer.t line 91. Koha::Item at t/db_dependent/Koha/SearchEngine/Indexer.t line 93. Elasticsearch at t/db_dependent/Koha/SearchEngine/Indexer.t line 91. Koha::Item at t/db_dependent/Koha/SearchEngine/Indexer.t line 93. Elasticsearch at t/db_dependent/Koha/SearchEngine/Indexer.t line 91. Koha::Item at t/db_dependent/Koha/SearchEngine/Indexer.t line 93. Elasticsearch at t/db_dependent/Koha/SearchEngine/Indexer.t line 91. Koha::Item at t/db_dependent/Koha/SearchEngine/Indexer.t line 93. t/db_dependent/Koha/SearchEngine/Indexer.t .. ok All tests successful. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #122 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 121389 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121389&action=edit Bug 22690: (QA follow-up) Silence manually generated warnings In our test setup we mock the index_records() to produce warnings like this: Koha::Item at t/db_dependent/Koha/SearchEngine/Indexer.t line 93. By wrapping all our item creations to warnings_are{} we can silence them. Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #124 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 121392 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121392&action=edit Bug 22690: (QA follow-up) Index also source biblio when calling move_to_biblio() We need to update the search index record for the old biblio where the item was moved from to keep the item info in search index up-to-date. To test: 1) $ prove t/db_dependent/Koha/SearchEngine/Indexer.t Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |28479 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28479 [Bug 28479] TestBuilder.pm uses incorrect method for checking if objects to be created exists -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #126 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Created attachment 121474 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121474&action=edit Bug 22690: (QA follow-up) Make bib-level hold object actually bib-level We need to pass undef itemnumber to build_object() to actually have a hold without an item tied to it. Otherwise build_object() will create automatically an item for us (thus making it an item-level hold) To test: $ prove t/db_dependent/Koha/Item.t -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #128 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- Note: please be sure to include the patch from bug 28479, it fixes one of the warnings in TestBuilder.pm -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #120853|0 |1 is obsolete| | --- Comment #130 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 121517 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121517&action=edit Bug 22690: Add more tests - Tests for adopt_items_from_biblio - Tests for the relationship between items and acquisition orders - Tests for indexer calls in adopt_items_from_biblio 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> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121392|0 |1 is obsolete| | --- Comment #132 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 121519 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121519&action=edit Bug 22690: (QA follow-up) Index also source biblio when calling move_to_biblio() We need to update the search index record for the old biblio where the item was moved from to keep the item info in search index up-to-date. To test: 1) $ prove t/db_dependent/Koha/SearchEngine/Indexer.t Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #134 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Thanks for the followup, no additional warnings and the test plan still works :) --- QA tools: FAIL C4/Items.pm OK critic OK forbidden patterns OK git manipulation OK pod FAIL pod coverage POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666 OK spelling OK valid That looks a false positive, a function was deleted, so it's POD --- (In reply to Joonas Kylmälä from comment #127)

Please give one more look and mark this as PQA & sign-off if all looks good to you.

Code is way over my head so I can't go further than this. But at least the QAer only has the code to review. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121516|0 |1 is obsolete| | --- Comment #135 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 122846 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=122846&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121518|0 |1 is obsolete| | --- Comment #137 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 122848 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=122848&action=edit Bug 22690: (QA follow-up) Silence manually generated warnings In our test setup we mock the index_records() to produce warnings like this: Koha::Item at t/db_dependent/Koha/SearchEngine/Indexer.t line 93. By wrapping all our item creations to warnings_are{} we can silence them. Signed-off-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> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121520|0 |1 is obsolete| | --- Comment #139 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 122850 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=122850&action=edit Bug 22690: (QA follow-up) Make bib-level hold object actually bib-level We need to pass undef itemnumber to build_object() to actually have a hold without an item tied to it. Otherwise build_object() will create automatically an item for us (thus making it an item-level hold) To test: $ prove t/db_dependent/Koha/Item.t Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|josef.moravec@gmail.com |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #142 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 122887 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=122887&action=edit Bug 22690: Add more tests - Tests for adopt_items_from_biblio - Tests for the relationship between items and acquisition orders - Tests for indexer calls in adopt_items_from_biblio 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> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #144 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 122889 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=122889&action=edit Bug 22690: (QA follow-up) Index also source biblio when calling move_to_biblio() We need to update the search index record for the old biblio where the item was moved from to keep the item info in search index up-to-date. To test: 1) $ prove t/db_dependent/Koha/SearchEngine/Indexer.t Signed-off-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> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #146 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Patches rebased. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #148 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 122936 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=122936&action=edit Bug 22690: Remove MoveItemFromBiblio import Added in the meanwhile by bug 17600. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA --- Comment #150 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- 1. Why Koha::Item->item_orders is not named Koha::Item->orders? It returns a Koha::Acquisition::Orders. 2. in move_to_biblio 1215 return unless $self->biblionumber != $to_biblio->biblionumber; I am a fan of unless, not when there is a negation in the test :) return if $self->biblionumber == $to_biblio->biblionumber; Read much better IMO. 3. There is Koha::Item->move_to_biblio($biblio) and Koha::Biblio->adopt_items_from_biblio($biblio) Don't you think Koha::Biblio->adopt_items_from_biblio could be Koha::Items->move_to_biblio actually? It would be more consistent and flexible. 4. in move_to_biblio you are calling, on a DBIC rs, ->update, then update_all: 1237 $hold_fill_target->update({ biblionumber => $to_biblionumber }); 1254 $linktrackers->update_all({ biblionumber => $to_biblionumber }); It's not consistent, is there a good reason for that? Please provide a fast reply, I can write the follow-up patches if needed. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #152 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- (In reply to Jonathan Druart from comment #150)

1. Why Koha::Item->item_orders is not named Koha::Item->orders? It returns a Koha::Acquisition::Orders.

This should certainly be named 'orders', annoyed I overlooked that during QA :(

2. in move_to_biblio 1215 return unless $self->biblionumber != $to_biblio->biblionumber; I am a fan of unless, not when there is a negation in the test :) return if $self->biblionumber == $to_biblio->biblionumber; Read much better IMO.

Agreed.

3. There is Koha::Item->move_to_biblio($biblio) and Koha::Biblio->adopt_items_from_biblio($biblio) Don't you think Koha::Biblio->adopt_items_from_biblio could be Koha::Items->move_to_biblio actually? It would be more consistent and flexible.

Hmmm, I think so long as the method name is clear I was happy for it to live in either class. I do see what you mean though and for consistency, I certainly like that.

4. in move_to_biblio you are calling, on a DBIC rs, ->update, then update_all: 1237 $hold_fill_target->update({ biblionumber => $to_biblionumber });

1254 $linktrackers->update_all({ biblionumber => $to_biblionumber });

It's not consistent, is there a good reason for that?

Please provide a fast reply, I can write the follow-up patches if needed.

I think this is simply because we didn't have a Koha:: based resultset yet and he didn't want to add further complexity to the patch by adding that new class as well. Having said that, it's fairly trivial to add such a class so long as it's a basic one, so perhaps we should for consistency. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #122886|0 |1 is obsolete| | --- Comment #154 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 123029 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123029&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #122888|0 |1 is obsolete| | --- Comment #156 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 123031 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123031&action=edit Bug 22690: (QA follow-up) Silence manually generated warnings In our test setup we mock the index_records() to produce warnings like this: Koha::Item at t/db_dependent/Koha/SearchEngine/Indexer.t line 93. By wrapping all our item creations to warnings_are{} we can silence them. Signed-off-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> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #122890|0 |1 is obsolete| | --- Comment #158 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 123033 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123033&action=edit Bug 22690: (QA follow-up) Make bib-level hold object actually bib-level We need to pass undef itemnumber to build_object() to actually have a hold without an item tied to it. Otherwise build_object() will create automatically an item for us (thus making it an item-level hold) To test: $ prove t/db_dependent/Koha/Item.t Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #122939|0 |1 is obsolete| | --- Comment #160 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 123035 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123035&action=edit Bug 22690: Remove MoveItemFromBiblio import Added in the meanwhile by bug 17600. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #162 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 123037 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123037&action=edit Bug 22690: (QA follow-up) Improve negation syntax -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #164 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 123039 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123039&action=edit Bug 22690: (QA follow-up) Clarify uses of DBIC -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #166 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 123041 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123041&action=edit Bug 22690: DBIC Schema Updates -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Passed QA -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #169 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- RM comments addressed in followups. I also took the opportunity to add the Koha::TrackedLink(s) classes and use those Koha::Object(s) based accessors to clarify some of the code found within move_to_biblio. With this change, the odd call to update_all has been replaced with our Koha::Objects based automatic trigger handling update call (without passing no_triggers yet). The other DBIC resultset acting call is against a single row so update is correct.. I believe. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Passed QA -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #172 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Thanks for catching that Joonas :) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 --- Comment #174 from Joonas Kylmälä <joonas.kylmala@helsinki.fi> --- I also get the following test failure:

root@kohadevbox:koha(master)$ prove t/db_dependent/Koha/Item.t t/db_dependent/Koha/Item.t .. # No tests run! t/db_dependent/Koha/Item.t .. 1/12 # Failed test 'No tests run for subtest "tracked_links relationship"' # at t/db_dependent/Koha/Item.t line 56. Can't locate object method "_new_from_dbic" via package "Koha::TrackedLinks" (perhaps you forgot to load "Koha::TrackedLinks"?) at /kohadevbox/koha/Koha/Item.pm line 1220. # Looks like your test exited with 255 just after 1.

-- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #123029|0 |1 is obsolete| | --- Comment #176 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 123481 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123481&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22690 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #123031|0 |1 is obsolete| | --- Comment #178 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 123483 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123483&action=edit Bug 22690: (QA follow-up) Silence manually generated warnings In our test setup we mock the index_records() to produce warnings like this: Koha::Item at t/db_dependent/Koha/SearchEngine/Indexer.t line 93. By wrapping all our item creations to warnings_are{} we can silence them. Signed-off-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> -- You are receiving this mail because: You are watching all bug changes.