mysociety / fixmystreet

This is mySociety's popular map-based reporting platform: easy to install in new countries and regions
http://fixmystreet.org/
Other
507 stars 235 forks source link

Automatic e-mail subscription #1550

Open jonkri opened 7 years ago

jonkri commented 7 years ago

Is it possible to specify an e-mail address that will automatically be subscribed to new problems reported to a certain category or body?

We have found that body users might sometimes miss updates, for instance in the case when a body user might quit his or her job. By enabling an automatic e-mail subscription to a shared e-mail account, things would be easier to keep track of, for municipalities working with e-mail.

What do you think of this idea?

I was thinking that some kind of database trigger might solve this problem on the SQL-level, but I was wondering if there was some cleaner way of doing this.

Thanks!

dracos commented 7 years ago

The "alert types" were originally written quite generically, so new types of alert could be added. This was pretty premature optimisation though, as it wasn't really used at all in that way, and I can't actually say if it was written in such a way that an alert such as you ask for would be possible! But theoretically, I'd suggest it might be, with parameter of body/category. You could create a new alert_type entry, and then add alerts associated with that alert type, and it's possible it might work… But it would need plenty of testing, and sorry, that's probably not enough to go on.

jonkri commented 7 years ago

Ah, I see!

Thanks anyway, @dracos!

Have a nice weekend! :)

jonkri commented 6 years ago

@dracos:

Do you see a simple way to automatically e-mail all contacts associated with a given category, on every problem update, except the contact making the update?

A simple hack could do for now (a Swedish municipality would like to have this functionality in place before the end of the year), but I imagine that it could be a useful "boolean" setting for bodies in general, that would allow certain municipalities to opt in to being notified of every update, whether they are subscribed to the given report or not.

Anyway, thanks for anything you can do. :)

dracos commented 6 years ago

What do you mean by "all contacts associated with a given category"?

jonkri commented 6 years ago

Hi, @dracos!

Sorry for being unclear!

I mean all e-mail addresses (typically one) specified as the contact for the category in question. For example, if "info@mymunicipality.se" is the designated receiver of problems reported in the category "Park", then "info@mymunicipality.se" should receive an e-mail on any updates, even if this e-mail address is not subscribed to updates. (Any subscribers should also receive the update.)

One reason for this is that sometimes a staff user leaves the municipality, and then any future staff users (recipients of e-mail) won't be notified of updates in old reports (such as the reports reopening). With a setting like the one above, the new recipient would become notified of the updates. As an added benefit, it would also allow municipalities that opt-in the possibility of receiving all updates through e-mail.

Does that clear things up? :)

jonkri commented 2 years ago

Just to add to the benefits of this: If there is a delay in solving a problem and the user submits an update, the municipality won't receive the update (unless the municipality is subribed to updates), which can cause the municipality to miss out on useful information. Also, some users assume that updates are sent the same way reports are, which can cause misunderstandings when municipalities don't receive all updates.

jonkri commented 1 year ago

What is the reasoning behind not sending updates to the body (when the body is not subscribed to updates)?

Also, since new reports are sent to bodies, I'm thinking that it (from the user's perspective) could be expected that updates would be sent as well.

Any thoughts on this would be appreciated. Thank you!

dracos commented 1 year ago

The site does or should at least say that updates aren't sent - in the UK, we did not think that councils receiving reports would appreciate also receiving updates through the same channel with potentially no way of matching them up. When we integrate with a backend, we do try and send updates there. But yes, I mean, in theory there's no reason why updates couldn't also be sent to a body should you want to do so.

jonkri commented 1 year ago

The site does or should at least say that updates aren't sent - in the UK, we did not think that councils receiving reports would appreciate also receiving updates through the same channel with potentially no way of matching them up. When we integrate with a backend, we do try and send updates there.

That makes sense, and I think it's a good default for us as well not to send updates.

But yes, I mean, in theory there's no reason why updates couldn't also be sent to a body should you want to do so.

Do you have any pointers on how we could go about allowing bodies to opt in to receiving all updates?

Thanks! 🙏

dracos commented 1 year ago

There is an update hook called update_email_shortlisted_user (bit misnamed now), that two of our cobrands use to do something, one more relevant than the other:

jonkri commented 1 year ago

Thanks for that, @dracos!

A related question: is it possible to have a message go out when a report is assigned?

One municipality has started using the report assigning functionality, but at the moment they have to both assign the report in the FixMyStreet UI and send an e-mail to the assignee. It would save the time and make the process a bit less error-prone if they could just assign the report and have the e-mail be sent automatically.

dracos commented 1 year ago

There isn't current functionality to do that - I guess most straightforward way of adding it would be to adjust the two places assignment happens, in My.pm in bulk_assign and Report.pm in inspect, to call a cobrand hook, and then that hook could send an email.

jonkri commented 2 months ago

Thanks as ever for your advice, @dracos!

I think our municipalities would appreciate having emails sent to users when they are assigned. I also think we may use the update_email_shortlisted_user functionality from TfL.pm.

I put together a proof of concept. Please let me know if you happen to have any thoughts on it.

