PR Updates

Author: mitchdawson1982

feedback/forms.py

@@ -1,7 +1,7 @@
 from django import forms
 from django.forms.widgets import RadioSelect, Textarea, TextInput
 
-from .models import Feedback, ReportIssue
+from .models import Feedback, Issue
 
 
 def formfield(field, **kwargs):
@@ -25,9 +25,9 @@ class Meta:
         formfield_callback = formfield
 
 
-class ReportIssueForm(forms.ModelForm):
+class IssueForm(forms.ModelForm):
     class Meta:
-        model = ReportIssue
+        model = Issue
         fields = [
             "reason",
             "additional_info",

feedback/migrations/0010_issue_delete_reportissue.py

@@ -0,0 +1,67 @@
+# Generated by Django 5.1.2 on 2024-10-22 09:23
+
+import uuid
+
+import django.core.validators
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("feedback", "0009_alter_reportissue_additional_info_and_more"),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name="Issue",
+            fields=[
+                (
+                    "id",
+                    models.UUIDField(
+                        default=uuid.uuid4,
+                        editable=False,
+                        primary_key=True,
+                        serialize=False,
+                        unique=True,
+                    ),
+                ),
+                ("created", models.DateTimeField(auto_now_add=True)),
+                ("modified", models.DateTimeField(auto_now=True)),
+                (
+                    "reason",
+                    models.CharField(
+                        choices=[
+                            ("Link is broken", "Broken Link"),
+                            ("Owner is incorrect", "Incorrect Owner"),
+                            ("Contact is outdated", "Outdated Contact"),
+                            ("Other", "Other"),
+                        ],
+                        max_length=50,
+                        verbose_name="What is wrong with this page?",
+                    ),
+                ),
+                (
+                    "additional_info",
+                    models.TextField(
+                        validators=[django.core.validators.MinLengthValidator(10)],
+                        verbose_name="Can you provide more detail?",
+                    ),
+                ),
+                ("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(
+                        blank=True,
+                        max_length=250,
+                        validators=[django.core.validators.EmailValidator()],
+                    ),
+                ),
+            ],
+        ),
+        migrations.DeleteModel(
+            name="ReportIssue",
+        ),
+    ]

feedback/models.py

@@ -26,11 +26,11 @@ class Feedback(models.Model):
     )
 
 
-class ReportIssue(models.Model):
+class Issue(models.Model):
     class IssueChoices(models.TextChoices):
-        BROKEN_LINK = "Broken link"
-        INCORRECT_OWNER = "Incorrect owner"
-        OUTDATED_CONTACT = "Outdated contact"
+        BROKEN_LINK = "Link is broken"
+        INCORRECT_OWNER = "Owner is incorrect"
+        OUTDATED_CONTACT = "Contact is outdated"
         OTHER = "Other"
 
     id = models.UUIDField(
@@ -42,7 +42,7 @@ class IssueChoices(models.TextChoices):
     reason = models.CharField(
         max_length=50,
         choices=IssueChoices.choices,
-        verbose_name=_("Reason"),
+        verbose_name=_("What is wrong with this page?"),
         null=False,
         blank=False,
     )

feedback/service.py

@@ -1,9 +1,10 @@
 import logging
+import threading
 
 from django.conf import settings
 from notifications_python_client.notifications import NotificationsAPIClient
 
-from feedback.models import ReportIssue
+from feedback.models import Issue
 
 log = logging.getLogger(__name__)
 
@@ -12,7 +13,15 @@
 )
 
 
