Skip to content

Commit

Permalink
Remove need for category_extras ajax call.
Browse files Browse the repository at this point in the history 
Add by_category output to the /report/new/ajax call, containing all the data
that /report/new/category_extras returns for one category. Then alter the JS
to use that data immediately when needed.
  • Loading branch information
@dracos
dracos committed Aug 28, 2018
1 parent 60629c9 commit 22c12ab
Show file tree
Hide file tree
Showing 7 changed files with 154 additions and 111 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Expand Up @@ -11,6 +11,7 @@
- “Report another problem here” button on report confirmation page #2198
- Button in nav bar now makes it easier to report again in the same location #2195
- Shrink OpenLayers library a bit. #2217
- Remove need for separate per-category ajax call. #1201
- Admin improvements:
- Mandatory defect type selection if defect raised.
- Send login email button on user edit page #2041
Expand Down
103 changes: 55 additions & 48 deletions perllib/FixMyStreet/App/Controller/Report/New.pm
View file
Expand Up @@ -188,10 +188,8 @@ sub report_form_ajax : Path('ajax') : Args(0) {

# work out the location for this report and do some checks
if ( ! $c->forward('determine_location') ) {
my $body = encode_json({ error => $c->stash->{location_error} });
$c->res->content_type('application/json; charset=utf-8');
$c->res->body($body);
return;
$c->stash->{json_response} = { error => $c->stash->{location_error} };
$c->detach('send_json_response');
}

$c->forward('setup_categories_and_bodies');
Expand Down Expand Up @@ -220,52 +218,52 @@ sub report_form_ajax : Path('ajax') : Args(0) {
$contribute_as->{body} = $ca_body if $ca_body;
}

my $body = encode_json(
{
bodies => \@list_of_names,
councils_text => $councils_text,
councils_text_private => $councils_text_private,
category => $category,
extra_name_info => $extra_name_info,
titles_list => $extra_titles_list,
%$contribute_as ? (contribute_as => $contribute_as) : (),
$top_message ? (top_message => $top_message) : (),
unresponsive => $c->stash->{unresponsive}->{ALL} || '',
}
);
my %by_category;
foreach my $contact (@{$c->stash->{category_options}}) {
next if ref $contact eq 'HASH'; # Ignore the 'Pick a category' line
my $cat = $c->stash->{category} = $contact->category;
my $body = $c->forward('by_category_ajax_data', [ 'all', $cat ]);
$by_category{$cat} = $body;
}

$c->res->content_type('application/json; charset=utf-8');
$c->res->body($body);
$c->stash->{json_response} = {
bodies => \@list_of_names,
councils_text => $councils_text,
councils_text_private => $councils_text_private,
category => $category,
extra_name_info => $extra_name_info,
titles_list => $extra_titles_list,
%$contribute_as ? (contribute_as => $contribute_as) : (),
$top_message ? (top_message => $top_message) : (),
unresponsive => $c->stash->{unresponsive}->{ALL} || '',
by_category => \%by_category,
};
$c->detach('send_json_response');
}

sub category_extras_ajax : Path('category_extras') : Args(0) {
my ( $self, $c ) = @_;

$c->forward('initialize_report');
if ( ! $c->forward('determine_location') ) {
my $body = encode_json({ error => _("Sorry, we could not find that location.") });
$c->res->content_type('application/json; charset=utf-8');
$c->res->body($body);
return 1;
$c->stash->{json_response} = { error => _("Sorry, we could not find that location.") };
$c->detach('send_json_response');
}
$c->forward('setup_categories_and_bodies');
$c->forward('setup_report_extra_fields');
$c->forward('check_for_category');

$c->forward('check_for_category');
my $category = $c->stash->{category} || "";
$category = '' if $category eq _('-- Pick a category --');

my $bodies = $c->forward('contacts_to_bodies', [ $category ]);
$c->stash->{json_response} = $c->forward('by_category_ajax_data', [ 'one', $category ]);
$c->forward('send_json_response');
}

