From b411918412c4055ac1f12ec134257b96de90fcd5 Mon Sep 17 00:00:00 2001 From: Michael Gage Date: Thu, 30 Aug 2012 14:50:56 -0400 Subject: [PATCH 1/4] Be more careful about replacing _ with spaces in set names Instead of replacing _ with a space a in $name itself we do the replacement in $display_name The replacement is now only for the purposes of display and $name does not have spaces which can sometimes cause problems. this fixes a bug in which students (but only students) could not print pdf copies from the Course homepage. --- lib/WeBWorK/ContentGenerator/ProblemSets.pm | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/WeBWorK/ContentGenerator/ProblemSets.pm b/lib/WeBWorK/ContentGenerator/ProblemSets.pm index 8a238413cc..bb48dd3e4a 100644 --- a/lib/WeBWorK/ContentGenerator/ProblemSets.pm +++ b/lib/WeBWorK/ContentGenerator/ProblemSets.pm @@ -384,10 +384,10 @@ sub setListRow { $interactiveURL =~ s|/quiz_mode/|/proctored_quiz_mode/| if ( defined( $set->assignment_type() ) && $set->assignment_type() eq 'proctored_gateway' ); - - $name =~ s/_/ /g; + my $display_name = $name; + $display_name =~ s/_/ /g; # this is the link to the homework assignment - my $interactive = CGI::a({-href=>$interactiveURL}, "$name"); + my $interactive = CGI::a({-href=>$interactiveURL}, "$display_name"); my $control = ""; @@ -395,7 +395,7 @@ sub setListRow { my $status = ''; if ( $gwtype ) { if ( $gwtype == 1 ) { - unless (ref($problemRecords[0]) ) {warn "Error: problem not defined in set $name"; return()} + unless (ref($problemRecords[0]) ) {warn "Error: problem not defined in set $display_name"; return()} if ( $problemRecords[0]->num_correct() + $problemRecords[0]->num_incorrect() >= ( ( !($set->attempts_per_version()) ) ? 0 : $set->attempts_per_version() ) ) { @@ -413,7 +413,7 @@ sub setListRow { # reset the link to give the test number my $vnum = $set->version_id; $interactive = CGI::a({-href=>$interactiveURL}, - $r->maketext("[_1] (test [_2])", $name, $vnum)); + $r->maketext("[_1] (test [_2])", $display_name, $vnum)); } else { my $t = time(); if ( $t < $set->open_date() ) { @@ -421,24 +421,24 @@ sub setListRow { if ( $preOpenSets ) { # reset the link $interactive = CGI::a({-href=>$interactiveURL}, - $r->maketext("Take [_1] test", $name)); + $r->maketext("Take [_1] test", $display_name)); } else { $control = ""; - $interactive = $r->maketext("[_1] test", $name); + $interactive = $r->maketext("[_1] test", $display_name); } } elsif ( $t < $set->due_date() ) { $status = $r->maketext("now open, due ") . $self->formatDateTime($set->due_date,undef,$ce->{studentDateDisplayFormat}); $setIsOpen = 1; $interactive = CGI::a({-href=>$interactiveURL}, - $r->maketext("Take [_1] test", $name)); + $r->maketext("Take [_1] test", $display_name)); } else { $status = $r->maketext("Closed"); if ( $authz->hasPermissions( $user, "record_answers_after_due_date" ) ) { $interactive = CGI::a({-href=>$interactiveURL}, - $r->maketext("Take [_1] test", $name)); + $r->maketext("Take [_1] test", $display_name)); } else { - $interactive = $r->maketext("[_1] test", $name); + $interactive = $r->maketext("[_1] test", $display_name); } } } From ea0e340abaf6d3d86ffa45778e8a43118fe1bf61 Mon Sep 17 00:00:00 2001 From: Michael Gage Date: Wed, 5 Sep 2012 10:53:52 -0400 Subject: [PATCH 2/4] Add link to codemirror2 at htdocs from theme/math3 This is probably not the best way to include codemirror2 in every theme but it's a start --- htdocs/themes/math3/codemirror2 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) mode change 160000 => 120000 htdocs/themes/math3/codemirror2 diff --git a/htdocs/themes/math3/codemirror2 b/htdocs/themes/math3/codemirror2 deleted file mode 160000 index 73edae2aaa..0000000000 --- a/htdocs/themes/math3/codemirror2 +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 73edae2aaabf867f45119804b7816a339762414e diff --git a/htdocs/themes/math3/codemirror2 b/htdocs/themes/math3/codemirror2 new file mode 120000 index 0000000000..d4cffd905a --- /dev/null +++ b/htdocs/themes/math3/codemirror2 @@ -0,0 +1 @@ +../../codemirror2 \ No newline at end of file From 5d233ba2f48816744f806f1b4684a4546f4d2697 Mon Sep 17 00:00:00 2001 From: Michael Gage Date: Wed, 5 Sep 2012 10:55:15 -0400 Subject: [PATCH 3/4] Fix bugs and improve error messages in SendMail.pm It's a wonder that anything worked here. It's clear that errors in mailing would not have been reported. The actual bug seems to be that the value $r needs to be passed explicitly into cleanup_register(). This was not being done. It is not clear why this used to work. The error checking routine has been beefed up and to some extent unit tested. The unit tests can be repeated by uncommenting the warn messages. --- .../ContentGenerator/Instructor/SendMail.pm | 75 +++++++++++-------- 1 file changed, 43 insertions(+), 32 deletions(-) diff --git a/lib/WeBWorK/ContentGenerator/Instructor/SendMail.pm b/lib/WeBWorK/ContentGenerator/Instructor/SendMail.pm index 32b8cdcedf..b71ec28f68 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/SendMail.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/SendMail.pm @@ -417,21 +417,21 @@ sub initialize { my $result_message = eval { $self->mail_message_to_recipients() }; if ($@) { # add the die message to the result message - $result_message = "An error occurred while trying to send email.\n" + $result_message .= "An error occurred while trying to send email.\n" . "The error message is:\n\n$@\n\n"; # and also write it to the apache log - $self->r->log->error("An error occurred while trying to send email: $@"); + $r->log->error("An error occurred while trying to send email: $@\n"); } # this could fail too... eval { $self->email_notification($result_message) }; if ($@) { - $self->r->log->error("An error occured while trying to send the email notification: $@"); + $r->log->error("An error occured while trying to send the email notification: $@\n"); } }; if (MP2) { - $r->connection->pool->cleanup_register($post_connection_action); + $r->connection->pool->cleanup_register($post_connection_action, $r); } else { - $r->post_connection($post_connection_action); + $r->post_connection($post_connection_action, $r); } } else { $self->addbadmessage(CGI::p("Didn't recognize button $action")); @@ -781,44 +781,55 @@ sub mail_message_to_recipients { my $merge_file = $self->{merge_file}; my $result_message = ''; my $failed_messages = 0; - + my $error_messages = ''; foreach my $recipient (@recipients) { - # warn "FIXME sending email to $recipient"; - my $error_messages = ''; + $error_messages = ''; + my $ur = $self->{db}->getUser($recipient); #checked unless ($ur) { - $error_messages .= "Record for user $recipient not found\n"; + $error_messages .= "Record for user $recipient not found\n"; next; } - unless ($ur->email_address) { - $error_messages .="User $recipient does not have an email address -- skipping\n"; + unless ($ur->email_address=~/\S/) { #unless address contains a non-blank charachter + $error_messages .="User $recipient does not have an email address -- skipping\n"; next; } + #warn "\nDEBUG: sending email to $recipient with email address ",$ur->email_address,"\n"; + my $msg = eval { $self->process_message($ur,$rh_merge_data) }; $error_messages .= "There were errors in processing user $recipient, merge file $merge_file. \n$@\n" if $@; - my $mailer = Mail::Sender->new({ - from => $ce->{mail}{smtpSender}, - fake_from => $from, - to => $ur->email_address, - smtp => $self->{smtpServer}, - subject => $subject, - headers => "X-Remote-Host: ".$self->{remote_host}, - }); - unless (ref $mailer) { - $error_messages .= "Failed to create a mailer for user $recipient: $Mail::Sender::Error\n"; + #warn "message is ok"; + my $mailer = eval{ Mail::Sender->new({ + from => $ce->{mail}{smtpSender}, + fake_from => $from, + to => $ur->email_address, + smtp => $self->{smtpServer}, + subject => $subject, + headers => "X-Remote-Host: ".$self->{remote_host}, + }) + }; + if ($@) { + $error_messages .= "Failed to create a mailer for user $recipient: $Mail::Sender::Error\n$@\n"; next; } - unless (ref $mailer->Open()) { - $error_messages .= "Failed to open the mailer for user $recipient: $Mail::Sender::Error\n"; + #warn "DEBUG: mailer created as $mailer\n"; + unless (ref($mailer) and $mailer->Open()) { + $error_messages .= "Failed to open the mailer for user $recipient: $@\n $Mail::Sender::Error\n"; next; } - my $MAIL = $mailer->GetHandle() || ($error_messages .= "Couldn't get mailer handle \n"); - print $MAIL $msg || ($error_messages .= "Couldn't print to $MAIL"); - close $MAIL || ($error_messages .= "Couldn't close $MAIL"); - #warn "FIXME mailed to $recipient: ", $ur->email_address, " from $from subject $subject Errors: $error_messages"; - $failed_messages++ if $error_messages; - $result_message .= $error_messages; - } + #warn "DEBUG: mailer opened\n"; + my $MAIL = $mailer->GetHandle() || ($error_messages .= "$recipient: Couldn't get mailer handle \n"); + print $MAIL $msg || ($error_messages .= "$recipient: Couldn't print to mail $MAIL\n"); + close $MAIL || ($error_messages .= "$recipient: Couldn't close mail $MAIL -- possibly a badly formed address: ".$ur->email_address."\n"); + #warn "DEBUG: mailed to $recipient: ", $ur->email_address, " from $from subject $subject. Errors:\n $error_messages\n\n"; + #FIXME -- allow this list to be turned off with a "verbose" flag + $result_message .= "Msg sent to $recipient at ".$ur->email_address."\n" unless $error_messages; + } continue { #update failed messages before continuing loop + if ($error_messages) { + $failed_messages++; + $result_message .= $error_messages; + } + } my $courseName = $self->r->urlpath->arg("courseID"); my $number_of_recipients = scalar(@recipients) - $failed_messages; $result_message = <{defaultFrom}; + warn "\ninstructor message \"". $self->{subject}."\" sent to ", $self->{defaultFrom},"\n"; } sub getRecord { From 6f952e8dd8b9663456315de94ba6c8df56d2ec1d Mon Sep 17 00:00:00 2001 From: Michael Gage Date: Wed, 5 Sep 2012 12:10:47 -0400 Subject: [PATCH 4/4] Comment out codemirror in themes math2 and math3 These two themes are not yet ready to play with codemirror. The theme ubc does seem to play well with codemirror. --- htdocs/themes/math2/system.template | 3 ++- htdocs/themes/math3/system.template | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/htdocs/themes/math2/system.template b/htdocs/themes/math2/system.template index 8149427e99..f42ab3687d 100644 --- a/htdocs/themes/math2/system.template +++ b/htdocs/themes/math2/system.template @@ -30,10 +30,11 @@ /images/favicon.ico"/> /themes/math2/math2.css"/> +/themes/math2/codemirror2/lib/codemirror.css"> /themes/math2/codemirror2/mode/pg/pg.css"> /themes/math2/codemirror2/mode/math/math.css"> - +--> /css/tabber.css"/> diff --git a/htdocs/themes/math3/system.template b/htdocs/themes/math3/system.template index 18e892e9aa..2263687386 100644 --- a/htdocs/themes/math3/system.template +++ b/htdocs/themes/math3/system.template @@ -29,10 +29,11 @@ /images/favicon.ico"/> /themes/math3/math3.css"/> +/themes/math3/codemirror2/lib/codemirror.css"> /themes/math3/codemirror2/mode/pg/pg.css"> /themes/math3/codemirror2/mode/math/math.css"> - +--> /css/tabber.css"/>