[Bug 10996] New: Cannot exclude numeric subfields on export
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10996 Bug ID: 10996 Summary: Cannot exclude numeric subfields on export Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: minor Priority: P5 - low Component: Tools Assignee: gmcharlt@gmail.com Reporter: colin.campbell@ptfs-europe.com QA Contact: testopia@bugs.koha-community.org Tools export allows you to specify tags and subfields to strip from exported records. However if the subfields you specify are numeric they are not stripped Cause the regexp used to parse the exclusion list considers all numerics to be part of the tag number so an attempt to strip $9 from tag 110 ends up in stripping tag 1109!! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10996 Colin Campbell <colin.campbell@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Assignee|gmcharlt@gmail.com |colin.campbell@ptfs-europe. | |com --- Comment #1 from Colin Campbell <colin.campbell@ptfs-europe.com> --- Created attachment 21806 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=21806&action=edit Proposed Patch To test export a record containing a numeric subfield (e.g. 100 $9 ) with exclusion specified thus 1009. Without patch the $9 is in the export. With patch it should be stripped -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10996 I'm just a bot <gitbot@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |gitbot@bugs.koha-community. | |org When did the bot| |2013-10-05 last check this| | --- Comment #2 from I'm just a bot <gitbot@bugs.koha-community.org> --- Patch applied cleanly, go forth and signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10996 Paola Rossi <paola.rossi@cineca.it> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #21806|0 |1 is obsolete| | --- Comment #3 from Paola Rossi <paola.rossi@cineca.it> --- Created attachment 21934 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=21934&action=edit Cannot exclude numeric subfields on export -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10996 Paola Rossi <paola.rossi@cineca.it> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |paola.rossi@cineca.it --- Comment #4 from Paola Rossi <paola.rossi@cineca.it> --- I sign off the patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10996 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10996 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #21934|0 |1 is obsolete| | --- Comment #5 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 21974 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=21974&action=edit Bug 10996 Allow numeric subfields to be stripped on export Regex assumed all numeric characters were part of the tag number This is obviously false as $9 would be a likely candidate to be removed on export. Constrain the tag by length The code can be any printing ascii character other than space according to LoC's website Also changed regexp to no longer allow a zero length tag number which is nonsensical. The old regex would accept shorter than 3 digit tags but these were not stripped so I've removed that option considering it a bug not a feature NB assumption that the code makes that a tag is always numeric is incorrect but works in practice. Handling non-numeric tags is a 'To be done' Made code dependent on the regex succeeding. Picking up results from a previous regex on failure can lead to weird hard to identify bugs Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Passes koha-qa.pl, works as advertised. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10996 --- Comment #6 from Galen Charlton <gmcharlt@gmail.com> --- Comment on attachment 21974 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=21974 Bug 10996 Allow numeric subfields to be stripped on export Review of attachment 21974: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=10996&attachment=21974) ----------------------------------------------------------------- ::: tools/export.pl @@ +365,5 @@
+ my $subfield = $2; + + # skip if this record doesn't have this field + next if not defined $record->field($field); + if ($subfield) {
This is not the right test -- consider somebody who wants to exclude a subfield $0 (zero) from export. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10996 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA CC| |gmcharlt@gmail.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10996 Colin Campbell <colin.campbell@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #21974|0 |1 is obsolete| | --- Comment #7 from Colin Campbell <colin.campbell@ptfs-europe.com> --- Created attachment 22009 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=22009&action=edit Amended patch Changed test to be definedness only so it meets the case Galen spotted -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10996 --- Comment #8 from Colin Campbell <colin.campbell@ptfs-europe.com> --- (In reply to Galen Charlton from comment #6)
Comment on attachment 21974 [details] [review] Bug 10996 Allow numeric subfields to be stripped on export
Review of attachment 21974 [details] [review]: -----------------------------------------------------------------
::: tools/export.pl @@ +365,5 @@
+ my $subfield = $2; + + # skip if this record doesn't have this field + next if not defined $record->field($field); + if ($subfield) {
This is not the right test -- consider somebody who wants to exclude a subfield $0 (zero) from export.
Amended patch to cover this case. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10996 Colin Campbell <colin.campbell@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #9 from Colin Campbell <colin.campbell@ptfs-europe.com> --- Reverting status -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10996 Paola Rossi <paola.rossi@cineca.it> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #10 from Paola Rossi <paola.rossi@cineca.it> --- Now the patch let a user exclude a subfield $0 (zero) from export, as required. However, when the tag is repeatable and a biblio has two tags of these, koha let the user exclude the numeric subfield of the first tag, but not the second one (case of the subfield $4 in two different 700 of a biblio). So I pass the patch to "Failed QA" status. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10996 --- Comment #11 from Colin Campbell <colin.campbell@ptfs-europe.com> --- Actually irrespective of this patch testing reveals if you specify a subfield then it only gets stripped from the first occurence. Looks like the existing functionality was in need of fixing -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10996 Colin Campbell <colin.campbell@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #22009|0 |1 is obsolete| | --- Comment #12 from Colin Campbell <colin.campbell@ptfs-europe.com> --- Created attachment 22586 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=22586&action=edit Amended patch Expanded the patch to ensure that all occurences of the tag are processed not just the first as previously was the case -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10996 Colin Campbell <colin.campbell@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #13 from Colin Campbell <colin.campbell@ptfs-europe.com> --- Reset status to needs signoff. Would be grateful if folks could retest and confirm that deletes are being applied to all occurences -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10996 Paola Rossi <paola.rossi@cineca.it> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #22586|0 |1 is obsolete| | --- Comment #14 from Paola Rossi <paola.rossi@cineca.it> --- Created attachment 22596 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=22596&action=edit Cannot exclude numeric subfields on export The deletes are being applied to all occurences. I sign off the patch. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10996 Paola Rossi <paola.rossi@cineca.it> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10996 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA CC| |jonathan.druart@biblibre.co | |m QA Contact|testopia@bugs.koha-communit |jonathan.druart@biblibre.co |y.org |m Severity|minor |normal --- Comment #15 from Jonathan Druart <jonathan.druart@biblibre.com> --- Good catch Paola! QA comment: The patch works great, the code was completely buggy! Maybe a good thing should be to replace the syntax XXXY with XXX$Y (but XXXY is used in several places into Koha it seems to me). Marked as Pased QA. switch the severity to normal. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10996 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #22596|0 |1 is obsolete| | --- Comment #16 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 23067 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23067&action=edit Bug 10996 Allow numeric subfields to be stripped on export Regex assumed all numeric characters were part of the tag number This is obviously false as $9 would be a likely candidate to be removed on export. Constrain the tag by length The code can be any printing ascii character other than space according to LoC's website Also changed regexp to no longer allow a zero length tag number which is nonsensical. The old regex would accept shorter than 3 digit tags but these were not stripped so I've removed that option considering it a bug not a feature NB assumption that the code makes that a tag is always numeric is incorrect but works in practice. Handling non-numeric tags is a 'To be done' Made code dependent on the regex succeeding. Picking up results from a previous regex on failure can lead to weird hard to identify bugs In the course of testing Paola Rossi <paola.rossi@cineca.it> spotted that the delete operation was flawed, only removing the first occurence of the specified tag/tag subfield. Reworked the delete loop to operate on all occurences Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10996 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #17 from Galen Charlton <gmcharlt@gmail.com> --- Pushed to master. Thanks, Colin! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10996 Fridolin SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolyn.somers@biblibre.co | |m Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10996 Fridolin SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable --- Comment #18 from Fridolin SOMERS <fridolyn.somers@biblibre.com> --- This patch has been pushed to 3.14.x, will be in 3.14.1 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10996 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com --- Comment #19 from Tomás Cohen Arazi <tomascohen@gmail.com> --- This patch has been pushed to 3.12.x, will be in 3.12.8. Thanks Colin! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10996 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |12860 --- Comment #20 from Jonathan Druart <jonathan.druart@biblibre.com> --- This patch introduced a bug, see bug 12860 for a fix. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org