On bug 27837 we ensured that the value for permanent_location was always passed through to the edit Unfortunately - when performing a batch mod the field is not set if we don't want to change it or clear it Forcing the pass through then means we clear the field on every update To recreate: 1 - In frameworks add a new subfield to 952, "C" - make it editable in items tab and visible in intranet/editor 2 - In mappings map that subfield to items.permanent_location 3 - Set an item to have differing shelving location and permanent_location 4 - This saves correctly for an individual item 5 - Edit the item using Tools->batch item modification, updating the note field 6 - Note the permanet_location is removed 7 - Check the DB, the field is set to NULL
Created attachment 123654 [details] [review] Bug 28832: Don't batch modify fields that we don't update In bug 27837 we made sure to always pass through the permanent_location if it was passed in unlike other fields which are not passed if they have no value. During batch mod, however, fields that aren't editied have no value, so forcing permanent_location to pass through forces blanking it. This patch alters the script to only pass thgouhr for edit the fields that have been updated or cleared. To test: 1 - In frameworks add a new subfield to 952, "C" - make it editable in items tab and visible in intranet/editor 2 - In mappings map that subfield to items.permanent_location 3 - Set an item to have differing shelving location and permanent_location 4 - This saves correctly for an individual item 5 - Edit the item using Tools->batch item modification, updating the note field 6 - Note the permanet_location is removed 7 - Check the DB, the field is set to NULL 8 - Apply patch 9 - Repeat 10 - Permanent location is not cleared 11 - Verify that clearing fields works as before, including permanent_location
Created attachment 123966 [details] [review] Bug 28832: Don't batch modify fields that we don't update In bug 27837 we made sure to always pass through the permanent_location if it was passed in unlike other fields which are not passed if they have no value. During batch mod, however, fields that aren't editied have no value, so forcing permanent_location to pass through forces blanking it. This patch alters the script to only pass thgouhr for edit the fields that have been updated or cleared. To test: 1 - In frameworks add a new subfield to 952, "C" - make it editable in items tab and visible in intranet/editor 2 - In mappings map that subfield to items.permanent_location 3 - Set an item to have differing shelving location and permanent_location 4 - This saves correctly for an individual item 5 - Edit the item using Tools->batch item modification, updating the note field 6 - Note the permanet_location is removed 7 - Check the DB, the field is set to NULL 8 - Apply patch 9 - Repeat 10 - Permanent location is not cleared 11 - Verify that clearing fields works as before, including permanent_location Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Created attachment 123982 [details] [review] Bug 28832: Don't batch modify fields that we don't update In bug 27837 we made sure to always pass through the permanent_location if it was passed in unlike other fields which are not passed if they have no value. During batch mod, however, fields that aren't editied have no value, so forcing permanent_location to pass through forces blanking it. This patch alters the script to only pass thgouhr for edit the fields that have been updated or cleared. To test: 1 - In frameworks add a new subfield to 952, "C" - make it editable in items tab and visible in intranet/editor 2 - In mappings map that subfield to items.permanent_location 3 - Set an item to have differing shelving location and permanent_location 4 - This saves correctly for an individual item 5 - Edit the item using Tools->batch item modification, updating the note field 6 - Note the permanet_location is removed 7 - Check the DB, the field is set to NULL 8 - Apply patch 9 - Repeat 10 - Permanent location is not cleared 11 - Verify that clearing fields works as before, including permanent_location Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Deb Stephenson <dstephen@dubuque.lib.ia.us>
All the repeated fields need to be spliced off for this to work. Now for example this breaks the regex replace functionality. If I try to change the copynumber with: t - Copy number s/.*/ TTT / using the batchmod tool it doesn't work after this patch. I think the indicator field modification might be broken now as well (didn't check but looking at the batchMod.pl code it seems so)
Created attachment 125075 [details] [review] Bug 28832: (follow-up) Handle regex fields too
Created attachment 125076 [details] [review] Bug 28832: Don't batch modify fields that we don't update In bug 27837 we made sure to always pass through the permanent_location if it was passed in unlike other fields which are not passed if they have no value. During batch mod, however, fields that aren't editied have no value, so forcing permanent_location to pass through forces blanking it. This patch alters the script to only pass thgouhr for edit the fields that have been updated or cleared. To test: 1 - In frameworks add a new subfield to 952, "C" - make it editable in items tab and visible in intranet/editor 2 - In mappings map that subfield to items.permanent_location 3 - Set an item to have differing shelving location and permanent_location 4 - This saves correctly for an individual item 5 - Edit the item using Tools->batch item modification, updating the note field 6 - Note the permanet_location is removed 7 - Check the DB, the field is set to NULL 8 - Apply patch 9 - Repeat 10 - Permanent location is not cleared 11 - Verify that clearing fields works as before, including permanent_location Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Deb Stephenson <dstephen@dubuque.lib.ia.us> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Created attachment 125077 [details] [review] Bug 28832: (follow-up) Handle regex fields too Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Repeated the original test plan and then did a second batch edit, successfully editing a field via regex, confirmed that edit worked and did not mess up the shelving locations.
The indicator code was not handled here but on a closer look it seems unused, and for some reason the same indicator code is copied all over Koha codebase unused as well. I will open a separate ticket to remove the indicator related code with items.
After applying the patches modifying the item fields to values that evaluate in perl to false is impossible. The conditions in > unless( $values[$i] || $searches[$i] || grep { $subfields[$i] } @disabled ){ should be the same as just above it, i.e. if the value is empty string or not. To reproduce: 1) Try to change inventory field to 0 and notice it is not modified.
After bug 28445 I don't recreate the problem. Can you confirm, Nick?
(In reply to Jonathan Druart from comment #11) > After bug 28445 I don't recreate the problem. Can you confirm, Nick? Yes, however, it still affects 21.05
Created attachment 127982 [details] [review] Bug 28832: (follow-up) Don't remove false values, only empty strings
(In reply to Joonas Kylmälä from comment #10) > After applying the patches modifying the item fields to values that evaluate > in perl to false is impossible. The conditions in > > > unless( $values[$i] || $searches[$i] || grep { $subfields[$i] } @disabled ){ > > should be the same as just above it, i.e. if the value is empty string or > not. Updated > To reproduce: > 1) Try to change inventory field to 0 and notice it is not modified. I actually find this is true from the item editor as well - it is not caused by this patchset
I am sorry, this no longer applies - can you please rebase? Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 28832: Don't batch modify fields that we don't update Using index info to reconstruct a base tree... M tools/batchMod.pl Falling back to patching base and 3-way merge... Auto-merging tools/batchMod.pl CONFLICT (content): Merge conflict in tools/batchMod.pl error: Failed to merge in the changes. Patch failed at 0001 Bug 28832: Don't batch modify fields that we don't update The copy of the patch that failed is found in: .git/rebase-apply/patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-28832-Dont-batch-modify-fields-that-we-dont-up-dHzj6t.patch
(In reply to Katrin Fischer from comment #15) > I am sorry, this no longer applies - can you please rebase? > Applies cleanly to 21.05.x for me
I missed that this was for an older version only - I have added to the description to make it more visible.
We are eagerly waiting for this bug. Any status update?
Created attachment 129991 [details] [review] Bug 28832: Don't batch modify fields that we don't update In bug 27837 we made sure to always pass through the permanent_location if it was passed in unlike other fields which are not passed if they have no value. During batch mod, however, fields that aren't editied have no value, so forcing permanent_location to pass through forces blanking it. This patch alters the script to only pass thgouhr for edit the fields that have been updated or cleared. To test: 1 - In frameworks add a new subfield to 952, "C" - make it editable in items tab and visible in intranet/editor 2 - In mappings map that subfield to items.permanent_location 3 - Set an item to have differing shelving location and permanent_location 4 - This saves correctly for an individual item 5 - Edit the item using Tools->batch item modification, updating the note field 6 - Note the permanet_location is removed 7 - Check the DB, the field is set to NULL 8 - Apply patch 9 - Repeat 10 - Permanent location is not cleared 11 - Verify that clearing fields works as before, including permanent_location Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Deb Stephenson <dstephen@dubuque.lib.ia.us> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 129992 [details] [review] Bug 28832: (follow-up) Handle regex fields too Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 129993 [details] [review] Bug 28832: (follow-up) Don't remove false values, only empty strings Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 129994 [details] [review] Bug 28832: (QA follow-up) Tiny comment spelling fix Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This works as expected for me at 21.05. QA scripts are also happy now I've fixed a minor typo spelling error. I'm happy this works now, Passing QA
Pushed to 21.05.x for 21.05.11
20.11.x also affected. Backported: Pushed to 20.11.x branch for 20.11.15
Not backported to 19.11.x