diff options
22 files changed, 125 insertions, 279 deletions
diff --git a/db/alert_types.sql b/db/alert_types.sql index 4022ddaf7..471fd905f 100644 --- a/db/alert_types.sql +++ b/db/alert_types.sql @@ -93,7 +93,7 @@ values ('postcode_local_problems_state', '', '', 'problem_find_nearby(?, ?, ?) as nearby,problem', 'nearby.problem_id = problem.id and problem.non_public = ''f'' and problem.state in (?)', 'created desc', '{{title}}, {{confirmed}}', '/report/{{id}}', '{{detail}}', 'alert-problem-nearby'); --- New problems sent to a particular council +-- New problems sent to a particular body insert into alert_type (ref, head_sql_query, head_table, head_title, head_link, head_description, @@ -107,8 +107,7 @@ values ('council_problems', '', '', ''fixed'', ''fixed - council'', ''fixed - user'', ''closed'', ''action scheduled'', ''not responsible'', ''duplicate'', ''unable to fix'', ''internal referral'' ) AND - (bodies_str like ''%''||?||''%'' or bodies_str is null) and - areas like ''%,''||?||'',%''', + regexp_split_to_array(bodies_str, '','') && ARRAY[?]', 'created desc', '{{title}}, {{confirmed}}', '/report/{{id}}', '{{detail}}', 'alert-problem-council' ); @@ -128,7 +127,7 @@ values ('ward_problems', '', '', ''fixed'', ''fixed - council'', ''fixed - user'', ''closed'', ''action scheduled'', ''not responsible'', ''duplicate'', ''unable to fix'', ''internal referral'' ) AND - (bodies_str like ''%''||?||''%'' or bodies_str is null) and + (regexp_split_to_array(bodies_str, '','') && ARRAY[?] or bodies_str is null) and areas like ''%,''||?||'',%''', 'created desc', '{{title}}, {{confirmed}}', '/report/{{id}}', '{{detail}}', 'alert-problem-ward' diff --git a/db/alert_types_eha.pl b/db/alert_types_eha.pl deleted file mode 100644 index b090522e6..000000000 --- a/db/alert_types_eha.pl +++ /dev/null @@ -1,28 +0,0 @@ -# This file is only here so that the strings from alert_types_eha.sql -# get into the .po translation file. - -# New problems anywhere on the site - _('New reports on reportemptyhomes.com'), - -# New fixed problems anywhere on the site - _('Properties recently reported as put back to use on reportemptyhomes.com'), - _('The latest properties reported back to use by users'), - -# New problems around a location - _('New local reports on reportemptyhomes.com'), - _('The latest local reports reported by users'), - -# New problems around a postcode - _('New reports on reportemptyhomes.com near {{POSTCODE}}'), - -# New problems sent to a particular council - _('New reports to {{COUNCIL}} on reportemptyhomes.com'), - _('The latest reports for {{COUNCIL}} reported by users'), - -# New problems within a particular ward sent to a particular council - _('New reports for {{COUNCIL}} within {{WARD}} ward on reportemptyhomes.com'), - _('The latest reports for {{COUNCIL}} within {{WARD}} ward reported by users'), - -# New problems within a particular voting area (ward, constituency, whatever) - _('New reports within {{NAME}}\'s boundary on reportemptyhomes.com'), - _('The latest reports within {{NAME}}\'s boundary reported by users'), diff --git a/db/alert_types_eha.sql b/db/alert_types_eha.sql deleted file mode 100644 index e4ec0c986..000000000 --- a/db/alert_types_eha.sql +++ /dev/null @@ -1,95 +0,0 @@ --- New updates on a particular problem report -insert into alert_type -(ref, head_sql_query, head_table, - head_title, head_link, head_description, - item_table, item_where, item_order, - item_title, item_link, item_description, template) -values ('new_updates', 'select * from problem where id=?', 'problem', - 'Updates on {{title}}', '/', 'Updates on {{title}}', - 'comment', 'comment.state=''confirmed''', 'created desc', - 'Update by {{name}}', '/report/{{problem_id}}#comment_{{id}}', '{{text}}', 'alert-update'); - --- New problems anywhere on the site -insert into alert_type -(ref, head_sql_query, head_table, - head_title, head_link, head_description, - item_table, item_where, item_order, - item_title, item_link, item_description, template) -values ('new_problems', '', '', - 'New reports on reportemptyhomes.com', '/', 'The latest empty properties reported by users', - 'problem', 'problem.state in (''confirmed'', ''investigating'', ''planned'', ''in progress'', ''fixed'', ''fixed - council'', ''fixed - user'', ''closed'')', 'created desc', - '{{title}}, {{confirmed}}', '/report/{{id}}', '{{detail}}', 'alert-problem'); - --- New fixed problems anywhere on the site -insert into alert_type -(ref, head_sql_query, head_table, - head_title, head_link, head_description, - item_table, item_where, item_order, - item_title, item_link, item_description, template) -values ('new_fixed_problems', '', '', - 'Properties recently reported as put back to use on reportemptyhomes.com', '/', 'The latest properties reported back to use by users', - 'problem', 'problem.state in (''fixed'', ''sixed - council'', ''fixed - user'')', 'lastupdate desc', - '{{title}}, {{confirmed}}', '/report/{{id}}', '{{detail}}', 'alert-problem'); - --- New problems around a location -insert into alert_type -(ref, head_sql_query, head_table, - head_title, head_link, head_description, - item_table, item_where, item_order, - item_title, item_link, item_description, template) -values ('local_problems', '', '', - 'New local reports on reportemptyhomes.com', '/', 'The latest local reports reported by users', - 'problem_find_nearby(?, ?, ?) as nearby,problem', 'nearby.problem_id = problem.id and problem.state in (''confirmed'', ''investigating'', ''planned'', ''in progress'', ''fixed'', ''fixed - council'', ''fixed - user'', ''closed'')', 'created desc', - '{{title}}, {{confirmed}}', '/report/{{id}}', '{{detail}}', 'alert-problem-nearby'); - --- New problems around a postcode -insert into alert_type -(ref, head_sql_query, head_table, - head_title, head_link, head_description, - item_table, item_where, item_order, - item_title, item_link, item_description, template) -values ('postcode_local_problems', '', '', - 'New reports on reportemptyhomes.com near {{POSTCODE}}', '/', 'The latest local reports reported by users', - 'problem_find_nearby(?, ?, ?) as nearby,problem', 'nearby.problem_id = problem.id and problem.state in (''confirmed'', ''investigating'', ''planned'', ''in progress'', ''fixed'', ''fixed - council'', ''fixed - user'', ''closed'')', 'created desc', - '{{title}}, {{confirmed}}', '/report/{{id}}', '{{detail}}', 'alert-problem-nearby'); - --- New problems sent to a particular council -insert into alert_type -(ref, head_sql_query, head_table, - head_title, head_link, head_description, - item_table, item_where, item_order, - item_title, item_link, item_description, template) -values ('council_problems', '', '', - 'New reports to {{COUNCIL}} on reportemptyhomes.com', '/reports', 'The latest reports for {{COUNCIL}} reported by users', - 'problem', 'problem.state in (''confirmed'', ''investigating'', ''planned'', ''in progress'', ''fixed'', ''fixed - council'', ''fixed - user'', ''closed'') and (bodies_str like ''%''||?||''%'' - or bodies_str is null) and areas like ''%,''||?||'',%''', 'created desc', - '{{title}}, {{confirmed}}', '/report/{{id}}', '{{detail}}', 'alert-problem-council' -); - --- New problems within a particular ward sent to a particular council -insert into alert_type -(ref, head_sql_query, head_table, - head_title, head_link, head_description, - item_table, item_where, item_order, - item_title, item_link, item_description, template) -values ('ward_problems', '', '', - 'New reports for {{COUNCIL}} within {{WARD}} ward on reportemptyhomes.com', '/reports', - 'The latest reports for {{COUNCIL}} within {{WARD}} ward reported by users', - 'problem', 'problem.state in (''confirmed'', ''investigating'', ''planned'', ''in progress'', ''fixed'', ''fixed - council'', ''fixed - user'', ''closed'') and (bodies_str like ''%''||?||''%'' - or bodies_str is null) and areas like ''%,''||?||'',%''', 'created desc', - '{{title}}, {{confirmed}}', '/report/{{id}}', '{{detail}}', 'alert-problem-ward' -); - --- New problems within a particular voting area (ward, constituency, whatever) -insert into alert_type -(ref, head_sql_query, head_table, - head_title, head_link, head_description, - item_table, item_where, item_order, - item_title, item_link, item_description, template) -values ('area_problems', '', '', - 'New reports within {{NAME}}''s boundary on reportemptyhomes.com', '/reports', - 'The latest reports within {{NAME}}''s boundary reported by users', 'problem', - 'problem.state in (''confirmed'', ''investigating'', ''planned'', ''in progress'', ''fixed'', ''fixed - council'', ''fixed - user'', ''closed'') and areas like ''%,''||?||'',%''', 'created desc', - '{{title}}, {{confirmed}}', '/report/{{id}}', '{{detail}}', 'alert-problem-area' -); - diff --git a/db/schema.sql b/db/schema.sql index 5d42d57cf..ea2698b0e 100644 --- a/db/schema.sql +++ b/db/schema.sql @@ -207,6 +207,7 @@ create index problem_state_latitude_longitude_idx on problem(state, latitude, lo create index problem_user_id_idx on problem ( user_id ); create index problem_external_body_idx on problem(lower(external_body)); create index problem_radians_latitude_longitude_idx on problem(radians(latitude), radians(longitude)); +create index problem_bodies_str_array_idx on problem USING gin(regexp_split_to_array(bodies_str, ',')); create table questionnaire ( id serial not null primary key, diff --git a/db/schema_0035-bodies_str-tidying.sql b/db/schema_0035-bodies_str-tidying.sql index c4c7badaf..f5e1dbbdd 100644 --- a/db/schema_0035-bodies_str-tidying.sql +++ b/db/schema_0035-bodies_str-tidying.sql @@ -7,4 +7,25 @@ update problem bodies_str = split_part(bodies_str, '|', 1) where bodies_str like '%|%'; +create index problem_bodies_str_array_idx on problem USING gin(regexp_split_to_array(bodies_str, ',')); + +UPDATE alert_type set item_where = + 'problem.non_public = ''f'' and problem.state in + (''confirmed'', ''investigating'', ''planned'', ''in progress'', + ''fixed'', ''fixed - council'', ''fixed - user'', ''closed'', + ''action scheduled'', ''not responsible'', ''duplicate'', ''unable to fix'', + ''internal referral'' ) AND + regexp_split_to_array(bodies_str, '','') && ARRAY[?]' + WHERE ref = 'council_problems'; + +UPDATE alert_type set item_where = + 'problem.non_public = ''f'' and problem.state in + (''confirmed'', ''investigating'', ''planned'', ''in progress'', + ''fixed'', ''fixed - council'', ''fixed - user'', ''closed'', + ''action scheduled'', ''not responsible'', ''duplicate'', ''unable to fix'', + ''internal referral'' ) AND + (regexp_split_to_array(bodies_str, '','') && ARRAY[?] or bodies_str is null) and + areas like ''%,''||?||'',%''' + WHERE ref = 'ward_problems'; + commit; diff --git a/notes/cobranding.txt b/notes/cobranding.txt index 3e902abc8..a62747397 100644 --- a/notes/cobranding.txt +++ b/notes/cobranding.txt @@ -10,8 +10,7 @@ NB: this is moderately specific to producing cobrands for UK councils 1: copy an exiting perllib/FixMyStreet/Cobrand/ file to perllib/FixMyStreet/Cobrand/ExampleCom.pm * Change package name at top of file * Change following functions accordingly: - site_restriction - problems_clause + body_restriction enter_postcode_text area_check base_url diff --git a/perllib/FixMyStreet/App/Controller/Admin.pm b/perllib/FixMyStreet/App/Controller/Admin.pm index 0134dad5d..f9ea383f8 100644 --- a/perllib/FixMyStreet/App/Controller/Admin.pm +++ b/perllib/FixMyStreet/App/Controller/Admin.pm @@ -70,8 +70,6 @@ sub index : Path : Args(0) { return $c->cobrand->admin(); } - my $site_restriction = $c->cobrand->site_restriction(); - my $problems = $c->cobrand->problems->summary_count; my %prob_counts = @@ -85,7 +83,7 @@ sub index : Path : Args(0) { for ( FixMyStreet::DB::Result::Problem->visible_states() ); $c->stash->{total_problems_users} = $c->cobrand->problems->unique_users; - my $comments = $c->model('DB::Comment')->summary_count( $site_restriction ); + my $comments = $c->model('DB::Comment')->summary_count( $c->cobrand->body_restriction ); my %comment_counts = map { $_->state => $_->get_column('state_count') } $comments->all; @@ -150,7 +148,6 @@ sub config_page : Path( 'config' ) : Args(0) { sub timeline : Path( 'timeline' ) : Args(0) { my ($self, $c) = @_; - my $site_restriction = $c->cobrand->site_restriction(); my %time; $c->model('DB')->schema->storage->sql_maker->quote_char( '"' ); @@ -171,7 +168,7 @@ sub timeline : Path( 'timeline' ) : Args(0) { push @{$time{$_->whenanswered->epoch}}, { type => 'quesAnswered', date => $_->whenanswered, obj => $_ } if $_->whenanswered; } - my $updates = $c->model('DB::Comment')->timeline( $site_restriction ); + my $updates = $c->model('DB::Comment')->timeline( $c->cobrand->body_restriction ); foreach ($updates->all) { push @{$time{$_->created->epoch}}, { type => 'update', date => $_->created, obj => $_} ; @@ -538,8 +535,6 @@ sub reports : Path('reports') { if (my $search = $c->get_param('search')) { $c->stash->{searched} = $search; - my $site_restriction = $c->cobrand->site_restriction; - my $search_n = 0; $search_n = int($search) if $search =~ /^\d+$/; @@ -616,9 +611,10 @@ sub reports : Path('reports') { } if (@$query) { - my $updates = $c->model('DB::Comment')->search( + my $updates = $c->model('DB::Comment') + ->to_body($c->cobrand->body_restriction) + ->search( { - %{ $site_restriction }, -or => $query, }, { @@ -650,8 +646,6 @@ sub reports : Path('reports') { sub report_edit : Path('report_edit') : Args(1) { my ( $self, $c, $id ) = @_; - my $site_restriction = $c->cobrand->site_restriction; - my $problem = $c->cobrand->problems->search( { id => $id } )->first; $c->detach( '/page_error_404_not_found' ) @@ -874,13 +868,9 @@ sub users: Path('users') : Args(0) { sub update_edit : Path('update_edit') : Args(1) { my ( $self, $c, $id ) = @_; - my $site_restriction = $c->cobrand->site_restriction; - my $update = $c->model('DB::Comment')->search( - { - id => $id, - %{$site_restriction}, - } - )->first; + my $update = $c->model('DB::Comment') + ->to_body($c->cobrand->body_restriction) + ->search({ id => $id })->first; $c->detach( '/page_error_404_not_found' ) unless $update; @@ -1121,9 +1111,6 @@ sub stats : Path('stats') : Args(0) { my $bymonth = $c->get_param('bymonth'); $c->stash->{bymonth} = $bymonth; - my ( %body, %dates ); - $body{bodies_str} = { like => $c->get_param('body') } - if $c->get_param('body'); $c->stash->{selected_body} = $c->get_param('body'); @@ -1154,14 +1141,12 @@ sub stats : Path('stats') : Args(0) { ); } - my $p = $c->cobrand->problems->search( + my $p = $c->cobrand->problems->to_body($c->get_param('body'))->search( { -AND => [ $field => { '>=', $start_date}, $field => { '<=', $end_date + $one_day }, ], - %body, - %dates, }, \%select, ); diff --git a/perllib/FixMyStreet/App/Controller/Dashboard.pm b/perllib/FixMyStreet/App/Controller/Dashboard.pm index c3aa35008..faddaa89e 100644 --- a/perllib/FixMyStreet/App/Controller/Dashboard.pm +++ b/perllib/FixMyStreet/App/Controller/Dashboard.pm @@ -89,6 +89,7 @@ sub index : Path : Args(0) { my ( $self, $c ) = @_; my $body = $c->forward('check_page_allowed'); + $c->stash->{body} = $body; # Set up the data for the dropdowns @@ -112,7 +113,6 @@ sub index : Path : Args(0) { $c->stash->{category} = $c->get_param('category'); my %where = ( - bodies_str => $body->id, # XXX Does this break in a two tier council? Restriction needs looking at... 'problem.state' => [ FixMyStreet::DB::Result::Problem->visible_states() ], ); $where{areas} = { 'like', '%,' . $c->stash->{ward} . ',%' } @@ -155,7 +155,7 @@ sub index : Path : Args(0) { %$prob_where, 'me.confirmed' => { '>=', $dtf->format_datetime( $now->clone->subtract( days => 30 ) ) }, }; - my $problems_rs = $c->cobrand->problems->search( $params ); + my $problems_rs = $c->cobrand->problems->to_body($body)->search( $params ); my @problems = $problems_rs->all; my %problems; @@ -270,12 +270,14 @@ sub export_as_csv { sub updates_search : Private { my ( $self, $c, $time ) = @_; + my $body = $c->stash->{body}; + my $params = { %{$c->stash->{where}}, 'me.confirmed' => { '>=', $time }, }; - my $comments = $c->model('DB::Comment')->search( + my $comments = $c->model('DB::Comment')->to_body($body)->search( $params, { group_by => [ 'problem_state' ], @@ -302,7 +304,7 @@ sub updates_search : Private { my $col = shift @$vars; my $substmt = "select min(id) from comment where me.problem_id=comment.problem_id and problem_state in ('" . join("','", @$vars) . "')"; - $comments = $c->model('DB::Comment')->search( + $comments = $c->model('DB::Comment')->to_body($body)->search( { %$params, problem_state => $vars, 'me.id' => \"= ($substmt)", @@ -319,7 +321,7 @@ sub updates_search : Private { $counts{$col} = int( ($comments->get_column('time')||0) / 60 / 60 / 24 + 0.5 ); } - $counts{fixed_user} = $c->model('DB::Comment')->search( + $counts{fixed_user} = $c->model('DB::Comment')->to_body($body)->search( { %$params, mark_fixed => 1, problem_state => undef }, { join => 'problem' } )->count; @@ -327,7 +329,7 @@ sub updates_search : Private { %{$c->stash->{prob_where}}, 'me.confirmed' => { '>=', $time }, }; - $counts{total} = $c->cobrand->problems->search( $params )->count; + $counts{total} = $c->cobrand->problems->to_body($body)->search( $params )->count; $params = { %{$c->stash->{prob_where}}, @@ -335,7 +337,7 @@ sub updates_search : Private { state => 'confirmed', '(select min(id) from comment where me.id=problem_id and problem_state is not null)' => undef, }; - $counts{not_marked} = $c->cobrand->problems->search( $params )->count; + $counts{not_marked} = $c->cobrand->problems->to_body($body)->search( $params )->count; return \%counts; } diff --git a/perllib/FixMyStreet/App/Controller/My.pm b/perllib/FixMyStreet/App/Controller/My.pm index 83d5f7adb..9db9386f5 100644 --- a/perllib/FixMyStreet/App/Controller/My.pm +++ b/perllib/FixMyStreet/App/Controller/My.pm @@ -37,10 +37,6 @@ sub my : Path : Args(0) { my $params = { state => [ keys %$states ], }; - $params = { - %{ $c->cobrand->problems_clause }, - %$params - } if $c->cobrand->problems_clause; my $category = $c->get_param('filter_category'); if ( $category ) { @@ -48,7 +44,9 @@ sub my : Path : Args(0) { $c->stash->{filter_category} = $category; } - my $rs = $c->user->problems->search( $params, { + my $rs = $c->user->problems + ->to_body($c->cobrand->body_restriction) + ->search( $params, { order_by => { -desc => 'confirmed' }, rows => 50 } )->page( $p_page ); diff --git a/perllib/FixMyStreet/App/Controller/Reports.pm b/perllib/FixMyStreet/App/Controller/Reports.pm index 4582843bd..25d5cd82e 100644 --- a/perllib/FixMyStreet/App/Controller/Reports.pm +++ b/perllib/FixMyStreet/App/Controller/Reports.pm @@ -269,9 +269,7 @@ sub rss_ward : Path('/rss/reports') : Args(2) { # Problems sent to a council $c->stash->{type} = 'council_problems'; $c->stash->{title_params} = { COUNCIL => $c->stash->{body}->name }; - # XXX This looks up in both bodies_str and areas, but is only using body ID. - # This will not work properly in any install where body IDs are not === area IDs. - $c->stash->{db_params} = [ $c->stash->{body}->id, $c->stash->{body}->id ]; + $c->stash->{db_params} = [ $c->stash->{body}->id ]; } # Send on to the RSS generation @@ -417,29 +415,16 @@ sub load_and_group_problems : Private { $where->{category} = $category; } + my $problems = $c->cobrand->problems; + if ($c->stash->{ward}) { $where->{areas} = { 'like', '%,' . $c->stash->{ward}->{id} . ',%' }; - $where->{bodies_str} = [ - undef, - $c->stash->{body}->id, - { 'like', $c->stash->{body}->id . ',%' }, - { 'like', '%,' . $c->stash->{body}->id }, - ]; + $problems = $problems->to_body($c->stash->{body}); } elsif ($c->stash->{body}) { - # XXX FixMyStreet used to have the following line so that reports not - # currently sent anywhere could still be listed in the appropriate - # (body/area), as they were the same. Now they're not, not sure if - # there's a way to do this easily. - #$where->{areas} = { 'like', '%,' . $c->stash->{body}->id . ',%' }; - $where->{bodies_str} = [ - # undef, - $c->stash->{body}->id, - { 'like', $c->stash->{body}->id . ',%' }, - { 'like', '%,' . $c->stash->{body}->id }, - ]; + $problems = $problems->to_body($c->stash->{body}); } - my $problems = $c->cobrand->problems->search( + $problems = $problems->search( $where, { order_by => $c->cobrand->reports_ordering, @@ -463,7 +448,6 @@ sub load_and_group_problems : Private { } } else { # Add to bodies it was sent to - # XXX Assumes body ID matches "council ID" my $bodies = $problem->bodies_str_ids; foreach ( @$bodies ) { next if $_ != $c->stash->{body}->id; diff --git a/perllib/FixMyStreet/App/Controller/Rss.pm b/perllib/FixMyStreet/App/Controller/Rss.pm index 7aafc99ff..b58b79937 100755 --- a/perllib/FixMyStreet/App/Controller/Rss.pm +++ b/perllib/FixMyStreet/App/Controller/Rss.pm @@ -264,9 +264,9 @@ sub add_row : Private { (my $link = $alert_type->item_link) =~ s/{{(.*?)}}/$row->{$1}/g; (my $desc = _($alert_type->item_description)) =~ s/{{(.*?)}}/$row->{$1}/g; - my $hashref_restriction = $c->cobrand->site_restriction; + my $hashref_restriction = $c->cobrand->body_restriction; my $base_url = $c->cobrand->base_url; - if ( $hashref_restriction && $hashref_restriction->{bodies_str} && $row->{bodies_str} && $row->{bodies_str} ne $hashref_restriction->{bodies_str} ) { + if ( $hashref_restriction && $row->{bodies_str} && $row->{bodies_str} ne $hashref_restriction ) { $base_url = $c->config->{BASE_URL}; } my $url = $base_url . $link; diff --git a/perllib/FixMyStreet/Cobrand/Default.pm b/perllib/FixMyStreet/Cobrand/Default.pm index c3185ea05..8cd392073 100644 --- a/perllib/FixMyStreet/Cobrand/Default.pm +++ b/perllib/FixMyStreet/Cobrand/Default.pm @@ -45,15 +45,6 @@ sub country { return ''; } -=head1 problems_clause - -Returns a hash for a query to be used by problems (and elsewhere in joined -queries) to restrict results for a cobrand. - -=cut - -sub problems_clause {} - =head1 problems Returns a ResultSet of Problems, restricted to a subset if we're on a cobrand @@ -66,20 +57,20 @@ sub problems { return $self->{c}->model('DB::Problem'); } -=head1 site_restriction +=head1 body_restriction -Return a site key and a hash of extra query parameters if the cobrand uses a -subset of the FixMyStreet data. Parameter is any extra data the cobrand needs. -Returns a site key of 0 and an empty hash if the cobrand uses all the data. +Return an extra query parameter to restrict reports to those sent to a +particular body. =cut -sub site_restriction { return {}; } +sub body_restriction {} + sub site_key { return 0; } =head2 restriction -Return a restriction to pull out data saved while using the cobrand site. +Return a restriction to data saved while using this specific cobrand site. =cut diff --git a/perllib/FixMyStreet/Cobrand/SeeSomething.pm b/perllib/FixMyStreet/Cobrand/SeeSomething.pm index 75f1c32e8..9ae15fd8d 100644 --- a/perllib/FixMyStreet/Cobrand/SeeSomething.pm +++ b/perllib/FixMyStreet/Cobrand/SeeSomething.pm @@ -10,14 +10,9 @@ sub council_name { return 'See Something Say Something'; } sub council_url { return 'seesomething'; } sub area_types { [ 'MTD' ] } -sub site_restriction { +sub body_restriction { my $self = shift; - return { bodies_str => { IN => $self->council_id } }; -} - -sub problems_clause { - my $self = shift; - return { bodies_str => { IN => $self->council_id } }; + return $self->council_id; } sub area_check { diff --git a/perllib/FixMyStreet/Cobrand/UKCouncils.pm b/perllib/FixMyStreet/Cobrand/UKCouncils.pm index e653ae522..2c231bc39 100644 --- a/perllib/FixMyStreet/Cobrand/UKCouncils.pm +++ b/perllib/FixMyStreet/Cobrand/UKCouncils.pm @@ -22,10 +22,11 @@ sub path_to_web_templates { ]; } -sub site_restriction { +sub body_restriction { my $self = shift; - return { bodies_str => sprintf('%d', $self->council_id) }; + return $self->council_id; } + sub site_key { my $self = shift; return $self->council_url; @@ -35,23 +36,9 @@ sub restriction { return { cobrand => shift->moniker }; } -# Different function to site_restriction due to two-tier use -sub problems_clause { - my $self = shift; - - if ($self->is_two_tier) { - return { bodies_str => { - like => ('%' . $self->council_id . '%') - }}; - } - else { - return { bodies_str => sprintf('%d', $self->council_id) }; - } -} - sub problems { my $self = shift; - return $self->{c}->model('DB::Problem')->search( $self->problems_clause ); + return $self->{c}->model('DB::Problem')->to_body($self->council_id); } sub base_url { diff --git a/perllib/FixMyStreet/DB/ResultSet/AlertType.pm b/perllib/FixMyStreet/DB/ResultSet/AlertType.pm index d2264139a..dde30b59c 100644 --- a/perllib/FixMyStreet/DB/ResultSet/AlertType.pm +++ b/perllib/FixMyStreet/DB/ResultSet/AlertType.pm @@ -70,8 +70,6 @@ sub email_alerts ($) { # this is for the new_updates alerts next if $row->{non_public} and $row->{user_id} != $row->{alert_user_id}; - my $hashref_restriction = $cobrand->site_restriction( $row->{cobrand_data} ); - FixMyStreet::App->model('DB::AlertSent')->create( { alert_id => $row->{alert_id}, parameter => $row->{item_id}, @@ -90,8 +88,9 @@ sub email_alerts ($) { $data{state_message} = _("This report is currently marked as open."); } + my $hashref_restriction = $cobrand->body_restriction; my $url = $cobrand->base_url( $row->{alert_cobrand_data} ); - if ( $hashref_restriction && $hashref_restriction->{bodies_str} && $row->{bodies_str} ne $hashref_restriction->{bodies_str} ) { + if ( $hashref_restriction && $row->{bodies_str} ne $hashref_restriction ) { $url = mySociety::Config::get('BASE_URL'); } # this is currently only for new_updates @@ -172,7 +171,7 @@ sub email_alerts ($) { my $longitude = $alert->parameter; my $latitude = $alert->parameter2; - my $hashref_restriction = $cobrand->site_restriction( $alert->cobrand_data ); + my $hashref_restriction = $cobrand->body_restriction; my $d = mySociety::Gaze::get_radius_containing_population($latitude, $longitude, 200000); # Convert integer to GB locale string (with a ".") $d = mySociety::Locale::in_gb_locale { @@ -197,7 +196,7 @@ sub email_alerts ($) { parameter => $row->{id}, } ); my $url = $cobrand->base_url( $alert->cobrand_data ); - if ( $hashref_restriction && $hashref_restriction->{bodies_str} && $row->{bodies_str} ne $hashref_restriction->{bodies_str} ) { + if ( $hashref_restriction && $row->{bodies_str} ne $hashref_restriction ) { $url = mySociety::Config::get('BASE_URL'); } $data{data} .= $url . "/report/" . $row->{id} . " - $row->{title}\n\n"; diff --git a/perllib/FixMyStreet/DB/ResultSet/Comment.pm b/perllib/FixMyStreet/DB/ResultSet/Comment.pm index abdc46868..270501efc 100644 --- a/perllib/FixMyStreet/DB/ResultSet/Comment.pm +++ b/perllib/FixMyStreet/DB/ResultSet/Comment.pm @@ -4,19 +4,24 @@ use base 'DBIx::Class::ResultSet'; use strict; use warnings; +sub to_body { + my ($rs, $body_restriction) = @_; + return FixMyStreet::DB::ResultSet::Problem::to_body($rs, $body_restriction); +} + + sub timeline { - my ( $rs, $restriction ) = @_; + my ( $rs, $body_restriction ) = @_; my $prefetch = FixMyStreet::App->model('DB')->schema->storage->sql_maker->quote_char ? [ qw/user/ ] : []; - return $rs->search( + return $rs->to_body($body_restriction)->search( { state => 'confirmed', created => { '>=', \"current_timestamp-'7 days'::interval" }, - %{ $restriction }, }, { prefetch => $prefetch, @@ -25,10 +30,10 @@ sub timeline { } sub summary_count { - my ( $rs, $restriction ) = @_; + my ( $rs, $body_restriction ) = @_; - return $rs->search( - $restriction, + return $rs->to_body($body_restriction)->search( + undef, { group_by => ['me.state'], select => [ 'me.state', { count => 'me.id' } ], diff --git a/perllib/FixMyStreet/DB/ResultSet/Nearby.pm b/perllib/FixMyStreet/DB/ResultSet/Nearby.pm index 670673ab3..a6b00ef7b 100644 --- a/perllib/FixMyStreet/DB/ResultSet/Nearby.pm +++ b/perllib/FixMyStreet/DB/ResultSet/Nearby.pm @@ -19,12 +19,10 @@ sub nearby { if $interval; $params->{id} = { -not_in => $ids } if $ids; - $params = { - %{ $c->cobrand->problems_clause }, - %$params - } if $c->cobrand->problems_clause; $params->{category} = $category if $category; + $rs = FixMyStreet::DB::ResultSet::Problem::to_body($rs, $c->cobrand->body_restriction); + my $attrs = { prefetch => 'problem', bind => [ $mid_lat, $mid_lon, $dist ], diff --git a/perllib/FixMyStreet/DB/ResultSet/Problem.pm b/perllib/FixMyStreet/DB/ResultSet/Problem.pm index 98bd7c68f..8f0d23080 100644 --- a/perllib/FixMyStreet/DB/ResultSet/Problem.pm +++ b/perllib/FixMyStreet/DB/ResultSet/Problem.pm @@ -20,6 +20,18 @@ sub set_restriction { $site_key = $key; } +sub to_body { + my ($rs, $bodies) = @_; + return $rs unless $bodies; + unless (ref $bodies eq 'ARRAY') { + $bodies = [ map { ref $_ ? $_->id : $_ } $bodies ]; + } + $rs = $rs->search( + \[ "regexp_split_to_array(bodies_str, ',') && ?", [ {} => $bodies ] ] + ); + return $rs; +} + # Front page statistics sub recent_fixed { @@ -320,9 +332,8 @@ sub send_reports { $cobrand->process_additional_metadata_for_email($row, \%h); } - my @bodies = split /,/, $row->bodies_str; my $bodies = FixMyStreet::App->model("DB::Body")->search( - { id => \@bodies }, + { id => $row->bodies_str_ids }, { order_by => 'name' }, ); diff --git a/perllib/Open311/GetServiceRequestUpdates.pm b/perllib/Open311/GetServiceRequestUpdates.pm index f7b758137..1e5f4dc6b 100644 --- a/perllib/Open311/GetServiceRequestUpdates.pm +++ b/perllib/Open311/GetServiceRequestUpdates.pm @@ -48,12 +48,12 @@ sub fetch { $self->suppress_alerts( $body->suppress_alerts ); $self->system_user( $body->comment_user ); - $self->update_comments( $o, { areas => $body->areas }, ); + $self->update_comments( $o, $body ); } } sub update_comments { - my ( $self, $open311, $body_details ) = @_; + my ( $self, $open311, $body ) = @_; my @args = (); @@ -63,7 +63,7 @@ sub update_comments { push @args, $self->start_date; push @args, $self->end_date; # default to asking for last 2 hours worth if not Bromley - } elsif ( ! $body_details->{areas}->{$AREA_ID_BROMLEY} ) { + } elsif ( ! $body->areas->{$AREA_ID_BROMLEY} ) { my $end_dt = DateTime->now(); my $start_dt = $end_dt->clone; $start_dt->add( hours => -2 ); @@ -75,7 +75,7 @@ sub update_comments { my $requests = $open311->get_service_request_updates( @args ); unless ( $open311->success ) { - warn "Failed to fetch ServiceRequest Updates for " . join(",", keys %{$body_details->{areas}}) . ":\n" . $open311->error + warn "Failed to fetch ServiceRequest Updates for " . $body->name . ":\n" . $open311->error if $self->verbose; return 0; } @@ -90,10 +90,8 @@ sub update_comments { my $problem; my $criteria = { external_id => $request_id, - # XXX This assumes that areas will actually only be one area. - bodies_str => { like => '%' . join(",", keys %{$body_details->{areas}}) . '%' }, }; - $problem = FixMyStreet::App->model('DB::Problem')->search( $criteria ); + $problem = FixMyStreet::App->model('DB::Problem')->to_body($body)->search( $criteria ); if (my $p = $problem->first) { my $c = $p->comments->search( { external_id => $request->{update_id} } ); diff --git a/perllib/Open311/GetUpdates.pm b/perllib/Open311/GetUpdates.pm index 5007a1f82..bc55086f0 100644 --- a/perllib/Open311/GetUpdates.pm +++ b/perllib/Open311/GetUpdates.pm @@ -17,9 +17,8 @@ sub get_updates { api_key => $body->api_key ); - my $reports = FixMyStreet::App->model('DB::Problem')->search( + my $reports = FixMyStreet::App->model('DB::Problem')->to_body($body)->search( { - bodies_str => { like => "\%" . $body->id . "\%" }, state => { 'IN', [qw/confirmed fixed/] }, -and => [ external_id => { '!=', undef }, diff --git a/t/cobrand/two_tier.t b/t/cobrand/two_tier.t index ad664aabd..b3d6ca7db 100644 --- a/t/cobrand/two_tier.t +++ b/t/cobrand/two_tier.t @@ -6,11 +6,11 @@ use FixMyStreet; use FixMyStreet::Cobrand; my @cobrands = ( - [ hart => '%2333%' ], - [ oxfordshire => '%2237%' ], - [ eastsussex => '%2224%' ], - [ stevenage => '%2347%' ], - [ warwickshire => '%2243%' ], + [ hart => 2333 ], + [ oxfordshire => 2237 ], + [ eastsussex => 2224 ], + [ stevenage => 2347 ], + [ warwickshire => 2243 ], ); FixMyStreet::override_config { @@ -18,11 +18,10 @@ FixMyStreet::override_config { }, sub { for my $c (@cobrands) { - my ($m, $like) = @$c; + my ($m, $id) = @$c; my $cobrand = FixMyStreet::Cobrand->get_class_for_moniker($m); - my $problems_clause = $cobrand->problems_clause; - is_deeply $problems_clause, - { bodies_str => { like => $like } }, "problems_clause for $m"; + my $body_restriction = $cobrand->body_restriction; + is $body_restriction, $id, "body_restriction for $m"; } }; diff --git a/t/open311/getservicerequestupdates.t b/t/open311/getservicerequestupdates.t index dac10d69b..0ab5b232d 100644 --- a/t/open311/getservicerequestupdates.t +++ b/t/open311/getservicerequestupdates.t @@ -18,6 +18,11 @@ my $user = FixMyStreet::App->model('DB::User')->find_or_create( } ); +my %bodies = ( + 2482 => FixMyStreet::App->model("DB::Body")->new({ id => 2482 }), + 2651 => FixMyStreet::App->model("DB::Body")->new({ id => 2651 }), +); + my $requests_xml = qq{<?xml version="1.0" encoding="utf-8"?> <service_requests_updates> <request_update> @@ -342,9 +347,8 @@ for my $test ( $problem->state( $test->{start_state} ); $problem->update; - my $council_details = { areas => { 2482 => 1 } }; my $update = Open311::GetServiceRequestUpdates->new( system_user => $user ); - $update->update_comments( $o, $council_details ); + $update->update_comments( $o, $bodies{2482} ); is $problem->comments->count, 1, 'comment count'; $problem->discard_changes; @@ -382,9 +386,8 @@ foreach my $test ( $problem->comments->delete; - my $council_details = { areas => { 2482 => 1 } }; my $update = Open311::GetServiceRequestUpdates->new( system_user => $user ); - $update->update_comments( $o, $council_details ); + $update->update_comments( $o, $bodies{2482} ); my $comment = $problem->comments->first; is $comment->created, $dt, 'created date set to date from XML'; @@ -451,9 +454,8 @@ for my $test ( my $o = Open311->new( jurisdiction => 'mysociety', endpoint => 'http://example.com', test_mode => 1, test_get_returns => { 'servicerequestupdates.xml' => $local_requests_xml } ); - my $council_details = { areas => { $test->{area_id} => 1 } }; my $update = Open311::GetServiceRequestUpdates->new( system_user => $user ); - $update->update_comments( $o, $council_details ); + $update->update_comments( $o, $bodies{$test->{area_id}} ); is $problem->comments->count, $test->{p1_comments}, 'comment count for first problem'; is $problem2->comments->count, $test->{p2_comments}, 'comment count for second problem'; @@ -491,8 +493,7 @@ subtest 'using start and end date' => sub { end_date => $end_dt, ); - my $council_details = { areas => { 2482 => 1 } }; - $update->update_comments( $o, $council_details ); + $update->update_comments( $o, $bodies{2482} ); my $start = $start_dt . ''; my $end = $end_dt . ''; @@ -551,18 +552,17 @@ subtest 'check that existing comments are not duplicated' => sub { system_user => $user, ); - my $council_details = { areas => { 2482 => 1 } }; - $update->update_comments( $o, $council_details ); + $update->update_comments( $o, $bodies{2482} ); $problem->discard_changes; is $problem->comments->count, 2, 'two comments after fetching updates'; - $update->update_comments( $o, $council_details ); + $update->update_comments( $o, $bodies{2482} ); $problem->discard_changes; is $problem->comments->count, 2, 're-fetching updates does not add comments'; $problem->comments->delete; - $update->update_comments( $o, $council_details ); + $update->update_comments( $o, $bodies{2482} ); $problem->discard_changes; is $problem->comments->count, 2, 'if comments are deleted then they are added'; }; @@ -612,8 +612,7 @@ foreach my $test ( { system_user => $user, ); - my $council_details = { areas => { 2482 => 1 } }; - $update->update_comments( $o, $council_details ); + $update->update_comments( $o, $bodies{2482} ); $problem->discard_changes; is $problem->comments->count, 2, 'two comments after fetching updates'; @@ -678,8 +677,7 @@ foreach my $test ( { suppress_alerts => $test->{suppress_alerts}, ); - my $council_details = { areas => { 2482 => 1 } }; - $update->update_comments( $o, $council_details ); + $update->update_comments( $o, $bodies{2482} ); $problem->discard_changes; my $alerts_sent = FixMyStreet::App->model('DB::AlertSent')->search( |