Add sort order options to list pages.

Includes newest, oldest, least/most recently updated, and most comments.

The default remains the same, which is last updated on /reports, and
newest on /my and /around (the latter plus not-in-view
sorted-by-distance ones).

13 files changed, 111 insertions, 48 deletions

diff --git a/perllib/FixMyStreet/App/Controller/Around.pm b/perllib/FixMyStreet/App/Controller/Around.pm
index cd96c3b5d..b4f94bb35 100644
--- a/perllib/FixMyStreet/App/Controller/Around.pm
+++ b/perllib/FixMyStreet/App/Controller/Around.pm
@@ -176,13 +176,16 @@ sub display_location : Private {
 
     # Check the category to filter by, if any, is valid
     $c->forward('check_and_stash_category');
+    $c->forward( '/reports/stash_report_sort', [ 'created-desc' ]);
 
     # get the map features
     my ( $on_map_all, $on_map, $nearby, $distance ) =
       FixMyStreet::Map::map_features( $c,
         latitude => $latitude, longitude => $longitude,
         interval => $interval, categories => $c->stash->{filter_category},
-        states => $c->stash->{filter_problem_states} );
+        states => $c->stash->{filter_problem_states},
+        order => $c->stash->{sort_order},
+      );
 
     # copy the found reports to the stash
     $c->stash->{on_map}     = $on_map;