my $list_of_names = [ map { $_->name } ($category ? @$bodies : values %{$c->stash->{bodies_to_list}}) ];
my $vars = {
$category ? (list_of_names => $list_of_names) : (),
};
sub by_category_ajax_data : Private {
my ($self, $c, $type, $category) = @_;

my $category_extra = '';
my $category_extra_json = [];
my $generate;
my $unresponsive = '';
if ( $c->stash->{category_extras}->{$category} && @{ $c->stash->{category_extras}->{$category} } >= 1 ) {
$c->stash->{category_extras} = { $category => $c->stash->{category_extras}->{$category} };
$generate = 1;
Expand All @@ -276,27 +274,36 @@ sub category_extras_ajax : Path('category_extras') : Args(0) {
if ($c->stash->{report_extra_fields}) {
$generate = 1;
}

my $bodies = $c->forward('contacts_to_bodies', [ $category ]);
my $list_of_names = [ map { $_->name } ($category ? @$bodies : values %{$c->stash->{bodies_to_list}}) ];
my $vars = {
$category ? (list_of_names => $list_of_names) : (),
};

my $body = {
bodies => $list_of_names,
};

if ($generate) {
$category_extra = $c->render_fragment('report/new/category_extras.html', $vars);
$category_extra_json = $c->forward('generate_category_extra_json');
}
$body->{category_extra} = $c->render_fragment('report/new/category_extras.html', $vars);
$body->{category_extra_json} = $c->forward('generate_category_extra_json');

my $councils_text = $c->render_fragment( 'report/new/councils_text.html', $vars);
my $councils_text_private = $c->render_fragment( 'report/new/councils_text_private.html');
}

$unresponsive = $c->stash->{unresponsive}->{$category} || $c->stash->{unresponsive}->{ALL} || '';
my $unresponsive = $c->stash->{unresponsive}->{$category};
$unresponsive ||= $c->stash->{unresponsive}->{ALL} || '' if $type eq 'one';

my $body = encode_json({
category_extra => $category_extra,
councils_text => $councils_text,
councils_text_private => $councils_text_private,
category_extra_json => $category_extra_json,
unresponsive => $unresponsive,
bodies => $list_of_names,
});
# unresponsive must return empty string if okay, as that's what mobile app checks
if ($type eq 'one' || ($type eq 'all' && $unresponsive)) {
$body->{unresponsive} = $unresponsive;
if ($type eq 'all' && $unresponsive) {
$body->{councils_text} = $c->render_fragment( 'report/new/councils_text.html', $vars);
$body->{councils_text_private} = $c->render_fragment( 'report/new/councils_text_private.html');
}
}

$c->res->content_type('application/json; charset=utf-8');
$c->res->body($body);
return $body;
}

=head2 report_import
Expand Down
7 changes: 3 additions & 4 deletions t/app/controller/report_new.t
View file
Expand Up @@ -1669,10 +1669,9 @@ subtest "unresponsive body handling works" => sub {
# Test body-level send method
my $old_send = $contact1->body->send_method;
$contact1->body->update( { send_method => 'Refused' } );
$mech->get_ok('/report/new/ajax?latitude=55.952055&longitude=-3.189579'); # Edinburgh
my $body_id = $contact1->body->id;
ok $mech->content_like( qr{Edinburgh.*accept reports.*/unresponsive\?body=$body_id} );
my $extra_details = $mech->get_ok_json('/report/new/ajax?latitude=55.952055&longitude=-3.189579');
like $extra_details->{top_message}, qr{Edinburgh.*accept reports.*/unresponsive\?body=$body_id};
is $extra_details->{unresponsive}, $body_id, "unresponsive json set";
$extra_details = $mech->get_ok_json('/report/new/category_extras?category=Street%20lighting&latitude=55.952055&longitude=-3.189579');
is $extra_details->{unresponsive}, $body_id, "unresponsive json set";
Expand Down Expand Up @@ -1749,8 +1748,8 @@ subtest "unresponsive body handling works" => sub {
# And test per-category refusing
my $old_email = $contact3->email;
$contact3->update( { email => 'REFUSED' } );
$mech->get_ok('/report/new/category_extras?category=Trees&latitude=51.896268&longitude=-2.093063');
ok $mech->content_like( qr/Cheltenham.*Trees.*unresponsive.*category=Trees/ );
$extra_details = $mech->get_ok_json('/report/new/ajax?latitude=51.896268&longitude=-2.093063');
like $extra_details->{by_category}{$contact3->category}{category_extra}, qr/Cheltenham.*Trees.*unresponsive.*category=Trees/s;
$extra_details = $mech->get_ok_json('/report/new/category_extras?category=Trees&latitude=51.896268&longitude=-2.093063');
is $extra_details->{unresponsive}, $contact3->body->id, "unresponsive json set";

Expand Down
41 changes: 25 additions & 16 deletions t/app/controller/report_new_open311.t
View file
Expand Up @@ -249,13 +249,18 @@ subtest "Category extras omits description label when all fields are hidden" =>
ALLOWED_COBRANDS => [ { fixmystreet => '.' } ],
MAPIT_URL => 'http://mapit.uk/',
}, sub {
my $json = $mech->get_ok_json('/report/new/category_extras?category=Pothole&latitude=55.952055&longitude=-3.189579');
my $category_extra = $json->{category_extra};
contains_string($category_extra, "usrn");
contains_string($category_extra, "central_asset_id");
lacks_string($category_extra, "USRN", "Lacks 'USRN' label");
lacks_string($category_extra, "Asset ID", "Lacks 'Asset ID' label");
lacks_string($category_extra, "resolve your problem quicker, by providing some extra detail", "Lacks description text");
for (
{ url => '/report/new/ajax?' },
{ url => '/report/new/category_extras?category=Pothole' },
) {
my $json = $mech->get_ok_json($_->{url} . '&latitude=55.952055&longitude=-3.189579');
my $category_extra = $json->{by_category} ? $json->{by_category}{Pothole}{category_extra} : $json->{category_extra};
contains_string($category_extra, "usrn");
contains_string($category_extra, "central_asset_id");
lacks_string($category_extra, "USRN", "Lacks 'USRN' label");
lacks_string($category_extra, "Asset ID", "Lacks 'Asset ID' label");
lacks_string($category_extra, "resolve your problem quicker, by providing some extra detail", "Lacks description text");
}
};
};

Expand All @@ -266,15 +271,19 @@ subtest "Category extras includes description label for user" => sub {
}, sub {
$contact4->push_extra_fields({ description => 'Size?', code => 'size', required => 'true', automated => '', variable => 'true', order => '3' });
$contact4->update;

my $json = $mech->get_ok_json('/report/new/category_extras?category=Pothole&latitude=55.952055&longitude=-3.189579');
my $category_extra = $json->{category_extra};
contains_string($category_extra, "usrn");
contains_string($category_extra, "central_asset_id");
lacks_string($category_extra, "USRN", "Lacks 'USRN' label");
lacks_string($category_extra, "Asset ID", "Lacks 'Asset ID' label");
contains_string($category_extra, "Size?");
contains_string($category_extra, "resolve your problem quicker, by providing some extra detail", "Contains description text");
for (
{ url => '/report/new/ajax?' },
{ url => '/report/new/category_extras?category=Pothole' },
) {
my $json = $mech->get_ok_json($_->{url} . '&latitude=55.952055&longitude=-3.189579');
my $category_extra = $json->{by_category} ? $json->{by_category}{Pothole}{category_extra} : $json->{category_extra};
contains_string($category_extra, "usrn");
contains_string($category_extra, "central_asset_id");
lacks_string($category_extra, "USRN", "Lacks 'USRN' label");
lacks_string($category_extra, "Asset ID", "Lacks 'Asset ID' label");
contains_string($category_extra, "Size?");
contains_string($category_extra, "resolve your problem quicker, by providing some extra detail", "Contains description text");
}
};
};

