Don't ask for email when reporting issues

feedback/forms.py

@@ -1,5 +1,6 @@
 from django import forms
 from django.forms.widgets import RadioSelect, Textarea, TextInput
+from django.utils.translation import gettext as _
 
 from .models import Feedback, Issue
 
@@ -28,11 +29,7 @@ class Meta:
 class IssueForm(forms.ModelForm):
     class Meta:
         model = Issue
-        fields = [
-            "reason",
-            "additional_info",
-            "user_email",
-        ]
+        fields = ["reason", "additional_info"]
         widgets = {
             "reason": RadioSelect(attrs={"class": "govuk-radios__input"}),
             "additional_info": Textarea(
@@ -53,3 +50,13 @@ class Meta:
             ),
         }
         formfield_callback = formfield
+
+    send_email_to_reporter = forms.TypedChoiceField(
+        widget=RadioSelect(
+            attrs={"class": "govuk-radios__input", "required": "True"},
+        ),
+        choices=(("Yes", _("Yes")), ("No", _("No"))),
+        coerce=lambda x: x == "Yes",
+        required=True,
+        label=_("Do you want us to email you a copy of this report?"),
+    )

feedback/migrations/0002_remove_issue_user_email_issue_created_by.py

@@ -0,0 +1,29 @@
+# Generated by Django 5.1.2 on 2024-10-31 16:19
+
+import django.db.models.deletion
+from django.conf import settings
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("feedback", "0001_initial"),
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name="issue",
+            name="user_email",
+        ),
+        migrations.AddField(
+            model_name="issue",
+            name="created_by",
+            field=models.ForeignKey(
+                null=True,
+                on_delete=django.db.models.deletion.CASCADE,
+                to=settings.AUTH_USER_MODEL,
+            ),
+        ),
+    ]

feedback/models.py

@@ -1,4 +1,5 @@
-from django.core.validators import EmailValidator, MinLengthValidator
+from django.conf import settings
+from django.core.validators import MinLengthValidator
 from django.db import models
 from django.utils.translation import gettext as _
 
@@ -34,6 +35,10 @@ class IssueChoices(models.TextChoices):
     created = models.DateTimeField(auto_now_add=True)
     modified = models.DateTimeField(auto_now=True)
 
