diff options
| -rw-r--r-- | perllib/FixMyStreet/App/Controller/Alert.pm | 75 | ||||
| -rw-r--r-- | t/app/controller/alert_new.t | 73 | ||||
| -rw-r--r-- | templates/web/default/alert/list.html | 4 |
3 files changed, 73 insertions, 79 deletions
diff --git a/perllib/FixMyStreet/App/Controller/Alert.pm b/perllib/FixMyStreet/App/Controller/Alert.pm index d1759a330..ff92a7d2d 100644 --- a/perllib/FixMyStreet/App/Controller/Alert.pm +++ b/perllib/FixMyStreet/App/Controller/Alert.pm @@ -55,17 +55,14 @@ Target for subscribe form sub subscribe : Path('subscribe') : Args(0) { my ( $self, $c ) = @_; - if ( $c->req->param('rss') ) { - $c->detach('rss'); - } + $c->detach('rss') if $c->req->param('rss'); + # if it exists then it's been submitted so we should # go to subscribe email and let it work out the next step - elsif ( exists $c->req->params->{'rznvy'} ) { - $c->detach('subscribe_email'); - } - elsif ( $c->req->params->{'id'} ) { - $c->go('updates'); - } + $c->detach('subscribe_email') + if exists $c->req->params->{'rznvy'} || $c->req->params->{'alert'}; + + $c->go('updates') if $c->req->params->{'id'}; # shouldn't get to here but if we have then do something sensible $c->go('index'); @@ -116,25 +113,18 @@ Sign up to email alerts sub subscribe_email : Private { my ( $self, $c ) = @_; - my $type = $c->req->param('type'); - $c->stash->{email_type} = 'alert'; + $c->stash->{errors} = []; + $c->forward('process_user'); - my @errors; - push @errors, _('Please enter a valid email address') - unless is_valid_email( $c->req->param('rznvy') ); - push @errors, _('Please select the type of alert you want') + my $type = $c->req->param('type'); + push @{ $c->stash->{errors} }, _('Please select the type of alert you want') if $type && $type eq 'local' && !$c->req->param('feed'); - if (@errors) { - $c->stash->{errors} = \@errors; + if (@{ $c->stash->{errors} }) { $c->go('updates') if $type && $type eq 'updates'; $c->go('list') if $type && $type eq 'local'; $c->go('index'); } - my $email = $c->req->param('rznvy'); - $c->stash->{email} = $email; - $c->forward('process_user'); - if ( $type eq 'updates' ) { $c->forward('set_update_alert_options'); } @@ -200,14 +190,12 @@ sub create_alert : Private { $options->{cobrand_data} = $c->cobrand->extra_update_data(); $options->{lang} = $c->stash->{lang_code}; - if ( $c->user && $c->stash->{alert_user}->in_storage && $c->user->id == $c->stash->{alert_user}->id ) { - $options->{confirmed} = 1; - } - $alert = $c->model('DB::Alert')->new($options); $alert->insert(); } + $alert->confirm() if $c->user && $c->user->id == $alert->user->id; + $c->stash->{alert} = $alert; } @@ -303,6 +291,7 @@ sub send_confirmation_email : Private { $c->send_email( 'alert-confirm.txt', { to => $c->stash->{alert}->user->email } ); + $c->stash->{email_type} = 'alert'; $c->stash->{template} = 'email_sent.html'; } @@ -331,12 +320,46 @@ sub prettify_pc : Private { return 1; } +=head2 process_user + +Fetch/check email address + +=cut + sub process_user : Private { my ( $self, $c ) = @_; - my $email = $c->stash->{email}; + if ( $c->user_exists ) { + $c->stash->{alert_user} = $c->user->obj; + return; + } + + # Extract all the params to a hash to make them easier to work with + my %params = map { $_ => scalar $c->req->param($_) } + ( 'rznvy' ); # , 'password_register' ); + + # cleanup the email address + my $email = $params{rznvy} ? lc $params{rznvy} : ''; + $email =~ s{\s+}{}g; + + push @{ $c->stash->{errors} }, _('Please enter a valid email address') + unless is_valid_email( $email ); + my $alert_user = $c->model('DB::User')->find_or_new( { email => $email } ); $c->stash->{alert_user} = $alert_user; + +# # The user is trying to sign in. We only care about email from the params. +# if ( $c->req->param('submit_sign_in') ) { +# unless ( $c->forward( '/auth/sign_in', [ $email ] ) ) { +# $c->stash->{field_errors}->{password} = _('There was a problem with your email/password combination. Please try again.'); +# return 1; +# } +# my $user = $c->user->obj; +# $c->stash->{alert_user} = $user; +# return 1; +# } +# +# $alert_user->password( Utils::trim_text( $params{password_register} ) ); } =head2 setup_coordinate_rss_feeds diff --git a/t/app/controller/alert_new.t b/t/app/controller/alert_new.t index d0976b4bb..cb3b4b656 100644 --- a/t/app/controller/alert_new.t +++ b/t/app/controller/alert_new.t @@ -193,22 +193,11 @@ foreach my $test ( foreach my $test ( { desc => 'logged in user signing up', - user => 'test-sign-in@example.com', email => 'test-sign-in@example.com', type => 'council', param1 => 2651, param2 => 2651, confirmed => 1, - }, - { - desc => 'logged in user signing up with different email', - user => 'loggedin@example.com', - email => 'test-sign-in@example.com', - type => 'council', - param1 => 2651, - param2 => 2651, - confirmed => 0, - delete => 1, } ) { @@ -217,42 +206,18 @@ foreach my $test ( my $user = FixMyStreet::App->model('DB::User') - ->find_or_create( { email => $test->{user} } ); - - my $alert_user = - FixMyStreet::App->model('DB::User') - ->find( { email => $test->{email} } ); - - $mech->log_in_ok( $test->{user} ); + ->find_or_create( { email => $test->{email} } ); + $mech->log_in_ok( $test->{email} ); $mech->clear_emails_ok; - my $alert; - if ($alert_user) { - $alert = FixMyStreet::App->model('DB::Alert')->find( - { - user => $alert_user, - alert_type => $type - } - ); - - # clear existing data so we can be sure we're creating it - $alert->delete() if $alert; - } - $mech->get_ok('/alert/list?pc=EH991SP'); - - my $form_values = $mech->visible_form_values(); - ok $form_values->{rznvy} eq $test->{user}, - 'auto filled in correct email'; - - $mech->set_visible( [ radio => 'council:2651:City_of_Edinburgh' ], - [ text => $test->{email} ] ); + $mech->set_visible( [ radio => 'council:2651:City_of_Edinburgh' ] ); $mech->click('alert'); - $alert = FixMyStreet::App->model('DB::Alert')->find( + my $alert = FixMyStreet::App->model('DB::Alert')->find( { - user => $alert_user, + user => $user, alert_type => $type, parameter => $test->{param1}, parameter2 => $test->{param2}, @@ -261,13 +226,8 @@ foreach my $test ( ); ok $alert, 'New alert created with logged in user'; - - $mech->email_count_is( $test->{confirmed} ? 0 : 1 ); - - if ( $test->{delete} ) { - $mech->delete_user($user); - $mech->delete_user($alert_user); - } + $mech->email_count_is( 0 ); + $mech->delete_user($user); }; } @@ -391,17 +351,24 @@ subtest "Test normal alert signups and that alerts are sent" => sub { $user2->alerts->delete; for my $alert ( - { feed => 'local:55.951963:-3.189944', email_confirm => 1 }, - { feed => 'council:2651:City_of_Edinburgh', }, + { + fields => { + feed => 'local:55.951963:-3.189944', + rznvy => $user2->email, + }, + email_confirm => 1 + }, + { + fields => { + feed => 'council:2651:City_of_Edinburgh', + } + }, ) { $mech->get_ok( '/alert' ); $mech->submit_form_ok( { with_fields => { pc => 'EH11BB' } } ); $mech->submit_form_ok( { button => 'alert', - with_fields => { - rznvy => $user2->email, - feed => $alert->{feed}, - } + with_fields => $alert->{fields}, } ); if ( $alert->{email_confirm} ) { my $email = $mech->get_email; diff --git a/templates/web/default/alert/list.html b/templates/web/default/alert/list.html index 38007ea62..36bfaffd9 100644 --- a/templates/web/default/alert/list.html +++ b/templates/web/default/alert/list.html @@ -116,10 +116,14 @@ for the county council.' ) %] [% loc('or') %] </p> +[% UNLESS c.user_exists %] + <p> [% loc('Your email:') %] <input type="text" id="rznvy" name="rznvy" value="[% rznvy | html %]" size="30"> </p> +[% END %] + <p> <input type="submit" name="alert" value="[% loc('Subscribe me to an email alert') %]"> </p> |