Author: kake Date: 2012-07-05 08:37:57 +0100 (Thu, 05 Jul 2012) New Revision: 1421

Added: trunk/t/710_missing_metadata_map.t trunk/templates/search_results_leaflet.tt Modified: trunk/Build.PL trunk/MANIFEST trunk/lib/OpenGuides.pm trunk/templates/map_index_leaflet.tt trunk/templates/missing_metadata.tt trunk/wiki.cgi Log: Split out 'plot nodes on map' code, added map display to missing metadata search (fixes #296).

Modified: trunk/Build.PL =================================================================== --- trunk/Build.PL 2012-07-04 10:01:45 UTC (rev 1420) +++ trunk/Build.PL 2012-07-05 07:37:57 UTC (rev 1421) @@ -426,6 +426,7 @@ "read_only.tt", "recent_changes.tt", "search_results.tt", + "search_results_leaflet.tt", "site_index.tt", "search.tt", "spam_detected.tt",

Modified: trunk/MANIFEST =================================================================== --- trunk/MANIFEST 2012-07-04 10:01:45 UTC (rev 1420) +++ trunk/MANIFEST 2012-07-05 07:37:57 UTC (rev 1421) @@ -82,6 +82,7 @@ templates/read_only.tt templates/recent_changes.tt templates/search_results.tt +templates/search_results_leaflet.tt templates/site_index.tt templates/search.tt templates/spam_detected.tt @@ -158,6 +159,7 @@ t/707_send_email.t t/708_about.t t/709_host_blacklist.t +t/710_missing_metadata_map.t t/801_node_moderate_whitelist.t t/802_stylesheet.t t/803_show_delete_to_admins_only.t

Modified: trunk/lib/OpenGuides.pm =================================================================== --- trunk/lib/OpenGuides.pm 2012-07-04 10:01:45 UTC (rev 1420) +++ trunk/lib/OpenGuides.pm 2012-07-05 07:37:57 UTC (rev 1421) @@ -2240,12 +2240,17 @@ my $wiki = $self->wiki; my $formatter = $self->wiki->formatter; my $script_name = $self->config->script_name; + my $use_leaflet = $self->config->use_leaflet;

- my ($metadata_type, $metadata_value, $exclude_locales, $exclude_categories) - = @args{ qw( metadata_type metadata_value exclude_locales exclude_categories ) }; + my ( $metadata_type, $metadata_value, $exclude_locales, + $exclude_categories, $format) + = @args{ qw( metadata_type metadata_value exclude_locales + exclude_categories format ) }; + $format ||= "";

my @nodes; my $done_search = 0; + my $nodes_on_map;

# Only search if they supplied at least a metadata type if($metadata_type) { @@ -2257,45 +2262,67 @@ );

# Filter out redirects; also filter out locales/categories if required. - foreach my $node ( @all_nodes ) { + foreach my $node ( sort @all_nodes ) { next if ( $exclude_locales && $node =~ /^Locale / ); next if ( $exclude_categories && $node =~ /^Category / ); - my $content = $wiki->retrieve_node( $node ); - next if OpenGuides::Utils->detect_redirect( content => $content ); - push @nodes, $node; + my %data = $wiki->retrieve_node( $node ); + next if OpenGuides::Utils->detect_redirect( + content => $data{content} ); + my $node_param = $formatter->node_name_to_node_param( $node ); + my %this_node = ( + name => $node, + param => $node_param, + address => $data{metadata}{address}[0], + view_url => "$script_name?$node_param", + edit_url => "$script_name?id=$node_param;action=edit", + ); + if ( $format eq "map" && $use_leaflet ) { + my ( $wgs84_long, $wgs84_lat ) + = OpenGuides::Utils->get_wgs84_coords( + latitude => $data{metadata}{latitude}[0], + longitude => $data{metadata}{longitude}[0], + config => $self->config ); + if ( defined $wgs84_lat ) { + $this_node{has_geodata} = 1; + $this_node{wgs84_lat} = $wgs84_lat; + $this_node{wgs84_long} = $wgs84_long; + $nodes_on_map++; + } + } + push @nodes, %this_node; } }

- # Build nice edit etc links for our nodes - my @tt_nodes; - for my $node (sort @nodes) { - my %n; - - # Make the URLs - my $node_param = uri_escape( $formatter->node_name_to_node_param( $node ) ); - - # Save into the hash - $n{'name'} = $node; - $n{'view_url'} = $script_name . "?id=" . $node_param; - $n{'edit_url'} = $script_name . "?id=" . $node_param . ";action=edit"; - push @tt_nodes, %n; - } - # Set up our TT variables, including the search parameters my %tt_vars = ( not_editable => 1, not_deletable => 1, deter_robots => 1, - - nodes => @tt_nodes, + nodes => @nodes, done_search => $done_search, + no_nodes_on_map => !$nodes_on_map, metadata_type => $metadata_type, metadata_value => $metadata_value, exclude_locales => $exclude_locales, exclude_categories => $exclude_categories, - script_name => $script_name ); + + # Figure out the map boundaries and centre, if applicable. + if ( $format eq "map" ) { + if ( $use_leaflet ) { + my %minmaxdata = OpenGuides::Utils->get_wgs84_min_max( + nodes => @nodes ); + if ( scalar %minmaxdata ) { + %tt_vars = ( %tt_vars, %minmaxdata ); + } + $tt_vars{display_google_maps} = 1; # to get the JavaScript in + } + # Set the show_map var even if we don't have Leaflet enabled, so + # people aren't left wondering why there's no map. + $tt_vars{show_map} = 1; + } + return %tt_vars if $return_tt_vars;

# Render to the page @@ -2303,6 +2330,7 @@ id => "", template => "missing_metadata.tt", tt_vars => %tt_vars, + noheaders => $args{noheaders} || 0, ); return $output if $return_output; print $output;

Added: trunk/t/710_missing_metadata_map.t =================================================================== --- trunk/t/710_missing_metadata_map.t (rev 0) +++ trunk/t/710_missing_metadata_map.t 2012-07-05 07:37:57 UTC (rev 1421) @@ -0,0 +1,122 @@ +use strict; +use OpenGuides; +use OpenGuides::Test; +use Test::More; + +eval { require DBD::SQLite; }; +if ( $@ ) { + my ($error) = $@ =~ /^(.*?)\n/; + plan skip_all => "DBD::SQLite not available ($error)"; +} + +eval { require Test::HTML::Content; }; +if ( $@ ) { + plan skip_all => "Test::HTML::Content not available"; +} + +plan tests => 18; + +OpenGuides::Test::refresh_db(); + +my $config = OpenGuides::Test->make_basic_config; +my $guide = OpenGuides->new( config => $config ); +my $wiki = $guide->wiki; + +# Write four nodes, all with geodata and two with a phone number. +# Also write one with no geodata and no phone number. +# All five have summaries. +OpenGuides::Test->write_data( guide => $guide, node => "Page 1", + summary => "A page", latitude => 51, longitude => 1, phone => "123", + return_output => 1 ); +OpenGuides::Test->write_data( guide => $guide, node => "Page 2", + summary => "A page", latitude => 51.1, longitude => 1.1, phone => "456", + return_output => 1 ); +OpenGuides::Test->write_data( guide => $guide, node => "Page 3", + summary => "A page", latitude => 51.2, longitude => 1.2, + return_output => 1 ); +OpenGuides::Test->write_data( guide => $guide, node => "Page 4", + summary => "A page", latitude => 51.3, longitude => 1.3, + return_output => 1 ); +OpenGuides::Test->write_data( guide => $guide, node => "Page 5", + summary => "A page", return_output => 1 ); + +# Note: map output is only enabled if Leaflet is enabled. + +# Make sure the checkbox only appears if Leaflet is enabled. +$config->use_leaflet( 0 ); +my $output = $guide->show_missing_metadata( return_output => 1, + noheaders => 1 ); +Test::HTML::Content::no_tag( $output, "input", + { type => "checkbox", name => "format", value => "map" }, + "Map checkbox doesn't appear if Leaflet isn't enabled" ); + +$config->use_leaflet( 1 ); +$output = $guide->show_missing_metadata( return_output => 1, + noheaders => 1 ); +Test::HTML::Content::tag_ok( $output, "input", + { type => "checkbox", name => "format", value => "map" }, + "...but it does appear if it is" ); + +# If map is requested but Leaflet isn't enabled, there should be an apology. +$config->use_leaflet( 0 ); +$output = $guide->show_missing_metadata( return_output => 1, + noheaders => 1, metadata_type => "phone", format => "map" ); +Test::HTML::Content::tag_ok( $output, "span", + { class => "map_results_no_leaflet" }, + "span.map_results_no_leaflet appears if Leaflet's not enabled" ); + +$config->use_leaflet( 1 ); +$output = $guide->show_missing_metadata( return_output => 1, + noheaders => 1, metadata_type => "phone", format => "map" ); +Test::HTML::Content::no_tag( $output, "span", + { class => "map_results_no_leaflet" }, + "...but doesn't if it is" ); + +### All tests from here on are with Leaflet enabled. +$config->use_leaflet( 1 ); + +# Make sure that there's a message if nodes are returned but none of them +# have geodata. +$output = $guide->show_missing_metadata( return_output => 1, + noheaders => 1, metadata_type => "latitude", format => "map" ); +Test::HTML::Content::tag_ok( $output, "p", { class => "no_nodes_on_map" }, + "p.no_nodes_on_map appears if no returned nodes have geodata" ); + +# Make sure the map does show up properly if we have nodes with geodata. +# Pages 3 & 4 have geodata but no phone; Page 5 has neither phone nor geodata. +$output = $guide->show_missing_metadata( return_output => 1, + noheaders => 1, metadata_type => "phone", format => "map" ); +like( $output, qr/page\s+5\s+(not\s+on\s+map/i, + "when map shown, pages with no geodata are marked 'not on map'" ); +Test::HTML::Content::link_ok( $output, "?Page_5", "...and are linked to" ); +unlike( $output, qr/page\s+4\s+(not\s+on\s+map/i, + "pages with geodata are _not_ marked 'not on map'" ); +like( $output, qr/param:[\s"]*Page_4/, "...and are included in JavaScript" ); + +# Navbar shouldn't appear if map does. +Test::HTML::Content::no_tag( $output, "div", { id => "navbar" }, + "navbar doesn't appear when map shows" ); +Test::HTML::Content::no_tag( $output, "div", { id => "maincontent" }, + "...and no div#maincontent" ); +Test::HTML::Content::tag_ok( $output, "div", { id => "maincontent_no_navbar" }, + "...we get div#maincontent_no_navbar instead" ); + +# But it should be there when we're listing instead of mapping. +$output = $guide->show_missing_metadata( return_output => 1, + noheaders => 1, metadata_type => "phone" ); +Test::HTML::Content::tag_ok( $output, "div", { id => "navbar" }, + "navbar shown when no map" ); +Test::HTML::Content::tag_ok( $output, "div", { id => "maincontent" }, + "...div#maincontent too" ); +Test::HTML::Content::no_tag( $output, "div", { id => "maincontent_no_navbar" }, + "...but no div#maincontent_no_navbar" ); + +# It should also be there if we get no results from our search. +$output = $guide->show_missing_metadata( return_output => 1, + noheaders => 1, metadata_type => "summary", format => "map" ); +Test::HTML::Content::tag_ok( $output, "div", { id => "navbar" }, + "navbar shown when no search results" ); +Test::HTML::Content::tag_ok( $output, "div", { id => "maincontent" }, + "...div#maincontent too" ); +Test::HTML::Content::no_tag( $output, "div", { id => "maincontent_no_navbar" }, + "...but no div#maincontent_no_navbar" );

Modified: trunk/templates/map_index_leaflet.tt =================================================================== --- trunk/templates/map_index_leaflet.tt 2012-07-04 10:01:45 UTC (rev 1420) +++ trunk/templates/map_index_leaflet.tt 2012-07-05 07:37:57 UTC (rev 1421) @@ -1,5 +1,3 @@ -[% USE JSON.Escape %] - [% IF criteria %] [% page_title = "Map of $criteria_title - $site_name" %] [% ELSE %] @@ -27,76 +25,8 @@ </h2>

[% INCLUDE index_form.tt %] + [% INCLUDE search_results_leaflet.tt %]

- <div id="map_index_node_list"> - <ul> - [% IF no_nodes_on_map %] - [% FOREACH node = nodes %] - <li><a href="?[% node.param %]">[% node.name %]</a></li> - [% END %] - [% ELSE %] - [% i = 0 %] - [% FOREACH node = nodes %] - [% i = i + 1 %] - <li> - [% IF node.has_geodata %] - <a href="#" onclick="return show_marker( [% i %] )">[% node.name | html %]</a> - [% ELSE %] - [% node.name | html %] (not on map &#8212; - <a href="?[% node.param %]">view info</a>) - [% END %] - </li> - [% END %] - [% END %] - </ul> - </div> - - <div id="map_index_canvas"> - [% IF no_nodes_on_map %] - <p class="no_nodes_on_map">No pages with geodata - nothing to display - on map!</p> - [% ELSE %] - <span id="map_index_no_js"> - <p>Sorry! JavaScript is required to view the map - you may prefer to - <a href="[% feed_base %]">view the results as a list</a> instead.</p> - </span> - - <script type="text/javascript"> - - $('#map_index_no_js').hide(); - - map_div_id = 'map_index_canvas'; - centre_lat = [% centre_lat %]; - centre_long = [% centre_long %]; - min_lat = [% min_lat %]; - max_lat = [% max_lat %]; - min_long = [% min_long %]; - max_long = [% max_long %]; - - function add_markers() { - var node; - [% i = 0 %] - [% FOREACH node = nodes %] - [% i = i + 1 %] - [% IF node.has_geodata %] - node = { - name: [% node.name.json %], - param: [% node.param.json %], - [% address = node.node_data.metadata.address.list.first %] - [% IF address %] - address: [% address.json %], - [% END %] - lat: [% node.wgs84_lat.json %], - long: [% node.wgs84_long.json %], - }; - add_marker( [% i %], node ); - [% END %] - [% END %] - } - </script> - [% END %] - </div> - </div>

[% INCLUDE footer.tt %]

Modified: trunk/templates/missing_metadata.tt =================================================================== --- trunk/templates/missing_metadata.tt 2012-07-04 10:01:45 UTC (rev 1420) +++ trunk/templates/missing_metadata.tt 2012-07-05 07:37:57 UTC (rev 1421) @@ -1,8 +1,14 @@ [% INCLUDE header.tt page_title = "Missing Metadata - $site_name" %] [% INCLUDE banner.tt %] <div id="content"> -[% INCLUDE navbar.tt %] -<div id="maincontent"> + +[% IF no_nodes_on_map OR !show_map %] + [% INCLUDE navbar.tt %] + <div id="maincontent"> +[% ELSE %] + <div id="maincontent_no_navbar"> +[% END %] + <h2>Pages Missing Metadata</h2>

[% options = [ "category", "locale", "address", "postcode", "phone", "fax", @@ -43,6 +49,12 @@ <label for="exclude_categories">Exclude Categories:</label> <input type="checkbox" name="exclude_categories" id="exclude_categories" [% IF exclude_categories %]checked="true"[% END %] /> </div> + [% IF config.use_leaflet %] + <div> + <label for="show_map">Show results on a map:</label> + <input type="checkbox" name="format" value="map" id="show_map"[% IF show_map %] checked="true"[% END %] /> + </div> + [% END %] <div> <input type="submit" value="Find Nodes" /> </div> @@ -50,17 +62,26 @@

[% IF done_search %] <h3>Pages</h3> - <a name="nodes"></a> - <table id="nodes"> - <tr><th>Node name</th><th>View</th><th>Edit</th></tr> - [% FOREACH node = nodes %] - <tr> - <td><a href="[% node.view_url %]">[% node.name %]</a></td> - <td><a href="[% node.view_url %]">view</a></td> - <td><a href="[% node.edit_url %]">edit</a></td> - </tr> + [% IF show_map %] + [% IF config.use_leaflet %] + [% INCLUDE search_results_leaflet.tt %] + [% ELSE %] + <span class="map_results_no_leaflet">Sorry! Map output is only available + for sites using the Leaflet mapping library.</span> [% END %] - </table> + [% ELSE %] + <a name="nodes"></a> + <table id="nodes"> + <tr><th>Node name</th><th>View</th><th>Edit</th></tr> + [% FOREACH node = nodes %] + <tr> + <td><a href="[% node.view_url %]">[% node.name %]</a></td> + <td><a href="[% node.view_url %]">view</a></td> + <td><a href="[% node.edit_url %]">edit</a></td> + </tr> + [% END %] + </table> + [% END %] [% END %]

</div>

Added: trunk/templates/search_results_leaflet.tt =================================================================== --- trunk/templates/search_results_leaflet.tt (rev 0) +++ trunk/templates/search_results_leaflet.tt 2012-07-05 07:37:57 UTC (rev 1421) @@ -0,0 +1,70 @@ +[% USE JSON.Escape %] + +<div id="map_index_node_list"> + <ul> + [% IF no_nodes_on_map %] + [% FOREACH node = nodes %] + <li><a href="?[% node.param %]">[% node.name %]</a></li> + [% END %] + [% ELSE %] + [% i = 0 %] + [% FOREACH node = nodes %] + [% i = i + 1 %] + <li> + [% IF node.has_geodata %] + <a href="#" onclick="return show_marker( [% i %] )">[% node.name | html %]</a> + [% ELSE %] + [% node.name | html %] (not on map &#8212; + <a href="?[% node.param %]">view info</a>) + [% END %] + </li> + [% END %] + [% END %] + </ul> +</div> + +<div id="map_index_canvas"> + [% IF no_nodes_on_map %] + <p class="no_nodes_on_map">No pages with geodata - nothing to display + on map!</p> + [% ELSE %] + <span id="map_index_no_js"> + <p>Sorry! JavaScript is required to view the map - you may prefer to + <a href="[% feed_base %]">view the results as a list</a> instead.</p> + </span> + + <script type="text/javascript"> + + $('#map_index_no_js').hide(); + + map_div_id = 'map_index_canvas'; + centre_lat = [% centre_lat %]; + centre_long = [% centre_long %]; + min_lat = [% min_lat %]; + max_lat = [% max_lat %]; + min_long = [% min_long %]; + max_long = [% max_long %]; + + function add_markers() { + var node; + [% i = 0 %] + [% FOREACH node = nodes %] + [% i = i + 1 %] + [% IF node.has_geodata %] + node = { + name: [% node.name.json %], + param: [% node.param.json %], + [% address = node.node_data.metadata.address.list.first %] + [% IF address %] + address: [% address.json %], + [% END %] + lat: [% node.wgs84_lat.json %], + long: [% node.wgs84_long.json %], + }; + add_marker( [% i %], node ); + [% END %] + [% END %] + } + </script> + [% END %] +</div>

Modified: trunk/wiki.cgi =================================================================== --- trunk/wiki.cgi 2012-07-04 10:01:45 UTC (rev 1420) +++ trunk/wiki.cgi 2012-07-05 07:37:57 UTC (rev 1421) @@ -121,7 +121,8 @@ metadata_type => $q->param("metadata_type") || "", metadata_value => $q->param("metadata_value") || "", exclude_locales => $q->param("exclude_locales") || "", - exclude_categories => $q->param("exclude_categories") || "" + exclude_categories => $q->param("exclude_categories") || "", + format => $q->param( "format" ) || "", ); } elsif ( $action eq 'set_moderation' ) { $guide->set_node_moderation(