[Bucks] Add email alternative to templates

[MorayMySoc]

• Adds setting to allow email alternative text to be added
• Filters out alerts that need to send the email text from general alerts of added comments
• Update database with new fields
• Create email templates for email template text
• Updates documentation to include template email text

https:/mysociety/societyworks/issues/2817

[codecov]

Codecov Report

Merging #4001 (297465d) into master (edec516) will decrease coverage by 9.12%.
The diff coverage is n/a.

❗ Current head 297465d differs from pull request most recent head 6d2795b. Consider uploading reports for the commit 6d2795b to get more accurate results

@@            Coverage Diff             @@
##           master    #4001      +/-   ##
==========================================
- Coverage   82.82%   73.69%   -9.13%     
==========================================
  Files         362       55     -307     
  Lines       25270     4760   -20510     
  Branches     3809        0    -3809     
==========================================
- Hits        20929     3508   -17421     
+ Misses       3154     1252    -1902     
+ Partials     1187        0    -1187     

Impacted Files	Coverage Δ
...llib/FixMyStreet/App/Controller/Admin/Templates.pm
perllib/FixMyStreet/DB/Result/Comment.pm
perllib/FixMyStreet/DB/Result/ResponseTemplate.pm
perllib/FixMyStreet/Queue/Item/Report.pm
perllib/FixMyStreet/Script/Alerts.pm
perllib/FixMyStreet/Template.pm
perllib/Open311/UpdatesBase.pm
perllib/Utils.pm
perllib/FixMyStreet/App/Form/Wizard.pm
perllib/FixMyStreet/Cobrand/EastSussex.pm
... and 297 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[chrismytton]

Thanks Moray, this is looking really good! Sorry it's taken more so long to review this, I'm not familiar with the updates/alert sending code, so had to do some background research in order to review this properly!

I wonder if this could potentially be simplified by adding an email_text column to the comment table, rather than storing it as extra metadata. That way you wouldn't need to mess around with the RABX to get the email text back out. What do you think?

A few other things:

1. I'm not 100% convinced the template_email_alternative feature flag is needed. Is this just so other cobrand admins don't get surprised by an extra field on the response templates page?
2. What's the advantage of the use_template_email column on the response templates, rather than just checking to see if there is an email template and using that if present?
3. Why is the new templated_email_alert-update needed? Looks like you could change the item_text to use the email text in the same place that we're adding the "State changed to:" text (see code below) then you wouldn't need a separate email template.

fixmystreet/perllib/FixMyStreet/Script/Alerts.pm

Lines 117 to 129 in d005224

	if ($row->{is_new_update}) {
	# this might throw up the odd false positive but only in cases where the
	# state has changed and there was already update text
	if ($row->{item_problem_state} && $last_problem_state ne $row->{item_problem_state}) {
	my $state = FixMyStreet::DB->resultset("State")->display($row->{item_problem_state}, 1, $cobrand->moniker);
	my $update = _('State changed to:') . ' ' . $state;
	$row->{item_text} = $row->{item_text} ? $row->{item_text} . "\n\n" . $update :
	$update;
	$last_problem_state = $row->{item_problem_state};
	}
	next unless $row->{item_text};
	}

[MorayMySoc]

I've added an email_text column to the comments and removed the extra_data stuff. I've also removed the cobrand specifics. I have left the separate template in as I think that it is a different style of email as we discussed, but happy to reconsider. Also added that these emails should go to the reporter and any subscriber should continue to receive the subscription list - which I think is working. Added a test which makes me think it it anyway!

[chrismytton]

Definitely getting closer! I've left a couple of comments inline, as well as the query below about updates to the Perl DB models.

---

This seems to be missing the changes from db/rerun_dbic_loader.pl which adds the columns to perllib/FixMyStreet/DB/Result/Comment.pm and perllib/FixMyStreet/DB/Result/ResponseTemplate.pm. When you run that command it will generate a lot of extraneous fluff, you just need the bits adding the columns, as per the following:

diff --git a/perllib/FixMyStreet/DB/Result/Comment.pm b/perllib/FixMyStreet/DB/Result/Comment.pm
index b7af66505..61b17ade7 100644
--- a/perllib/FixMyStreet/DB/Result/Comment.pm
+++ b/perllib/FixMyStreet/DB/Result/Comment.pm
@@ -72,6 +72,8 @@ __PACKAGE__->add_columns(
   { data_type => "timestamp", is_nullable => 1 },
   "send_state",
   { data_type => "text", default_value => "unprocessed", is_nullable => 0 },
+  "private_email_text",
+  { data_type => "text", is_nullable => 1 },
 );
 __PACKAGE__->set_primary_key("id");
 __PACKAGE__->has_many(
diff --git a/perllib/FixMyStreet/DB/Result/ResponseTemplate.pm b/perllib/FixMyStreet/DB/Result/ResponseTemplate.pm
index 7d6dfe29f..768013510 100644
--- a/perllib/FixMyStreet/DB/Result/ResponseTemplate.pm
+++ b/perllib/FixMyStreet/DB/Result/ResponseTemplate.pm
@@ -41,6 +41,8 @@ __PACKAGE__->add_columns(
   { data_type => "text", is_nullable => 1 },
   "external_status_code",
   { data_type => "text", is_nullable => 1 },
+  "email_text",
+  { data_type => "text", is_nullable => 1 },
 );
 __PACKAGE__->set_primary_key("id");
 __PACKAGE__->add_unique_constraint("response_templates_body_id_title_key", ["body_id", "title"]);

[chrismytton]

Oh and I think rebasing this branch onto lastest master should hopefully fix the test failures!

[chrismytton]

Looks good to me! 👍

Let's get this change on staging and see what the client thinks 🙂

[MorayMySoc]

@dracos This latest commit to answer feedback on the FD ticket

[dracos]

A few comments, hopefully nothing major.

I should note that in FD, they have said "If Text is blank and text for email alert is populated, then we don't publish anything online but we do send an email to the customer" - whilst the rest of what they say is correct (for only Text, or both present), I don't believe this part is currently the functionality, and no-one has gone back to point this out, which will need doing. If text is blank, the template won't be used at all, as far as I can tell - if the admin allows blank text to be submitted, presumably that should be fixed.

[dracos]

Couple of small issues is all.

[dracos]

Looks good :)

[dracos]

Couple of tiny suggestions