Expand Down
2 changes: 2 additions & 0 deletions templates/web/base/js/translation_strings.html
View file
Expand Up @@ -38,6 +38,8 @@
how_to_send: '[% loc('How to send successful reports') | replace("'", "\\'") %]',
more_details: '[% loc('Details') | replace("'", "\\'") %]',

or: '[% loc(' or ') | replace("'", "\\'") %]',

geolocation_declined: '[% loc('You declined; please fill in the box above') | replace("'", "\\'") %]',
geolocation_no_position: '[% loc('Could not look up location') | replace("'", "\\'") %]',
geolocation_no_result: '[% loc('No result returned') | replace("'", "\\'") %]',
Expand Down
41 changes: 24 additions & 17 deletions web/cobrands/fixmystreet/assets.js
View file
Expand Up @@ -19,9 +19,7 @@ var fixmystreet = fixmystreet || {};
OpenLayers.Layer.VectorAsset = OpenLayers.Class(OpenLayers.Layer.Vector, {
initialize: function(name, options) {
OpenLayers.Layer.Vector.prototype.initialize.apply(this, arguments);
// Do in both locations so fixmystreet.bodies is up to date. Otherwise
// e.g. a layer can disappear the category change after it should.
$(fixmystreet).on('report_new:category_change:extras_received', this.update_layer_visibility.bind(this));
// Update layer based upon new data from category change
$(fixmystreet).on('report_new:category_change', this.update_layer_visibility.bind(this));
},

Expand Down Expand Up @@ -74,9 +72,7 @@ OpenLayers.Layer.VectorNearest = OpenLayers.Class(OpenLayers.Layer.VectorAsset,
OpenLayers.Layer.VectorAsset.prototype.initialize.apply(this, arguments);
$(fixmystreet).on('maps:update_pin', this.checkFeature.bind(this));
$(fixmystreet).on('assets:selected', this.checkFeature.bind(this));
// Might only be able to fill in fields once they've been returned from the server
$(fixmystreet).on('report_new:category_change:extras_received', this.changeCategory.bind(this));
// But also want to do it immediately in case it's hiding the form or something
// Update fields/etc from data now available from category change
$(fixmystreet).on('report_new:category_change', this.changeCategory.bind(this));
},

Expand Down Expand Up @@ -332,6 +328,13 @@ function check_zoom_message_visibility() {
}

function layer_visibilitychanged() {
if (this.fixmystreet.road) {
if (!this.getVisibility()) {
this.road_not_found();
}
return;
}

check_zoom_message_visibility.call(this);
var layers = fixmystreet.map.getLayersBy('assets', true);
var visible = 0;
Expand Down Expand Up @@ -624,7 +627,8 @@ fixmystreet.assets = {
}
});
}
if (!options.always_visible) {

if (!options.always_visible || options.road) {
asset_layer.events.register( 'visibilitychanged', asset_layer, layer_visibilitychanged);
}

Expand Down Expand Up @@ -804,22 +808,25 @@ return {
})();

