Author: kake Date: 2012-05-04 11:31:33 +0100 (Fri, 04 May 2012) New Revision: 1378

Modified: trunk/lib/OpenGuides.pm trunk/t/86_recent_changes.t trunk/t/87_more_recent_changes.t trunk/t/88_recent_changes_overtime.t Log: Made recent changes work properly when minor edits not shown (fixes #270).

Modified: trunk/lib/OpenGuides.pm =================================================================== --- trunk/lib/OpenGuides.pm 2012-05-02 18:24:58 UTC (rev 1377) +++ trunk/lib/OpenGuides.pm 2012-05-04 10:31:33 UTC (rev 1378) @@ -694,8 +694,10 @@ my $id = $args{id} || $self->config->home_name; my $return_output = $args{return_output} || 0; my (%tt_vars, %recent_changes); + # NB the $q stuff below should be removed - we should _always_ do this via + # an argument to the method. my $q = CGI->new; - my $since = $q->param("since"); + my $since = $args{since} || $q->param("since"); if ( $since ) { $tt_vars{since} = $since; my $t = localtime($since); # overloaded by Time::Piece @@ -703,60 +705,31 @@ my %criteria = ( since => $since ); $criteria{metadata_was} = { edit_type => "Normal edit" } unless $minor_edits; - my @rc = $self->{wiki}->list_recent_changes( %criteria ); - - my $base_url = $config->script_name . '?'; - - @rc = map { - { - name => CGI->escapeHTML($_->{name}), - last_modified => CGI->escapeHTML($_->{last_modified}), - version => CGI->escapeHTML($_->{version}), - comment => OpenGuides::Utils::parse_change_comment( - CGI->escapeHTML($_->{metadata}{comment}[0]), - $base_url, - ), - username => CGI->escapeHTML($_->{metadata}{username}[0]), - host => CGI->escapeHTML($_->{metadata}{host}[0]), - username_param => CGI->escape($_->{metadata}{username}[0]), - edit_type => CGI->escapeHTML($_->{metadata}{edit_type}[0]), - url => $base_url - . CGI->escape($wiki->formatter->node_name_to_node_param($_->{name})), - } - } @rc; + my @rc = $self->_get_recent_changes( + config => $config, criteria => %criteria ); if ( scalar @rc ) { $recent_changes{since} = @rc; } } else { + # Look at day, week, fortnight, month separately, but make sure things + # don't appear in e.g. "week" if we've already seen them in "day". + my %seen; for my $days ( [0, 1], [1, 7], [7, 14], [14, 30] ) { my %criteria = ( between_days => $days ); $criteria{metadata_was} = { edit_type => "Normal edit" } unless $minor_edits; - my @rc = $self->{wiki}->list_recent_changes( %criteria ); - - my $base_url = $config->script_name . '?'; - - @rc = map { - { - name => CGI->escapeHTML($_->{name}), - last_modified => CGI->escapeHTML($_->{last_modified}), - version => CGI->escapeHTML($_->{version}), - comment => OpenGuides::Utils::parse_change_comment( - CGI->escapeHTML($_->{metadata}{comment}[0]), - $base_url, - ), - username => CGI->escapeHTML($_->{metadata}{username}[0]), - host => CGI->escapeHTML($_->{metadata}{host}[0]), - username_param => CGI->escape($_->{metadata}{username}[0]), - edit_type => CGI->escapeHTML($_->{metadata}{edit_type}[0]), - url => $base_url - . CGI->escape($wiki->formatter->node_name_to_node_param($_->{name})), + my @rc = $self->_get_recent_changes( + config => $config, criteria => %criteria ); + my @filtered; + foreach my $node ( @rc ) { + next if $seen{$node->{name}}; + $seen{$node->{name}}++; + push @filtered, $node; + } + if ( scalar @filtered ) { + $recent_changes{$days->[1]} = @filtered; + } } - } @rc; - if ( scalar @rc ) { - $recent_changes{$days->[1]} = @rc; - } - } } $tt_vars{not_editable} = 1; $tt_vars{recent_changes} = %recent_changes; @@ -777,6 +750,49 @@ print $output; }

+sub _get_recent_changes { + my ( $self, %args ) = @_; + my $wiki = $self->wiki; + my $formatter = $wiki->formatter; + my $config = $self->config; + my %criteria = %{ $args{criteria} }; + + my @rc = $wiki->list_recent_changes( %criteria ); + my $base_url = $config->script_name . '?'; + + # If using metadata_was then we need to pick out just the most recent + # versions. + if ( $criteria{metadata_was} ) { + my %seen; + my @filtered; + foreach my $node ( @rc ) { + next if $seen{$node->{name}}; + $seen{$node->{name}}++; + push @filtered, $node; + } + @rc = @filtered; + } + + @rc = map { + { + name => CGI->escapeHTML($_->{name}), + last_modified => CGI->escapeHTML($_->{last_modified}), + version => CGI->escapeHTML($_->{version}), + comment => OpenGuides::Utils::parse_change_comment( + CGI->escapeHTML($_->{metadata}{comment}[0]), + $base_url, + ), + username => CGI->escapeHTML($_->{metadata}{username}[0]), + host => CGI->escapeHTML($_->{metadata}{host}[0]), + username_param => CGI->escape($_->{metadata}{username}[0]), + edit_type => CGI->escapeHTML($_->{metadata}{edit_type}[0]), + url => $base_url + . CGI->escape($formatter->node_name_to_node_param($_->{name})), + } + } @rc; + return @rc; +} + =item B<display_diffs>

$guide->display_diffs(

Modified: trunk/t/86_recent_changes.t =================================================================== --- trunk/t/86_recent_changes.t 2012-05-02 18:24:58 UTC (rev 1377) +++ trunk/t/86_recent_changes.t 2012-05-04 10:31:33 UTC (rev 1378) @@ -1,170 +1,130 @@ use strict; -use Wiki::Toolkit::Setup::SQLite; -use OpenGuides::Config; use OpenGuides; -use OpenGuides::Feed; -use OpenGuides::Utils; use OpenGuides::Test; use Test::More; -use OpenGuides::CGI; +use Time::Piece; +use Time::Seconds; +use Wiki::Toolkit::Store::Database;

- eval { require DBD::SQLite; }; if ( $@ ) { my ($error) = $@ =~ /^(.*?)\n/; plan skip_all => "DBD::SQLite could not be used - no database to test with. ($error)"; }

-eval { require Wiki::Toolkit::Search::Plucene; }; -if ( $@ ) { - plan skip_all => "Plucene not installed"; -} +plan tests => 16;

- -plan tests => 14; - OpenGuides::Test::refresh_db();

- -my $config = OpenGuides::Config->new( - vars => { - dbtype => "sqlite", - dbname => "t/node.db", - indexing_directory => "t/indexes", - script_name => "wiki.cgi", - script_url => "http://example.com/", - site_name => "Test Site", - template_path => "./templates", - home_name => "Home", - use_plucene => 1 - } -); - -# Basic sanity check first. -my $wiki = OpenGuides::Utils->make_wiki_object( config => $config ); - -my $feed = OpenGuides::Feed->new( wiki => $wiki, - config => $config ); - - -# Write the first version +my $config = OpenGuides::Test->make_basic_config; my $guide = OpenGuides->new( config => $config ); - -# Set up CGI parameters ready for a node write. -my $q = OpenGuides::Test->make_cgi_object( - content => "foo", - username => "bob", - comment => "First edit", - node_image => "image", - edit_type => "Normal edit", -); +my $wiki = $guide->wiki;

-my $output = $guide->commit_node( - return_output => 1, - id => "Wombats", - cgi_obj => $q, - ); +# Set things up so that we have the following nodes and edit types. All +# nodes added 11 days ago. +# - Red Lion, edited 9 days ago (normal), 3 days ago (normal) and today (minor) +# - Blue Lion, edited 7 days ago (minor), 5 days ago (normal) & today (minor) +setup_pages();

-# Check we have it -ok( $wiki->node_exists( "Wombats" ), "Wombats written" ); +# Check all went in OK. +my %red = $wiki->retrieve_node( "Red Lion" ); +my %blue = $wiki->retrieve_node( "Blue Lion" ); +ok( $wiki->node_exists( "Red Lion" ), "Red Lion written." ); +ok( $wiki->node_exists( "Blue Lion" ), "Blue Lion written." ); +is( $red{version}, 4, "Correct Red version." ); +is( $blue{version}, 4, "Correct Blue version." );

-my %node = $wiki->retrieve_node("Wombats"); -is( $node{version}, 1, "First version" ); +# Check recent changes output when minor edits switched on. +my $cookie = OpenGuides::CGI->make_prefs_cookie( config => $config, + show_minor_edits_in_rc => 1 ); +$ENV{HTTP_COOKIE} = $cookie;

+# First check default display. +my %tt_vars = $guide->display_recent_changes( return_tt_vars => 1 ); +my @nodes = extract_nodes( %tt_vars ); +my @names_vers = sort map { "$_->{name} (v$_->{version})" } @nodes; +is_deeply( @names_vers, [ "Blue Lion (v4)", "Red Lion (v4)" ], + "With minor edits: nodes returned only once however many times changed." ); +diag( "Found: " . join( ", ", @names_vers ) );

-# Now write a second version of it -$q->param( -name => "edit_type", -value => "Normal edit" ); -$q->param( -name => "checksum", -value => $node{checksum} ); -$q->param( -name => "comment", -value => "Second edit" ); -$output = $guide->commit_node( - return_output => 1, - id => "Wombats", - cgi_obj => $q, - ); +# Should see the same thing for past 10 days. +my $now = localtime; # overloaded by Time::Piece +my $tendays = $now - ( ONE_DAY * 10 ); +%tt_vars = $guide->display_recent_changes( return_tt_vars => 1, + since => $tendays->epoch ); +@nodes = extract_nodes( %tt_vars ); +@names_vers = sort map { "$_->{name} (v$_->{version})" } @nodes; +is_deeply( @names_vers, [ "Blue Lion (v4)", "Red Lion (v4)" ], + "...same result when looking at past 10 days" ); +diag( "Found: " . join( ", ", @names_vers ) );

-# Check it's as expected -%node = $wiki->retrieve_node("Wombats"); -is( $node{version}, 2, "First version" ); -is( $node{metadata}->{edit_type}[0], "Normal edit", "Right edit type" ); +# Check last day (both nodes edited minorly today, should show up). +my $yesterday = $now - ONE_DAY; +%tt_vars = $guide->display_recent_changes( return_tt_vars => 1, + since => $yesterday->epoch ); +@nodes = extract_nodes( %tt_vars ); +@names_vers = sort map { "$_->{name} (v$_->{version})" } @nodes; +is_deeply( @names_vers, [ "Blue Lion (v4)", "Red Lion (v4)" ], + "...and both nodes included when we look at past day." ); +diag( "Found: " . join( ", ", @names_vers ) );

-# Now try to commit some invalid data, and make sure we get an edit form back -$q = OpenGuides::Test->make_cgi_object( - content => "foo", - os_x => "fooooo", - username => "bob", - comment => "bar", - node_image => "image", - edit_type => "Minor tidying" -); +# Check last 4 days (again, both should show up since this is minor edits, +# but they should only show up once). +my $fourdays = $now - ( ONE_DAY * 4 ); +%tt_vars = $guide->display_recent_changes( return_tt_vars => 1, + since => $fourdays->epoch ); +@nodes = extract_nodes( %tt_vars ); +@names_vers = sort map { "$_->{name} (v$_->{version})" } @nodes; +is_deeply( @names_vers, [ "Blue Lion (v4)", "Red Lion (v4)" ], + "...and past 4 days" ); +diag( "Found: " . join( ", ", @names_vers ) );

-$output = $guide->commit_node( - return_output => 1, - id => "Wombats again", - cgi_obj => $q, - ); - -like( $output, qr/Your input was invalid/, - "Edit form displayed and invalid input message shown if invalid input" ); - -like( $output, qr/os_x must be integer/, - "Edit form displayed and os_x integer message displayed" ); - -my @nodes = $wiki->list_recent_changes( days => 1 ); - is( scalar @nodes, 1, - "By default each node returned only once however many times changed" ); - @nodes = $wiki->list_recent_changes( days => 1, include_all_changes => 1 ); - is( scalar @nodes, 2, - "...returned more than once when 'include_all_changes' set" ); - -# when minor_edits = 1 - -my $cookie = OpenGuides::CGI->make_prefs_cookie( - config => $config, - username => "bob", - include_geocache_link => 1, - preview_above_edit_box => 1, - omit_help_links => 1, - show_minor_edits_in_rc => 1, - default_edit_type => "tidying", - cookie_expires => "never", - track_recent_changes_views => 1, - is_admin => 1, -); -$output = $guide->display_recent_changes( return_output => 1 ); - -unlike ($output, qr/First edit/, "showing a page edit twice when show minor edits enabled. "); - - -# set show_minor_edits to 0. - $cookie = OpenGuides::CGI->make_prefs_cookie( - config => $config, - username => "bob", - include_geocache_link => 1, - preview_above_edit_box => 1, - omit_help_links => 1, - show_minor_edits_in_rc => 0, - default_edit_type => "tidying", - cookie_expires => "never", - track_recent_changes_views => 1, - is_admin => 1, -); +# Now test with minor edits switched off. +$cookie = OpenGuides::CGI->make_prefs_cookie( config => $config, + show_minor_edits_in_rc => 0 ); $ENV{HTTP_COOKIE} = $cookie;

+# First check default display. +%tt_vars = $guide->display_recent_changes( return_tt_vars => 1 ); +@nodes = extract_nodes( %tt_vars ); +@names_vers = sort map { "$_->{name} (v$_->{version})" } @nodes; +is_deeply( @names_vers, [ "Blue Lion (v3)", "Red Lion (v3)" ], + "Without minor edits: node returned only once however many times changed." ); +diag( "Found: " . join( ", ", @names_vers ) );

+# Should see the same thing for past 10 days. +%tt_vars = $guide->display_recent_changes( return_tt_vars => 1, + since => $tendays->epoch ); +@nodes = extract_nodes( %tt_vars ); +@names_vers = sort map { "$_->{name} (v$_->{version})" } @nodes; +is_deeply( @names_vers, [ "Blue Lion (v3)", "Red Lion (v3)" ], + "...same result when looking at past 10 days" ); +diag( "Found: " . join( ", ", @names_vers ) );

-$output = $guide->display_recent_changes( return_output => 1 ); +# Check last day (last normal edit 3 days ago - nothing should show up). +%tt_vars = $guide->display_recent_changes( return_tt_vars => 1, + since => $yesterday->epoch ); +@nodes = extract_nodes( %tt_vars ); +@names_vers = sort map { "$_->{name} (v$_->{version})" } @nodes; +ok( scalar @nodes == 0, + "...and nothing returned when no recent normal edits." ); +diag( "Found: " . join( ", ", @names_vers ) );

-TODO: { - local $TODO = "http://dev.openguides.org/ticket/270"; - unlike ($output, qr/First edit/, "showing a page edit twice when not showing minor edits"); -} +# Check last 4 days (should only see one normal edit, for Red Lion). +%tt_vars = $guide->display_recent_changes( return_tt_vars => 1, + since => $fourdays->epoch ); +@nodes = extract_nodes( %tt_vars ); +@names_vers = sort map { "$_->{name} (v$_->{version})" } @nodes; +is_deeply( @names_vers, [ "Red Lion (v3)" ], + "...and only normally-edited nodes returned for past 4 days" ); +diag( "Found: " . join( ", ", @names_vers ) );

# Now write a node that will Auto Create a locale, and check the # Recent Changes output with minor edits and admin links switched on. # We can't use OG::Test->write_data() for this, because it calls # make_cgi_object(), which overwrites REMOTE_ADDR (and we want to test # output of IP address). -$q = OpenGuides::Test->make_cgi_object(); +my $q = OpenGuides::Test->make_cgi_object(); $q->param( -name => "username", -value => "Anonymous" ); $q->param( -name => "locales", -value => "London" ); my $test_host = "198.51.100.255"; @@ -172,7 +132,7 @@ $guide->commit_node( id => "A Pub", cgi_obj => $q, return_output => 1 ); $ENV{HTTP_COOKIE} = OpenGuides::CGI->make_prefs_cookie( config => $config, show_minor_edits_in_rc => 1, is_admin => 1 ); -$output = $guide->display_recent_changes( return_output => 1 ); +my $output = $guide->display_recent_changes( return_output => 1 ); like( $output, qr|Auto\s+Create|, "Auto Create stuff shown on Recent Changes." ); unlike( $output, qr|host=;action=userstats|, @@ -190,3 +150,76 @@ like( $output, qr|$test_host|, "...also when admin links switched off" );

+sub setup_pages { + # We write directly to the database because that way we can fake the pages + # having been written in the past. Copied from test 062 in Wiki::Toolkit. + my $dbh = $wiki->store->dbh; + my $content_sth = $dbh->prepare( "INSERT INTO content + (node_id,version,text,modified) + VALUES (?,?,?,?)"); + my $node_sth = $dbh->prepare( "INSERT INTO node + (id,name,version,text,modified) + VALUES (?,?,?,?,?)"); + my $md_sth = $dbh->prepare( "INSERT INTO metadata + (node_id,version,metadata_type,metadata_value) + VALUES (?,?,?,?)"); + + # Red Lion first. + $node_sth->execute( 10, "Red Lion", 3, "red 2", + get_timestamp( days => 3 ) ) + or die $dbh->errstr; + $content_sth->execute( 10, 3, "red 3", get_timestamp( days => 3 ) ) + or die $dbh->errstr; + $content_sth->execute( 10, 2, "red 2", get_timestamp( days => 9 ) ) + or die $dbh->errstr; + $content_sth->execute( 10, 1, "red 1", get_timestamp( days => 11 ) ) + or die $dbh->errstr; + $md_sth->execute( 10, 3, "edit_type", "Normal edit" ); + $md_sth->execute( 10, 2, "edit_type", "Normal edit" ); + $md_sth->execute( 10, 1, "edit_type", "Normal edit" ); + $md_sth->execute( 10, 3, "comment", "Third red edit." ); + $md_sth->execute( 10, 2, "comment", "Second red edit." ); + $md_sth->execute( 10, 1, "comment", "First red edit." ); + + # Now write it as per usual. + OpenGuides::Test->write_data( guide => $guide, node => "Red Lion", + content => "red 4", edit_type => "Minor tidying", + comment => "Fourth red edit.", return_output => 1 ); + + # Now Blue Lion. + $node_sth->execute( 20, "Blue Lion", 3, "blue 3", + get_timestamp( days => 5 ) ) + or die $dbh->errstr; + $content_sth->execute( 20, 3, "blue 3", get_timestamp( days => 5 ) ) + or die $dbh->errstr; + $content_sth->execute( 20, 2, "blue 2", get_timestamp( days => 7 ) ) + or die $dbh->errstr; + $content_sth->execute( 20, 1, "blue 1", get_timestamp( days => 11 ) ) + or die $dbh->errstr; + $md_sth->execute( 20, 3, "edit_type", "Normal edit" ); + $md_sth->execute( 20, 2, "edit_type", "Minor tidying" ); + $md_sth->execute( 20, 1, "edit_type", "Normal edit" ); + $md_sth->execute( 20, 3, "comment", "Third blue edit." ); + $md_sth->execute( 20, 2, "comment", "Second blue edit." ); + $md_sth->execute( 20, 1, "comment", "First blue edit." ); + + # Now write it as per usual. + OpenGuides::Test->write_data( guide => $guide, node => "Blue Lion", + content => "blue 4", edit_type => "Minor tidying", + comment => "Fourth blue edit.", return_output => 1); +} + +sub get_timestamp { + my %args = @_; + my $days = $args{days}; + my $now = localtime; # overloaded by Time::Piece + my $time = $now - ( $days * ONE_DAY ); + return Wiki::Toolkit::Store::Database->_get_timestamp( $time ); +} + +sub extract_nodes { + my %tt_vars = @_; + my %rc = %{$tt_vars{recent_changes} }; + return @{ $rc{1} || [] }, @{ $rc{7} || [] }, @{ $rc{14} || [] }, + @{ $rc{since} || [] }; +}

Modified: trunk/t/87_more_recent_changes.t =================================================================== --- trunk/t/87_more_recent_changes.t 2012-05-02 18:24:58 UTC (rev 1377) +++ trunk/t/87_more_recent_changes.t 2012-05-04 10:31:33 UTC (rev 1378) @@ -149,12 +149,8 @@ ); $ENV{HTTP_COOKIE} = $cookie;

- -TODO: { - local $TODO = "http://dev.openguides.org/ticket/270"; $output = $guide->display_recent_changes( return_output => 1 ); like ($output, qr/<td class="recentchanges_node_name">/, "expecting a table defintion for an edit"); like ($output, qr/Second edit/, "expecting at least one edit"); unlike ($output, qr/First edit/, "showing a page edit twice when not showing minor edits"); unlike ($output, qr/Third edit/, "showing a page edit twice when not showing minor edits"); -}

Modified: trunk/t/88_recent_changes_overtime.t =================================================================== --- trunk/t/88_recent_changes_overtime.t 2012-05-02 18:24:58 UTC (rev 1377) +++ trunk/t/88_recent_changes_overtime.t 2012-05-04 10:31:33 UTC (rev 1378) @@ -153,10 +153,7 @@ $output = $guide->display_recent_changes( return_output => 1 ); # check recent changes renders properly like ($output, qr/24 hours/, "pages changed in the last 24 hours"); -TODO: { - local $TODO = "http://dev.openguides.org/ticket/270"; unlike ($output, qr/Echidnas rock/, "not showing multiple edits"); -} like ($output, qr/last week/, "edits in the last week"); like ($output, qr/last fortnight/, "edits in the last fornight"); unlike ($output, qr/last 30 days/, "no edits in the last 30 days");