[Bug 21975] New: Unnecessary substitutions in automatic item modification by age
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21975 Bug ID: 21975 Summary: Unnecessary substitutions in automatic item modification by age Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: minor Priority: P5 - low Component: Command-line Utilities Assignee: koha-bugs@lists.koha-community.org Reporter: fridolin.somers@biblibre.com QA Contact: testopia@bugs.koha-community.org CC: robin@catalyst.net.nz 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. -- 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=21975 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |11023 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11023 [Bug 11023] Automatic item modification by age (Was "Toggle new status for items") -- 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=21975 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |fridolin.somers@biblibre.co |ity.org |m Status|NEW |ASSIGNED -- 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=21975 --- Comment #1 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Created attachment 82983 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=82983&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21975 --- Comment #2 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Created attachment 82984 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=82984&action=edit Bug 21975: add Unit Test Added missing transaction rollback -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21975 --- Comment #3 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Created attachment 82985 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=82985&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21975 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch 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=21975 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #82983|0 |1 is obsolete| | --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 87575 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87575&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21975 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #82984|0 |1 is obsolete| | --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 87576 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87576&action=edit Bug 21975: add Unit Test Added missing transaction rollback 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=21975 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #82985|0 |1 is obsolete| | --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 87577 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87577&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21975 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com Status|Needs Signoff |Signed Off --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Well spotted, good tests added.. works as expected.. Signing off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21975 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de Status|Signed Off |Failed QA --- Comment #8 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21975 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|11023 |21206 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11023 [Bug 11023] Automatic item modification by age (Was "Toggle new status for items") https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21206 [Bug 21206] C4::Items - Remove GetItem -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21975 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |11023 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11023 [Bug 11023] Automatic item modification by age (Was "Toggle new status for items") -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21975 --- Comment #9 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Ah OK, caused by Bug 21206 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21975 Fridolin SOMERS <fridolin.somers@biblibre.com> 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=21975 --- Comment #10 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Created attachment 87876 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87876&action=edit Bug 21975 : (followup) get item datas from previous SQL query Forced by removal of C4::Items::GetItem from Bug 21206 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21975 --- Comment #11 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Created attachment 87877 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87877&action=edit Bug 21975 : (followup 2) avoid undef in eq -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21975 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #87575|0 |1 is obsolete| | --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 87879 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87879&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21975 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #87576|0 |1 is obsolete| | --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 87880 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87880&action=edit Bug 21975: add Unit Test Added missing transaction rollback 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=21975 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #87577|0 |1 is obsolete| | --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 87881 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87881&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21975 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #87876|0 |1 is obsolete| | --- Comment #15 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 87882 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87882&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21975 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #87877|0 |1 is obsolete| | --- Comment #16 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 87883 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87883&action=edit Bug 21975 : (followup 2) avoid undef in eq 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=21975 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #17 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Back to SO.. works for me. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21975 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21975 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #87879|0 |1 is obsolete| | Attachment #87882|0 |1 is obsolete| | Attachment #87883|0 |1 is obsolete| | --- Comment #18 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 87889 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87889&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21975 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #87880|0 |1 is obsolete| | --- Comment #19 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 87890 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87890&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21975 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #87881|0 |1 is obsolete| | --- Comment #20 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 87891 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87891&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21975 --- Comment #21 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 87892 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87892&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21975 --- Comment #22 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 87893 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87893&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21975 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |nick@bywatersolutions.com --- Comment #23 from Nick Clemens <nick@bywatersolutions.com> --- Awesome work all! Pushed to master for 19.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21975 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |19.05.00, 18.11.05 released in| | Status|Pushed to Master |Pushed to Stable --- Comment #24 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Pushed to 18.11.x for 18.11.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21975 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com --- Comment #25 from Lucas Gass <lucas@bywatersolutions.com> --- missing dependencies for 18.05.x, no backport -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org