From 69ed1cda6a315a46e3309dcf3035ad7229931829 Mon Sep 17 00:00:00 2001 From: Steven Day Date: Mon, 29 Jun 2015 12:03:17 +0100 Subject: Rename map filtering GET param, remove unnecessary query --- perllib/FixMyStreet/App/Controller/Around.pm | 17 +++++------------ perllib/FixMyStreet/App/Controller/My.pm | 2 +- perllib/FixMyStreet/App/Controller/Reports.pm | 4 ++-- perllib/FixMyStreet/Map.pm | 2 +- t/app/controller/around.t | 4 ++-- web/js/map-OpenLayers.js | 21 +++++++-------------- 6 files changed, 18 insertions(+), 32 deletions(-) diff --git a/perllib/FixMyStreet/App/Controller/Around.pm b/perllib/FixMyStreet/App/Controller/Around.pm index 7d382c228..c4040af95 100644 --- a/perllib/FixMyStreet/App/Controller/Around.pm +++ b/perllib/FixMyStreet/App/Controller/Around.pm @@ -240,7 +240,7 @@ sub check_location_is_acceptable : Private { =head2 check_and_stash_category -Check that the 'category' query param is valid, if it's present. Stores +Check that the 'filter_category' query param is valid, if it's present. Stores the validated string in the stash as filter_category. Puts all the valid categories in filter_categories on the stash. @@ -270,17 +270,10 @@ sub check_and_stash_category : Private { $c->stash->{filter_categories} = \@categories; - my $category = $c->req->param('category'); - if ( $category ) { - my $count = $c->model('DB::Contact')->not_deleted->search( - { - body_id => [ keys %bodies ], - category => $category - } - )->count; - if ( $count ) { - $c->stash->{filter_category} = $category; - } + my $category = $c->req->param('filter_category'); + my %categories_mapped = map { $_ => 1 } @categories; + if ( defined $category && $categories_mapped{$category} ) { + $c->stash->{filter_category} = $category; } } diff --git a/perllib/FixMyStreet/App/Controller/My.pm b/perllib/FixMyStreet/App/Controller/My.pm index 0cea3735a..0bcee7ac6 100644 --- a/perllib/FixMyStreet/App/Controller/My.pm +++ b/perllib/FixMyStreet/App/Controller/My.pm @@ -54,7 +54,7 @@ sub my : Path : Args(0) { %$params } if $c->cobrand->problems_clause; - my $category = $c->req->param('category'); + my $category = $c->req->param('filter_category'); if ( $category ) { $params->{category} = $category; $c->stash->{filter_category} = $category; diff --git a/perllib/FixMyStreet/App/Controller/Reports.pm b/perllib/FixMyStreet/App/Controller/Reports.pm index 82418fc48..e5f9d9507 100644 --- a/perllib/FixMyStreet/App/Controller/Reports.pm +++ b/perllib/FixMyStreet/App/Controller/Reports.pm @@ -137,7 +137,7 @@ sub ward : Path : Args(2) { } )->all; @categories = map { $_->category } @categories; $c->stash->{filter_categories} = \@categories; - $c->stash->{filter_category} = $c->req->param('category'); + $c->stash->{filter_category} = $c->req->param('filter_category'); my $pins = $c->stash->{pins}; @@ -394,7 +394,7 @@ sub load_and_group_problems : Private { my $page = $c->req->params->{p} || 1; my $type = $c->req->params->{t} || 'all'; - my $category = $c->req->params->{c} || $c->req->params->{category} || ''; + my $category = $c->req->params->{c} || $c->req->params->{filter_category} || ''; # Unlike the 't' query param, 'status' isn't affected by # the age of a report, so treat the filtering separately. diff --git a/perllib/FixMyStreet/Map.pm b/perllib/FixMyStreet/Map.pm index 7a9c8fa18..704b19bee 100644 --- a/perllib/FixMyStreet/Map.pm +++ b/perllib/FixMyStreet/Map.pm @@ -117,7 +117,7 @@ sub map_pins { my $bbox = $c->req->param('bbox'); my ( $min_lon, $min_lat, $max_lon, $max_lat ) = split /,/, $bbox; - my $category = $c->req->param('category'); + my $category = $c->req->param('filter_category'); # Filter reports by status, if present in query params my $states; diff --git a/t/app/controller/around.t b/t/app/controller/around.t index 4040d1024..03bcebf96 100644 --- a/t/app/controller/around.t +++ b/t/app/controller/around.t @@ -153,7 +153,7 @@ subtest 'check category and status filtering works on /ajax' => sub { my $pins = $json->{pins}; is scalar @$pins, 6, 'correct number of reports when no filters'; - $json = $mech->get_ok_json( '/ajax?category=Pothole&bbox=' . $bbox ); + $json = $mech->get_ok_json( '/ajax?filter_category=Pothole&bbox=' . $bbox ); $pins = $json->{pins}; is scalar @$pins, 2, 'correct number of Pothole reports'; @@ -161,7 +161,7 @@ subtest 'check category and status filtering works on /ajax' => sub { $pins = $json->{pins}; is scalar @$pins, 3, 'correct number of open reports'; - $json = $mech->get_ok_json( '/ajax?status=fixed&category=Vegetation&bbox=' . $bbox ); + $json = $mech->get_ok_json( '/ajax?status=fixed&filter_category=Vegetation&bbox=' . $bbox ); $pins = $json->{pins}; is scalar @$pins, 1, 'correct number of fixed Vegetation reports'; }; diff --git a/web/js/map-OpenLayers.js b/web/js/map-OpenLayers.js index c60ded3f9..a0ab4f34c 100644 --- a/web/js/map-OpenLayers.js +++ b/web/js/map-OpenLayers.js @@ -32,7 +32,7 @@ function fixmystreet_update_pin(lonlat) { // If the category filter appears on the map and the user has selected // something from it, then pre-fill the category field in the report, // if it's a value already present in the drop-down. - var category = $("#categories").val(); + var category = $("#filter_categories").val(); if (category !== undefined && $("#form_category option[value="+category+"]").length) { $("#form_category").val(category); } @@ -255,8 +255,8 @@ function fixmystreet_onload() { // If the category filter dropdown exists on the page set up the // event handlers to populate it and react to it changing - if ($("select#categories").length) { - $("body").on("change", "#categories", fms_categories_or_status_changed); + if ($("select#filter_categories").length) { + $("body").on("change", "#filter_categories", fms_categories_or_status_changed); } // Do the same for the status dropdown if ($("select#statuses").length) { @@ -412,7 +412,6 @@ $(function(){ fixmystreet.drag.deactivate(); $('#side-form').hide(); $('#side').show(); - $("select#categories").attr("name", "category"); $('#sub_map_links').show(); //only on mobile $('#mob_sub_map_links').remove(); @@ -527,15 +526,15 @@ OpenLayers.Control.PermalinkFMSz = OpenLayers.Class(OpenLayers.Control.Permalink // This class is used to get a JSON object from /ajax that contains // pins for the map and HTML for the sidebar. It does a fetch whenever the map // is dragged (modulo a buffer extending outside the viewport). -// This subclass is required so we can pass the 'category' and 'status' query +// This subclass is required so we can pass the 'filter_category' and 'status' query // params to /ajax if the user has filtered the map. OpenLayers.Protocol.FixMyStreet = OpenLayers.Class(OpenLayers.Protocol.HTTP, { read: function(options) { // Pass the values of the category and status fields as query params - var category = $("#categories").val(); - if (category !== undefined) { + var filter_category = $("#filter_categories").val(); + if (filter_category !== undefined) { options.params = options.params || {}; - options.params.category = category; + options.params.filter_category = filter_category; } var status = $("#statuses").val(); if (status !== undefined) { @@ -631,12 +630,6 @@ OpenLayers.Control.Click = OpenLayers.Class(OpenLayers.Control, { document.getElementById('side-form').style.display = 'block'; } $('#side').hide(); - // Although it's now hidden, the category filter for the map is still - // posted to the server when the report is sent. The name clashes with - // the category select element in the report form, which can cause - // issues with the wrong/no category being used for the report. - // Work around this by renaming the field when it's not shown. - $("select#categories").attr("name", "category_filter"); if (typeof heightFix !== 'undefined') { heightFix('#report-a-problem-sidebar', '.content', 26); } -- cgit v1.2.3