|
Description
Jonathan Druart
2021-05-25 12:12:56 UTC
Created attachment 121372 [details] [review] Bug 28445: WIP - Use the task queue for the batch delete and update items tool Created attachment 123221 [details] [review] Bug 28445: Isolate add item form to an include file To ease reusability Created attachment 123222 [details] [review] Bug 28445: Move controller code to Koha::UI::Form::Builder::Item To ease reusability Created attachment 123223 [details] [review] Bug 28445: Use Koha::UI::Form::Builder::Item from batchmod as well Now that it's reusable, let use it somewhere else! Created attachment 123224 [details] [review] Bug 28445: Adjust code to handle regexs Created attachment 123225 [details] [review] Bug 28445: Start form with empty library for batch mod Created attachment 123226 [details] [review] Bug 28445: Add the capability to provide more info to the background job detail view We already had the need for that, when bibliographic records are modified in batch we wanted to add a "Add to list" feature, and so pass a list of lists/virtual shelves to the template. Here we will want to pass the infos of the items that have been modified to display a table. Created attachment 123227 [details] [review] Bug 28445: Use the task queue for the batch delete and update items tool Here we go! Disclaimer: this patch is huge and does many things, but splitting it in several chunks would be time consuming and painful to rebase. However it adds many tests and isolate/refactor code to make it way more reusable. This patchset will make the "batch item modification" and "batch item deletion" features use the task queue (reminder: Since bug 28158, and so 21.05.00, we do no longer use the old "background job" functionality and the user does not get any info about the progress of the job). More than that, more of the code to build an item form and a list of items is now isolated in module (.pm) and include files (.inc) We are reusing the changes made by bug 27526 that simplifies the way we edit/create items (no more unecessary serialization Koha > MARC > MARCXML > XML > HTML) New module: * Koha::BackgroundJob::BatchDeleteItem Subclass for process item deletion in batch * Koha::BackgroundJob::BatchUpdateItem Subclass for process item modification in batch * Koha::Item::Attributes We needed an object to represent item's attributes that are not mapped with a koha field (aka "more subfields xml") This module will help us to create the marcxml from a hashref and the reverse. * Koha::UI::Form::Builder::Item The code that was used to build the add/edit item form is centralised in this module. In conjunction with the subfields_for_item BLOCK (from html_helpers.inc) it will be really easy to reuse this code in other places where the item form is used (acquisition and serials modules) * Koha::UI::Table::Builder::Items Same as previously for the table. We are now using this table from 3 different places (batch item mod, batch item del, backgroung job detail view) and the code is only in one place. To use with items_table_batchmod BLOCK (still from html_helpers.inc) This patch is fixing some bugs about repeatable subfields and regex. A UI change will reflect the limitation: if you want to apply a regex on a subfield you cannot add several subfields for the same subfield code. Test plan: Prepare the ground: - Make sure you are always using a bibliographic/item record using the framework you are modifying! - Add some subfields for items that are not mapped with a koha field (note that you can use 'é' for more fun, don't try more funny characters) - Make some subfields (mapped and not mapped with a kohafield) repeatable - Add default values to some of your subfields There are 4 main screens to test: 1. Add/edit item form The behaviour should be the same before and after this patch. See test plan from bug 27526. Those 2 prefs must be tested: * SubfieldsToAllowForRestrictedEditing * SubfieldsToUseWhenPrefill 2. Batch modification a. Fill some values, play with repeatable and regex. Note that the behaviour in master was buggy, only the first value was modified by the regex: * With subfield = "a | b" 1 value added with "new" => "new | b" * With subfield = "a | b" 2 new fields "new1","new2" => "new2 | b" Important note: For repeatable subfields, a regex will apply on the subfields in the "concatenated form". To apply the regex on all the different subfields of a given subfield code you must use the "g" modifier. This could be improved later, but keep in mind that it's not a regression or behaviour change. b. Play with the "Populate fields with default values from default framework" checkbox c. Use this tool to modify items and play with the different sysprefs that interfer with it: * NewItemsDefaultLocation * SubfieldsToAllowForRestrictedBatchmod * MaxItemsToDisplayForBatchMod * MaxItemsToProcessForBatchMod 3. Batch deletion a. Batch delete some items b. Check items out and try to delete them c. Use the "Delete records if no items remain" checkbox to delete bibliographic records without remaining items. d. Play with the following sysprefs and confirm that it works as expected: * MaxItemsToDisplayForBatchDel e. Stress the tool: Go to the confirmation screen with items that can be deleted, don't request the job to be processed right away, but check the item out before. 4. Background job detail view You must have seen it already if you are curious and tested the above. When a new modification or deletion batch is requested, the confirmation screen will tell you that the job has enqueued. A link to the progress of the job can be followed. On this screen you will be able to see the result of the job once it's fully processed. QA notes: * There are some FIXME's that are not blocker in my opinion. Feel free to discuss them if you have suggestions. * Do we still need MaxItemsToProcessForBatchMod? * Prior to this patchset we had a "Return to the cataloging module" link if we went from the cataloguing module and that the biblio was deleted. We cannot longer know if the biblio will be deleted but we could display a "Go to the cataloging module" link on the "job has been enqueued" screen regardless from where we were coming from. Created attachment 123228 [details] [review] Bug 28445: UI change for repeatable and regex This patch applies the changes describe in the main commit message about the "limitation" and "the behaviour in master was buggy". Created attachment 123229 [details] [review] Bug 28445: Fix encoding issue with the background job's report Remote branch, with all the dependencies applied, available for testing: https://gitlab.com/joubu/Koha/-/commits/bug_28445 Created attachment 123239 [details] [review] Bug 28445: Fix encoding issue with the background job's report Created attachment 123240 [details] [review] Bug 28445: Don't display the items if not finished yet > (note that you can use 'é' for more fun, don't try more funny characters)
This is not correct, any 4-byte character is supposed to work.
(In reply to Jonathan Druart from comment #14) > > (note that you can use 'é' for more fun, don't try more funny characters) > > This is not correct, any 4-byte character is supposed to work. Should that be 1-4 byte character since UTF-8 characters are all between 1-4 bytes long? (J'adore couper les cheveux en quatre... [U+1F605]) (In reply to David Cook from comment #15) > (In reply to Jonathan Druart from comment #14) > > > (note that you can use 'é' for more fun, don't try more funny characters) > > > > This is not correct, any 4-byte character is supposed to work. > > Should that be 1-4 byte character since UTF-8 characters are all between 1-4 > bytes long? Oui mais, "qui peut le plus peut le moins!" (who can do more, can do less) I'm a bit concerned by the way this patch set is handling the UI. A UI form builder module feels pretty regressive. I'm even seeing CSS embedded in a Perl module, which feels like it should be a hard no. What I'm seeing here is re-enforces the problem that we are coming up on in Koha over and over. Scalability. For 20 years Koha has been processing all data monolithically on the back end, and then presenting it to the user on page load. This worked fine when Koha was being used by small libraries. Now, Koha is being adopted by huge library system, and those designs that were great in the past are showing their age. Time and time again we find areas of Koha where a page simply cannot load because a library has so much data. That is why we as a community chose to implement a REST API. This is what allows us to make pages load quickly and dynamically using DataTables and other tools. I guess my question is, why? Why add a Perl module that's so tightly integrated with html generation that it has CSS embedded in it's code? Why not use the API? Why not make smarter objects and make cleaner TT templates? Thanks for your input. These patches are more from perfect, I agree. But the idea is to improve the current situation. And I think this move goes toward the following goals: isolate the code, centralise it, make it more robust by providing tests. These item form and table are used in several places over the code, and having this code in a module makes it reusable easily. After this move the idea is to reuse it where this code is duplicated. Once everything will use the same code it will be easier to move to a better solution. (In reply to Jonathan Druart from comment #18) > These patches are more from perfect, I agree. *Far* from perfect! :D As I didn't get a reply I will try to provide a more complete, and maybe more convincing, answer. I would like to highlight that those patches are not adding *new* code to Koha::UI. The code there is moved from current code that is duplicated in several places: additem and batchmod so far (for the add item form and the items table). To make the item batchmod tool use the task queue I had to reuse the same code from batchmod.pl, and instead of reusing the code as it, I preferred to make it more robust, and reusable from other places. The code is now in a module, covered by test, and centralized in a single place. The next move should be to reuse the same code (methods and BLOCK from inc) from the remaining places (acq and serial, detail pages, etc.) If we reach this point we will have all our add item forms and items tables using the exact same code, covered by tests. That will make any other improvements easier to implement. Please see this as a continuous improvement of the current situation, not as the perfect solution we want to have at the end. I totally agree we should be using the REST API routes to achieve that goal, but we are not ready at all for that. Building the table like it's done here is not achievable right now using the REST API, and was not the point anyway. We need the item batch mod tool back running in background mode for 21.11. (In reply to Jonathan Druart from comment #20) I understand your intentions. I'm sure we have as many possible solutions as we do developers. If Koha and its community are to continue to be successful we need to adapt as technology and trends change. It's important that we keep that in mind is all. I have reservations but this is definitely an improvement nevertheless. Created attachment 125801 [details] [review] Bug 28445: Isolate add item form to an include file To ease reusability Created attachment 125802 [details] [review] Bug 28445: Move controller code to Koha::UI::Form::Builder::Item To ease reusability Created attachment 125803 [details] [review] Bug 28445: Use Koha::UI::Form::Builder::Item from batchmod as well Now that it's reusable, let use it somewhere else! Created attachment 125804 [details] [review] Bug 28445: Adjust code to handle regexs Created attachment 125805 [details] [review] Bug 28445: Start form with empty library for batch mod Created attachment 125808 [details] [review] Bug 28445: Use the task queue for the batch delete and update items tool Here we go! Disclaimer: this patch is huge and does many things, but splitting it in several chunks would be time consuming and painful to rebase. However it adds many tests and isolate/refactor code to make it way more reusable. This patchset will make the "batch item modification" and "batch item deletion" features use the task queue (reminder: Since bug 28158, and so 21.05.00, we do no longer use the old "background job" functionality and the user does not get any info about the progress of the job). More than that, more of the code to build an item form and a list of items is now isolated in module (.pm) and include files (.inc) We are reusing the changes made by bug 27526 that simplifies the way we edit/create items (no more unecessary serialization Koha > MARC > MARCXML > XML > HTML) New module: * Koha::BackgroundJob::BatchDeleteItem Subclass for process item deletion in batch * Koha::BackgroundJob::BatchUpdateItem Subclass for process item modification in batch * Koha::Item::Attributes We needed an object to represent item's attributes that are not mapped with a koha field (aka "more subfields xml") This module will help us to create the marcxml from a hashref and the reverse. * Koha::UI::Form::Builder::Item The code that was used to build the add/edit item form is centralised in this module. In conjunction with the subfields_for_item BLOCK (from html_helpers.inc) it will be really easy to reuse this code in other places where the item form is used (acquisition and serials modules) * Koha::UI::Table::Builder::Items Same as previously for the table. We are now using this table from 3 different places (batch item mod, batch item del, backgroung job detail view) and the code is only in one place. To use with items_table_batchmod BLOCK (still from html_helpers.inc) This patch is fixing some bugs about repeatable subfields and regex. A UI change will reflect the limitation: if you want to apply a regex on a subfield you cannot add several subfields for the same subfield code. Test plan: Prepare the ground: - Make sure you are always using a bibliographic/item record using the framework you are modifying! - Add some subfields for items that are not mapped with a koha field (note that you can use 'é' for more fun, don't try more funny characters) - Make some subfields (mapped and not mapped with a kohafield) repeatable - Add default values to some of your subfields There are 4 main screens to test: 1. Add/edit item form The behaviour should be the same before and after this patch. See test plan from bug 27526. Those 2 prefs must be tested: * SubfieldsToAllowForRestrictedEditing * SubfieldsToUseWhenPrefill 2. Batch modification a. Fill some values, play with repeatable and regex. Note that the behaviour in master was buggy, only the first value was modified by the regex: * With subfield = "a | b" 1 value added with "new" => "new | b" * With subfield = "a | b" 2 new fields "new1","new2" => "new2 | b" Important note: For repeatable subfields, a regex will apply on the subfields in the "concatenated form". To apply the regex on all the different subfields of a given subfield code you must use the "g" modifier. This could be improved later, but keep in mind that it's not a regression or behaviour change. b. Play with the "Populate fields with default values from default framework" checkbox c. Use this tool to modify items and play with the different sysprefs that interfer with it: * NewItemsDefaultLocation * SubfieldsToAllowForRestrictedBatchmod * MaxItemsToDisplayForBatchMod * MaxItemsToProcessForBatchMod 3. Batch deletion a. Batch delete some items b. Check items out and try to delete them c. Use the "Delete records if no items remain" checkbox to delete bibliographic records without remaining items. d. Play with the following sysprefs and confirm that it works as expected: * MaxItemsToDisplayForBatchDel e. Stress the tool: Go to the confirmation screen with items that can be deleted, don't request the job to be processed right away, but check the item out before. 4. Background job detail view You must have seen it already if you are curious and tested the above. When a new modification or deletion batch is requested, the confirmation screen will tell you that the job has enqueued. A link to the progress of the job can be followed. On this screen you will be able to see the result of the job once it's fully processed. QA notes: * There are some FIXME's that are not blocker in my opinion. Feel free to discuss them if you have suggestions. * Do we still need MaxItemsToProcessForBatchMod? * Prior to this patchset we had a "Return to the cataloging module" link if we went from the cataloguing module and that the biblio was deleted. We cannot longer know if the biblio will be deleted but we could display a "Go to the cataloging module" link on the "job has been enqueued" screen regardless from where we were coming from. Created attachment 125810 [details] [review] Bug 28445: UI change for repeatable and regex This patch applies the changes describe in the main commit message about the "limitation" and "the behaviour in master was buggy". Created attachment 125811 [details] [review] Bug 28445: Fix encoding issue with the background job's report Created attachment 125812 [details] [review] Bug 28445: Don't display the items if not finished yet Created attachment 125817 [details] [review] Bug 28445: Improve robustness of tests * Could fail if not using the original sample data: build the AV we need instead of relying on what is in the DB * Libraries are ordered by name by default but if others have been added the test may fail Created attachment 125865 [details] [review] Bug 28445: Improve robustness of tests * Libraries are ordered by name by default but if others have been added the test may fail (In reply to Jonathan Druart from comment #33) > Created attachment 125865 [details] [review] [review] > Bug 28445: Improve robustness of tests > > * Libraries are ordered by name by default but if others have been added > the test may fail Part of this patch has been moved out as a follow-up of bug 27526. Notes for testing:
Edit the Default framework in order to ensure the new fields show during batch modification
Results:
Overall, things worked well, sent to background_jobs after an update, can return to batch mod
On additem.pl attempting to edit a repeated field only adds the value from the first field to all the repeated attempts, this is true on master as well
Blockers:
- Checkbox to 'delete' a subfield does not work, no changes are made
- If number of items to edit is over MaxItemsToDisplayForBatchMod, but below MaxItemsToProcessForBatchMod the form does not show and no edits can be made
- MaxItemsToDisplayForBatchMod is not respected on background_jobs.pl, all items are displayed
- Progress may not be updated - hard to tell, was 0 until page reload loaded the full item list
- Batch deletion failed: Cannot enqueue this job. (The error was: Can't use string ("on") as an ARRAY ref while "strict refs" in use at /kohadevbox/koha/Koha/BackgroundJob/BatchDeleteItem.pm line 214. , see the Koha log file for more information).
Created attachment 126046 [details] [review] Bug 28445: Fix subfields to blank The value of the checkbox was not correct Created attachment 126047 [details] [review] Bug 28445: Don't surround the whole batch with a transaction It makes more sense to commit when an item has successfully been modified and so move the transaction inside the loop. It also fixes the progress of the whole job. Thanks Nick! (In reply to Nick Clemens from comment #35) > On additem.pl attempting to edit a repeated field only adds the value from > the first field to all the repeated attempts, this is true on master as well I don't understand. Can you detail please? I made 952$u repeatable and I can create several subfields, edit, save. And it is working as I expect. > Blockers: > - Checkbox to 'delete' a subfield does not work, no changes are made Will be fixed! > - If number of items to edit is over MaxItemsToDisplayForBatchMod, but below > MaxItemsToProcessForBatchMod the form does not show and no edits can be made Cannot recreate. Set MaxItemsToDisplayForBatchMod=2, MaxItemsToProcessForBatchMod=4, edit 3 barcodes. I see "Too many items (3): You are editing more than 2 items in a batch, items will not be shown." and there is the form. > - MaxItemsToDisplayForBatchMod is not respected on background_jobs.pl, all > items are displayed On purpose, do you think we should restore it? In my understanding it was to avoid the table on the first screen. > - Progress may not be updated - hard to tell, was 0 until page reload loaded > the full item list It actually hide something different, the transaction was not made at the correct level. I think we should commit when an item has been modified. The previous behaviour was failing the whole job if something went wrong for an item. It's fixed now. > - Batch deletion failed: Cannot enqueue this job. (The error was: Can't use > string ("on") as an ARRAY ref while "strict refs" in use at > /kohadevbox/koha/Koha/BackgroundJob/BatchDeleteItem.pm line 214. , see the > Koha log file for more information). Hum, cannot recreate that, but line 214 is: 214 my $delete_biblios = @{ $args->{delete_biblios} || [] }; That does not seem related at all, can you retry please? Note that I have tested on top of the bug 27526 follow-up, not pushed yet. So results may vary. Patchrs applied for me. Did you try the remote branch, Nick? Otherwise I will resubmit tomorrow. Created attachment 126136 [details] [review] Bug 28445: Use the task queue for the batch delete and update items tool Here we go! Disclaimer: this patch is huge and does many things, but splitting it in several chunks would be time consuming and painful to rebase. However it adds many tests and isolate/refactor code to make it way more reusable. This patchset will make the "batch item modification" and "batch item deletion" features use the task queue (reminder: Since bug 28158, and so 21.05.00, we do no longer use the old "background job" functionality and the user does not get any info about the progress of the job). More than that, more of the code to build an item form and a list of items is now isolated in module (.pm) and include files (.inc) We are reusing the changes made by bug 27526 that simplifies the way we edit/create items (no more unecessary serialization Koha > MARC > MARCXML > XML > HTML) New module: * Koha::BackgroundJob::BatchDeleteItem Subclass for process item deletion in batch * Koha::BackgroundJob::BatchUpdateItem Subclass for process item modification in batch * Koha::Item::Attributes We needed an object to represent item's attributes that are not mapped with a koha field (aka "more subfields xml") This module will help us to create the marcxml from a hashref and the reverse. * Koha::UI::Form::Builder::Item The code that was used to build the add/edit item form is centralised in this module. In conjunction with the subfields_for_item BLOCK (from html_helpers.inc) it will be really easy to reuse this code in other places where the item form is used (acquisition and serials modules) * Koha::UI::Table::Builder::Items Same as previously for the table. We are now using this table from 3 different places (batch item mod, batch item del, backgroung job detail view) and the code is only in one place. To use with items_table_batchmod BLOCK (still from html_helpers.inc) This patch is fixing some bugs about repeatable subfields and regex. A UI change will reflect the limitation: if you want to apply a regex on a subfield you cannot add several subfields for the same subfield code. Test plan: Prepare the ground: - Make sure you are always using a bibliographic/item record using the framework you are modifying! - Add some subfields for items that are not mapped with a koha field (note that you can use 'é' for more fun, don't try more funny characters) - Make some subfields (mapped and not mapped with a kohafield) repeatable - Add default values to some of your subfields There are 4 main screens to test: 1. Add/edit item form The behaviour should be the same before and after this patch. See test plan from bug 27526. Those 2 prefs must be tested: * SubfieldsToAllowForRestrictedEditing * SubfieldsToUseWhenPrefill 2. Batch modification a. Fill some values, play with repeatable and regex. Note that the behaviour in master was buggy, only the first value was modified by the regex: * With subfield = "a | b" 1 value added with "new" => "new | b" * With subfield = "a | b" 2 new fields "new1","new2" => "new2 | b" Important note: For repeatable subfields, a regex will apply on the subfields in the "concatenated form". To apply the regex on all the different subfields of a given subfield code you must use the "g" modifier. This could be improved later, but keep in mind that it's not a regression or behaviour change. b. Play with the "Populate fields with default values from default framework" checkbox c. Use this tool to modify items and play with the different sysprefs that interfer with it: * NewItemsDefaultLocation * SubfieldsToAllowForRestrictedBatchmod * MaxItemsToDisplayForBatchMod * MaxItemsToProcessForBatchMod 3. Batch deletion a. Batch delete some items b. Check items out and try to delete them c. Use the "Delete records if no items remain" checkbox to delete bibliographic records without remaining items. d. Play with the following sysprefs and confirm that it works as expected: * MaxItemsToDisplayForBatchDel e. Stress the tool: Go to the confirmation screen with items that can be deleted, don't request the job to be processed right away, but check the item out before. 4. Background job detail view You must have seen it already if you are curious and tested the above. When a new modification or deletion batch is requested, the confirmation screen will tell you that the job has enqueued. A link to the progress of the job can be followed. On this screen you will be able to see the result of the job once it's fully processed. QA notes: * There are some FIXME's that are not blocker in my opinion. Feel free to discuss them if you have suggestions. * Do we still need MaxItemsToProcessForBatchMod? * Prior to this patchset we had a "Return to the cataloging module" link if we went from the cataloguing module and that the biblio was deleted. We cannot longer know if the biblio will be deleted but we could display a "Go to the cataloging module" link on the "job has been enqueued" screen regardless from where we were coming from. Created attachment 126137 [details] [review] Bug 28445: UI change for repeatable and regex This patch applies the changes describe in the main commit message about the "limitation" and "the behaviour in master was buggy". Created attachment 126138 [details] [review] Bug 28445: Fix encoding issue with the background job's report Created attachment 126139 [details] [review] Bug 28445: Don't display the items if not finished yet Created attachment 126140 [details] [review] Bug 28445: Improve robustness of tests * Libraries are ordered by name by default but if others have been added the test may fail Created attachment 126141 [details] [review] Bug 28445: Fix subfields to blank The value of the checkbox was not correct Created attachment 126142 [details] [review] Bug 28445: Don't surround the whole batch with a transaction It makes more sense to commit when an item has successfully been modified and so move the transaction inside the loop. It also fixes the progress of the whole job. (In reply to Jonathan Druart from comment #38) > (In reply to Nick Clemens from comment #35) > > On additem.pl attempting to edit a repeated field only adds the value from > > the first field to all the repeated attempts, this is true on master as well > > I don't understand. Can you detail please? > I made 952$u repeatable and I can create several subfields, edit, save. And > it is working as I expect. I added new subfield "A" - the values repeat On master I also made "o" repeatable and had same issue With these patches "o" worked fine, "A" always repeated first value and during batch mod when I used the "A" field with two values a single record was often skipped (18 of 19 items updated), or all 19 would be updated, but 18 would repeat first value and the 19th would get the second Something is weird there > > - MaxItemsToDisplayForBatchMod is not respected on background_jobs.pl, all > > items are displayed > > On purpose, do you think we should restore it? In my understanding it was to > avoid the table on the first screen. On master the items are not shown in results, I think we should do the same here - don't show more items than allowed in a batch modify job result > > - Batch deletion failed: Cannot enqueue this job. (The error was: Can't use > > string ("on") as an ARRAY ref while "strict refs" in use at > > /kohadevbox/koha/Koha/BackgroundJob/BatchDeleteItem.pm line 214. , see the > > Koha log file for more information). > > Hum, cannot recreate that, but line 214 is: > 214 my $delete_biblios = @{ $args->{delete_biblios} || [] }; > > That does not seem related at all, can you retry please? I get the error when I check "Delete records if no items remain." (In reply to Nick Clemens from comment #47) > (In reply to Jonathan Druart from comment #38) > > (In reply to Nick Clemens from comment #35) > > > On additem.pl attempting to edit a repeated field only adds the value from > > > the first field to all the repeated attempts, this is true on master as well > > > > I don't understand. Can you detail please? > > I made 952$u repeatable and I can create several subfields, edit, save. And > > it is working as I expect. > > I added new subfield "A" - the values repeat > On master I also made "o" repeatable and had same issue > With these patches "o" worked fine, "A" always repeated first value and > during batch mod when I used the "A" field with two values a single record > was often skipped (18 of 19 items updated), or all 19 would be updated, but > 18 would repeat first value and the 19th would get the second > Something is weird there I do recreate, but it's the expected behaviour I think. If you have "A" defined for the default framework, but not for BK, then the items using BK won't have the values added to "A". > > > - MaxItemsToDisplayForBatchMod is not respected on background_jobs.pl, all > > > items are displayed > > > > On purpose, do you think we should restore it? In my understanding it was to > > avoid the table on the first screen. > > On master the items are not shown in results, I think we should do the same > here - don't show more items than allowed in a batch modify job result Ok, will follow-up. > > > - Batch deletion failed: Cannot enqueue this job. (The error was: Can't use > > > string ("on") as an ARRAY ref while "strict refs" in use at > > > /kohadevbox/koha/Koha/BackgroundJob/BatchDeleteItem.pm line 214. , see the > > > Koha log file for more information). > > > > Hum, cannot recreate that, but line 214 is: > > 214 my $delete_biblios = @{ $args->{delete_biblios} || [] }; > > > > That does not seem related at all, can you retry please? > > I get the error when I check "Delete records if no items remain." Oops, silly mistake. Will fix. Created attachment 126217 [details] [review] Bug 28445: Don't surround the whole batch with a transaction It makes more sense to commit when an item has successfully been modified and so move the transaction inside the loop. It also fixes the progress of the whole job. Created attachment 126218 [details] [review] Bug 28445: delete_biblios is a boolean Will fix: Batch deletion failed: Cannot enqueue this job. (The error was: Can't use string ("on") as an ARRAY ref while "strict refs" in use at /kohadevbox/koha/Koha/BackgroundJob/BatchDeleteItem.pm line 214. , see the Koha log file for more information). Created attachment 126219 [details] [review] Bug 28445: Respect MaxItemsToDisplayForBatchMod from job detail view Created attachment 126223 [details] [review] Bug 28445: Split and join on ' | ' instead of '|' for repeatable Is that correct?! (In reply to Jonathan Druart from comment #48) > (In reply to Nick Clemens from comment #47) > > (In reply to Jonathan Druart from comment #38) > > > (In reply to Nick Clemens from comment #35) > > > > On additem.pl attempting to edit a repeated field only adds the value from > > > > the first field to all the repeated attempts, this is true on master as well > > > > > > I don't understand. Can you detail please? > > > I made 952$u repeatable and I can create several subfields, edit, save. And > > > it is working as I expect. > > > > I added new subfield "A" - the values repeat > > On master I also made "o" repeatable and had same issue > > With these patches "o" worked fine, "A" always repeated first value and > > during batch mod when I used the "A" field with two values a single record > > was often skipped (18 of 19 items updated), or all 19 would be updated, but > > 18 would repeat first value and the 19th would get the second > > Something is weird there > > I do recreate, but it's the expected behaviour I think. > If you have "A" defined for the default framework, but not for BK, then the > items using BK won't have the values added to "A". The problem was when using the background job then the cataloguing edit form. There was a mismatch with the separator for repeatable subfields. It's fixed now, but I am wondering if split/join on ' | ' instead of '|' is correct. (In reply to Jonathan Druart from comment #53) > It's fixed now, but I am wondering if split/join on ' | ' instead of '|' is > correct. Confirmed on 21.05, we were using spaces prior to bug 27526 and this one. Created attachment 126328 [details] [review] Bug 28445: Adjust code to handle regexs Created attachment 126329 [details] [review] Bug 28445: Start form with empty library for batch mod Created attachment 126330 [details] [review] Bug 28445: Use the task queue for the batch delete and update items tool Here we go! Disclaimer: this patch is huge and does many things, but splitting it in several chunks would be time consuming and painful to rebase. However it adds many tests and isolate/refactor code to make it way more reusable. This patchset will make the "batch item modification" and "batch item deletion" features use the task queue (reminder: Since bug 28158, and so 21.05.00, we do no longer use the old "background job" functionality and the user does not get any info about the progress of the job). More than that, more of the code to build an item form and a list of items is now isolated in module (.pm) and include files (.inc) We are reusing the changes made by bug 27526 that simplifies the way we edit/create items (no more unecessary serialization Koha > MARC > MARCXML > XML > HTML) New module: * Koha::BackgroundJob::BatchDeleteItem Subclass for process item deletion in batch * Koha::BackgroundJob::BatchUpdateItem Subclass for process item modification in batch * Koha::Item::Attributes We needed an object to represent item's attributes that are not mapped with a koha field (aka "more subfields xml") This module will help us to create the marcxml from a hashref and the reverse. * Koha::UI::Form::Builder::Item The code that was used to build the add/edit item form is centralised in this module. In conjunction with the subfields_for_item BLOCK (from html_helpers.inc) it will be really easy to reuse this code in other places where the item form is used (acquisition and serials modules) * Koha::UI::Table::Builder::Items Same as previously for the table. We are now using this table from 3 different places (batch item mod, batch item del, backgroung job detail view) and the code is only in one place. To use with items_table_batchmod BLOCK (still from html_helpers.inc) This patch is fixing some bugs about repeatable subfields and regex. A UI change will reflect the limitation: if you want to apply a regex on a subfield you cannot add several subfields for the same subfield code. Test plan: Prepare the ground: - Make sure you are always using a bibliographic/item record using the framework you are modifying! - Add some subfields for items that are not mapped with a koha field (note that you can use 'é' for more fun, don't try more funny characters) - Make some subfields (mapped and not mapped with a kohafield) repeatable - Add default values to some of your subfields There are 4 main screens to test: 1. Add/edit item form The behaviour should be the same before and after this patch. See test plan from bug 27526. Those 2 prefs must be tested: * SubfieldsToAllowForRestrictedEditing * SubfieldsToUseWhenPrefill 2. Batch modification a. Fill some values, play with repeatable and regex. Note that the behaviour in master was buggy, only the first value was modified by the regex: * With subfield = "a | b" 1 value added with "new" => "new | b" * With subfield = "a | b" 2 new fields "new1","new2" => "new2 | b" Important note: For repeatable subfields, a regex will apply on the subfields in the "concatenated form". To apply the regex on all the different subfields of a given subfield code you must use the "g" modifier. This could be improved later, but keep in mind that it's not a regression or behaviour change. b. Play with the "Populate fields with default values from default framework" checkbox c. Use this tool to modify items and play with the different sysprefs that interfer with it: * NewItemsDefaultLocation * SubfieldsToAllowForRestrictedBatchmod * MaxItemsToDisplayForBatchMod * MaxItemsToProcessForBatchMod 3. Batch deletion a. Batch delete some items b. Check items out and try to delete them c. Use the "Delete records if no items remain" checkbox to delete bibliographic records without remaining items. d. Play with the following sysprefs and confirm that it works as expected: * MaxItemsToDisplayForBatchDel e. Stress the tool: Go to the confirmation screen with items that can be deleted, don't request the job to be processed right away, but check the item out before. 4. Background job detail view You must have seen it already if you are curious and tested the above. When a new modification or deletion batch is requested, the confirmation screen will tell you that the job has enqueued. A link to the progress of the job can be followed. On this screen you will be able to see the result of the job once it's fully processed. QA notes: * There are some FIXME's that are not blocker in my opinion. Feel free to discuss them if you have suggestions. * Do we still need MaxItemsToProcessForBatchMod? * Prior to this patchset we had a "Return to the cataloging module" link if we went from the cataloguing module and that the biblio was deleted. We cannot longer know if the biblio will be deleted but we could display a "Go to the cataloging module" link on the "job has been enqueued" screen regardless from where we were coming from. Created attachment 126331 [details] [review] Bug 28445: UI change for repeatable and regex This patch applies the changes describe in the main commit message about the "limitation" and "the behaviour in master was buggy". Created attachment 126332 [details] [review] Bug 28445: Fix encoding issue with the background job's report Created attachment 126333 [details] [review] Bug 28445: Don't display the items if not finished yet Created attachment 126334 [details] [review] Bug 28445: Improve robustness of tests * Libraries are ordered by name by default but if others have been added the test may fail Created attachment 126335 [details] [review] Bug 28445: Fix subfields to blank The value of the checkbox was not correct Created attachment 126336 [details] [review] Bug 28445: Don't surround the whole batch with a transaction It makes more sense to commit when an item has successfully been modified and so move the transaction inside the loop. It also fixes the progress of the whole job. Created attachment 126337 [details] [review] Bug 28445: delete_biblios is a boolean Will fix: Batch deletion failed: Cannot enqueue this job. (The error was: Can't use string ("on") as an ARRAY ref while "strict refs" in use at /kohadevbox/koha/Koha/BackgroundJob/BatchDeleteItem.pm line 214. , see the Koha log file for more information). Created attachment 126338 [details] [review] Bug 28445: Respect MaxItemsToDisplayForBatchMod from job detail view Created attachment 126339 [details] [review] Bug 28445: Split and join on ' | ' instead of '|' for repeatable Is that correct?! Created attachment 126352 [details] [review] Bug 28445: Isolate add item form to an include file To ease reusability Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 126353 [details] [review] Bug 28445: Move controller code to Koha::UI::Form::Builder::Item To ease reusability Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 126354 [details] [review] Bug 28445: Use Koha::UI::Form::Builder::Item from batchmod as well Now that it's reusable, let use it somewhere else! Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 126355 [details] [review] Bug 28445: Adjust code to handle regexs Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 126356 [details] [review] Bug 28445: Start form with empty library for batch mod Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 126357 [details] [review] Bug 28445: Use the task queue for the batch delete and update items tool Here we go! Disclaimer: this patch is huge and does many things, but splitting it in several chunks would be time consuming and painful to rebase. However it adds many tests and isolate/refactor code to make it way more reusable. This patchset will make the "batch item modification" and "batch item deletion" features use the task queue (reminder: Since bug 28158, and so 21.05.00, we do no longer use the old "background job" functionality and the user does not get any info about the progress of the job). More than that, more of the code to build an item form and a list of items is now isolated in module (.pm) and include files (.inc) We are reusing the changes made by bug 27526 that simplifies the way we edit/create items (no more unecessary serialization Koha > MARC > MARCXML > XML > HTML) New module: * Koha::BackgroundJob::BatchDeleteItem Subclass for process item deletion in batch * Koha::BackgroundJob::BatchUpdateItem Subclass for process item modification in batch * Koha::Item::Attributes We needed an object to represent item's attributes that are not mapped with a koha field (aka "more subfields xml") This module will help us to create the marcxml from a hashref and the reverse. * Koha::UI::Form::Builder::Item The code that was used to build the add/edit item form is centralised in this module. In conjunction with the subfields_for_item BLOCK (from html_helpers.inc) it will be really easy to reuse this code in other places where the item form is used (acquisition and serials modules) * Koha::UI::Table::Builder::Items Same as previously for the table. We are now using this table from 3 different places (batch item mod, batch item del, backgroung job detail view) and the code is only in one place. To use with items_table_batchmod BLOCK (still from html_helpers.inc) This patch is fixing some bugs about repeatable subfields and regex. A UI change will reflect the limitation: if you want to apply a regex on a subfield you cannot add several subfields for the same subfield code. Test plan: Prepare the ground: - Make sure you are always using a bibliographic/item record using the framework you are modifying! - Add some subfields for items that are not mapped with a koha field (note that you can use 'é' for more fun, don't try more funny characters) - Make some subfields (mapped and not mapped with a kohafield) repeatable - Add default values to some of your subfields There are 4 main screens to test: 1. Add/edit item form The behaviour should be the same before and after this patch. See test plan from bug 27526. Those 2 prefs must be tested: * SubfieldsToAllowForRestrictedEditing * SubfieldsToUseWhenPrefill 2. Batch modification a. Fill some values, play with repeatable and regex. Note that the behaviour in master was buggy, only the first value was modified by the regex: * With subfield = "a | b" 1 value added with "new" => "new | b" * With subfield = "a | b" 2 new fields "new1","new2" => "new2 | b" Important note: For repeatable subfields, a regex will apply on the subfields in the "concatenated form". To apply the regex on all the different subfields of a given subfield code you must use the "g" modifier. This could be improved later, but keep in mind that it's not a regression or behaviour change. b. Play with the "Populate fields with default values from default framework" checkbox c. Use this tool to modify items and play with the different sysprefs that interfer with it: * NewItemsDefaultLocation * SubfieldsToAllowForRestrictedBatchmod * MaxItemsToDisplayForBatchMod * MaxItemsToProcessForBatchMod 3. Batch deletion a. Batch delete some items b. Check items out and try to delete them c. Use the "Delete records if no items remain" checkbox to delete bibliographic records without remaining items. d. Play with the following sysprefs and confirm that it works as expected: * MaxItemsToDisplayForBatchDel e. Stress the tool: Go to the confirmation screen with items that can be deleted, don't request the job to be processed right away, but check the item out before. 4. Background job detail view You must have seen it already if you are curious and tested the above. When a new modification or deletion batch is requested, the confirmation screen will tell you that the job has enqueued. A link to the progress of the job can be followed. On this screen you will be able to see the result of the job once it's fully processed. QA notes: * There are some FIXME's that are not blocker in my opinion. Feel free to discuss them if you have suggestions. * Do we still need MaxItemsToProcessForBatchMod? * Prior to this patchset we had a "Return to the cataloging module" link if we went from the cataloguing module and that the biblio was deleted. We cannot longer know if the biblio will be deleted but we could display a "Go to the cataloging module" link on the "job has been enqueued" screen regardless from where we were coming from. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 126358 [details] [review] Bug 28445: UI change for repeatable and regex This patch applies the changes describe in the main commit message about the "limitation" and "the behaviour in master was buggy". Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 126359 [details] [review] Bug 28445: Fix encoding issue with the background job's report Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 126360 [details] [review] Bug 28445: Don't display the items if not finished yet Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 126361 [details] [review] Bug 28445: Improve robustness of tests * Libraries are ordered by name by default but if others have been added the test may fail Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 126362 [details] [review] Bug 28445: Fix subfields to blank The value of the checkbox was not correct Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 126363 [details] [review] Bug 28445: Don't surround the whole batch with a transaction It makes more sense to commit when an item has successfully been modified and so move the transaction inside the loop. It also fixes the progress of the whole job. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 126364 [details] [review] Bug 28445: delete_biblios is a boolean Will fix: Batch deletion failed: Cannot enqueue this job. (The error was: Can't use string ("on") as an ARRAY ref while "strict refs" in use at /kohadevbox/koha/Koha/BackgroundJob/BatchDeleteItem.pm line 214. , see the Koha log file for more information). Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 126365 [details] [review] Bug 28445: Respect MaxItemsToDisplayForBatchMod from job detail view Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 126366 [details] [review] Bug 28445: Split and join on ' | ' instead of '|' for repeatable Is that correct?! Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 126367 [details] [review] Bug 28445: (follow-up) Display count of modified items when too many to display Created attachment 126374 [details] [review] Bug 28445: Isolate add item form to an include file To ease reusability Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 126375 [details] [review] Bug 28445: Move controller code to Koha::UI::Form::Builder::Item To ease reusability Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 126376 [details] [review] Bug 28445: Use Koha::UI::Form::Builder::Item from batchmod as well Now that it's reusable, let use it somewhere else! Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 126377 [details] [review] Bug 28445: Adjust code to handle regexs Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 126378 [details] [review] Bug 28445: Start form with empty library for batch mod Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 126379 [details] [review] Bug 28445: Use the task queue for the batch delete and update items tool Here we go! Disclaimer: this patch is huge and does many things, but splitting it in several chunks would be time consuming and painful to rebase. However it adds many tests and isolate/refactor code to make it way more reusable. This patchset will make the "batch item modification" and "batch item deletion" features use the task queue (reminder: Since bug 28158, and so 21.05.00, we do no longer use the old "background job" functionality and the user does not get any info about the progress of the job). More than that, more of the code to build an item form and a list of items is now isolated in module (.pm) and include files (.inc) We are reusing the changes made by bug 27526 that simplifies the way we edit/create items (no more unecessary serialization Koha > MARC > MARCXML > XML > HTML) New module: * Koha::BackgroundJob::BatchDeleteItem Subclass for process item deletion in batch * Koha::BackgroundJob::BatchUpdateItem Subclass for process item modification in batch * Koha::Item::Attributes We needed an object to represent item's attributes that are not mapped with a koha field (aka "more subfields xml") This module will help us to create the marcxml from a hashref and the reverse. * Koha::UI::Form::Builder::Item The code that was used to build the add/edit item form is centralised in this module. In conjunction with the subfields_for_item BLOCK (from html_helpers.inc) it will be really easy to reuse this code in other places where the item form is used (acquisition and serials modules) * Koha::UI::Table::Builder::Items Same as previously for the table. We are now using this table from 3 different places (batch item mod, batch item del, backgroung job detail view) and the code is only in one place. To use with items_table_batchmod BLOCK (still from html_helpers.inc) This patch is fixing some bugs about repeatable subfields and regex. A UI change will reflect the limitation: if you want to apply a regex on a subfield you cannot add several subfields for the same subfield code. Test plan: Prepare the ground: - Make sure you are always using a bibliographic/item record using the framework you are modifying! - Add some subfields for items that are not mapped with a koha field (note that you can use 'é' for more fun, don't try more funny characters) - Make some subfields (mapped and not mapped with a kohafield) repeatable - Add default values to some of your subfields There are 4 main screens to test: 1. Add/edit item form The behaviour should be the same before and after this patch. See test plan from bug 27526. Those 2 prefs must be tested: * SubfieldsToAllowForRestrictedEditing * SubfieldsToUseWhenPrefill 2. Batch modification a. Fill some values, play with repeatable and regex. Note that the behaviour in master was buggy, only the first value was modified by the regex: * With subfield = "a | b" 1 value added with "new" => "new | b" * With subfield = "a | b" 2 new fields "new1","new2" => "new2 | b" Important note: For repeatable subfields, a regex will apply on the subfields in the "concatenated form". To apply the regex on all the different subfields of a given subfield code you must use the "g" modifier. This could be improved later, but keep in mind that it's not a regression or behaviour change. b. Play with the "Populate fields with default values from default framework" checkbox c. Use this tool to modify items and play with the different sysprefs that interfer with it: * NewItemsDefaultLocation * SubfieldsToAllowForRestrictedBatchmod * MaxItemsToDisplayForBatchMod * MaxItemsToProcessForBatchMod 3. Batch deletion a. Batch delete some items b. Check items out and try to delete them c. Use the "Delete records if no items remain" checkbox to delete bibliographic records without remaining items. d. Play with the following sysprefs and confirm that it works as expected: * MaxItemsToDisplayForBatchDel e. Stress the tool: Go to the confirmation screen with items that can be deleted, don't request the job to be processed right away, but check the item out before. 4. Background job detail view You must have seen it already if you are curious and tested the above. When a new modification or deletion batch is requested, the confirmation screen will tell you that the job has enqueued. A link to the progress of the job can be followed. On this screen you will be able to see the result of the job once it's fully processed. QA notes: * There are some FIXME's that are not blocker in my opinion. Feel free to discuss them if you have suggestions. * Do we still need MaxItemsToProcessForBatchMod? * Prior to this patchset we had a "Return to the cataloging module" link if we went from the cataloguing module and that the biblio was deleted. We cannot longer know if the biblio will be deleted but we could display a "Go to the cataloging module" link on the "job has been enqueued" screen regardless from where we were coming from. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 126380 [details] [review] Bug 28445: UI change for repeatable and regex This patch applies the changes describe in the main commit message about the "limitation" and "the behaviour in master was buggy". Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 126381 [details] [review] Bug 28445: Fix encoding issue with the background job's report Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 126382 [details] [review] Bug 28445: Don't display the items if not finished yet Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 126383 [details] [review] Bug 28445: Improve robustness of tests * Libraries are ordered by name by default but if others have been added the test may fail Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 126384 [details] [review] Bug 28445: Fix subfields to blank The value of the checkbox was not correct Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 126385 [details] [review] Bug 28445: Don't surround the whole batch with a transaction It makes more sense to commit when an item has successfully been modified and so move the transaction inside the loop. It also fixes the progress of the whole job. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 126386 [details] [review] Bug 28445: delete_biblios is a boolean Will fix: Batch deletion failed: Cannot enqueue this job. (The error was: Can't use string ("on") as an ARRAY ref while "strict refs" in use at /kohadevbox/koha/Koha/BackgroundJob/BatchDeleteItem.pm line 214. , see the Koha log file for more information). Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 126387 [details] [review] Bug 28445: Respect MaxItemsToDisplayForBatchMod from job detail view Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 126388 [details] [review] Bug 28445: Split and join on ' | ' instead of '|' for repeatable Is that correct?! Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 126389 [details] [review] Bug 28445: (follow-up) Display count of modified items when too many to display Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Pushed to master for 21.11, thanks to everybody involved! Created attachment 127033 [details] [review] Bug 28445: Add tests for subfields_to_prefill I first wrote test for another thing but the code was actually correct. So submitting the tests. Note that the marc for more_subfields does not need to be passed, we can build it when needed. Last patch pushed to master. Created attachment 127037 [details] [review] Bug 28445: Correctly count modified items Created attachment 127038 [details] [review] Bug 28445: Remove modified_itemitemnumbers Wrong substitution Created attachment 127039 [details] [review] Bug 28445: Fix LostItem call and display errors Never a try without a catch, ever! Last three patches pushed to master. Created attachment 128007 [details] [review] Bug 28445: Add diag for failing test Temporary patch to help trying to fix the following random error: # Failed test 'Item types should be sorted by description and an empty entries should be shown' # at t/db_dependent/Koha/UI/Form/Builder/Item.t line 121. # Structures begin differing at: # $got->[12] = 'Be9T9FH5aO' # $expected->[12] = 'REF' # Looks like you failed 1 test of 2. # Failed test 'itemtypes' # at t/db_dependent/Koha/UI/Form/Builder/Item.t line 130. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 128008 [details] [review] Bug 28445: (follow-up) Add diag for failing test Last diag was not enough (Koha_Master_D11/457) # Failed test 'Item types should be sorted by description and an empty entries should be shown' # at t/db_dependent/Koha/UI/Form/Builder/Item.t line 127. # Structures begin differing at: # $got->[11] = 'X_vvQE5' # $expected->[11] = 'knHm32' # Itemtypes details: $VAR1 = [ # '', # 'EMVzKcGR', # 'x4S8_BaFCp', # 'Cp2BGkY', # 'BK', # 'CF', # 'CR', # 'bBvgmVAIzD', # 'WKyaQ_l', # 'Cjc0SKQz', # 'liic63', # 'X_vvQE5', # 'knHm32', # 'MP', # 'MX', # 'MU', # 'g_F5eNqA', # 'REF', # 'pkxaMSv', # 'VM' # ]; # $VAR2 = [ # 'bBvgmVAIzD', # 'BK', # 'CF', # 'Cjc0SKQz', # 'Cp2BGkY', # 'CR', # 'EMVzKcGR', # 'g_F5eNqA', # 'knHm32', # 'liic63', # 'MP', # 'MU', # 'MX', # 'pkxaMSv', # 'REF', # 'VM', # 'WKyaQ_l', # 'X_vvQE5', # 'x4S8_BaFCp' # ]; We needed the translated_description. It should be the same actually, but maybe something weird is happening. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 128009 [details] [review] Bug 28445: Prevent UI/Form/Builder/Item.t to fail randomly MySQL and Perl don't order strings with _ identically which cause a mismatch when comparing the itemtypes. Last 3 patches pushed to master (the first one has been pushed earlier actually). |