@@ -293,13 +296,16 @@ sub ajax : Path('/ajax') {
     my $interval = $all_pins ? undef : $c->cobrand->on_map_default_max_pin_age;
 
     $c->forward( '/reports/stash_report_filter_status' );
+    $c->forward( '/reports/stash_report_sort', [ 'created-desc' ]);
 
     # extract the data from the map
     my ( $on_map_all, $on_map_list, $nearby, $dist ) =
       FixMyStreet::Map::map_features($c,
           bbox => $bbox, interval => $interval,
           categories => [ $c->get_param_list('filter_category', 1) ],
-          states => $c->stash->{filter_problem_states} );
+          states => $c->stash->{filter_problem_states},
+          order => $c->stash->{sort_order},
+      );
 
     # create a list of all the pins
     my @pins = map {
diff --git a/perllib/FixMyStreet/App/Controller/My.pm b/perllib/FixMyStreet/App/Controller/My.pm
index b6f425ead..51f1687ee 100644
--- a/perllib/FixMyStreet/App/Controller/My.pm
+++ b/perllib/FixMyStreet/App/Controller/My.pm
@@ -57,6 +57,7 @@ sub get_problems : Private {
     my $p_page = $c->get_param('p') || 1;
 
     $c->forward( '/reports/stash_report_filter_status' );
+    $c->forward('/reports/stash_report_sort', [ 'created-desc' ]);
 
     my $pins = [];
     my $problems = [];
@@ -73,9 +74,9 @@ sub get_problems : Private {
     }
 
     my $rs = $c->stash->{problems_rs}->search( $params, {
-        order_by => { -desc => 'confirmed' },
+        order_by => $c->stash->{sort_order},
         rows => 50
-    } )->page( $p_page );
+    } )->include_comment_counts->page( $p_page );
 
     while ( my $problem = $rs->next ) {
         $c->stash->{has_content}++;
diff --git a/perllib/FixMyStreet/App/Controller/Reports.pm b/perllib/FixMyStreet/App/Controller/Reports.pm
index f05096525..813c2052d 100644
--- a/perllib/FixMyStreet/App/Controller/Reports.pm
+++ b/perllib/FixMyStreet/App/Controller/Reports.pm
@@ -365,6 +365,8 @@ sub check_canonical_url : Private {
 sub load_and_group_problems : Private {
     my ( $self, $c ) = @_;
 
+    $c->forward('stash_report_sort', [ $c->cobrand->reports_ordering ]);
+
     my $page = $c->get_param('p') || 1;
     # NB: If 't' is specified, it will override 'status'.
     my $type = $c->get_param('t') || 'all';
@@ -411,10 +413,10 @@ sub load_and_group_problems : Private {
     $problems = $problems->search(
         $where,
         {
-            order_by => $c->cobrand->reports_ordering,
+            order_by => $c->stash->{sort_order},
             rows => $c->cobrand->reports_per_page,
         }
-    )->page( $page );
+    )->include_comment_counts->page( $page );
     $c->stash->{pager} = $problems->pager;
 
     my ( %problems, @pins );
@@ -502,6 +504,27 @@ sub stash_report_filter_status : Private {
     return 1;
 }
 
+sub stash_report_sort : Private {
+    my ( $self, $c, $default ) = @_;
+
+    my %types = (
+        updated => 'lastupdate',
+        created => 'confirmed',
+        comments => 'comment_count',
+    );
+
+    my $sort = $c->get_param('sort') || $default;
+    $sort = $default unless $sort =~ /^((updated|created)-(desc|asc)|comments-desc)$/;
+    $sort =~ /^(updated|created|comments)-(desc|asc)$/;
+    my $order_by = $types{$1} || $1;
+    my $dir = $2;
+    $order_by = { -desc => $order_by } if $dir eq 'desc';
+
+    $c->stash->{sort_key} = $sort;
+    $c->stash->{sort_order} = $order_by;
+    return 1;
+}
+
 sub add_row {
     my ( $c, $problem, $body, $problems, $pins ) = @_;
     push @{$problems->{$body}}, $problem;
diff --git a/perllib/FixMyStreet/Cobrand/Default.pm b/perllib/FixMyStreet/Cobrand/Default.pm
index b63999b2f..13d5cb8f1 100644
--- a/perllib/FixMyStreet/Cobrand/Default.pm
+++ b/perllib/FixMyStreet/Cobrand/Default.pm
@@ -432,7 +432,7 @@ The order_by clause to use for reports on all reports page
 =cut
 
 sub reports_ordering {
-    return { -desc => 'lastupdate' };
+    return 'updated-desc';
 }
 
 =head2 on_map_list_limit
diff --git a/perllib/FixMyStreet/Cobrand/Oxfordshire.pm b/perllib/FixMyStreet/Cobrand/Oxfordshire.pm
index 989d6f622..a476b5e9b 100644
--- a/perllib/FixMyStreet/Cobrand/Oxfordshire.pm
+++ b/perllib/FixMyStreet/Cobrand/Oxfordshire.pm
@@ -100,7 +100,7 @@ sub problem_response_days {
 }
 
 sub reports_ordering {
-    return { -desc => 'confirmed' };
+    return 'created-desc';
 }
 
 sub pin_colour {
diff --git a/perllib/FixMyStreet/DB/ResultSet/Problem.pm b/perllib/FixMyStreet/DB/ResultSet/Problem.pm
index 723a6e7c2..f1ed50721 100644
--- a/perllib/FixMyStreet/DB/ResultSet/Problem.pm
+++ b/perllib/FixMyStreet/DB/ResultSet/Problem.pm
@@ -140,27 +140,27 @@ sub _recent {
 # Problems around a location
 
 sub around_map {
-    my ( $rs, $min_lat, $max_lat, $min_lon, $max_lon, $interval, $limit, $categories, $states ) = @_;
+    my ( $rs, $limit, %p) = @_;
     my $attr = {
-        order_by => { -desc => 'created' },
+        order_by => $p{order},
     };
     $attr->{rows} = $limit if $limit;
 
-    unless ( $states ) {
-        $states = FixMyStreet::DB::Result::Problem->visible_states();
+    unless ( $p{states} ) {
+        $p{states} = FixMyStreet::DB::Result::Problem->visible_states();
     }
 
     my $q = {
             non_public => 0,
-            state => [ keys %$states ],
-            latitude => { '>=', $min_lat, '<', $max_lat },
-            longitude => { '>=', $min_lon, '<', $max_lon },
+            state => [ keys %{$p{states}} ],
+            latitude => { '>=', $p{min_lat}, '<', $p{max_lat} },
+            longitude => { '>=', $p{min_lon}, '<', $p{max_lon} },
     };
-    $q->{'current_timestamp - lastupdate'} = { '<', \"'$interval'::interval" }
-        if $interval;
-    $q->{category} = $categories if $categories && @$categories;
+    $q->{'current_timestamp - lastupdate'} = { '<', \"'$p{interval}'::interval" }
+        if $p{interval};
+    $q->{category} = $p{categories} if $p{categories} && @{$p{categories}};
 
-    my @problems = mySociety::Locale::in_gb_locale { $rs->search( $q, $attr )->all };
+    my @problems = mySociety::Locale::in_gb_locale { $rs->search( $q, $attr )->include_comment_counts->all };
     return \@problems;
 }
 
@@ -238,4 +238,16 @@ sub send_reports {
     return FixMyStreet::Script::Reports::send($site_override);
 }
 
+sub include_comment_counts {
+    my $rs = shift;
+    my $order_by = $rs->{attrs}{order_by};
+    return $rs unless ref $order_by eq 'HASH' && $order_by->{-desc} eq 'comment_count';
+    $rs->search({}, {
+        '+select' => [ {
+            "" => \'(select count(*) from comment where problem_id=me.id and state=\'confirmed\')',
+            -as => 'comment_count'
+        } ]
+    });
+}
+
 1;
diff --git a/perllib/FixMyStreet/Map.pm b/perllib/FixMyStreet/Map.pm
index f7caf51d7..b6b618efb 100644
--- a/perllib/FixMyStreet/Map.pm
+++ b/perllib/FixMyStreet/Map.pm
@@ -91,10 +91,9 @@ sub map_features {
     # list of problems around map can be limited, but should show all pins
     my $around_limit = $c->cobrand->on_map_list_limit || undef;
 
-    my @around_args = @p{"min_lat", "max_lat", "min_lon", "max_lon", "interval"};
-    my $on_map_all = $c->cobrand->problems_on_map->around_map( @around_args, undef, $p{categories}, $p{states} );
+    my $on_map_all = $c->cobrand->problems_on_map->around_map( undef, %p );
     my $on_map_list = $around_limit
-        ? $c->cobrand->problems_on_map->around_map( @around_args, $around_limit, $p{categories}, $p{states} )
+        ? $c->cobrand->problems_on_map->around_map( $around_limit, %p )
         : $on_map_all;
 
     my $dist = FixMyStreet::Gaze::get_radius_containing_population( $p{latitude}, $p{longitude} );
diff --git a/templates/web/base/reports/_list-filters.html b/templates/web/base/reports/_list-filters.html
index cda5a3dc2..5ca483a1f 100644
--- a/templates/web/base/reports/_list-filters.html
+++ b/templates/web/base/reports/_list-filters.html
@@ -29,6 +29,18 @@
             <input type="submit" name="filter_update" value="[% loc('Go') %]">
         </p>
 
+        <p class="report-list-filters">
+            <label for="sort">[% loc('Sort by') %]</label>
+            <select class="form-control" name="sort" id="sort">
+                <option value="created-desc"[% ' selected' IF sort_key == 'created-desc' %]>[% loc('Newest') %]</option>
+                <option value="created-asc"[% ' selected' IF sort_key == 'created-asc' %]>[% loc('Oldest') %]</option>
+                <option value="updated-desc"[% ' selected' IF sort_key == 'updated-desc' %]>[% loc('Recently updated') %]</option>
+                <option value="updated-asc"[% ' selected' IF sort_key == 'updated-asc' %]>[% loc('Least recently updated') %]</option>
+                <option value="comments-desc"[% ' selected' IF sort_key == 'comments-desc' %]>[% loc('Most commented') %]</option>
+            </select>
+            <input type="submit" name="filter_update" value="[% loc('Go') %]">
+        </p>
+
 [% IF use_form_wrapper %]
     </form>
 [% END %]
diff --git a/web/cobrands/fixmystreet/fixmystreet.js b/web/cobrands/fixmystreet/fixmystreet.js
index 9000d34e7..b8dfa94f9 100644
--- a/web/cobrands/fixmystreet/fixmystreet.js
+++ b/web/cobrands/fixmystreet/fixmystreet.js
@@ -1231,7 +1231,7 @@ $(function() {
                     // location.href is something like foo.com/around?pc=abc-123,
                     // which we pass into fixmystreet.display.reports_list() as a fallback
                     // in case the list isn't already in the DOM.
-                    $('#filter_categories').add('#statuses').find('option')
+                    $('#filter_categories').add('#statuses').add('#sort').find('option')
                         .prop('selected', function() { return this.defaultSelected; })
                         .trigger('change.multiselect');
                     fixmystreet.display.reports_list(location.href);
@@ -1242,6 +1242,7 @@ $(function() {
                 } else if ('filter_change' in e.state) {
                     $('#filter_categories').val(e.state.filter_change.filter_categories);
                     $('#statuses').val(e.state.filter_change.statuses);
+                    $('#sort').val(e.state.filter_change.sort);
                     $('#filter_categories').add('#statuses')
                         .trigger('change.filters').trigger('change.multiselect');
                 } else if ('hashchange' in e.state) {
diff --git a/web/cobrands/sass/_base.scss b/web/cobrands/sass/_base.scss
index f75a53c33..7cb47af6a 100644
--- a/web/cobrands/sass/_base.scss
+++ b/web/cobrands/sass/_base.scss
@@ -10,7 +10,7 @@ $itemlist_item_background: #f6f6f6 !default;
 $itemlist_item_background_hover: #e6e6e6 !default;
 $col_big_numbers: #ccc !default;
 
-$form-control-border-color: #aaaaaa;
+$form-control-border-color: #aaaaaa !default;
 
 @import "_mixins";
 @import "_report_list";
diff --git a/web/cobrands/sass/_multiselect.scss b/web/cobrands/sass/_multiselect.scss
index 5ee15ead8..9dda17fea 100644
--- a/web/cobrands/sass/_multiselect.scss
+++ b/web/cobrands/sass/_multiselect.scss
@@ -47,7 +47,7 @@
     text-overflow: ellipsis;
     vertical-align: -0.5em;
     background-color: #fff;
-    border: 1px solid #aaa;
+    border: 1px solid $form-control-border-color;
     border-radius: 4px;
     box-shadow: 0 1px 3px rgba(0, 0, 0, 0.2);
     cursor: default;
diff --git a/web/cobrands/sass/_report_list.scss b/web/cobrands/sass/_report_list.scss
index 54c2d4bf2..87af99c0f 100644
--- a/web/cobrands/sass/_report_list.scss
+++ b/web/cobrands/sass/_report_list.scss
@@ -6,6 +6,10 @@
   font-size: 0.85em;
   line-height: 1.8em;
 
+  & + & {
+    padding-top: 0.25em;
+  }
+
   label {
     // Override default label styling
     font-weight: normal;
@@ -18,23 +22,24 @@
     vertical-align: top;
   }
 
-  select {
+  .form-control {
     display: inline-block;
-    width: auto;
-    color: inherit;
-    font-family: inherit;
-    font-size: 1em;
-    border: 1px solid #ced7c4;
-    max-width: 13em;
     margin: 0 0.2em;
-  }
 
-  .form-control {
-    margin-bottom: 0;
+    // Visually match the teeny tiny .multi-select-buttons
+    font-size: 0.875em;
+    background-color: #fff;
+    color: #000;
+    max-width: 10em;
+    vertical-align: top;
+    height: 1.8em;
+    line-height: 1.8em;
+    padding: 0;
   }
 
   .multi-select-container {
     margin: 0 0.2em;
+    color: #000;
   }
 
   .multi-select-menu {
diff --git a/web/js/map-OpenLayers.js b/web/js/map-OpenLayers.js
index 02a0d7727..3de8e4d1f 100644
--- a/web/js/map-OpenLayers.js
+++ b/web/js/map-OpenLayers.js
@@ -286,7 +286,11 @@ var fixmystreet = fixmystreet || {};
 
     function replace_query_parameter(qs, id, key) {
         var value = $('#' + id).val();
-        value ? qs[key] = value.join(',') : delete qs[key];
+        if (value) {
+            qs[key] = (typeof value === 'string') ? value : value.join(',');
+        } else {
+            delete qs[key];
+        }
         return value;
     }
 
@@ -297,6 +301,7 @@ var fixmystreet = fixmystreet || {};
         var qs = parse_query_string();
         var filter_categories = replace_query_parameter(qs, 'filter_categories', 'filter_category');
         var filter_statuses = replace_query_parameter(qs, 'statuses', 'status');
+        var sort_key = replace_query_parameter(qs, 'sort', 'sort');
         delete qs['p'];
         var new_url;
         if ($.isEmptyObject(qs)) {
@@ -307,7 +312,7 @@ var fixmystreet = fixmystreet || {};
             new_url = location.href + '?' + $.param(qs);
         }
         history.pushState({
-            filter_change: { 'filter_categories': filter_categories, 'statuses': filter_statuses }
+            filter_change: { 'filter_categories': filter_categories, 'statuses': filter_statuses, 'sort': sort_key }
         }, null, new_url);
     }
 
@@ -522,8 +527,10 @@ var fixmystreet = fixmystreet || {};
             // Set up the event handlers to populate the filters and react to them changing
             $("#filter_categories").on("change.filters", categories_or_status_changed);
             $("#statuses").on("change.filters", categories_or_status_changed);
+            $("#sort").on("change.filters", categories_or_status_changed);
             $("#filter_categories").on("change.user", categories_or_status_changed_history);
             $("#statuses").on("change.user", categories_or_status_changed_history);
+            $("#sort").on("change.user", categories_or_status_changed_history);
         } else if (fixmystreet.page == 'new') {
             drag.activate();
         }
@@ -792,17 +799,14 @@ OpenLayers.Strategy.FixMyStreetFixed = OpenLayers.Class(OpenLayers.Strategy.Fixe
 // 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 filter_category = $("#filter_categories").val();
-        if (filter_category !== undefined) {
-            options.params = options.params || {};
-            options.params.filter_category = filter_category;
-        }
-        var status = $("#statuses").val();
-        if (status !== undefined) {
-            options.params = options.params || {};
-            options.params.status = status;
-        }
+        // Pass the values of the category, status, and sort fields as query params
+        $.each({ filter_category: 'filter_categories', status: 'statuses', sort: 'sort' }, function(key, id) {
+            var val = $('#' + id).val();
+            if (val !== undefined) {
+                options.params = options.params || {};
+                options.params[key] = val;
+            }
+        });
         return OpenLayers.Protocol.HTTP.prototype.read.apply(this, [options]);
     },
     CLASS_NAME: "OpenLayers.Protocol.FixMyStreet"