[Tfl] Check roles set before setting user body.

mysociety · Apr 30, 2024 · 2bb344c · 2bb344c
1 parent ede6acc
commit 2bb344c
Show file tree

Hide file tree

Showing 3 changed files with 8 additions and 3 deletions.
diff --git a/perllib/FixMyStreet/App/Controller/Auth.pm b/perllib/FixMyStreet/App/Controller/Auth.pm
@@ -337,7 +337,7 @@ sub process_login : Private {
         %{ $user->get_extra() },
         %{ $data->{extra} }
     }) if $data->{extra};
-    $c->cobrand->call_hook(roles_from_oidc => $user, $c->session->{oauth}{roles}) if $c->session->{oauth}{roles};
+    $c->cobrand->call_hook(roles_from_oidc => $user, $c->session->{oauth}{roles});
     $user->update_or_insert;
     $c->authenticate( { $type => $data->{$type}, $ver => 1 }, 'no_password' );
 

diff --git a/perllib/FixMyStreet/Cobrand/TfL.pm b/perllib/FixMyStreet/Cobrand/TfL.pm
@@ -208,11 +208,11 @@ sub user_from_oidc {
 sub roles_from_oidc {
     my ($self, $user, $roles) = @_;
 
+    return unless $roles && @$roles;
+
     $user->user_roles->delete;
     $user->from_body($self->body->id);
 
-    return unless $roles && @$roles;
-
     my %role_map = (
         BasicEditorViewers => 'Streetcare - Basic Editor Viewers',
         Contractor => 'Streetcare - Contractor',

diff --git a/t/cobrand/tfl.t b/t/cobrand/tfl.t
@@ -513,6 +513,7 @@ subtest "test bus report creation outside London, .com" => sub {
     $mech->get_ok('/report/new?latitude=51.345714&longitude=-0.227959');
     $mech->content_lacks('Bus things');
     $mech->host('tfl.fixmystreet.com');
+    $mech->log_out_ok;
 };
 
 subtest "test bus report creation outside London" => sub {
@@ -528,19 +529,23 @@ subtest "test bus report creation outside London" => sub {
                 title => 'Test outwith London',
                 detail => 'Test report details.',
                 name => 'Joe Bloggs',
+                username_register => 'test@example.org',
                 may_show_name => '1',
                 category => 'Bus stops',
             }
         },
         "submit good details"
     );
+    my $link = $mech->get_link_from_email;
+    $mech->get_ok($link);
     $mech->content_contains('Your issue is on its way to Transport for London');
     is_deeply $mech->page_errors, [], "check there were no errors";
 
     my $report = FixMyStreet::DB->resultset("Problem")->find({ title => 'Test outwith London'});
     ok $report, "Found the report";
     is $report->state, 'confirmed', "report confirmed";
     is $report->bodies_str, $body->id;
+    is $report->user->from_body, undef;
     $report->delete;
 
     $mech->log_out_ok;