Skip to content

Commit

Permalink
Item11587: eliminate calls to readTopicText, rework tests for changes…
Browse files Browse the repository at this point in the history
… to email system
  • Loading branch information
cdot committed May 7, 2017
1 parent c5328f2 commit 60ab182
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 63 deletions.
13 changes: 6 additions & 7 deletions lib/Foswiki/Plugins/ActionTrackerPlugin/ActionNotify.pm
Expand Up @@ -30,8 +30,8 @@ sub actionNotify {
# Assign SESSION so that Func methods work
$Foswiki::Plugins::SESSION = $session;

if ( $expr =~ /\bweb="([^" ,*]+)"/ ) {
Foswiki::Func::pushTopicContext( $1, 'WebHome' );
if ( $expr =~ /\bweb=(["'])([^" ,*]+?)\1/ ) {
Foswiki::Func::pushTopicContext( $2, 'WebHome' );
}

Foswiki::Plugins::ActionTrackerPlugin::lazyInit( $session->{webName},
Expand Down Expand Up @@ -232,7 +232,7 @@ sub _loadWebNotify {

my $list = {};
my $mainweb = Foswiki::Func::getMainWebname();
my $text = Foswiki::Func::readTopicText( $web, $topicname, undef, 1 );
my ( $m, $text ) = Foswiki::Func::readTopic( $web, $topicname );
foreach my $line ( split( /\r?\n/, $text ) ) {
if ( $line =~ /^\s+\*\s([\w\.]+)\s+-\s+([\w\-\.\+]+\@[\w\-\.\+]+)/o ) {
my $who = $1;
Expand Down Expand Up @@ -299,8 +299,7 @@ sub _getMailAddress {

# LEGACY - Try and expand groups the old way
if ( !$addresses && Foswiki::Func::topicExists( $inweb, $intopic ) ) {
my $text =
Foswiki::Func::readTopicText( $inweb, $intopic, undef, 1 );
my ( $m, $text ) = Foswiki::Func::readTopic( $inweb, $intopic );
if ( $intopic =~ m/Group$/o ) {

# If it's a Group topic, match * Set GROUP =
Expand Down Expand Up @@ -433,14 +432,14 @@ sub _findChangesInTopic {
$oldrev =~ s/\d+\.(\d+)/$1/o;

# Recover the action set at that date
my $text = Foswiki::Func::readTopicText( $theWeb, $theTopic, $oldrev, 1 );
my ( $m, $text ) = Foswiki::Func::readTopic( $theWeb, $theTopic, $oldrev );

my $oldActions =
Foswiki::Plugins::ActionTrackerPlugin::ActionSet::load( $theWeb,
$theTopic, $text );

# Recover the current action set.
$text = Foswiki::Func::readTopicText( $theWeb, $theTopic, undef, 1 );
( $m, $text ) = Foswiki::Func::readTopic( $theWeb, $theTopic );
my $currentActions =
Foswiki::Plugins::ActionTrackerPlugin::ActionSet::load( $theWeb,
$theTopic, $text );
Expand Down
12 changes: 3 additions & 9 deletions lib/Foswiki/Plugins/ActionTrackerPlugin/ActionSet.pm
Expand Up @@ -333,15 +333,9 @@ sub allActionsInWeb {

foreach my $topic ( keys %$grep ) {

# SMELL: always read the text, because it's faster in the current
# impl to find the perms embedded in it
my $text =
Foswiki::Func::readTopicText( $web, $topic, undef, $internal );
next
unless $internal
|| Foswiki::Func::checkAccessPermission( 'VIEW',
Foswiki::Func::getWikiName(),
$text, $topic, $web );
my ( $m, $text ) = Foswiki::Func::readTopic( $web, $topic );
next unless $internal || $m->haveAccess('VIEW');

my $tacts =
Foswiki::Plugins::ActionTrackerPlugin::ActionSet::load( $web, $topic,
$text );
Expand Down
10 changes: 7 additions & 3 deletions test/unit/ActionTrackerPlugin/ActionNotifyTests.pm
Expand Up @@ -242,7 +242,10 @@ sub test_B_NotifyLate {
}

my $ok = "";
while ( $html = shift(@FoswikiFnTestCase::mails) ) {
my $mail;
while ( $mail = shift(@FoswikiFnTestCase::mails) ) {

my $html = $mail->as_string();

# Ensure all macros are expanded in the output
$this->assert_does_not_match( qr/%\w+%/, $html, $html );
Expand Down Expand Up @@ -357,8 +360,9 @@ sub test_C_ChangedSince {
}
$this->assert( 0, "$mess\n" );
}
while ( $html = shift(@FoswikiFnTestCase::mails) ) {
my $re = qr/^From: /m;
while ( my $mail = shift(@FoswikiFnTestCase::mails) ) {
my $html = $mail->as_string();
my $re = qr/^From: /m;
$this->assert_matches( $re, $html );
$re = qr/^Subject: .*Changes to actions /m;
$this->assert_matches( $re, $html );
Expand Down
116 changes: 72 additions & 44 deletions test/unit/ActionTrackerPlugin/ActionTests.pm
Expand Up @@ -8,6 +8,7 @@ use Foswiki::Plugins::ActionTrackerPlugin::Action;
use Foswiki::Plugins::ActionTrackerPlugin::Format;
use Time::ParseDate;
use CGI;
use URI::URL;

sub new {
my $self = shift()->SUPER::new(@_);
Expand Down Expand Up @@ -479,24 +480,43 @@ sub test_HTMLFormattingOpen {

$fmt =
new Foswiki::Plugins::ActionTrackerPlugin::Format( "", "| \$edit |", "" );
my $url =
"$Foswiki::cfg{DefaultUrlHost}$Foswiki::cfg{ScriptUrlPath}/edit$Foswiki::cfg{ScriptSuffix}/Test/Topic\\?skin=$skin;nowysiwyg=1;origin=$this->{test_web}.$this->{test_topic}%23Test:Topic:AcTion0;atp_action=AcTion0;.*";

my $url = new URI::URL(
"$Foswiki::cfg{DefaultUrlHost}$Foswiki::cfg{ScriptUrlPath}/edit$Foswiki::cfg{ScriptSuffix}/Test/Topic"
);

$s = $fmt->formatHTMLTable( [$action] );
$this->assert(
$s =~ m(<td><a name=\"AcTion0\" /> <a href="(.*?)">edit</a> </td>),
$s );
$this->assert( $1, $s );
$this->assert_matches( qr(^$url), $1 );
$s =~
m(<td><a name=(["'])AcTion0\1\s*/>\s*<a[^>]* href=(["'])(.*?)\2[^>]*>edit</a>\s*</td>),
$s
);
my $href = $3;
$this->assert_matches( qr(^$url), $href );
my $goturl = new URI::URL($href);
my %p = map { /(.*?)=(.*)/; $1 => $2; } split( /[;&]/, $goturl->query );
$this->assert_equals( $skin, $p{skin} );
$this->assert_num_equals( 1, $p{nowysiwyg} );
$this->assert_equals(
"$this->{test_web}.$this->{test_topic}#Test:Topic:AcTion0",
$p{origin} );
$this->assert_equals( "AcTion0", $p{atp_action} );

$fmt =
new Foswiki::Plugins::ActionTrackerPlugin::Format( "", "| \$edit |", "" );
$s = $fmt->formatHTMLTable( [$action], 'atp' );
$this->assert_not_null($s);
$this->assert(
$s =~ m(<td><a name=\"AcTion0\" />\s*<a (.*?)>edit</a>\s*</td>), $s );
my $x = $1;
$this->assert_matches( qr/href="$url\d+"/, $x );
$this->assert_matches( qr/class="atp_edit/, $x );
$s =~
m(<td><a name=(["'])AcTion0\1\s*/>\s*<a([^>]*) href=(["'])(.*?)\3[^>]*>edit</a>\s*</td>),
$s
);
my $more = $2;
$href = $4;
$this->assert_matches( qr(^$url), $href );
$goturl = new URI::URL($href);
%p = map { /(.*?)=(.*)/; $1 => $2; } split( /[;&]/, $goturl->query );
$this->assert_matches( qr/\batp_edit\b/, $more );

$fmt = new Foswiki::Plugins::ActionTrackerPlugin::Format( "",
"| \$web.\$topic |", "" );
Expand Down Expand Up @@ -893,7 +913,7 @@ sub test_CreateFromQuery {
$chosen =~ s/ who="$this->{users_web}\.Who"//o;
$chosen =~ s/ created="2003-05-03"//o;
$chosen =~ s/ uid="UID"//o;
$this->assert_matches( qr/^%ACTION{\s*}% Text %ENDACTION%$/, $chosen, );
$this->assert_matches( qr/^%ACTION\{\s*\}% Text %ENDACTION%$/, $chosen, );
}

sub test_FormatForEditHidden {
Expand All @@ -902,46 +922,54 @@ sub test_FormatForEditHidden {
my $action =
new Foswiki::Plugins::ActionTrackerPlugin::Action( "Web", "Topic", 9,
<<EG, "Text" );
state="open" creator="$this->{users_web}.Creator" notify="$this->{users_web}.Notifyee" closer="$this->{users_web}.Closer" due="4-May-2003" closed="2-May-2003" who="$this->{users_web}.Who" created="3-May-2003" uid="UID"
state="open" creator="$this->{users_web}.Creator" notify="$this->{users_web}.Notifyee" closer="$this->{users_web}.Closer" due="4-May-2003" closed="3-May-2003" who="$this->{users_web}.Who" created="2-May-2003" uid="UID"
EG
my $fmt =
new Foswiki::Plugins::ActionTrackerPlugin::Format( "|Who|", "|\$who|",
"cols", "", "" );
my $s = $action->formatForEdit($fmt);

# only the who field should be a text; the rest should be hiddens
$s =~ s(<input (.*?name="state".*?)/>)();
my $glim = $1;
$this->assert_matches( qr/type="hidden"/, $glim );
$this->assert_matches( qr/value="open"/, $glim );
$s =~ s(<input (.*?name="creator".*?)/>)();
$glim = $1;
$this->assert_matches( qr/type="hidden"/, $glim );
$this->assert_matches( qr/value="$this->{users_web}\.Creator"/, $glim );
$s =~ s(<input (.*?name="notify".*?)/>)();
$glim = $1;
$this->assert_matches( qr/type="hidden"/, $glim );
$this->assert_matches( qr/value="$this->{users_web}\.Notifyee"/, $glim );
$s =~ s(<input (.*?name="closer".*?)/>)();
$glim = $1;
$this->assert_matches( qr/type="hidden"/, $glim );
$this->assert_matches( qr/value="$this->{users_web}\.Closer"/, $glim );
$s =~ s(<input (.*?name="due".*?)/>)();
$glim = $1;
$this->assert_matches( qr/type="hidden"/, $glim );
$this->assert_matches( qr/value="0?4 May 2003"/, $glim );
$s =~ s(<input (.*?name="closed".*?)/>)();
$glim = $1;
$this->assert_matches( qr/type="hidden"/, $glim );
$this->assert_matches( qr/value="0?2 May 2003"/, $glim );
$s =~ s(<input (.*?name="created".*?)/>)();
$glim = $1;
$this->assert_matches( qr/type="hidden"/, $glim );
$this->assert_matches( qr/value="0?3 May 2003"/, $glim );
$s =~ s(<input (.*?name="uid".*?)/>)();
$glim = $1;
$this->assert_matches( qr/type="hidden"/, $glim );
$this->assert_matches( qr/value="UID"/, $glim );
while ( $s =~ s!(<input [^>]*/>)!! ) {
my $i = $1;
$i =~ /name=(["'])(\w+)\1/;
my $name = $2;
my ( $type, $value, $more ) = ( 'hidden', '', '' );
if ( $name eq 'state' ) {
$value = 'open';
}
elsif ( $2 eq 'creator' ) {
$value = "$this->{users_web}.Creator";
}
elsif ( $name eq 'notify' ) {
$value = "$this->{users_web}.Notifyee";
}
elsif ( $name eq 'closer' ) {
$value = "$this->{users_web}.Closer";
}
elsif ( $name eq 'due' ) {
$value = "04 May 2003";
}
elsif ( $name eq 'closed' ) {
$value = "03 May 2003";
}
elsif ( $name eq 'created' ) {
$value = "02 May 2003";
}
elsif ( $name eq 'uid' ) {
$value = "UID";
}
elsif ( $name eq 'who' ) {
$value = "$this->{users_web}.Who";
$type = 'text';
$more = "size='35'";
}
else {
$this->assert( 0, $i );
}
$this->assert_html_equals(
"<input name='$name' type='$type' value='$value' $more/>", $i );
}
$this->assert_does_not_match( qr/name="text"/, $s );
$this->assert_does_not_match( qr/type="hidden"/, $s );
}
Expand Down

0 comments on commit 60ab182

Please sign in to comment.