perllib/FixMyStreet/App/Controller/Report.pm:

         # set assignment
         my $assigned = ($c->get_param('assignment'));
         if ($assigned && $assigned eq 'unassigned') {
             # take off shortlist
             my $current_assignee = $problem->shortlisted_user;
             $current_assignee->remove_from_planned_reports($problem)
                 if $current_assignee;
         } elsif ($assigned) {
-            my $assignee = $c->model('DB::User')->find({ id => $assigned });
-            $assignee->add_to_planned_reports($problem);
+            my $current_assignee = $problem->shortlisted_user;
+            my $new_assignee = $c->model('DB::User')->find({ id => $assigned });
+            $new_assignee->add_to_planned_reports($problem);
+            # Call report_inspect_assigned if there's a new assignee
+            if (!$current_assignee || $current_assignee->id ne $new_assignee->id) {
+                $c->cobrand->call_hook('report_inspect_assigned', $problem, $new_assignee);
+            }
        }
        $problem->non_public($c->get_param('non_public') ? 1 : 0);
        if ($problem->non_public) {
            $problem->get_photoset->delete_cached(plus_updates => 1);
        }

perllib/FixMyStreet/App/Controller/My.pm:

     if (@bulk_reports) {
         if ($assignee eq 'unassigned') {
             # take off shortlist
             my @problems = $c->model('DB::Problem')->search({ id => { -in => [ @bulk_reports ]} });
             foreach my $problem (@problems) {
                 my $current_assignee = $problem->shortlisted_user;
                 $current_assignee->remove_from_planned_reports($problem)
                     if $current_assignee;
             }
         } else {
             # add to shortlist
             my $inspector = $c->model('DB::User')->find({ id => $assignee });
             # ... if actually an inspector:
             if ($inspector->has_body_permission_to('report_inspect')) {
                 foreach my $report (@bulk_reports) {
                     $c->forward( '/report/load_problem_or_display_error', [ $report ] );
+                    my $current_assignee = $c->stash->{problem}->shortlisted_user;
                     $inspector->add_to_planned_reports($c->stash->{problem});
+                    # Call report_inspect_assigned if there's a new assignee
+                    if (!$current_assignee || $current_assignee->id ne $inspector->id) {
+                        $c->cobrand->call_hook('report_inspect_assigned', $c->stash->{problem}, $inspector);
+                    }
+                }
             }
         }
     }

perllib/FixMyStreet/Cobrand/FixaMinGata.pm:

sub report_inspect_assigned {
    my ($self, $problem, $assignee) = @_;
    my $c = $self->{c};
    my $cobrand = $c->cobrand;
    $c->send_email('assignment.txt', {
        to => [ [ $assignee->email, $assignee->name ] ],
        report => $problem,
        cobrand => $cobrand,
        problem_url => $cobrand->base_url . $problem->url,
    });
}

templates/email/fixamingata/assignment.html:

[%

title = report.title | html;
email_summary = "Tilldelning - " _ title;
email_columns = 2;

PROCESS '_email_settings.html';

INCLUDE '_email_top.html';

%]

<th style="[% td_style %][% primary_column_style %]" id="primary_column">
  [% start_padded_box | safe %]
  <h1 style="[% h1_style %]">Du har tilldelats rapporten [% title %]</h1>
  <p style="margin: 20px auto; text-align: center">
    <a style="[% button_style %]" href="[% problem_url %]">Se rapporten</a>
  </p>
  [% end_padded_box | safe %]
</th>
[% WRAPPER '_email_sidebar.html' object = report %]
    <h2 style="[% h2_style %]">[% title | html %]</h2>
    [% report.detail | html_para_email(secondary_p_style) %]
[% END %]

[% INCLUDE '_email_bottom.html' %]

templates/email/fixamingata/assignment.txt:

Subject: Tilldelning - '[% report.title %]'

Du har tilldelats rapporten [% title %].

För att se rapporten, klicka på följande länk:

[% problem_url %]

The “Unsubscribe from alerts about this report” link is necessarily displayed in the alert updates sent via update_email_shortlisted_user. This is problamatic for two reasons. First, the user may not actually be subscribed. Second, as long as the user stays assigned, it won't matter if the user unsubscribes (the alerts will be sent anyway).

By the way, I tried to pass $cobrand->find_closest($problem) to send_email in FixaMinGata.pm but didn't get anything that could be visibly printed in the email for some reason. Not sure why.

Here is my adaption of the update_email_shortlisted_user function from TfL.pm:

sub update_email_shortlisted_user {
    my ($self, $update) = @_;
    my $c = $self->{c};
    my $cobrand = $c->cobrand;
    my $shortlisted_by = $update->problem->shortlisted_user;
    if ($shortlisted_by && $shortlisted_by->id ne $update->user_id) {
        $c->send_email('alert-update.txt', {
            to => [ [ $shortlisted_by->email, $shortlisted_by->name ] ],
            report => $update->problem,
            cobrand => $cobrand,
            problem_url => $cobrand->base_url . $update->problem->url,
            data => [ {
                item_photo => $update->photo,
                item_text => $update->text,
                item_name => $update->name,
                item_anonymous => $update->anonymous,
            } ],
        });
    }
}

The assignment alert will be sent even if a user assigns themselves. That may be unnecessary.