Skip to content

Commit

Permalink
Fix issues with Open/Closed translated too early.
Browse files Browse the repository at this point in the history
It was possible for _hardcoded_states to be reached with no locale
selected (e.g. bin/update-all-reports), causing an error trying to
translate Open/Closed. But translating those there wasn't correct,
because that would then be cached and returned even if a different
language was being used. Caching was ignored in testing, which did
not help.

We no longer translate the state names in their objects, only upon
display at the point that we will know the locale. We can't simply
return the translation of Open/Closed because there may be entries
in the translation table as well.
  • Loading branch information
dracos committed Sep 15, 2017
1 parent 4314930 commit dc76133
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 6 deletions.
12 changes: 9 additions & 3 deletions perllib/FixMyStreet/DB/ResultSet/State.pm
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ 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") });
# These are translated on use, not here
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);
}

Expand All @@ -23,7 +24,7 @@ sub states {
my $rs = shift;

my $states = Memcached::get('states');
if ($states && !FixMyStreet->test_mode) {
if ($states) {
# Need to reattach schema
$states->[0]->result_source->schema( $rs->result_source->schema ) if $states->[0];
return $states;
Expand Down Expand Up @@ -62,10 +63,15 @@ sub display {
'fixed - council' => _("Fixed - Council"),
'fixed - user' => _("Fixed - User"),
};
my $translate_now = {
confirmed => _("Open"),
closed => _("Closed"),
};
$label = 'fixed' if $single_fixed && $label =~ /^fixed - (council|user)$/;
return $unchanging->{$label} if $unchanging->{$label};
my ($state) = $rs->_filter(sub { $_->label eq $label });
return $label unless $state;
$state->name($translate_now->{$label}) if $translate_now->{$label};
return $state->msgstr;
}

Expand Down
27 changes: 24 additions & 3 deletions t/app/model/state.t
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use FixMyStreet::Test;
use Test::More;
use FixMyStreet::Cobrand;

my $rs = FixMyStreet::DB->resultset('State');
my $trans_rs = FixMyStreet::DB->resultset('Translation');
Expand All @@ -14,6 +14,9 @@ for (
$trans_rs->create({ tbl => 'state', col => 'name', object_id => $obj->id,
lang => $lang, msgstr => "$lang $_->{label}" });
}
$trans_rs->create({ tbl => 'state', col => 'name', object_id => -1, lang => 'en-gb', msgstr => "Open Eng trans" });

$rs->clear;

my $states = $rs->states;
my %states = map { $_->label => $_ } @$states;
Expand All @@ -25,6 +28,8 @@ subtest 'Open/closed database data is as expected' => sub {
is @$closed, 5;
};

# No language set at this point

is $rs->display('investigating'), 'Investigating';
is $rs->display('bad'), 'bad';
is $rs->display('confirmed'), 'Open';
Expand All @@ -41,6 +46,9 @@ subtest 'default name is untranslated' => sub {
};

subtest 'msgstr gets translated if available when the language changes' => sub {
FixMyStreet::DB->schema->lang('en-gb');
is $states{confirmed}->name, 'Open';
is $states{confirmed}->msgstr, 'Open Eng trans';
FixMyStreet::DB->schema->lang('de');
is $states{'in progress'}->name, 'In progress';
is $states{'in progress'}->msgstr, 'de in progress';
Expand All @@ -50,13 +58,26 @@ subtest 'msgstr gets translated if available when the language changes' => sub {
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';

FixMyStreet::override_config {
LANGUAGES => [ 'en-gb,English,en_GB', 'nb,Norwegian,nb_NO' ],
}, sub {
subtest 'translation of open works both ways (file/db)' => sub {
# Note at this point the states have been cached
my $cobrand = FixMyStreet::Cobrand->get_class_for_moniker('default')->new;
my $lang = $cobrand->set_lang_and_domain('nb', 1, FixMyStreet->path_to('locale')->stringify);
is $lang, 'nb';
is $rs->display('confirmed'), "Åpen";
$lang = $cobrand->set_lang_and_domain('en-gb', 1, FixMyStreet->path_to('locale')->stringify);
is $lang, 'en-gb';
is $rs->display('confirmed'), "Open Eng trans";
};
};

done_testing();

0 comments on commit dc76133

Please sign in to comment.