http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8218 --- Comment #1 from Sophie MEYNIEUX <sophie.meynieux@biblibre.com> --- Created attachment 10039 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=10039&action=edit Bug 8218 : Adding SanitizeEntity and entity_clean sub which replace & by & in a record Bug 8218 : adding script for maintenance to clean any & in strings -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8218 Sophie MEYNIEUX <sophie.meynieux@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|gmcharlt@gmail.com |sophie.meynieux@biblibre.co | |m -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8218 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |chris@bigballofwax.co.nz --- Comment #3 from Chris Cormack <chris@bigballofwax.co.nz> --- Follow up is textual changes on, so setting status to signed off -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8218 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |jonathan.druart@biblibre.co | |m Assignee|sophie.meynieux@biblibre.co |alex.arnaud@biblibre.com |m | --- Comment #5 from Jonathan Druart <jonathan.druart@biblibre.com> --- QA Comments: 1/ Please use use strict; use warnings; OR use Modern::Perl; 2/ Perltidied for misc/maintenance/batchSanitizeEntity.pl 3/ Bad encoding (N°) Technically: 4/ $count += 1; => $count++; 5/ +# We remove spaces +my @biblio; +foreach my $bib(@biblios) { + $bib =~ s/(^\s*|\s*$)//g; + next if $bib eq ""; + push @biblio, $bib; +} +@biblios = @biblio; The second tmp array is useless: # We remove spaces s/(^\s*|\s*$)//g for @biblios; # Remove empty lines @biblios = grep {!/^$/} @biblios; 6/ if ($biblios =~ /\//) is more readable as: if ( $biblio =~ m|/| ) for example 7/ + while (my $bib = $sth-> fetchrow_array()){ There is a useless space ;-) Marking as Failed QA for 1 and 2 -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8218 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8218 Christophe Croullebois <christophe.croullebois@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #10588|0 |1 is obsolete| | Attachment #10589|0 |1 is obsolete| | Attachment #11744|0 |1 is obsolete| | Assignee|alex.arnaud@biblibre.com |christophe.croullebois@bibl | |ibre.com --- Comment #8 from Christophe Croullebois <christophe.croullebois@biblibre.com> --- Created attachment 12591 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=12591&action=edit this patch is finalised Two subs written by Alex Arnaud in C4/Charset.pm, SanitizeEntity and entity_clean I had created two scripts for Biblibre, one to be used in cron, it reindexes all transformed records. Sophie Meynieux brought together the two scripts, she has created a batch mode, usefull but without the reindexing So I have rewrited it and I have integrated "Fixing copyright headers and license and description" written by Chris Cormack and "Followup: Perltidy and QA FIX" written by Jonathan Druart Thx to all -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8218 Christophe Croullebois <christophe.croullebois@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #12591|0 |1 is obsolete| | --- Comment #10 from Christophe Croullebois <christophe.croullebois@biblibre.com> --- Created attachment 13678 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=13678&action=edit patch finalized ok rebased. thx -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8218 --- Comment #11 from Kyle M Hall <kyle@bywatersolutions.com> --- I'm not sure if I am testing this right. Can you please provide a test plan? What I did: 1) Picked a record, added "Includes index. amp;amp;" in a 505a note. 2) Ran script with --run --where search 3) Script says it has fixed 1 biblio 4) The field still looks the same to me in the marc editor. Also: * In the help '-where' should be '--where' * Shouldn't the line '--run' => \$run, be 'run' => \$run, ? -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8218 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA CC| |bgkriegel@gmail.com --- Comment #13 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- The script works as described, at last changes &amp; to & But search as explained by help do not works, or description is incorrect --batch do not prevent script to ask confirmation --cron reindexing fails, rebuild_zebra complains about wrong arguments, I think for newline in system() argument. Also, koha-qa complains: pod errors in C4/Charset.pm forbidden pattern (====) in misc/maintenance/batchSanitizeEntity.pl -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8218 Christophe Croullebois <christophe.croullebois@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8218 Christophe Croullebois <christophe.croullebois@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #15826|0 |1 is obsolete| | --- Comment #16 from Christophe Croullebois <christophe.croullebois@biblibre.com> --- Created attachment 16122 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=16122&action=edit proposed patch Hi Bernardo, thx for your help. Now it must be ok. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8218 Christophe Croullebois <christophe.croullebois@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #16122|0 |1 is obsolete| | --- Comment #18 from Christophe Croullebois <christophe.croullebois@biblibre.com> --- Created attachment 16720 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=16720&action=edit proposed patch -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8218 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #19 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Hi, it works with search, but... 1) --batch option continues to ask confirmation 2) koha-qa complains. You are using Modern::Perl, so you must change the way to open a file change this open( FILE, "$biblios" ) || die("Can't open $biblios"); to this open my $FILE,"<","$biblios" or die("Can't open $biblios"); and the closing/reading accordingly. 3) You are checking for a filename assuming a '/' in filename (absolute) if ( $biblios =~ m|/| ) change to if ( -e $filename ) to allow any file name, then, if not exists, check if the resulting string is composed by numbers. 4) You are reading the file with @bibliofile = <FILE>; that reads 1 line only 5) --where filename do not work I put 3 biblionumber on a file, run command and the result was weird, prints that was about to check 100 bibs, and enter endless loop Not ready yet. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8218 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8218 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #26754|0 |1 is obsolete| | --- Comment #22 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 26755 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26755&action=edit Bug 8218 : Add a maintenance script to sanitize biblio records This patch adds: - a new maintenance script batch_sanitize_records - a new subroutine C4::Charset::SanitizeRecord - new unit tests for the new subroutine Test plan: 1/ prove t/db_dependent/Charset.t 2/ Create a record containing "&amp;" (could be follow with as many 'amp;' as you want) in one of its fields and the same for the field linked to biblioitems.url. The url should not be sanitized, it cans contain "&". 3/ Launch the maintenance script with the -h paramters to see how to use it. 4/ Launch the script using the different parameters: --filename=FILENAME --biblionumbers='XXX' --auto-search The auto-search permits to sanitize all records containing "&amp;" in the marcxml field. Use the verbose flag for testing. Without the --confirm flag, nothing is done. 5/ Use the --confirm flag and verify in the biblioitems.marcxml field that the record has been sanitized. 6/ Try the --reindex flag to reindex records which have been modified. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8218 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #24 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- I would like to suggest an adjustment for this report: The confirm flag is not doing what I would expect it to do. I would expect that after running the sanitize check on the record, confirm would determine if the record gets saved. But looking at the code, I see that you just skip the whole record when confirm is off. Note that the print messages are not that clear too. You say "will be processed" but skip the record. After that you report 0 records to clean. But I knew that I had at least one record.. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8218 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #26755|0 |1 is obsolete| | Attachment #29792|0 |1 is obsolete| | --- Comment #25 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 29793 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=29793&action=edit Bug 8218 : Add a maintenance script to sanitize biblio records This patch adds: - a new maintenance script batch_sanitize_records - a new subroutine C4::Charset::SanitizeRecord - new unit tests for the new subroutine Test plan: 1/ prove t/db_dependent/Charset.t 2/ Create a record containing "&amp;" (could be follow with as many 'amp;' as you want) in one of its fields and the same for the field linked to biblioitems.url. The url should not be sanitized, it cans contain "&". 3/ Launch the maintenance script with the -h paramters to see how to use it. 4/ Launch the script using the different parameters: --filename=FILENAME --biblionumbers='XXX' --auto-search The auto-search permits to sanitize all records containing "&amp;" in the marcxml field. Use the verbose flag for testing. Without the --confirm flag, nothing is done. 5/ Use the --confirm flag and verify in the biblioitems.marcxml field that the record has been sanitized. 6/ Try the --reindex flag to reindex records which have been modified. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8218 M. de Rooy <m.de.rooy@rijksmuseum.nl> 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=8218 --- Comment #28 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 29796 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=29796&action=edit Bug 8218: Follow-up for some small typos Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8218 --- Comment #30 from Jonathan Druart <jonathan.druart@biblibre.com> --- Thank you Marcel for your follow-up and signoff! -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8218 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #29795|0 |1 is obsolete| | Attachment #29798|0 |1 is obsolete| | --- Comment #31 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 30120 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30120&action=edit [PASSED QA] Bug 8218 : Add a maintenance script to sanitize biblio records This patch adds: - a new maintenance script batch_sanitize_records - a new subroutine C4::Charset::SanitizeRecord - new unit tests for the new subroutine Test plan: 1/ prove t/db_dependent/Charset.t 2/ Create a record containing "&amp;" (could be follow with as many 'amp;' as you want) in one of its fields and the same for the field linked to biblioitems.url. The url should not be sanitized, it may contain "&". 3/ Launch the maintenance script with the -h parameter to see how to use it. 4/ Launch the script using the different parameters: --filename=FILENAME --biblionumbers='XXX' --auto-search The auto-search permits to sanitize all records containing "&amp;" in the marcxml field. Use the verbose flag for testing. Without the --confirm flag, nothing is done. 5/ Use the --confirm flag and verify in the biblioitems.marcxml field that the record has been sanitized. 6/ Try the --reindex flag to reindex records which have been modified. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8218 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_3_18_candidate Status|Passed QA |Failed QA CC| |tomascohen@gmail.com --- Comment #33 from Tomás Cohen Arazi <tomascohen@gmail.com> --- I'm willing to push the feature, because I like it, and it doesn't break people's funcionality, and hence marked as rel_3_18_candidate. This means you are not catch by feature freeze. Can you please work a bit on the fact that the feature is about ampersands only? I propose - keeping the SanitizeRecord name general. - renaming _entity_clean as _clean_ampersand - renaming the script to sanitize_records.pl - add a --fix-ampersand switch (the only one FOR NOW, enabled by default) so it is obvious what the script does. - make POD and usage reflect this changes. I mark it FAILED-QA until the authors answer this proposal. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8218 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Passed QA --- Comment #35 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Tomás Cohen Arazi from comment #33)

I'm willing to push the feature, because I like it, and it doesn't break people's funcionality, and hence marked as rel_3_18_candidate. This means you are not catch by feature freeze.

Can you please work a bit on the fact that the feature is about ampersands only? I propose

- keeping the SanitizeRecord name general. - renaming _entity_clean as _clean_ampersand - renaming the script to sanitize_records.pl - add a --fix-ampersand switch (the only one FOR NOW, enabled by default) so it is obvious what the script does. - make POD and usage reflect this changes.

I mark it FAILED-QA until the authors answer this proposal.

I agree, all done in the last patch, back to passed QA. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8218 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de Severity|new feature |enhancement -- You are receiving this mail because: You are watching all bug changes.