Automatic item modification by age cronjob is based on rules with conditions and substitutions. When substitution value is equal to actual item value, the code should not call C4::Items::ModItem. It adds unnecessary action log and entry in zebraqueue. With a rule than can impact all catalogue you can explode your database with action logs.
Created attachment 82983 [details] [review] Bug 21975: get item field in substitutions substitutions field stores the item field with 'items.' prefix. Looks like call of C4::Items::ModItem() works but it will not if ORM object is used in the futur. So I prefer fix it now and it is needed for main patch. Test plan : Run t/db_dependent/Items/AutomaticItemModificationByAge.t
Created attachment 82984 [details] [review] Bug 21975: add Unit Test Added missing transaction rollback
Created attachment 82985 [details] [review] Bug 21975: Avoid unnecessary substitutions in automatic item modification by age Automatic item modification by age cronjob is based on rules with conditions and substitutions. When substitution value is equal to actual item value, the code should not call C4::Items::ModItem. It adds unnecessary action log and entry in zebraqueue. With a rule than can impact all catalogue you can explode your database with action logs. Test plan : 1) Run prove t/db_dependent/Items/AutomaticItemModificationByAge.t 2) Define a item modification by age with no condition 3) Run several times misc/cronjobs/automatic_item_modification_by_age.pl 4) Check it creates only one entry in action_logs and zebraqueue
Created attachment 87575 [details] [review] Bug 21975: get item field in substitutions substitutions field stores the item field with 'items.' prefix. Looks like call of C4::Items::ModItem() works but it will not if ORM object is used in the futur. So I prefer fix it now and it is needed for main patch. Test plan : Run t/db_dependent/Items/AutomaticItemModificationByAge.t Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 87576 [details] [review] Bug 21975: add Unit Test Added missing transaction rollback Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 87577 [details] [review] Bug 21975: Avoid unnecessary substitutions in automatic item modification by age Automatic item modification by age cronjob is based on rules with conditions and substitutions. When substitution value is equal to actual item value, the code should not call C4::Items::ModItem. It adds unnecessary action log and entry in zebraqueue. With a rule than can impact all catalogue you can explode your database with action logs. Test plan : 1) Run prove t/db_dependent/Items/AutomaticItemModificationByAge.t 2) Define a item modification by age with no condition 3) Run several times misc/cronjobs/automatic_item_modification_by_age.pl 4) Check it creates only one entry in action_logs and zebraqueue Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Well spotted, good tests added.. works as expected.. Signing off
Fails QA script, can you please check? 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 FAIL valid BEGIN failed--compilation aborted Compilation failed in require Global symbol "$item" requires explicit package name (did you forget to declare "my $item"?) FAIL t/db_dependent/Items/AutomaticItemModificationByAge.t OK critic OK forbidden patterns OK git manipulation OK pod OK spelling FAIL valid Global symbol "$item" requires explicit package name (did you forget to declare "my $item"?) Compilation failed in require
Ah OK, caused by Bug 21206
Created attachment 87876 [details] [review] Bug 21975 : (followup) get item datas from previous SQL query Forced by removal of C4::Items::GetItem from Bug 21206
Created attachment 87877 [details] [review] Bug 21975 : (followup 2) avoid undef in eq
Created attachment 87879 [details] [review] Bug 21975: get item field in substitutions substitutions field stores the item field with 'items.' prefix. Looks like call of C4::Items::ModItem() works but it will not if ORM object is used in the futur. So I prefer fix it now and it is needed for main patch. Test plan : Run t/db_dependent/Items/AutomaticItemModificationByAge.t Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 87880 [details] [review] Bug 21975: add Unit Test Added missing transaction rollback Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 87881 [details] [review] Bug 21975: Avoid unnecessary substitutions in automatic item modification by age Automatic item modification by age cronjob is based on rules with conditions and substitutions. When substitution value is equal to actual item value, the code should not call C4::Items::ModItem. It adds unnecessary action log and entry in zebraqueue. With a rule than can impact all catalogue you can explode your database with action logs. Test plan : 1) Run prove t/db_dependent/Items/AutomaticItemModificationByAge.t 2) Define a item modification by age with no condition 3) Run several times misc/cronjobs/automatic_item_modification_by_age.pl 4) Check it creates only one entry in action_logs and zebraqueue Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 87882 [details] [review] Bug 21975 : (followup) get item datas from previous SQL query Forced by removal of C4::Items::GetItem from Bug 21206 Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 87883 [details] [review] Bug 21975 : (followup 2) avoid undef in eq Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Back to SO.. works for me.
Created attachment 87889 [details] [review] Bug 21975: get item field in substitutions substitutions field stores the item field with 'items.' prefix. Looks like call of C4::Items::ModItem() works but it will not if ORM object is used in the futur. So I prefer fix it now and it is needed for main patch. Test plan : Run t/db_dependent/Items/AutomaticItemModificationByAge.t Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 87890 [details] [review] Bug 21975: add Unit Test Added missing transaction rollback Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 87891 [details] [review] Bug 21975: Avoid unnecessary substitutions in automatic item modification by age Automatic item modification by age cronjob is based on rules with conditions and substitutions. When substitution value is equal to actual item value, the code should not call C4::Items::ModItem. It adds unnecessary action log and entry in zebraqueue. With a rule than can impact all catalogue you can explode your database with action logs. Test plan : 1) Run prove t/db_dependent/Items/AutomaticItemModificationByAge.t 2) Define a item modification by age with no condition 3) Run several times misc/cronjobs/automatic_item_modification_by_age.pl 4) Check it creates only one entry in action_logs and zebraqueue Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 87892 [details] [review] Bug 21975: (follow-up) get item datas from previous SQL query Forced by removal of C4::Items::GetItem from Bug 21206 Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 87893 [details] [review] Bug 21975: (follow-up) avoid undef in eq Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Awesome work all! Pushed to master for 19.05
Pushed to 18.11.x for 18.11.05
missing dependencies for 18.05.x, no backport