$(fixmystreet).on('body_overrides:change', function() {
var councils_text = $('#js-councils_text').html();
var single_body_only = $('#single_body_only').val(),
do_not_send = $('#do_not_send').val(),
bodies = fixmystreet.bodies;

var single_body_only = $('#single_body_only').val();
if (single_body_only) {
councils_text = councils_text.replace(/<strong>.*<\/strong>/, '<strong>' + single_body_only + '</strong>');
bodies = [ single_body_only ];
}

var do_not_send = $('#do_not_send').val();
if (do_not_send) {
do_not_send = fixmystreet.utils.csv_to_array(do_not_send);
for (var i=0; i<do_not_send.length; i++) {
// XXX Translations
councils_text = councils_text.replace(new RegExp('or <strong>' + do_not_send[i] + '</strong>'), '');
councils_text = councils_text.replace(new RegExp('<strong>' + do_not_send[i] + '</strong> or '), '');
}
var lookup = {};
$.map(do_not_send, function(val) {
lookup[val] = 1;
});
bodies = OpenLayers.Array.filter(bodies, function(b) {
return !lookup[b];
});
}

$('#js-councils_text').html(councils_text);
fixmystreet.update_public_councils_text(
$('#js-councils_text').html(), bodies);
});

0 comments on commit 22c12ab

Please sign in to comment.