[Bug 16519] New: Do not use global variables in [opac-]addbybiblionumbers.pl
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16519 Bug ID: 16519 Summary: Do not use global variables in [opac-]addbybiblionumbers.pl Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: ASSIGNED Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: jonathan.druart@bugs.koha-community.org Reporter: jonathan.druart@bugs.koha-community.org QA Contact: testopia@bugs.koha-community.org Depends on: 16517, 16518 To avoid bug like bug 16518 and to ease the readability/maintainability of these scripts, it would be great to see them rewritten not to use global variables. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16517 [Bug 16517] A server error is raised when creating a new list with an existing name https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16518 [Bug 16518] opac-addbybiblionumber is not plack safe -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16519 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16519 --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 51484 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51484&action=edit Bug 16519: Replace 'our' with 'my' in [opac-]addbybiblionumbers.pl To avoid bug like bug 16518 and to ease the readability/maintainability of these scripts, this patch replaces the use of 'our' with 'my' to avoid the use of global variables. Basically the code has been moved from subroutines to the appropriate places. Test plan: At the intranet and OPAC sides 1/ Add items to a list 2/ Add items to a list using an existing name 3/ Add items to a list you don't have right on it (by modifying the biblionumber in the url) 4/ At the OPAC, use the opac-addbybiblionumber.pl without being logged in to add items to a list -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16519 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |veron@veron.ch Status|Needs Signoff |Failed QA --- Comment #2 from Marc Véron <veron@veron.ch> --- Sorry, QA tools complain: FAIL opac/opac-addbybiblionumber.pl FAIL valid Global symbol "$bib" requires explicit package name opac/opac-addbybiblionumber.pl had compilation errors. Found = in conditional, should be == Can't modify scalar in scalar assignment -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16519 Jonathan Druart <jonathan.druart@bugs.koha-community.org> 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=16519 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #51484|0 |1 is obsolete| | --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 51796 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51796&action=edit Bug 16519: Replace 'our' with 'my' in [opac-]addbybiblionumbers.pl To avoid bug like bug 16518 and to ease the readability/maintainability of these scripts, this patch replaces the use of 'our' with 'my' to avoid the use of global variables. Basically the code has been moved from subroutines to the appropriate places. Test plan: At the intranet and OPAC sides 1/ Add items to a list 2/ Add items to a list using an existing name 3/ Add items to a list you don't have right on it (by modifying the biblionumber in the url) 4/ At the OPAC, use the opac-addbybiblionumber.pl without being logged in to add items to a list -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16519 --- Comment #4 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Shame on me, sorry about that. I have decided to rename the $bib var after my tests, but I have missed one! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16519 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #51796|0 |1 is obsolete| | --- Comment #5 from Marc Véron <veron@veron.ch> --- Created attachment 51903 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=51903&action=edit Bug 16519: Replace 'our' with 'my' in [opac-]addbybiblionumbers.pl To avoid bug like bug 16518 and to ease the readability/maintainability of these scripts, this patch replaces the use of 'our' with 'my' to avoid the use of global variables. Basically the code has been moved from subroutines to the appropriate places. Test plan: At the intranet and OPAC sides 1/ Add items to a list 2/ Add items to a list using an existing name 3/ Add items to a list you don't have right on it (by modifying the biblionumber in the url) 4/ At the OPAC, use the opac-addbybiblionumber.pl without being logged in to add items to a list Signed-off-by: Marc Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16519 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16519 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Hm Not sure if you are improving the code here. Wasn't it possible to move a lot of this code to some module shared by both scripts? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16519 --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Marcel de Rooy from comment #6)
Hm Not sure if you are improving the code here. Wasn't it possible to move a lot of this code to some module shared by both scripts?
With this code this script will now follow the same pattern as other scripts. So yes, I think it improves the code. Everything is possible :) I did not want to add too much changes just for a var scope problem. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16519 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA Patch complexity|--- |Medium patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16519 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #51903|0 |1 is obsolete| | --- Comment #8 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 55662 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=55662&action=edit [PASSED QA] Bug 16519: Replace 'our' with 'my' in [opac-]addbybiblionumbers.pl To avoid bug like bug 16518 and to ease the readability/maintainability of these scripts, this patch replaces the use of 'our' with 'my' to avoid the use of global variables. Basically the code has been moved from subroutines to the appropriate places. Test plan: At the intranet and OPAC sides 1/ Add items to a list 2/ Add items to a list using an existing name 3/ Add items to a list you don't have right on it (by modifying the biblionumber in the url) 4/ At the OPAC, use the opac-addbybiblionumber.pl without being logged in to add items to a list Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16519 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |kyle@bywatersolutions.com --- Comment #9 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to master for 16.11, thanks Jonathan! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16519 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |frederic@tamil.fr Status|Pushed to Master |Pushed to Stable --- Comment #10 from Frédéric Demians <frederic@tamil.fr> --- Pushed in 16.05. Will be in 16.05.05. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org