-def send_notifications(issue: ReportIssue) -> None:
+def send_notifications(issue: Issue) -> None:
+    if settings.NOTIFY_ENABLED:
+        # 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,))
+        t.start()
+
+
+def send(issue: Issue) -> None:
 
     personalisation = {
         "assetOwner": (

feedback/views.py

@@ -1,12 +1,10 @@
 import logging
-import threading
 
-from django.conf import settings
 from django.http import HttpResponse
 from django.shortcuts import redirect, render
 from django.utils.translation import gettext as _
 
-from .forms import FeedbackForm, ReportIssueForm
+from .forms import FeedbackForm, IssueForm
 from .service import send_notifications
 
 log = logging.getLogger(__name__)
@@ -43,19 +41,16 @@ def thank_you_view(request) -> HttpResponse:
 
 def report_issue_view(request) -> HttpResponse:
     if request.method == "POST":
-        form = ReportIssueForm(request.POST)
+        form = IssueForm(request.POST)
         if form.is_valid():
             issue = form.save(commit=False)
             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")
             issue.save()
 
-            if settings.NOTIFY_ENABLED:
-                # Spawn a thread to process the sending of notifcations and avoid potential delays
-                # returning a response to the user.
-                t = threading.Thread(target=send_notifications, args=(issue,))
-                t.start()
+            # Call the send notifications service
+            send_notifications(issue=issue)
 
             return redirect("feedback:thanks")
 
@@ -65,7 +60,9 @@ def report_issue_view(request) -> HttpResponse:
                 request,
                 "report_issue.html",
                 {
-                    "h1_value": _("Report an issue on Find MOJ data"),
+                    "h1_value": _(
+                        f"Report an issue with {request.session.get('entity_name')}"
+                    ),
                     "form": form,
                 },
             )
@@ -77,13 +74,13 @@ def report_issue_view(request) -> HttpResponse:
         request.session["entity_url"] = entity_url
         request.session["data_owner_email"] = _(request.GET.get("data_owner_email", ""))
 
-        form = ReportIssueForm()
+        form = IssueForm()
 
     return render(
         request,
         "report_issue.html",
         {
-            "h1_value": _("Report an issue on Find MOJ data"),
+            "h1_value": _(f"Report an issue with {entity_name}"),
             "form": form,
             "entity_name": entity_name,
             "entity_url": entity_url,

tests/feedback/test_forms.py

@@ -1,7 +1,7 @@
 import pytest
 
-from feedback.forms import FeedbackForm, ReportIssueForm
-from feedback.models import Feedback, ReportIssue
+from feedback.forms import FeedbackForm, IssueForm
+from feedback.models import Feedback, Issue
 
 
 def test_invalid_feedback_form():
@@ -27,16 +27,16 @@ def test_feedback_form_saves_to_db():
 
 
 def test_valid_report_issue_form():
-    assert ReportIssueForm(
+    assert IssueForm(
         {
             "reason": "Other",
             "additional_info": "a" * 10,
         }
-    ).is_valid
+    ).is_valid()
 
 
 def test_report_issue_form_invalid_additinal_info_length():
-    form = ReportIssueForm(
+    form = IssueForm(
         {
             "reason": "Other",
             "additional_info": "a" * 9,
@@ -50,7 +50,7 @@ def test_report_issue_form_invalid_additinal_info_length():
 
 
 def test_report_issue_form_invalid_user_email():
-    form = ReportIssueForm(
+    form = IssueForm(
         {"reason": "Other", "additional_info": "a" * 10, "user_email": "invalid_email"}
     )
     assert not form.is_valid()
@@ -59,15 +59,15 @@ def test_report_issue_form_invalid_user_email():
 
 @pytest.mark.django_db
 def test_report_issue_form_saves_to_db():
-    form = ReportIssueForm(
+    form = IssueForm(
         {
             "reason": "Other",
             "additional_info": "a" * 10,
         }
     )
     form.save()
 
-    saved = ReportIssue.objects.first()
+    saved = Issue.objects.first()
     assert saved
     assert saved.reason == "Other"
     assert saved.additional_info == "a" * 10

tests/feedback/test_notify_service.py

@@ -2,8 +2,8 @@
 
 import pytest
 
-from feedback.models import ReportIssue
-from feedback.service import send_notifications
+from feedback.models import Issue
+from feedback.service import send
 
 
 @pytest.mark.django_db
@@ -18,10 +18,10 @@ def test_send_all_notifications(mock_notifications_client):
         "user_email": "userx@justice.gov.uk",
     }
 
-    issue = ReportIssue.objects.create(**data)
+    issue = Issue.objects.create(**data)
     assert issue
 
-    send_notifications(issue=issue)
+    send(issue=issue)
 
     assert mock_notifications_client.call_count == 3
 
@@ -37,10 +37,10 @@ def test_send_notifications_no_data_owner_email(mock_notifications_client):
         "user_email": "userx@justice.gov.uk",
     }
 
-    issue = ReportIssue.objects.create(**data)
+    issue = Issue.objects.create(**data)
     assert issue
 
-    send_notifications(issue=issue)
+    send(issue=issue)
 
     assert mock_notifications_client.call_count == 2
 
@@ -56,10 +56,10 @@ def test_send_all_notifications_no_user_email(mock_notifications_client):
         "data_owner_email": "entity_owner@justice.gov.uk",
     }
 
-    issue = ReportIssue.objects.create(**data)
+    issue = Issue.objects.create(**data)
     assert issue
 
-    send_notifications(issue=issue)
+    send(issue=issue)
 
     assert mock_notifications_client.call_count == 2
 
@@ -74,9 +74,9 @@ def test_send_all_notifications_no_user_or_data_owner_email(mock_notifications_c
         "entity_url": "http://localhost/my_entity",
     }
 
-    issue = ReportIssue.objects.create(**data)
+    issue = Issue.objects.create(**data)
     assert issue
 
-    send_notifications(issue=issue)
+    send(issue=issue)
 
     assert mock_notifications_client.call_count == 1