diff options
author | Matthew Somerville <matthew-github@dracos.co.uk> | 2017-08-22 16:27:32 +0100 |
---|---|---|
committer | Matthew Somerville <matthew-github@dracos.co.uk> | 2017-08-31 15:37:09 +0100 |
commit | 27110ac506f071ad0c5925ef74de1321514c23c8 (patch) | |
tree | 61699c019d14a0b6abf4db20734b93f7c09d6093 | |
parent | b43813d8f3c5bae07f0c7b9fe120bff32611d8bb (diff) |
Remove hardcoded states from Problem model.
We keep the internal states hardcoded, plus the core open (confirmed)
and closed ones, but the remainder are moved to the database.
-rwxr-xr-x | bin/fixmystreet.com/fixture | 2 | ||||
-rwxr-xr-x | bin/update-schema | 19 | ||||
-rw-r--r-- | db/fixture.sql (renamed from db/alert_types.sql) | 10 | ||||
-rw-r--r-- | db/schema.sql | 2 | ||||
-rw-r--r-- | db/schema_0054-add-state-table.sql | 12 | ||||
-rw-r--r-- | perllib/FixMyStreet/DB/Result/Problem.pm | 46 | ||||
-rw-r--r-- | perllib/FixMyStreet/DB/Result/State.pm | 12 | ||||
-rw-r--r-- | perllib/FixMyStreet/DB/ResultSet/State.pm | 77 | ||||
-rw-r--r-- | perllib/FixMyStreet/TestAppProve.pm | 2 | ||||
-rw-r--r-- | t/app/controller/report_inspect.t | 4 | ||||
-rw-r--r-- | t/app/model/state.t | 62 |
11 files changed, 202 insertions, 46 deletions
diff --git a/bin/fixmystreet.com/fixture b/bin/fixmystreet.com/fixture index 6df675f7c..6cf5ad199 100755 --- a/bin/fixmystreet.com/fixture +++ b/bin/fixmystreet.com/fixture @@ -54,7 +54,7 @@ BEGIN END $func$; }) or die $!; - $db->dbh->do( scalar FixMyStreet->path_to('db/alert_types.sql')->slurp ) or die $!; + $db->dbh->do( scalar FixMyStreet->path_to('db/fixture.sql')->slurp ) or die $!; $db->dbh->do( scalar FixMyStreet->path_to('db/generate_secret.sql')->slurp ) or die $!; say "Emptied database"; } diff --git a/bin/update-schema b/bin/update-schema index 9a4f05813..a27bbbba1 100755 --- a/bin/update-schema +++ b/bin/update-schema @@ -40,6 +40,7 @@ BEGIN { } use FixMyStreet; +use FixMyStreet::Cobrand; use FixMyStreet::DB; use mySociety::MaPit; use Getopt::Long; @@ -112,7 +113,7 @@ if ($upgrade && $current_version eq 'EMPTY') { if ($commit) { run_statements(get_statements("$bin_dir/../db/schema.sql")); run_statements(get_statements("$bin_dir/../db/generate_secret.sql")); - run_statements(get_statements("$bin_dir/../db/alert_types.sql")); + run_statements(get_statements("$bin_dir/../db/fixture.sql")); } } elsif ($upgrade) { if ($version) { @@ -139,12 +140,28 @@ if ($upgrade && $current_version eq 'EMPTY') { my $area_ids = $db->dbh->selectcol_arrayref('SELECT area_id FROM body_areas'); if ( @$area_ids ) { my $areas = mySociety::MaPit::call('areas', $area_ids); + $db->txn_begin; foreach (values %$areas) { $db->dbh->do('UPDATE body SET name=? WHERE id=?', {}, $_->{name}, $_->{id}); } $db->txn_commit; } } + + if ( $commit && $current_version lt '0054' ) { + $nothing = 0; + print "States created, importing names\n"; + my @avail = FixMyStreet::Cobrand->available_cobrand_classes; + # Pick first available cobrand and language for database name import + my $cobrand = $avail[0] ? FixMyStreet::Cobrand::class($avail[0]) : 'FixMyStreet::Cobrand::Default'; + my $lang = $cobrand->new->set_lang_and_domain(undef, 1, FixMyStreet->path_to('locale')->stringify); + my $names = $db->dbh->selectcol_arrayref('SELECT name FROM state'); + $db->txn_begin; + foreach (@$names) { + $db->dbh->do('UPDATE state SET name=? WHERE name=?', {}, _($_), $_); + } + $db->txn_commit; + } } if ($downgrade) { diff --git a/db/alert_types.sql b/db/fixture.sql index 84f446f2c..840906223 100644 --- a/db/alert_types.sql +++ b/db/fixture.sql @@ -1,3 +1,13 @@ +INSERT INTO state (label, type, name) VALUES ('investigating', 'open', 'Investigating'); +INSERT INTO state (label, type, name) VALUES ('in progress', 'open', 'In progress'); +INSERT INTO state (label, type, name) VALUES ('planned', 'open', 'Planned'); +INSERT INTO state (label, type, name) VALUES ('action scheduled', 'open', 'Action scheduled'); +INSERT INTO state (label, type, name) VALUES ('unable to fix', 'closed', 'No further action'); +INSERT INTO state (label, type, name) VALUES ('not responsible', 'closed', 'Not responsible'); +INSERT INTO state (label, type, name) VALUES ('duplicate', 'closed', 'Duplicate'); +INSERT INTO state (label, type, name) VALUES ('internal referral', 'closed', 'Internal referral'); +INSERT INTO state (label, type, name) VALUES ('fixed', 'fixed', 'Fixed'); + -- New updates on a particular problem report insert into alert_type (ref, head_sql_query, head_table, diff --git a/db/schema.sql b/db/schema.sql index 12e66b53a..fedab2b9d 100644 --- a/db/schema.sql +++ b/db/schema.sql @@ -519,6 +519,6 @@ CREATE TABLE report_extra_fields ( CREATE TABLE state ( id serial not null primary key, label text not null unique, - type text not null check (type = 'open' OR type = 'closed'), + type text not null check (type = 'open' OR type = 'closed' OR type = 'fixed'), name text not null unique ); diff --git a/db/schema_0054-add-state-table.sql b/db/schema_0054-add-state-table.sql index c052f145a..c4be36015 100644 --- a/db/schema_0054-add-state-table.sql +++ b/db/schema_0054-add-state-table.sql @@ -3,10 +3,20 @@ BEGIN; CREATE TABLE state ( id serial not null primary key, label text not null unique, - type text not null check (type = 'open' OR type = 'closed'), + type text not null check (type = 'open' OR type = 'closed' OR type = 'fixed'), name text not null unique ); +INSERT INTO state (label, type, name) VALUES ('investigating', 'open', 'Investigating'); +INSERT INTO state (label, type, name) VALUES ('in progress', 'open', 'In progress'); +INSERT INTO state (label, type, name) VALUES ('planned', 'open', 'Planned'); +INSERT INTO state (label, type, name) VALUES ('action scheduled', 'open', 'Action scheduled'); +INSERT INTO state (label, type, name) VALUES ('unable to fix', 'closed', 'No further action'); +INSERT INTO state (label, type, name) VALUES ('not responsible', 'closed', 'Not responsible'); +INSERT INTO state (label, type, name) VALUES ('duplicate', 'closed', 'Duplicate'); +INSERT INTO state (label, type, name) VALUES ('internal referral', 'closed', 'Internal referral'); +INSERT INTO state (label, type, name) VALUES ('fixed', 'fixed', 'Fixed'); + ALTER TABLE problem DROP CONSTRAINT problem_state_check; ALTER TABLE comment DROP CONSTRAINT comment_problem_state_check; diff --git a/perllib/FixMyStreet/DB/Result/Problem.pm b/perllib/FixMyStreet/DB/Result/Problem.pm index a74a04828..fcffc1e97 100644 --- a/perllib/FixMyStreet/DB/Result/Problem.pm +++ b/perllib/FixMyStreet/DB/Result/Problem.pm @@ -220,15 +220,8 @@ HASHREF. =cut sub open_states { - my $states = { - 'confirmed' => 1, - 'investigating' => 1, - 'in progress' => 1, - 'planned' => 1, - 'action scheduled' => 1, - }; - - return wantarray ? keys %{$states} : $states; + my @states = map { $_->label } @{FixMyStreet::DB->resultset("State")->open}; + return wantarray ? @states : { map { $_ => 1 } @states }; } =head2 @@ -242,13 +235,9 @@ HASHREF. =cut sub fixed_states { - my $states = { - 'fixed' => 1, - 'fixed - user' => 1, - 'fixed - council' => 1, - }; - - return wantarray ? keys %{ $states } : $states; + my @states = map { $_->label } @{FixMyStreet::DB->resultset("State")->fixed}; + push @states, 'fixed - user', 'fixed - council' if @states; + return wantarray ? @states : { map { $_ => 1 } @states }; } =head2 @@ -262,18 +251,10 @@ HASHREF. =cut sub closed_states { - my $states = { - 'closed' => 1, - 'unable to fix' => 1, - 'not responsible' => 1, - 'duplicate' => 1, - 'internal referral' => 1, - }; - - return wantarray ? keys %{$states} : $states; + my @states = map { $_->label } @{FixMyStreet::DB->resultset("State")->closed}; + return wantarray ? @states : { map { $_ => 1 } @states }; } - =head2 @states = FixMyStreet::DB::Problem::all_states(); @@ -289,21 +270,10 @@ sub all_states { 'hidden' => 1, 'partial' => 1, 'unconfirmed' => 1, - 'confirmed' => 1, - 'investigating' => 1, - 'in progress' => 1, - 'planned' => 1, - 'action scheduled' => 1, - 'fixed' => 1, 'fixed - council' => 1, 'fixed - user' => 1, - 'unable to fix' => 1, - 'not responsible' => 1, - 'duplicate' => 1, - 'closed' => 1, - 'internal referral' => 1, }; - + map { $states->{$_->label} = 1 } @{FixMyStreet::DB->resultset("State")->states}; return wantarray ? keys %{$states} : $states; } diff --git a/perllib/FixMyStreet/DB/Result/State.pm b/perllib/FixMyStreet/DB/Result/State.pm index 6a689d595..b8a35d42b 100644 --- a/perllib/FixMyStreet/DB/Result/State.pm +++ b/perllib/FixMyStreet/DB/Result/State.pm @@ -33,6 +33,16 @@ __PACKAGE__->add_unique_constraint("state_name_key", ["name"]); # Created by DBIx::Class::Schema::Loader v0.07035 @ 2017-08-22 15:17:43 # DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:dvtAOpeYqEF9T3otHHgLqw +use Moo; +use namespace::clean; + +with 'FixMyStreet::Roles::Translatable'; + +sub msgstr { + my $self = shift; + my $lang = $self->result_source->schema->lang; + return $self->name unless $lang && $self->translated->{name}{$lang}; + return $self->translated->{name}{$lang}{msgstr}; +} -# You can replace this text with custom code or comments, and it will be preserved on regeneration 1; diff --git a/perllib/FixMyStreet/DB/ResultSet/State.pm b/perllib/FixMyStreet/DB/ResultSet/State.pm new file mode 100644 index 000000000..03bbb5e3b --- /dev/null +++ b/perllib/FixMyStreet/DB/ResultSet/State.pm @@ -0,0 +1,77 @@ +package FixMyStreet::DB::ResultSet::State; +use base 'DBIx::Class::ResultSet'; + +use strict; +use warnings; +use Memcached; + +sub _hardcoded_states { + my $rs = shift; + my $open = $rs->new({ id => -1, label => 'confirmed', type => 'open', name => _("Open") }); + my $closed = $rs->new({ id => -2, label => 'closed', type => 'closed', name => _("Closed") }); + return ($open, $closed); +} + +# As states will change rarely, and then only through the admin, +# we cache these in the package on first use, and clear on update. + +sub clear { + Memcached::set('states', ''); +} + +sub states { + my $rs = shift; + + my $states = Memcached::get('states'); + if ($states && !FixMyStreet->test_mode) { + # Need to reattach schema + $states->[0]->result_source->schema( $rs->result_source->schema ) if $states->[0]; + return $states; + } + + # Pick up and cache any translations + my $q = $rs->result_source->schema->resultset("Translation")->search({ + tbl => 'state', + col => 'name', + }); + my %trans; + $trans{$_->object_id}{$_->lang} = { id => $_->id, msgstr => $_->msgstr } foreach $q->all; + + my @states = ($rs->_hardcoded_states, $rs->search(undef, { order_by => 'label' })->all); + $_->translated->{name} = $trans{$_->id} || {} foreach @states; + $states = \@states; + Memcached::set('states', $states); + return $states; +} + +# Some functions to provide filters on the above data + +sub open { [ $_[0]->_filter(sub { $_->type eq 'open' }) ] } +sub closed { [ $_[0]->_filter(sub { $_->type eq 'closed' }) ] } +sub fixed { [ $_[0]->_filter(sub { $_->type eq 'fixed' }) ] } + +# We sometimes have only a state label to display, no associated object. +# This function can be used to return that label's display name. + +sub display { + my ($rs, $label) = @_; + my $unchanging = { + unconfirmed => _("Unconfirmed"), + hidden => _("Hidden"), + partial => _("Partial"), + 'fixed - council' => _("Fixed - Council"), + 'fixed - user' => _("Fixed - User"), + }; + return $unchanging->{$label} if $unchanging->{$label}; + my ($state) = $rs->_filter(sub { $_->label eq $label }); + return $label unless $state; + return $state->msgstr; +} + +sub _filter { + my ($rs, $fn) = @_; + my $states = $rs->states; + grep &$fn, @$states; +} + +1; diff --git a/perllib/FixMyStreet/TestAppProve.pm b/perllib/FixMyStreet/TestAppProve.pm index f6e09fbe9..7a387547d 100644 --- a/perllib/FixMyStreet/TestAppProve.pm +++ b/perllib/FixMyStreet/TestAppProve.pm @@ -75,7 +75,7 @@ sub run { $SIG{__WARN__} = sub { print STDERR @_ if $_[0] !~ m/NOTICE: CREATE TABLE/; }; $dbh->do( path('db/schema.sql')->slurp ) or die $!; - $dbh->do( path('db/alert_types.sql')->slurp ) or die $!; + $dbh->do( path('db/fixture.sql')->slurp ) or die $!; $dbh->do( path('db/generate_secret.sql')->slurp ) or die $!; $SIG{__WARN__} = $tmpwarn; diff --git a/t/app/controller/report_inspect.t b/t/app/controller/report_inspect.t index 33486aa28..687692679 100644 --- a/t/app/controller/report_inspect.t +++ b/t/app/controller/report_inspect.t @@ -57,7 +57,7 @@ FixMyStreet::override_config { }; subtest "test basic inspect submission" => sub { - $mech->submit_form_ok({ button => 'save', with_fields => { traffic_information => 'Yes', state => 'Action Scheduled', include_update => undef } }); + $mech->submit_form_ok({ button => 'save', with_fields => { traffic_information => 'Yes', state => 'Action scheduled', include_update => undef } }); $report->discard_changes; my $alert = FixMyStreet::App->model('DB::Alert')->find( { user => $user, alert_type => 'new_updates', confirmed => 1, } @@ -264,7 +264,7 @@ FixMyStreet::override_config { subtest "Oxfordshire-specific traffic management options are shown" => sub { $report->update({ state => 'confirmed' }); $mech->get_ok("/report/$report_id"); - $mech->submit_form_ok({ button => 'save', with_fields => { traffic_information => 'Signs and Cones', state => 'Action Scheduled', include_update => undef } }); + $mech->submit_form_ok({ button => 'save', with_fields => { traffic_information => 'Signs and Cones', state => 'Action scheduled', include_update => undef } }); $report->discard_changes; is $report->state, 'action scheduled', 'report state changed'; is $report->get_extra_metadata('traffic_information'), 'Signs and Cones', 'report data changed'; diff --git a/t/app/model/state.t b/t/app/model/state.t new file mode 100644 index 000000000..1653e36e2 --- /dev/null +++ b/t/app/model/state.t @@ -0,0 +1,62 @@ +use FixMyStreet::Test; +use Test::More; + +my $rs = FixMyStreet::DB->resultset('State'); +my $trans_rs = FixMyStreet::DB->resultset('Translation'); + +for ( + { label => 'in progress', lang => 'de' }, + { label => 'investigating', lang => 'fr' }, + { label => 'duplicate', lang => 'de' }, +) { + my $lang = $_->{lang}; + my $obj = $rs->find({ label => $_->{label} }); + $trans_rs->create({ tbl => 'state', col => 'name', object_id => $obj->id, + lang => $lang, msgstr => "$lang $_->{label}" }); +} + +my $states = $rs->states; +my %states = map { $_->label => $_ } @$states; + +subtest 'Open/closed database data is as expected' => sub { + my $open = $rs->open; + is @$open, 5; + my $closed = $rs->closed; + is @$closed, 5; +}; + +is $rs->display('investigating'), 'Investigating'; +is $rs->display('bad'), 'bad'; +is $rs->display('confirmed'), 'Open'; +is $rs->display('closed'), 'Closed'; +is $rs->display('fixed - council'), 'Fixed - Council'; +is $rs->display('fixed - user'), 'Fixed - User'; +is $rs->display('fixed'), 'Fixed'; + +subtest 'default name is untranslated' => sub { + is $states{'in progress'}->name, 'In progress'; + is $states{'in progress'}->msgstr, 'In progress'; + is $states{'action scheduled'}->name, 'Action scheduled'; + is $states{'action scheduled'}->msgstr, 'Action scheduled'; +}; + +subtest 'msgstr gets translated if available when the language changes' => sub { + FixMyStreet::DB->schema->lang('de'); + is $states{'in progress'}->name, 'In progress'; + is $states{'in progress'}->msgstr, 'de in progress'; + is $states{'investigating'}->name, 'Investigating'; + is $states{'investigating'}->msgstr, 'Investigating'; + is $states{'unable to fix'}->name, 'No further action'; + is $states{'unable to fix'}->msgstr, 'No further action'; +}; + +$rs->clear; + +is_deeply [ sort FixMyStreet::DB::Result::Problem->open_states ], + ['action scheduled', 'confirmed', 'in progress', 'investigating', 'planned'], 'open states okay'; +is_deeply [ sort FixMyStreet::DB::Result::Problem->closed_states ], + ['closed', 'duplicate', 'internal referral', 'not responsible', 'unable to fix'], 'closed states okay'; +is_deeply [ sort FixMyStreet::DB::Result::Problem->fixed_states ], + ['fixed', 'fixed - council', 'fixed - user'], 'fixed states okay'; + +done_testing(); |