Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Hoisting group fetching, take 2 #5189

Merged
merged 3 commits into from
Sep 27, 2024
Merged

Hoisting group fetching, take 2 #5189

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
323f936
Remove some old unneeded mixins.
dracos Sep 24, 2024
4625835
Slightly refactor category creation in fixture script.
dracos Sep 25, 2024
2218b8f
Capture group from hoisted categories.
dracos Sep 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .cypress/cypress/integration/category_tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ describe('Basic categories', function() {
'Footpath/bridleway away from road',
'Graffiti',
'Offensive graffiti',
'Licensing',
'G|Licensing',
'Parks/landscapes',
'Pavements',
'Potholes',
Expand Down Expand Up @@ -75,7 +75,7 @@ describe('Basic categories', function() {
cy.get('[value="Abandoned vehicles"]').should('not.be.visible');
cy.get('[value="Bus stops"]').should('not.be.visible');
cy.get('[value="Flyposting"]').should('not.be.visible');
cy.get('[value="Licensing"]').should('be.visible');
cy.get('[value="G|Licensing"]').should('be.visible');
cy.get('[value="Dropped Kerbs"]').should('be.visible');
cy.get('[value="Skips"]').should('be.visible');
cy.get('[value="Street lighting"]').should('be.visible');
Expand All @@ -84,7 +84,7 @@ describe('Basic categories', function() {
cy.get('[value="Abandoned vehicles"]').should('not.be.visible');
cy.get('[value="Bus stops"]').should('be.visible');
cy.get('[value="Flyposting"]').should('not.be.visible');
cy.get('[value="Licensing"]').should('be.visible');
cy.get('[value="G|Licensing"]').should('be.visible');
cy.get('[value="Dropped Kerbs"]').should('not.be.visible');
cy.get('[value="Skips"]').should('be.visible');
cy.get('[value="Road traffic signs"]').should('be.visible');
Expand Down
28 changes: 28 additions & 0 deletions .cypress/cypress/integration/gloucestershire.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
describe('Gloucestershire cobrand', function(){
describe('making a report as a new user', function() {
before(function(){
cy.server();
cy.route('/report/new/ajax*').as('report-ajax');

cy.visit('http://gloucestershire.localhost:3001/report/new?longitude=-2.093063&latitude=51.896268');
cy.contains('Gloucestershire County Council');

cy.wait('@report-ajax');
});

it('does not display extra message when selecting a "road" category', function(){
cy.pickCategory('A pothole in road');
});

it('clicks through to photo section', function(){
cy.nextPageReporting();
cy.contains('Drag photos here').should('be.visible');
});

it('clicks through to public details page', function(){
cy.nextPageReporting();
cy.contains('Public details').should('be.visible');
cy.contains('Cheltenham Borough Council').should('not.be.visible');
});
});
});
1 change: 1 addition & 0 deletions bin/browser-tests
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ BEGIN {
bromley
buckinghamshire
fixmystreet
gloucestershire
hackney
highwaysengland
hounslow
Expand Down
135 changes: 46 additions & 89 deletions bin/fixmystreet.com/fixture
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,17 +133,52 @@ if ($opt->test_fixtures) {

foreach (
{ area_id => 2504, categories => ['Damaged, dirty, or missing bin', 'Signs and bollards', 'Busking and Street performance'], name => 'Westminster City Council', cobrand => 'westminster' },
{ area_id => 2482, categories => ['Lamp Column Damaged', 'Sign Light Not Working'], name => 'Bromley Council', cobrand => 'bromley' },
{ area_id => 2482, categories => [
{ group => 'Street Lighting and Road Signs', category => 'Sign Light Not Working' },
{ group => 'Street Lighting and Road Signs', category => 'Lamp Column Damaged' },
], name => 'Bromley Council', cobrand => 'bromley' },
{ area_id => 164186, categories => ['Shelter Damaged', 'Very Urgent'], name => 'Northamptonshire Highways', cobrand => 'northamptonshire' },
{ area_id => 163793, categories => ['Flytipping', 'Roads', 'Parks', 'Snow and ice problem/winter salting', 'Grass cutting', 'Hedge problem'], name => 'Buckinghamshire Council', cobrand => 'buckinghamshire' },
{ area_id => 53822, categories => [ 'Grass cutting', 'Hedge problem' ], name => 'Adstock Parish Council' }, # Buckinghamshire parish council
{ area_id => 163793, categories => [
'Flytipping', 'Roads',
{ group => 'Grass, hedges and weeds', category => 'Grass cutting' },
{ group => 'Grass, hedges and weeds', category => 'Hedge problem' },
{ group => 'Roads & Pavements', category => 'Parks' },
{ group => 'Roads & Pavements', category => 'Snow and ice problem/winter salting' },
], name => 'Buckinghamshire Council', cobrand => 'buckinghamshire' },
{ area_id => 53822, categories => [
{ group => 'Grass, hedges and weeds', category => 'Grass cutting' },
{ group => 'Grass, hedges and weeds', category => 'Hedge problem' },
], name => 'Adstock Parish Council' }, # Buckinghamshire parish council
{ area_id => 164186, categories => [ 'Graffiti' ], name => 'West Northamptonshire Council' },
{ area_id => 2483, categories => [ 'Potholes', 'Other' ], name => 'Hounslow Borough Council', cobrand => 'hounslow' },
{ area_id => 2483, categories => [
'Potholes', { category => 'Other', non_public => 1 },
], name => 'Hounslow Borough Council', cobrand => 'hounslow' },
{ area_id => 2508, categories => [ 'Potholes', 'Other' ], name => 'Hackney Council', cobrand => 'hackney' },
{ area_id => 2636, categories => [ 'Potholes', 'Private', 'Extra' ], name => 'Isle of Wight Council', cobrand => 'isleofwight' },
{ area_id => 2636, categories => [
{ category => 'Potholes', send_method => 'Triage' },
{ category => 'Private', send_method => 'Triage', non_public => 1 },
{ category => 'Extra', send_method => 'Triage' },
], name => 'Isle of Wight Council', cobrand => 'isleofwight' },
{ area_id => 2232, categories => ['Damaged/missing cats eye'], name => 'Lincolnshire County Council', cobrand => 'lincolnshire' },
{ area_id => 2566, categories => [ 'General fly tipping', 'Fallen branch', 'Light Out', 'Light Dim', 'Fallen Tree', 'Damaged Tree', 'Pothole' ], name => 'Peterborough City Council', cobrand => 'peterborough' },
{ area_id => 2498, categories => [ 'Incorrect timetable', 'Glass broken', 'Mobile Crane Operation', 'Roadworks', 'Other (TfL)' ], name => 'TfL', cobrand => 'tfl' },
{ area_id => 2226, categories => [
{ group => 'A pothole in pavement', category => 'A pothole in pavement' },
{ group => 'A pothole in road', category => 'A pothole in road' },
],
name => 'Gloucestershire County Council', cobrand => 'gloucestershire' },
{ area_id => 2326, categories => ['Flyposting', 'Graffiti'],
name => 'Cheltenham Borough Council' },
{ area_id => 2566, categories => [
{ group => 'Trees', category => 'Fallen Tree' },
{ group => 'Trees', category => 'Damaged Tree' },
{ group => 'Street lighting', category => 'Light Out' },
{ group => 'Street lighting', category => 'Light Dim' },
'General fly tipping', 'Fallen branch', 'Pothole',
], name => 'Peterborough City Council', cobrand => 'peterborough' },
{ area_id => 2498, categories => [
{ group => 'Bus Stops and Shelters', category => 'Incorrect timetable' },
{ group => 'Bus Stops and Shelters', category => 'Glass broken' },
'Mobile Crane Operation', 'Roadworks', 'Other (TfL)'
], name => 'TfL', cobrand => 'tfl' },
{ area_id => 2237, categories => [ 'Flytipping', 'Roads', 'Parks', 'Lamp Out of Light' ], name => 'Oxfordshire County Council', cobrand => 'oxfordshire' },
{ area_id => 2551, categories => [ 'Abandoned vehicles', 'Dog fouling', 'Blocked drain' ], name => 'Bath and North East Somerset Council', cobrand => 'bathnes' },
{ area_id => 2238, categories => [ 'Flytipping', 'Roads', 'Parks' ], name => 'Shropshire Council', cobrand => 'shropshire' },
Expand All @@ -164,24 +199,6 @@ if ($opt->test_fixtures) {
}
}

for my $cat_name ('Fallen Tree', 'Damaged Tree') {
my $cat = FixMyStreet::DB->resultset('Contact')->find({
body => $bodies->{2566},
category => $cat_name,
});
$cat->set_extra_metadata( group => 'Trees' );
$cat->update;
}

for my $cat_name ('Light Out', 'Light Dim') {
my $cat = FixMyStreet::DB->resultset('Contact')->find({
body => $bodies->{2566},
category => $cat_name,
});
$cat->set_extra_metadata( group => 'Street lighting' );
$cat->update;
}

my $drain = FixMyStreet::DB->resultset('Contact')->find({
body => $bodies->{2551},
category => 'Blocked drain',
Expand Down Expand Up @@ -229,13 +246,6 @@ if ($opt->test_fixtures) {
});
$child_cat->update;

$child_cat = FixMyStreet::DB->resultset("Contact")->find({
body => $bodies->{163793},
category => 'Grass cutting',
});
$child_cat->set_extra_metadata(group => 'Grass, hedges and weeds');
$child_cat->update;

for my $cat ('Parks', 'Snow and ice problem/winter salting') {
$child_cat = FixMyStreet::DB->resultset("Contact")->find({
body => $bodies->{163793},
Expand All @@ -250,17 +260,9 @@ if ($opt->test_fixtures) {
variable => 'true',
automated => 'hidden_field',
});
$child_cat->set_extra_metadata(group => 'Roads & Pavements');
$child_cat->update;
}

$child_cat = FixMyStreet::DB->resultset("Contact")->find({
body => $bodies->{163793},
category => 'Hedge problem',
});
$child_cat->set_extra_metadata(group => 'Grass, hedges and weeds');
$child_cat->update;

$child_cat = FixMyStreet::DB->resultset("Contact")->find({
body => $bodies->{53822},
category => 'Grass cutting',
Expand All @@ -277,48 +279,17 @@ if ($opt->test_fixtures) {
});
$child_cat->update;

for my $cat_name ('Grass cutting', 'Hedge problem') {
$child_cat = FixMyStreet::DB->resultset("Contact")->find({
body => $bodies->{53822},
category => $cat_name,
});
$child_cat->set_extra_metadata(group => 'Grass, hedges and weeds');
if ($cat_name eq 'Hedge problem') {
$child_cat->set_extra_metadata(prefer_if_multiple => 1);
}
$child_cat->update;
}

$child_cat = FixMyStreet::DB->resultset("Contact")->find({
body => $bodies->{2483},
category => 'Other',
});
$child_cat->update({
non_public => 1
body => $bodies->{53822},
category => 'Hedge problem',
});
$child_cat->set_extra_metadata(prefer_if_multiple => 1);
$child_cat->update;

$child_cat = FixMyStreet::DB->resultset("Contact")->find({
body => $bodies->{2636},
category => 'Potholes',
});
$child_cat->update({
send_method => 'Triage'
});
$child_cat = FixMyStreet::DB->resultset("Contact")->find({
body => $bodies->{2636},
category => 'Private',
});
$child_cat->update({
non_public => 1,
send_method => 'Triage'
});
$child_cat = FixMyStreet::DB->resultset("Contact")->find({
body => $bodies->{2636},
category => 'Extra',
});
$child_cat->update({
send_method => 'Triage'
});
$child_cat->set_extra_fields({
code => 'extra',
datatype => 'string',
Expand Down Expand Up @@ -383,7 +354,6 @@ if ($opt->test_fixtures) {
body => $bodies->{2482},
category => 'Lamp Column Damaged',
});
$child_cat->set_extra_metadata(group => 'Street Lighting and Road Signs');
$child_cat->set_extra_fields({
code => 'prow_reference',
datatype => 'string',
Expand All @@ -394,13 +364,6 @@ if ($opt->test_fixtures) {
});
$child_cat->update;

$child_cat = FixMyStreet::DB->resultset("Contact")->find({
body => $bodies->{2482},
category => 'Sign Light Not Working',
});
$child_cat->set_extra_metadata(group => 'Street Lighting and Road Signs');
$child_cat->update;

$child_cat = FixMyStreet::DB->resultset("Contact")->find({
body => $bodies->{2504},
category => 'Damaged, dirty, or missing bin',
Expand Down Expand Up @@ -477,12 +440,6 @@ if ($opt->test_fixtures) {

FixMyStreet::DB->resultset('BodyArea')->create({ body_id => $bodies->{2498}->id, area_id => 2457 });
FixMyStreet::DB->resultset('BodyArea')->create({ body_id => $bodies->{2498}->id, area_id => 2483 });
$child_cat = FixMyStreet::DB->resultset("Contact")->find({ body => $bodies->{2498}, category => 'Incorrect timetable' });
$child_cat->set_extra_metadata(group => ['Bus Stops and Shelters']);
$child_cat->update;
$child_cat = FixMyStreet::DB->resultset("Contact")->find({ body => $bodies->{2498}, category => 'Glass broken' });
$child_cat->set_extra_metadata(group => ['Bus Stops and Shelters']);
$child_cat->update;
$child_cat = FixMyStreet::DB->resultset("Contact")->find({ body => $bodies->{2498}, category => 'Roadworks' });
$child_cat->set_extra_fields({ code => 'extra', datatype => 'string', order => 1, variable => 'true' });
$child_cat->update;
Expand Down
1 change: 1 addition & 0 deletions perllib/FixMyStreet/App/Controller/Report.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -843,6 +843,7 @@ sub stash_category_groups : Private {
(my $id = $_) =~ s/[^a-zA-Z]+//g;
if (@{$category_groups{$_}} == 1) {
my $contact = $category_groups{$_}[0];
$contact->set_extra_metadata(hoisted => $_);
push @list, [ $contact->category_display, $contact ];
} else {
my $cats = $category_groups{$_};
Expand Down
21 changes: 12 additions & 9 deletions perllib/FixMyStreet/App/Controller/Report/New.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1836,19 +1836,22 @@ sub generate_map : Private {
sub check_for_category : Private {
my ( $self, $c, $opts ) = @_;

my $category;
if (!$opts->{with_group}) {
$category = $c->get_param('category') || '';
} else {
# Group is either an actual group, or a category that wasn't in a group
my $group = $c->get_param('category') || $c->get_param('filter_group') || '';
if (any { $_->{name} && $group eq $_->{name} } @{$c->stash->{category_groups}}) {
my $category = $c->get_param('category') || '';
if ($opts->{with_group}) {
if (my ($group) = $category =~ /^G\|(.*)/) {
# A top-level group
$c->stash->{group} = $c->stash->{filter_group} = $group;
(my $group_id = $group) =~ s/[^a-zA-Z]+//g;
my $cat_param = "category.$group_id";
$category = $c->get_param($cat_param);
} else {
$category = $group;
} elsif ($category =~ /^H\|(.*?)\|(.*)/) {
# A hoisted to top-level category
($group, $category) = ($1, $2);
$c->stash->{group} = $c->stash->{filter_group} = $group;
} elsif (!$category && ($group = $c->get_param('filter_group'))) {
if (any { $_->{name} && $group eq $_->{name} } @{$c->stash->{category_groups}}) {
$c->stash->{group} = $c->stash->{filter_group} = $group;
}
}
}
$category ||= $c->stash->{report}->category || '';
Expand Down
8 changes: 7 additions & 1 deletion perllib/FixMyStreet/DB/Factories.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ __PACKAGE__->fields({
contacts => __PACKAGE__->callback(sub {
my $categories = shift->get('categories');
push @$categories, 'Other' unless @$categories;
[ map { FixMyStreet::DB::Factory::Contact->get_fields({ category => $_ }) } @$categories ];
[ map { FixMyStreet::DB::Factory::Contact->get_fields(ref $_ ? $_ : { category => $_ }) } @$categories ];
}),
});

Expand All @@ -233,6 +233,8 @@ use parent -norequire, "FixMyStreet::DB::Factory::Base";

__PACKAGE__->resultset(FixMyStreet::DB->resultset("Contact"));

__PACKAGE__->exclude(['group']);

__PACKAGE__->fields({
body_id => __PACKAGE__->callback(sub {
my $fields = shift;
Expand All @@ -248,6 +250,10 @@ __PACKAGE__->fields({
editor => 'Factory',
whenedited => \'current_timestamp',
note => 'Created by factory',
extra => __PACKAGE__->callback(sub {
my $group = shift->get('group');
return { group => $group } if $group;
}),
});

sub key_field { 'id' }
Expand Down
6 changes: 3 additions & 3 deletions t/app/controller/auth_social.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ for my $state ( 'refused', 'no email', 'existing UID', 'okay' ) {
$mech->submit_form_ok( { with_fields => { pc => $test->{pc} || 'SW1A1AA' } }, "submit location" );
$mech->follow_link_ok( { text_regex => qr/skip this step/i, }, "follow 'skip this step' link" );
$mech->submit_form(with_fields => {
category => 'Bins',
category => 'G|Bins',
'category.Bins' => 'Damaged bin',
title => 'Test title',
detail => 'Test detail',
Expand Down Expand Up @@ -174,7 +174,7 @@ for my $state ( 'refused', 'no email', 'existing UID', 'okay' ) {
if ($page eq 'report') {
$mech->content_contains('/report/new');
$mech->content_contains('Salt bin');
$mech->content_contains('name="category" value="Bins" data-subcategory="Bins" checked');
$mech->content_like(qr{value="G|Bins"\s+data-subcategory="Bins" checked});
$mech->content_contains('name="category.Bins" data-category_display="Damaged bin" value=\'Damaged bin\' checked');
} elsif ($page eq 'update') {
$mech->content_contains('/report/update');
Expand Down Expand Up @@ -320,7 +320,7 @@ for my $tw_state ( 'refused', 'existing UID', 'no email' ) {
$mech->submit_form_ok( { with_fields => { pc => 'SW1A1AA' } }, "submit location" );
$mech->follow_link_ok( { text_regex => qr/skip this step/i, }, "follow 'skip this step' link" );
$mech->submit_form(with_fields => {
category => 'Bins',
category => 'G|Bins',
'category.Bins' =>'Damaged bin',
title => 'Test title',
detail => 'Test detail',
Expand Down
Loading
Loading