kohabug 1993 - task scheduler improvements

[1] Removed hardcoded path to original developer's
    Koha configuration file; now checks KOHA_CONF.
[2] Now attempts to figure out if a task was successfully
    added to the task queue; if it wasn't, the failure
    is now reported in the  task scheduler page.
[3] When traveling to the task scheduler page from a
    saved report, the report in question is now
    selected by default.
[4] Removed the button to edit a queued task; this is
    not supported.

This patch is essentially a bandage; I've added a BUGS
section and some FIXMEs to the POD for C4::Scheduler
detailing some issues - a refactoring of the task
scheduler is in order at some point.

Signed-off-by: Joshua Ferraro <jmf@liblime.com>

diff --git a/C4/Scheduler.pm b/C4/Scheduler.pm
index 92982fb..40e4dfb 100644 (file)
--- a/C4/Scheduler.pm
+++ b/C4/Scheduler.pm
@@ -85,13 +85,47 @@ sub get_at_job {
 
 =item add_at_job ($time,$command)
 
-Given a timestamp and a command this will schedule the job to run at that time
+Given a timestamp and a command this will schedule the job to run at that time.
+
+Returns true if the job is added to the queue and false otherwise.
 
 =cut
 
 sub add_at_job {
        my ($time,$command) = @_;
+    # FIXME - a description of the task to be run 
+    # may be a better tag, since the tag is displayed
+    # in the job list that the administrator sees - e.g.,
+    # "run report foo, send to foo@bar.com"
        Schedule::At::add(TIME => $time, COMMAND => $command, TAG => $command);
+
+    # FIXME - this method of checking whether the job was added
+    # to the queue is less than perfect:
+    #
+    # 1. Since the command is the tag, it is possible that there is
+    #    already a job in the queue with the same tag.  However, since
+    #    the tag is what displays in the job list, we can't just
+    #    give it a unique ID.
+    # 2. Schedule::At::add() is supposed to return a non-zero
+    #    value if it fails to add a job - however, it does
+    #    not check all error conditions - in particular, it does
+    #    not check the return value of the "at" run; it basically
+    #    complains only if it can't find at.
+    # 3. Similary, Schedule::At::add() does not do something more useful,
+    #    such as returning the job ID.  To be fair, it is possible
+    #    that 'at' does not allow this in any portable way.
+    # 4. Although unlikely, it is possible that a job could be added
+    #    and completed instantly, thus dropping off the queue.
+    my $job_found = 0;
+    eval {
+           my %jobs = Schedule::At::getJobs(TAG => $command);
+        $job_found = scalar(keys %jobs) > 0;
+    };
+    if ($@) {
+        return 0;
+    } else {
+        return $job_found;
+    }
 }
 
 sub remove_at_job {
@@ -102,6 +136,24 @@ sub remove_at_job {
 1;
 __END__
 
+=head1 BUGS
+
+At some point C<C4::Scheduler> should be refactored:
+
+=over 4
+
+=item At and C<Schedule::At> does not work on Win32.
+
+=item At is not installed by default on all platforms.
+
+=item The At queue used by Koha is owned by the httpd user.  If multiple
+Koha databases share an Apache instance on a server, everybody can
+see everybody's jobs.
+
+=item There is no support for scheduling a job to run more than once.
+
+=back
+
 =head1 AUTHOR
 
 Chris Cormack <crc@liblime.com>

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/scheduler.tmpl b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/scheduler.tmpl
index f5d0dcc..f09a02e 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/scheduler.tmpl
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/scheduler.tmpl
@@ -15,6 +15,11 @@
     <div class="yui-b">
 
 <div id="main">
+
+<!-- TMPL_IF NAME="job_add_failed" -->
+<div class="dialog message">Failed to add scheduled task</div>
+<!-- /TMPL_IF -->
+
 <form name="form1" action="scheduler.pl" method="post">
 <input type="hidden" name="mode" value="job_add" />
 
@@ -84,8 +89,12 @@
 </li>
 <li><label for="report">Report:</label>
 <select name="report" id="report">
-<!-- TMPL_LOOP NAME="savedreports" -->                                   
-<option value="<!-- TMPL_VAR NAME="id" -->"><!-- TMPL_VAR NAME="report_name"--></option>                  
+<!-- TMPL_LOOP NAME="savedreports" -->
+    <!-- TMPL_IF NAME="selected" -->
+        <option value="<!-- TMPL_VAR NAME="id" -->" selected="selected"><!-- TMPL_VAR NAME="report_name"--></option>
+    <!-- TMPL_ELSE -->
+        <option value="<!-- TMPL_VAR NAME="id" -->"><!-- TMPL_VAR NAME="report_name"--></option>
+    <!-- /TMPL_IF -->
 <!-- /TMPL_LOOP -->
 </select>
 </li>
@@ -117,7 +126,7 @@
 <tr>
 <td><!-- TMPL_VAR NAME="TIME" --></td>
 <td><!-- TMPL_VAR NAME="TAG" --></td>
-<td><input type="submit" value="Edit"/>&nbsp;<input type="submit" name="delete" value="Delete" /></td>
+<td><input type="submit" name="delete" value="Delete" /></td>
 </tr>
 <input type="hidden" name="jobid" value="<!-- TMPL_VAR NAME="JOBID" -->" />
 </form>

diff --git a/tools/scheduler.pl b/tools/scheduler.pl
index 779098b..674a405 100755 (executable)
--- a/tools/scheduler.pl
+++ b/tools/scheduler.pl
@@ -21,14 +21,14 @@ use strict;
 use C4::Context;
 use C4::Scheduler;
 use C4::Reports;
-use C4::Auth;                                                                                                                                              
-use CGI;                                                                                                                                                   
-use C4::Output;                                                                                                                                            
+use C4::Auth;
+use CGI;
+use C4::Output;
 
 my $input = new CGI;
 
 my $base = C4::Context->config('intranetdir');
-my $CONFIG_NAME="/home/crc/koha/etc/koha-production.xml";
+my $CONFIG_NAME = $ENV{'KOHA_CONF'};
 
 my ( $template, $borrowernumber, $cookie ) = get_template_and_user(
            {
@@ -42,6 +42,7 @@ my ( $template, $borrowernumber, $cookie ) = get_template_and_user(
        );
 
 my $mode=$input->param('mode');
+my $id = $input->param('id');
 
 if ($mode eq 'job_add') {
        my $startday   = $input->param('startday');
@@ -60,7 +61,9 @@ if ($mode eq 'job_add') {
            add_cron_job($start,$command);
        }
        else {
-           add_at_job($start,$command);
+           unless (add_at_job($start,$command)) {
+            $template->param(job_add_failed => 1);
+        }
        }
 }
 
@@ -78,7 +81,14 @@ my @jobloop;
 }
 
 @jobloop = sort {$a->{TIME} cmp $b->{TIME}} @jobloop;
-my $reports = get_saved_reports();                                                                                                                     
+
+my $reports = get_saved_reports();
+if (defined $id) {
+    foreach my $report (@$reports) {
+        $report->{'selected'} = 1 if $report->{'id'} eq $id;
+    }
+}
+
 $template->param( 'savedreports' => $reports ); 
 $template->param(JOBS => \@jobloop);
 my $time = localtime(time);