Attachment 25024 Details for Bug 11603 – Bug 11603: Refactoring

[patch] Bug 11603: Refactoring

Bug-11603-Refactoring.patch (text/plain), 8.35 KB, created by Jonathan Druart on 2014-02-04 14:16:16 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2014-02-04 14:16:16 UTC

Size: 8.35 KB

patch

obsolete

>From 1ed020f2ad5236a9b89a87b9db54442111b040c5 Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@biblibre.com>
>Date: Wed, 22 Jan 2014 17:29:33 +0100
>Subject: [PATCH] Bug 11603: Refactoring
>
>A lot of code is duplicated for the 3 supported formats html, csv and
>ods.
>
>This patch refactores some of the code and makes it more readable.
>---
> misc/cronjobs/gather_print_notices.pl |  192 +++++++++++++++------------------
> 1 file changed, 85 insertions(+), 107 deletions(-)
>
>diff --git a/misc/cronjobs/gather_print_notices.pl b/misc/cronjobs/gather_print_notices.pl
>index 032b0f6..2f8f7b9 100755
>--- a/misc/cronjobs/gather_print_notices.pl
>+++ b/misc/cronjobs/gather_print_notices.pl
>@@ -101,34 +101,41 @@ foreach my $message (@all_messages) {
>     $message->{'content'} = $_;
> }
> 
>-print_notices_html({
>+print_notices({
>     messages => \@all_messages,
>     split => $split,
>     output_directory => $output_directory,
>+    format => 'html',
> }) if $html;
> 
>-print_notices_csv({
>+print_notices({
>     messages => \@all_messages,
>     split => $split,
>     output_directory => $output_directory,
>+    format => 'csv',
> }) if $csv;
> 
>-print_notices_ods({
>+print_notices({
>     messages => \@all_messages,
>     split => $split,
>     output_directory => $output_directory,
>+    format => 'ods',
> }) if $ods;
> 
>-sub print_notices_html {
>+sub print_notices {
>     my ( $params ) = @_;
> 
>     my $messages = $params->{messages};
>     my $split = $params->{split};
>     my $output_directory = $params->{output_directory};
>     my $set_status = $params->{set_status} // 1;
>-    my @filenames;
>+    my $format = $params->{format} // 'html';
>+
>+    die "Format $format is not known"
>+        unless $format =~ m[^html$|^csv$|^ods$];
>+
>+    my ( @filenames, $messages_by_branch );
> 
>-    my $messages_by_branch;
>     if ( $split ) {
>         foreach my $message (@$messages) {
>             push( @{ $messages_by_branch->{ $message->{'branchcode'} } }, $message );
>@@ -139,24 +146,26 @@ sub print_notices_html {
> 
>     while ( my ( $branchcode, $branch_messages ) = each %$messages_by_branch ) {
>         my $filename = $split
>-            ? 'holdnotices-' . $today->output('iso') . "-$branchcode.html"
>-            : 'holdnotices-' . $today->output('iso') . ".html";
>-
>-        my $template =
>-          C4::Templates::gettemplate( 'batch/print-notices.tmpl', 'intranet',
>-            new CGI );
>-
>-        $template->param(
>-            stylesheet => C4::Context->preference("NoticeCSS"),
>-            today      => $today->output(),
>-            messages   => $branch_messages,
>-        );
>-
>-        my $output_file = File::Spec->catdir( $output_directory, $filename )
>-        open my $OUTPUT, '>', $output_file
>-            or die "Could not open $output_file: $!";
>-        print $OUTPUT $template->output;
>-        close $OUTPUT;
>+            ? 'holdnotices-' . $today->output('iso') . "-$branchcode.$format"
>+            : 'holdnotices-' . $today->output('iso') . ".$format";
>+
>+        my $filepath = File::Spec->catdir( $output_directory, $filename );
>+        if ( $format eq 'html' ) {
>+            generate_html({
>+                messages => $branch_messages,
>+                filepath => $filepath,
>+            });
>+        } elsif ( $format eq 'csv' ) {
>+            generate_csv ({
>+                messages => $branch_messages,
>+                filepath => $filepath,
>+            });
>+        } elsif ( $format eq 'ods' ) {
>+            generate_ods ({
>+                messages => $branch_messages,
>+                filepath => $filepath,
>+            });
>+        }
> 
>         if ( $set_status ) {
>             foreach my $message ( @$branch_messages ) {
>@@ -173,104 +182,73 @@ sub print_notices_html {
>     return \@filenames;
> }
> 
>-sub print_notices_csv {
>+sub generate_html {
>     my ( $params ) = @_;
>-
>     my $messages = $params->{messages};
>-    my $split = $params->{split};
>-    my $output_directory = $params->{output_directory};
>-    my $set_status = $params->{set_status} // 1;
>-    my @filenames;
>+    my $filepath = $params->{filepath};
>+
>+    my $template =
>+      C4::Templates::gettemplate( 'batch/print-notices.tmpl', 'intranet',
>+        new CGI );
>+
>+    $template->param(
>+        stylesheet => C4::Context->preference("NoticeCSS"),
>+        today      => $today->output(),
>+        messages   => $messages,
>+    );
>+
>+    open my $OUTPUT, '>', $filepath
>+        or die "Could not open $filepath: $!";
>+    print $OUTPUT $template->output;
>+    close $OUTPUT;
>+}
> 
>-    my $messages_by_branch;
>-    if ( $split ) {
>-        foreach my $message (@$messages) {
>-            push( @{ $messages_by_branch->{ $message->{'branchcode'} } }, $message );
>+sub generate_csv {
>+    my ( $params ) = @_;
>+    my $messages = $params->{messages};
>+    my $filepath = $params->{filepath};
>+
>+    open my $OUTPUT, '>', $filepath
>+        or die "Could not open $filepath: $!";
>+    my ( @csv_lines, $headers );
>+    foreach my $message ( @$messages ) {
>+        my @lines = split /\n/, $message->{content};
>+
>+        # We don't have headers, get them
>+        unless ( $headers ) {
>+            $headers = $lines[0];
>+            chomp $headers;
>+            say $OUTPUT $headers;
>         }
>-    } else {
>-        $messages_by_branch->{all_branches} = $messages;
>-    }
> 
>-    while ( my ( $branchcode, $branch_messages ) = each %$messages_by_branch ) {
>-        my $filename = $split
>-            ? 'holdnotices-' . $today->output('iso') . "-$branchcode.csv"
>-            : 'holdnotices-' . $today->output('iso') . ".csv";
>-
>-        open my $OUTPUT, '>', File::Spec->catdir( $output_directory, $filename );
>-        my ( @csv_lines, $headers );
>-        foreach my $message ( @$branch_messages ) {
>-            my @lines = split /\n/, $message->{content};
>-
>-            # We don't have headers, get them
>-            unless ( $headers ) {
>-                $headers = $lines[0];
>-                chomp $headers;
>-                say $OUTPUT $headers;
>-            }
>-
>-            shift @lines;
>-            for my $line ( @lines ) {
>-                chomp $line;
>-                next if $line =~ /^\s$/;
>-                say $OUTPUT $line;
>-            }
>-
>-            if ( $set_status ) {
>-                C4::Letters::_set_message_status(
>-                    {
>-                        message_id => $message->{'message_id'},
>-                        status => 'sent'
>-                    }
>-                );
>-            }
>+        shift @lines;
>+        for my $line ( @lines ) {
>+            chomp $line;
>+            next if $line =~ /^\s$/;
>+            say $OUTPUT $line;
>         }
>-        close $OUTPUT;
>-        push @filenames, $filename;
>     }
>-    return \@filenames;
> }
> 
>-sub print_notices_ods {
>+sub generate_ods {
>     my ( $params ) = @_;
>-
>     my $messages = $params->{messages};
>-    my $split = $params->{split};
>-    my $output_directory = $params->{output_directory};
>-    my $set_status = $params->{set_status} // 1;
>-    my @filenames;
>-
>-    my $messages_by_branch;
>-    if ( $split ) {
>-        foreach my $message (@$messages) {
>-            push( @{ $messages_by_branch->{ $message->{'branchcode'} } }, $message );
>-        }
>-    } else {
>-        $messages_by_branch->{all_branches} = $messages;
>-    }
>+    my $filepath = $params->{filepath};
> 
>-    while ( my ( $branchcode, $branch_messages ) = each %$messages_by_branch ) {
>-        my $filename = $split
>-            ? 'holdnotices-' . $today->output('iso') . "-$branchcode.ods"
>-            : 'holdnotices-' . $today->output('iso') . ".ods";
>-
>-        my $tmpdir = File::Temp->newdir();
>-
>-        my ( $csv_filename ) = @{ print_notices_csv({
>-            messages => $branch_messages,
>-            output_directory => $tmpdir,
>-            set_status => 0,
>-        }) };
>+    my $tmpdir = File::Temp->newdir();
> 
>-        my $ods_output_filepath = File::Spec->catdir( $output_directory, $filename );
>-        my $csv_filepath = File::Spec->catdir( $tmpdir, $csv_filename );
>+    my ( $csv_filename ) = @{ print_notices({
>+        messages => $messages,
>+        output_directory => $tmpdir,
>+        set_status => 0,
>+        format => 'csv',
>+    }) };
> 
>-        qx|
>-            $csv2ods_cmd -i $csv_filepath -o $ods_output_filepath -d $delimiter
>-        |;
>+    my $csv_filepath = File::Spec->catdir( $tmpdir, $csv_filename );
> 
>-        push @filenames, $filename;
>-    }
>-    return \@filenames;
>+    qx|
>+        $csv2ods_cmd -i $csv_filepath -o $filepath -d $delimiter
>+    |;
> }
> 
> =head1 NAME
>-- 
>1.7.10.4

Actions: View | Diff | Splinter Review

Attachments on bug 11603: 25021 | 25022 | 25023 | 25024 | 25640 | 25642 | 30980 | 30981 | 30982 | 30983 | 37799 | 37812 | 37813 | 37814 | 37815 | 37816 | 37817 | 37829 | 37830 | 37831 | 37832 | 37833 | 37834 | 37837 | 38381 | 38385 | 38386 | 38387 | 38388 | 38389 | 38390 | 38391 | 38392 | 38403 | 38411 | 38412 | 38413 | 38414 | 38415 | 38416 | 38417 | 38418 | 38419 | 39033 | 39034 | 39035 | 39036 | 39037 | 39038 | 39039 | 39040 | 39041