+    created_by = models.ForeignKey(
+        settings.AUTH_USER_MODEL, on_delete=models.CASCADE, blank=False, null=True
+    )
+
     reason = models.CharField(
         max_length=50,
         choices=IssueChoices.choices,
@@ -50,6 +55,3 @@ class IssueChoices(models.TextChoices):
     entity_name = models.CharField(max_length=250)
     entity_url = models.CharField(max_length=250)
     data_owner_email = models.CharField(max_length=250)
-    user_email = models.CharField(
-        max_length=250, validators=[EmailValidator()], null=False, blank=True
-    )

feedback/service.py

@@ -13,22 +13,24 @@ def get_notify_api_client() -> NotificationsAPIClient:
     return NotificationsAPIClient(settings.NOTIFY_API_KEY)
 
 
-def send_notifications(issue: Issue) -> None:
+def send_notifications(issue: Issue, send_email_to_reporter: bool) -> None:
     if settings.NOTIFY_ENABLED:
         client = get_notify_api_client()
         # Spawn a thread to process the sending of notifcations and avoid potential delays
         # returning a response to the user.
-        t = threading.Thread(target=send, args=(issue, client))
+        t = threading.Thread(target=send, args=(issue, client, send_email_to_reporter))
         t.start()
 
 
-def send(issue: Issue, client: NotificationsAPIClient) -> None:
+def send(
+    issue: Issue, client: NotificationsAPIClient, send_email_to_reporter: bool
+) -> None:
 
     personalisation = {
         "assetOwner": (
             issue.data_owner_email if issue.data_owner_email else "Data Catalog Team"
         ),
-        "userEmail": issue.user_email,
+        "userEmail": issue.created_by.email if issue.created_by else "",
         "assetName": issue.entity_name,
         "userMessage": issue.additional_info,
         "assetUrl": issue.entity_url,
@@ -47,11 +49,11 @@ def send(issue: Issue, client: NotificationsAPIClient) -> None:
         )
 
     # Notify Sender
-    if issue.user_email:
+    if issue.created_by and send_email_to_reporter:
         notify(
             personalisation=personalisation,
             template_id=settings.NOTIFY_SENDER_TEMPLATE_ID,
-            email_address=issue.user_email,
+            email_address=issue.created_by.email,
             reference=reference,
             client=client,
         )

feedback/templates/report_issue.html

@@ -11,16 +11,16 @@ <h1 class="govuk-heading-xl">{{h1_value}}</h1>
   <div class="govuk-grid-column-two-thirds">
     <form action="{{ request.path }}" method="post" novalidate>
       {% if form.errors %}
-        <div class="govuk-error-summary" data-module="govuk-error-summary">
-          <div role="alert">
-            <h2 class="govuk-error-summary__title">
-              {% translate "There is a problem" %}
-            </h2>
-            <div class="govuk-error-summary__body">
-              <p class="govuk-body">{% translate 'Make sure you have filled in all the fields.' %}</p>
-            </div>
+      <div class="govuk-error-summary" data-module="govuk-error-summary">
+        <div role="alert">
+          <h2 class="govuk-error-summary__title">
+            {% translate "There is a problem" %}
+          </h2>
+          <div class="govuk-error-summary__body">
+            <p class="govuk-body">{% translate 'Make sure you have filled in all the fields.' %}</p>
           </div>
         </div>
+      </div>
       {% endif %}
 
       <div class="govuk-form-group {% if form.reason.errors %}govuk-form-group-errors{% endif %}">
@@ -31,18 +31,18 @@ <h2 class="govuk-fieldset__heading">
             </h2>
           </legend>
           {% for error in form.reason.errors %}
-            <p id="passport-issued-error" class="govuk-error-message">
-              <span class="govuk-visually-hidden">Error:</span> {{error}}
-            </p>
+          <p id="passport-issued-error" class="govuk-error-message">
+            <span class="govuk-visually-hidden">Error:</span> {{error}}
+          </p>
           {% endfor %}
           <div class="govuk-radios" data-module="govuk-radios">
             {% for radio in form.reason %}
-              <div class="govuk-radios__item">
-                {{radio.tag}}
-                <label class="govuk-label govuk-radios__label" for="{{radio.id_for_label}}">
-                  {{radio.choice_label}}
-                </label>
-              </div>
+            <div class="govuk-radios__item">
+              {{radio.tag}}
+              <label class="govuk-label govuk-radios__label" for="{{radio.id_for_label}}">
+                {{radio.choice_label}}
+              </label>
+            </div>
             {% endfor %}
           </div>
         </fieldset>
@@ -58,26 +58,36 @@ <h1 class="govuk-label-wrapper">
           {% translate 'Please provide as much information as possible about the issue.' %}
         </div>
         {% for error in form.additional_info.errors %}
-          <p id="additional-info-issued-error" class="govuk-error-message">
-            <span class="govuk-visually-hidden">Error:</span> {{error}}
-          </p>
+        <p id="additional-info-issued-error" class="govuk-error-message">
+          <span class="govuk-visually-hidden">Error:</span> {{error}}
+        </p>
         {% endfor %}
         {{ form.additional_info }}
       </div>
 
-      <div class="govuk-form-group {% if form.user_email.errors %}govuk-form-group-errors{% endif %}">
-        <label class="govuk-label" for="email">
-          Email address
-        </label>
-        <div id="email-hint" class="govuk-hint">
-          We’ll use this for future follow up.
-        </div>
-        {% for error in form.user_email.errors %}
+      <div class="govuk-form-group">
+        <fieldset class="govuk-fieldset">
+          <legend class="govuk-fieldset__legend govuk-fieldset__legend--l">
+            <h1 class="govuk-fieldset__heading">
+              {{form.send_email_to_reporter.label}}
+            </h1>
+          </legend>
+          {% for error in form.send_email_to_reporter.errors %}
           <p id="passport-issued-error" class="govuk-error-message">
             <span class="govuk-visually-hidden">Error:</span> {{error}}
           </p>
-        {% endfor %}
-        {{ form.user_email}}
+          {% endfor %}
+          <div class="govuk-radios" data-module="govuk-radios">
+            {% for radio in form.send_email_to_reporter %}
+            <div class="govuk-radios__item">
+              {{radio.tag}}
+              <label class="govuk-label govuk-radios__label" for="send_email_to_reporter">
+                {{ radio.choice_label }}
+              </label>
+            </div>
+            {% endfor %}
+          </div>
+        </fieldset>
       </div>
 
       <button type="submit" class="govuk-button" data-module="govuk-button">

feedback/views.py

@@ -48,10 +48,19 @@ def report_issue_view(request) -> HttpResponse:
             issue.entity_name = request.session.get("entity_name")
             issue.entity_url = request.session.get("entity_url")
             issue.data_owner_email = request.session.get("data_owner_email")
+
+            # in production, there should always be a signed in user,
+            # but this may not be the case in local development/unit tests
+            if not request.user.is_anonymous:
+                issue.created_by = request.user
+
             issue.save()
 
             # Call the send notifications service
-            send_notifications(issue=issue)
+            send_notifications(
+                issue=issue,
+                send_email_to_reporter=form.cleaned_data["send_email_to_reporter"],
+            )
 
             return redirect("feedback:thanks")