diff --git a/docs/changelog/7.x.x.txt b/docs/changelog/7.x.x.txt index 45e3f2fac..0e3000df7 100644 --- a/docs/changelog/7.x.x.txt +++ b/docs/changelog/7.x.x.txt @@ -19,6 +19,7 @@ - fix: Data Form Text Area Box Non-Existent (Wouter van Oijen / ProcoliX) - Added an error message to the FileUrl macro to help users figure out why it doesn't work. + - Fixed bugs in the GroupAdd and GroupDelete macros. - Fixed a cross-Matrix linking problem when you have two or more Matricies on one site with the same category names. - Deleted a template that was accidentally added to the core. diff --git a/lib/WebGUI/Macro/GroupAdd.pm b/lib/WebGUI/Macro/GroupAdd.pm index 1be988f92..9b965fa46 100644 --- a/lib/WebGUI/Macro/GroupAdd.pm +++ b/lib/WebGUI/Macro/GroupAdd.pm @@ -43,20 +43,19 @@ An optional template for formatting the text and link. #------------------------------------------------------------------- sub process { - my @param = @_; - my $session = shift; - return "" if ($param[0] eq ""); - return "" if ($param[1] eq ""); + my ($session, $groupName, $text, $template) = @_; + return "" if ($groupName eq ""); + return "" if ($text eq ""); return "" if ($session->user->userId eq '1'); - my $g = WebGUI::Group->find($param[0]); + my $g = WebGUI::Group->find($session, $groupName); return "" unless defined $g->getId; return "" unless ($g->autoAdd); return "" if ($session->user->isInGroup($g->getId)); my %var = (); $var{'group.url'} = $session->url->page("op=autoAddToGroup;groupId=".$g->getId); - $var{'group.text'} = $param[1]; - if ($param[2]) { - return WebGUI::Asset::Template->newByUrl($session,$param[2])->process(\%var); + $var{'group.text'} = $text; + if ($template) { + return WebGUI::Asset::Template->newByUrl($session,$template)->process(\%var); } else { return WebGUI::Asset::Template->new($session,"PBtmpl0000000000000040")->process(\%var); } diff --git a/lib/WebGUI/Macro/GroupDelete.pm b/lib/WebGUI/Macro/GroupDelete.pm index 5116102c8..1c64e8e06 100644 --- a/lib/WebGUI/Macro/GroupDelete.pm +++ b/lib/WebGUI/Macro/GroupDelete.pm @@ -43,20 +43,19 @@ An optional template for formatting the text and link. #------------------------------------------------------------------- sub process { - my $session = shift; - my @param = @_; - return "" if ($param[0] eq ""); - return "" if ($param[1] eq ""); + my ($session, $groupName, $text, $template) = @_; + return "" if ($groupName eq ""); + return "" if ($text eq ""); return "" if ($session->user->userId eq '1'); - my $g = WebGUI::Group->find($param[0]); - return "" if ($g->getId eq ""); + my $g = WebGUI::Group->find($session, $groupName); + return "" unless defined $g->getId; return "" unless ($g->autoDelete); return "" unless ($session->user->isInGroup($g->getId)); my %var = (); - $var{'group.url'} = $session->url->page("op=autoDeleteFromGroup;groupId=".$g->getId); - $var{'group.text'} = $param[1]; - if ($param[2]) { - return WebGUI::Asset::Template->newByUrl($session,$param[2])->process(\%var); + $var{'group.url'} = $session->url->page("op=autoDeleteFromGroup;groupId=".$g->getId); + $var{'group.text'} = $text; + if ($template) { + return WebGUI::Asset::Template->newByUrl($session,$template)->process(\%var); } else { return WebGUI::Asset::Template->new($session,"PBtmpl0000000000000041")->process(\%var); } diff --git a/t/Macro/GroupAdd.t b/t/Macro/GroupAdd.t index 4009cd225..90f912ceb 100644 --- a/t/Macro/GroupAdd.t +++ b/t/Macro/GroupAdd.t @@ -19,6 +19,7 @@ use WebGUI::Macro_Config; use Data::Dumper; use Test::More; # increment this value for each test you create +use HTML::TokeParser; my $session = WebGUI::Test->session; @@ -27,9 +28,8 @@ unless ($session->config->get('macros')->{'GroupAdd'}) { } my $homeAsset = WebGUI::Asset->getDefault($session); -my ($groups, $users) = setupTest($session); +my ($versionTag, $template, $groups, $users) = setupTest($session, $homeAsset); -##Add more Asset configurations here. my @testSets = ( { comment => 'Empty macro call returns null string', @@ -76,6 +76,57 @@ my @testSets = ( empty => 1, userId => $users->[2]->userId, }, + { + comment => 'Group without autoAdd returns null string', + macroText => q!^GroupAdd("%s","%s");!, + groupName => $groups->[1]->name, + text => 'Join up!', + template => '', + empty => 1, + userId => $users->[2]->userId, + }, + { + comment => 'Existing member of group sees empty string', + macroText => q!^GroupAdd("%s","%s");!, + groupName => $groups->[0]->name, + text => 'Join up!', + template => '', + empty => 1, + userId => $users->[0]->userId, + }, + { + comment => 'Non-member of group sees text and link', + macroText => q!^GroupAdd("%s","%s");!, + groupName => $groups->[0]->name, + groupId => $groups->[0]->getId, + text => 'Join up!', + template => '', + empty => 0, + userId => $users->[2]->userId, + parser => \&simpleHTMLParser, + }, + { + comment => 'Member of different group sees text and link', + macroText => q!^GroupAdd("%s","%s");!, + groupName => $groups->[0]->name, + groupId => $groups->[0]->getId, + text => 'Join up!', + template => '', + empty => 0, + userId => $users->[1]->userId, + parser => \&simpleHTMLParser, + }, + { + comment => 'Custom template check', + macroText => q!^GroupAdd("%s","%s","%s");!, + groupName => $groups->[0]->name, + groupId => $groups->[0]->getId, + text => 'Join up!', + template => $template->get('url'), + empty => 0, + userId => $users->[1]->userId, + parser => \&simpleTextParser, + }, ); my $numTests = 0; @@ -95,11 +146,15 @@ foreach my $testSet (@testSets) { is($output, '', $testSet->{comment}); } else { + my ($url, $text) = $testSet->{parser}->($output); + is($text, $testSet->{text}, 'TEXT: '.$testSet->{comment}); + my $expectedUrl = $session->url->page('op=autoAddToGroup;groupId='.$testSet->{groupId}); + is($url, $expectedUrl, 'URL: '.$testSet->{comment}); } } sub setupTest { - my ($session) = @_; + my ($session, $defaultNode) = @_; my @groups; ##Two groups, one with Group Add and one without $groups[0] = WebGUI::Group->new($session, "new"); @@ -114,14 +169,52 @@ sub setupTest { $users[0]->addToGroups([$groups[0]->getId]); $users[1]->addToGroups([$groups[1]->getId]); - return \@groups, \@users; + my $versionTag = WebGUI::VersionTag->getWorking($session); + $versionTag->set({name=>"GroupAdd test"}); + my $properties = { + title => 'GroupAdd test template', + className => 'WebGUI::Asset::Template', + url => 'groupadd-test', + namespace => 'Macro/GroupAdd', + template => "HREF=\nLABEL=", + # '1234567890123456789012' + id => 'GroupAdd001100Template', + }; + my $asset = $defaultNode->addChild($properties, $properties->{id}); + $versionTag->commit; + + return $versionTag, $asset, \@groups, \@users; } -#END { ##Clean-up after yourself, always -# foreach my $testGroup (@{ $groups }, ) { -# $testGroup->delete if (defined $testGroup and ref $testGroup eq 'WebGUI::Group'); -# } -# foreach my $dude (@{ $users }, ) { -# $dude->delete if (defined $dude and ref $dude eq 'WebGUI::User'); -# } -#} +sub simpleHTMLParser { + my ($text) = @_; + my $p = HTML::TokeParser->new(\$text); + + my $token = $p->get_tag("a"); + my $url = $token->[1]{href} || "-"; + my $label = $p->get_trimmed_text("/a"); + + return ($url, $label); +} + +sub simpleTextParser { + my ($text) = @_; + + my ($url) = $text =~ /^HREF=(.+)$/m; + my ($label) = $text =~ /^LABEL=(.+)$/m; + + return ($url, $label); +} + + +END { ##Clean-up after yourself, always + foreach my $testGroup (@{ $groups }, ) { + $testGroup->delete if (defined $testGroup and ref $testGroup eq 'WebGUI::Group'); + } + foreach my $dude (@{ $users }, ) { + $dude->delete if (defined $dude and ref $dude eq 'WebGUI::User'); + } + if (defined $versionTag and ref $versionTag eq 'WebGUI::VersionTag') { + $versionTag->rollback; + } +} diff --git a/t/Macro/GroupDelete.t b/t/Macro/GroupDelete.t new file mode 100644 index 000000000..cd694d895 --- /dev/null +++ b/t/Macro/GroupDelete.t @@ -0,0 +1,219 @@ +#------------------------------------------------------------------- +# WebGUI is Copyright 2001-2006 Plain Black Corporation. +#------------------------------------------------------------------- +# Please read the legal notices (docs/legal.txt) and the license +# (docs/license.txt) that came with this distribution before using +# this software. +#------------------------------------------------------------------- +# http://www.plainblack.com info@plainblack.com +#------------------------------------------------------------------- + +use FindBin; +use strict; +use lib "$FindBin::Bin/../lib"; + +use WebGUI::Test; +use WebGUI::Macro; +use WebGUI::Session; +use WebGUI::Macro_Config; +use Data::Dumper; + +use Test::More; # increment this value for each test you create +use HTML::TokeParser; + +my $session = WebGUI::Test->session; + +unless ($session->config->get('macros')->{'GroupDelete'}) { + Macro_Config::insert_macro($session, 'GroupDelete', 'GroupDelete'); +} + +my $homeAsset = WebGUI::Asset->getDefault($session); +my ($versionTag, $template, $groups, $users) = setupTest($session, $homeAsset); + +my @testSets = ( + { + comment => 'Empty macro call returns null string', + macroText => q!^GroupDelete();!, + groupName => '', + text => '', + template => '', + empty => 1, + userId => $users->[2]->userId, + }, + { + comment => 'Empty group returns null string', + macroText => q!^GroupDelete("%s","%s");!, + groupName => '', + text => 'Bow out', + template => '', + empty => 1, + userId => $users->[2]->userId, + }, + { + comment => 'Empty text returns null string', + macroText => q!^GroupDelete("%s","%s");!, + groupName => $groups->[0]->name, + text => '', + template => '', + empty => 1, + userId => $users->[2]->userId, + }, + { + comment => 'Visitor sees empty string with valid group and text', + macroText => q!^GroupDelete("%s","%s");!, + groupName => $groups->[0]->name(), + text => 'Bow out', + template => '', + empty => 1, + userId => 1, + }, + { + comment => 'Non-existant group returns null string', + macroText => q!^GroupDelete("%s","%s");!, + groupName => "Dudes of the day", + text => 'Bow out', + template => '', + empty => 1, + userId => $users->[2]->userId, + }, + { + comment => 'Group without autoDelete returns null string', + macroText => q!^GroupDelete("%s","%s");!, + groupName => $groups->[1]->name, + text => 'Bow out', + template => '', + empty => 1, + userId => $users->[2]->userId, + }, + { + comment => 'Non-member of group sees empty string', + macroText => q!^GroupDelete("%s","%s");!, + groupName => $groups->[0]->name, + text => 'Bow out', + template => '', + empty => 1, + userId => $users->[1]->userId, + }, + { + comment => 'Member of different group sees empty string', + macroText => q!^GroupDelete("%s","%s");!, + groupName => $groups->[0]->name, + groupId => $groups->[0]->getId, + text => 'Bow out', + template => '', + empty => 1, + userId => $users->[2]->userId, + }, + { + comment => 'Member of group sees text and link', + macroText => q!^GroupDelete("%s","%s");!, + groupName => $groups->[0]->name, + groupId => $groups->[0]->getId, + text => 'Bow out', + template => '', + empty => 0, + userId => $users->[0]->userId, + parser => \&simpleHTMLParser, + }, + { + comment => 'Custom template check', + macroText => q!^GroupDelete("%s","%s","%s");!, + groupName => $groups->[0]->name, + groupId => $groups->[0]->getId, + text => 'Bow out', + template => $template->get('url'), + empty => 0, + userId => $users->[0]->userId, + parser => \&simpleTextParser, + }, +); + +my $numTests = 0; + +foreach my $testSet (@testSets) { + $numTests += 1 + ($testSet->{empty} == 0); +} + +plan tests => $numTests; + +foreach my $testSet (@testSets) { + $session->user({ userId => $testSet->{userId} }); + my $output = sprintf $testSet->{macroText}, + $testSet->{groupName}, $testSet->{text}, $testSet->{template}; + WebGUI::Macro::process($session, \$output); + if ($testSet->{empty}) { + is($output, '', $testSet->{comment}); + } + else { + my ($url, $text) = $testSet->{parser}->($output); + is($text, $testSet->{text}, 'TEXT: '.$testSet->{comment}); + my $expectedUrl = $session->url->page('op=autoDeleteFromGroup;groupId='.$testSet->{groupId}); + is($url, $expectedUrl, 'URL: '.$testSet->{comment}); + } +} + +sub setupTest { + my ($session, $defaultNode) = @_; + my @groups; + ##Two groups, one with Group Delete and one without + $groups[0] = WebGUI::Group->new($session, "new"); + $groups[0]->name('AutoDelete Group'); + $groups[0]->autoDelete(1); + $groups[1] = WebGUI::Group->new($session, "new"); + $groups[1]->name('Regular Old Group'); + $groups[1]->autoDelete(0); + + ##Three users. One in each group and one with no group membership + my @users = map { WebGUI::User->new($session, "new") } 0..2; + $users[0]->addToGroups([$groups[0]->getId]); + $users[1]->addToGroups([$groups[1]->getId]); + + my $versionTag = WebGUI::VersionTag->getWorking($session); + $versionTag->set({name=>"GroupDelete test"}); + my $properties = { + title => 'GroupDelete test template', + className => 'WebGUI::Asset::Template', + url => 'groupdelete-test', + namespace => 'Macro/GroupDelete', + template => "HREF=\nLABEL=", + # '1234567890123456789012' + id => 'GroupDelete001Template', + }; + my $asset = $defaultNode->addChild($properties, $properties->{id}); + $versionTag->commit; + + return $versionTag, $asset, \@groups, \@users; +} + +sub simpleHTMLParser { + my ($text) = @_; + my $p = HTML::TokeParser->new(\$text); + + my $token = $p->get_tag("a"); + my $url = $token->[1]{href} || "-"; + my $label = $p->get_trimmed_text("/a"); + + return ($url, $label); +} + +sub simpleTextParser { + my ($text) = @_; + + my ($url) = $text =~ /^HREF=(.+)$/m; + my ($label) = $text =~ /^LABEL=(.+)$/m; + + return ($url, $label); +} + + +END { ##Clean-up after yourself, always + foreach my $testGroup (@{ $groups }, ) { + $testGroup->delete if (defined $testGroup and ref $testGroup eq 'WebGUI::Group'); + } + foreach my $dude (@{ $users }, ) { + $dude->delete if (defined $dude and ref $dude eq 'WebGUI::User'); + } + if (defined $versionTag and ref $versionTag eq 'WebGUI::VersionTag') { + $versionTag->rollback; + } +}