diff --git a/controlpanel/api/admin.py b/controlpanel/api/admin.py index ad9e199fb..016cfc36a 100644 --- a/controlpanel/api/admin.py +++ b/controlpanel/api/admin.py @@ -3,7 +3,7 @@ from simple_history.admin import SimpleHistoryAdmin # First-party/Local -from controlpanel.api.models import App, IPAllowlist, S3Bucket, User +from controlpanel.api.models import App, Feedback, IPAllowlist, S3Bucket, User def make_migration_pending(modeladmin, request, queryset): @@ -58,7 +58,12 @@ class IPAllowlistAdmin(SimpleHistoryAdmin): history_list_display = ("description", "contact", "allowed_ip_ranges") +class FeedbackAdmin(admin.ModelAdmin): + list_display = ("satisfaction_rating", "suggestions", "date_added") + + admin.site.register(App, AppAdmin) admin.site.register(S3Bucket, S3Admin) admin.site.register(User, UserAdmin) admin.site.register(IPAllowlist, IPAllowlistAdmin) +admin.site.register(Feedback, FeedbackAdmin) diff --git a/controlpanel/api/aws.py b/controlpanel/api/aws.py index 63d24be64..1468f7313 100644 --- a/controlpanel/api/aws.py +++ b/controlpanel/api/aws.py @@ -656,8 +656,7 @@ def create(self, bucket_name, is_data_warehouse=False): # https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/s3.html?highlight=s3#S3.BucketVersioning # noqa: E501 versioning = bucket.Versioning() versioning.enable() - # Set bucket lifecycle. Send non-current versions of files to glacier - # storage after 30 days. + # Set bucket lifecycle. Set to intelligent tiering # https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/s3.html#S3.Client.put_bucket_lifecycle_configuration # noqa: E501 self.apply_lifecycle_config(bucket_name, s3_client) if is_data_warehouse: @@ -800,6 +799,10 @@ def exists(self, bucket_name): return True + def write_to_bucket(self, bucket_name, key, data): + s3_client = self.boto3_session.client("s3") + s3_client.put_object(Bucket=bucket_name, Key=key, Body=data) + class AWSPolicy(AWSService): def create_policy(self, name, path, policy_document=None): diff --git a/controlpanel/api/migrations/0048_feedback.py b/controlpanel/api/migrations/0048_feedback.py new file mode 100644 index 000000000..979f92758 --- /dev/null +++ b/controlpanel/api/migrations/0048_feedback.py @@ -0,0 +1,43 @@ +# Generated by Django 5.1.2 on 2024-12-03 10:35 + +# Third-party +import django.utils.timezone +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("api", "0047_app_cloud_platform_role_arn"), + ] + + operations = [ + migrations.CreateModel( + name="Feedback", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, primary_key=True, serialize=False, verbose_name="ID" + ), + ), + ( + "satisfaction_rating", + models.IntegerField( + choices=[ + (5, "Very satisfied"), + (4, "Satisfied"), + (3, "Neither satisfied or dissatisfied"), + (2, "Dissatisfied"), + (1, "Very dissatisfied"), + ] + ), + ), + ("suggestions", models.TextField()), + ("date_added", models.DateTimeField(default=django.utils.timezone.now)), + ], + options={ + "db_table": "control_panel_api_feedback", + }, + ), + ] diff --git a/controlpanel/api/migrations/0049_alter_feedback_suggestions.py b/controlpanel/api/migrations/0049_alter_feedback_suggestions.py new file mode 100644 index 000000000..176717c1b --- /dev/null +++ b/controlpanel/api/migrations/0049_alter_feedback_suggestions.py @@ -0,0 +1,19 @@ +# Generated by Django 5.1.2 on 2024-12-10 15:29 + +# Third-party +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("api", "0048_feedback"), + ] + + operations = [ + migrations.AlterField( + model_name="feedback", + name="suggestions", + field=models.TextField(blank=True), + ), + ] diff --git a/controlpanel/api/models/__init__.py b/controlpanel/api/models/__init__.py index 4b49f607d..81cd1f7a6 100644 --- a/controlpanel/api/models/__init__.py +++ b/controlpanel/api/models/__init__.py @@ -8,6 +8,7 @@ from controlpanel.api.models.app import App from controlpanel.api.models.app_ip_allowlist import AppIPAllowList from controlpanel.api.models.apps3bucket import AppS3Bucket +from controlpanel.api.models.feedback import Feedback from controlpanel.api.models.iam_managed_policy import IAMManagedPolicy from controlpanel.api.models.parameter import Parameter from controlpanel.api.models.policys3bucket import PolicyS3Bucket diff --git a/controlpanel/api/models/feedback.py b/controlpanel/api/models/feedback.py new file mode 100644 index 000000000..6739b80f6 --- /dev/null +++ b/controlpanel/api/models/feedback.py @@ -0,0 +1,25 @@ +# Third-party +from django.db import models +from django.utils import timezone + + +class Feedback(models.Model): + SATISFACTION_RATINGS = [ + (5, "Very satisfied"), + (4, "Satisfied"), + (3, "Neither satisfied or dissatisfied"), + (2, "Dissatisfied"), + (1, "Very dissatisfied"), + ] + + satisfaction_rating = models.IntegerField( + choices=SATISFACTION_RATINGS, + null=False, + blank=False, + ) + + suggestions = models.TextField(blank=True) + date_added = models.DateTimeField(default=timezone.now) + + class Meta: + db_table = "control_panel_api_feedback" diff --git a/controlpanel/cli/management/commands/feedback_csv.py b/controlpanel/cli/management/commands/feedback_csv.py new file mode 100644 index 000000000..0c428bad0 --- /dev/null +++ b/controlpanel/cli/management/commands/feedback_csv.py @@ -0,0 +1,65 @@ +# Standard library +import csv +from datetime import datetime, timedelta +from io import StringIO + +# Third-party +from django.conf import settings +from django.core.management.base import BaseCommand + +# First-party/Local +from controlpanel.api.aws import AWSBucket +from controlpanel.api.models import Feedback + + +class Command(BaseCommand): + help = "Writes a csv file with the feedback data to an S3 Bucket" + csv_headings = ["Satisfaction Rating", "Suggestions", "Date Added"] + + def add_arguments(self, parser): + parser.add_argument( + "--weeks", + "-w", + type=int, + default=2, + help="Get feedback over an x week period from today's date", + ) + parser.add_argument("--all", "-a", action="store_true", help="Get all feedback received") + + def handle(self, *args, **options): + today = datetime.today() + + if options["all"]: + feedback_items = Feedback.objects.all() + else: + self.stdout.write(f"weeks: {options['weeks']}") + timeframe = today - timedelta(weeks=options["weeks"]) + feedback_items = Feedback.objects.filter(date_added__gte=timeframe) + + if not feedback_items: + self.stdout.write(f"No feedback found for the past {options['weeks']} weeks") + return + + filename = f"feedback_{today}.csv" + csv_buffer = StringIO() + writer = csv.writer(csv_buffer, delimiter=",", quotechar='"', quoting=csv.QUOTE_MINIMAL) + writer.writerow(self.csv_headings) + for feedback in feedback_items: + row = [ + feedback.get_satisfaction_rating_display(), + feedback.suggestions, + feedback.date_added.date(), + ] + writer.writerow(row) + + try: + csv_value = csv_buffer.getvalue() + bucket = AWSBucket() + + if not bucket.exists(settings.FEEDBACK_BUCKET_NAME): + bucket.create(settings.FEEDBACK_BUCKET_NAME) + + bucket.write_to_bucket(settings.FEEDBACK_BUCKET_NAME, filename, csv_value) + self.stdout.write(f"Feedback data written to {settings.FEEDBACK_BUCKET_NAME}") + except Exception as e: + self.stdout.write(f"Failed to write to S3 bucket: {e}") diff --git a/controlpanel/frontend/forms.py b/controlpanel/frontend/forms.py index c688ace24..65d2e93b2 100644 --- a/controlpanel/frontend/forms.py +++ b/controlpanel/frontend/forms.py @@ -16,6 +16,7 @@ from controlpanel.api.models import ( QUICKSIGHT_EMBED_PERMISSION, App, + Feedback, S3Bucket, Tool, User, @@ -655,3 +656,12 @@ def grant_access(self): self.user.user_permissions.add(permission) else: self.user.user_permissions.remove(permission) + + +class FeedbackForm(forms.ModelForm): + class Meta: + model = Feedback + fields = [ + "satisfaction_rating", + "suggestions", + ] diff --git a/controlpanel/frontend/jinja2/base.html b/controlpanel/frontend/jinja2/base.html index a52666baa..3daa89397 100644 --- a/controlpanel/frontend/jinja2/base.html +++ b/controlpanel/frontend/jinja2/base.html @@ -75,6 +75,14 @@ {% endblock %} {% block beforeContent %} +
+

+ + Provide your Feedback to help us improve this service. + +

+
+ {% if not hide_nav and request.user.is_authenticated %} {{ mojPrimaryNavigation({ "label": "Primary navigation", diff --git a/controlpanel/frontend/jinja2/feedback-create.html b/controlpanel/frontend/jinja2/feedback-create.html new file mode 100644 index 000000000..4a50d1386 --- /dev/null +++ b/controlpanel/frontend/jinja2/feedback-create.html @@ -0,0 +1,80 @@ +{% from "error-message/macro.html" import govukErrorMessage %} +{% from "label/macro.html" import govukLabel %} +{% from "radios/macro.html" import govukRadios %} +{% from "includes/auth0-connections-form.html" import auth0_connections_form with context %} + + +{% extends "base.html" %} + +{% set page_title = "Feedback" %} + +{% block content %} +

Give feedback on the Analytical Platform

+ + +
+ {{ csrf_input }} + + {{ govukRadios({ + "name": "satisfaction_rating", + "fieldset": { + "legend": { + "text": "Satisfaction survey", + "classes": "govuk-fieldset__legend--l", + }, + }, + "items": [ + { + "value": 5, + "text": "Very satisfied", + "checked": form.satisfaction_rating.value() == "5" + }, + { + "value": 4, + "text": "Satisfied", + "checked": form.satisfaction_rating.value() == "4" + }, + { + "value": 3, + "text": "Neither satisfied or dissatisfied", + "checked": form.satisfaction_rating.value() == "3" + }, + { + "value": 2, + "text": "Dissatisfied", + "checked": form.satisfaction_rating.value() == "2" + }, + { + "value": 1, + "text": "Very dissatisfied", + "checked": form.satisfaction_rating.value() == "1" + }, + ], + "errorMessage": { "text": form.errors.get("satisfaction_rating") } if form.errors.get("satisfaction_rating") else {} + }) }} + + + +
+

+ +

+
+ Do not include personal or financial information, like your National Insurance number or credit card details. +
+ + {% if form.errors.get("suggestions") %} + {{ govukErrorMessage({"text": form.errors.get("suggestions")}) }} + {% endif %} + + +
+ + +
+ +
+
+{% endblock %} diff --git a/controlpanel/frontend/jinja2/feedback-thanks.html b/controlpanel/frontend/jinja2/feedback-thanks.html new file mode 100644 index 000000000..9bcbf5db1 --- /dev/null +++ b/controlpanel/frontend/jinja2/feedback-thanks.html @@ -0,0 +1,12 @@ +{% from "error-message/macro.html" import govukErrorMessage %} +{% from "includes/auth0-connections-form.html" import auth0_connections_form with context %} + + +{% extends "base.html" %} + +{% set page_title = "Thank you" %} + +{% block content %} +

Thank you for your feedback

+

Your feedback will help us improve the service.

+{% endblock %} diff --git a/controlpanel/frontend/mixins.py b/controlpanel/frontend/mixins.py index aa8fe3c91..27f56bce6 100644 --- a/controlpanel/frontend/mixins.py +++ b/controlpanel/frontend/mixins.py @@ -1,5 +1,10 @@ +# Standard library +import csv +from datetime import datetime + # Third-party from django.contrib import messages +from django.http import HttpResponse from rules.contrib.views import PermissionRequiredMixin # First-party/Local @@ -28,3 +33,33 @@ def form_valid(self, form): def get_success_url(self): messages.success(self.request, self.success_message) return super().get_success_url() + + +class CsvWriterMixin(OIDCLoginRequiredMixin, PermissionRequiredMixin): + """ + Allows exporting a list of models to a CSV file. + """ + + http_method_names = ["post"] + permission_required = "api.is_superuser" + filename = "" + csv_headings = [] + model_attributes = [] + + def write_csv(self, models): + timestamp = datetime.now().strftime("%Y-%m-%d-%H-%M-%S") + + response = HttpResponse( + content_type="text/csv", + headers={ + "Content-Disposition": f'attachment; filename="{self.filename}_{timestamp}.csv"' + }, + ) + + writer = csv.writer(response) + writer.writerow(self.csv_headings) + for model in models: + row = [model[attribute] for attribute in self.model_attributes] + writer.writerow(row) + + return response diff --git a/controlpanel/frontend/urls.py b/controlpanel/frontend/urls.py index 67f69fee3..22bdeb343 100644 --- a/controlpanel/frontend/urls.py +++ b/controlpanel/frontend/urls.py @@ -283,4 +283,6 @@ ), path("parameters//delete/", views.ParameterDelete.as_view(), name="delete-parameter"), path("quicksight/", views.QuicksightView.as_view(), name="quicksight"), + path("feedback/", views.CreateFeedback.as_view(), name="feedback-create"), + path("feedback/thanks", views.FeedbackThanks.as_view(), name="feedback-thanks"), ] diff --git a/controlpanel/frontend/views/__init__.py b/controlpanel/frontend/views/__init__.py index 23f9a6e60..31a62e43c 100644 --- a/controlpanel/frontend/views/__init__.py +++ b/controlpanel/frontend/views/__init__.py @@ -61,6 +61,7 @@ UpdateIAMManagedPolicyAccessLevel, WebappBucketList, ) +from controlpanel.frontend.views.feedback import CreateFeedback, FeedbackThanks from controlpanel.frontend.views.help import Help from controlpanel.frontend.views.ip_allowlist import ( IPAllowlistCreate, diff --git a/controlpanel/frontend/views/app.py b/controlpanel/frontend/views/app.py index dc738ae78..59f02be77 100644 --- a/controlpanel/frontend/views/app.py +++ b/controlpanel/frontend/views/app.py @@ -44,7 +44,7 @@ RemoveCustomerByEmailForm, UpdateAppAuth0ConnectionsForm, ) -from controlpanel.frontend.mixins import PolicyAccessMixin +from controlpanel.frontend.mixins import CsvWriterMixin, PolicyAccessMixin from controlpanel.frontend.views.apps_mng import AppManager from controlpanel.oidc import OIDCLoginRequiredMixin @@ -78,8 +78,10 @@ def get_queryset(self): ) -class AppAdminCSV(OIDCLoginRequiredMixin, PermissionRequiredMixin, View): - permission_required = "api.is_superuser" +class AppAdminCSV(CsvWriterMixin, View): + filename = "app_admins" + csv_headings = ["App Name", "Admins", "Emails"] + model_attributes = ["name", "users", "emails"] def post(self, request, *args, **kwargs): apps = ( @@ -95,19 +97,7 @@ def post(self, request, *args, **kwargs): .order_by("name") ) - timestamp = datetime.now().strftime("%Y-%m-%d-%H-%M-%S") - - response = HttpResponse( - content_type="text/csv", - headers={"Content-Disposition": f'attachment; filename="app_admins_{timestamp}.csv"'}, - ) - - writer = csv.writer(response) - writer.writerow(["App Name", "Admins", "Emails"]) - for app in apps: - writer.writerow([app["name"], app["users"], app["emails"]]) - - return response + return self.write_csv(apps) class AppDetail(OIDCLoginRequiredMixin, PermissionRequiredMixin, DetailView): diff --git a/controlpanel/frontend/views/feedback.py b/controlpanel/frontend/views/feedback.py new file mode 100644 index 000000000..d4a7d3be1 --- /dev/null +++ b/controlpanel/frontend/views/feedback.py @@ -0,0 +1,22 @@ +# Third-party +from django.urls import reverse_lazy +from django.views.generic import TemplateView +from django.views.generic.edit import CreateView, FormMixin + +# First-party/Local +from controlpanel.api.models import Feedback +from controlpanel.frontend.forms import FeedbackForm +from controlpanel.oidc import OIDCLoginRequiredMixin + + +class CreateFeedback(OIDCLoginRequiredMixin, CreateView): + form_class = FeedbackForm + model = Feedback + template_name = "feedback-create.html" + + def get_success_url(self): + return reverse_lazy("feedback-thanks") + + +class FeedbackThanks(OIDCLoginRequiredMixin, TemplateView): + template_name = "feedback-thanks.html" diff --git a/controlpanel/settings/common.py b/controlpanel/settings/common.py index d00de2fb2..837649ea7 100644 --- a/controlpanel/settings/common.py +++ b/controlpanel/settings/common.py @@ -608,3 +608,4 @@ DPR_DATABASE_NAME = os.environ.get("DPR_DATABASE_NAME", None) S3_ARCHIVE_BUCKET_NAME = "dev-archive-folder" +FEEDBACK_BUCKET_NAME = f"{ENV}-{os.environ.get('FEEDBACK_BUCKET_NAME')}" diff --git a/controlpanel/settings/test.py b/controlpanel/settings/test.py index 71b606d1d..eba1132a2 100644 --- a/controlpanel/settings/test.py +++ b/controlpanel/settings/test.py @@ -38,3 +38,5 @@ QUICKSIGHT_ASSUMED_ROLE = "arn:aws:iam::123456789012:role/quicksight_test" OIDC_CPANEL_API_AUDIENCE = "test-audience" + +FEEDBACK_BUCKET_NAME = "test-feedback-bucket" diff --git a/requirements.txt b/requirements.txt index 63a846838..5a2075bcd 100644 --- a/requirements.txt +++ b/requirements.txt @@ -29,13 +29,12 @@ mozilla-django-oidc==4.0.1 psycopg2-binary==2.9.9 PyNaCl==1.5.0 pytest==8.0.0 -pytest-django==4.8.0 +pytest-django==4.9.0 python-dotenv==1.0.1 python-jose==3.3.0 -pyyaml==6.0.1 +pyyaml==6.0.2 rules==3.3 -sentry-sdk==2.17.0 +sentry-sdk==2.19.2 slackclient==2.9.4 urllib3==2.2.3 -uvicorn[standard]==0.28.0 - +uvicorn[standard]==0.32.1 diff --git a/tests/api/cli/test_feedback_csv.py b/tests/api/cli/test_feedback_csv.py new file mode 100644 index 000000000..970b5ff74 --- /dev/null +++ b/tests/api/cli/test_feedback_csv.py @@ -0,0 +1,37 @@ +# Standard library +from unittest.mock import MagicMock, PropertyMock, call, patch + +# Third-party +from django.core.management import call_command + + +@patch("controlpanel.cli.management.commands.feedback_csv.AWSBucket") +def test_feedback_csv_no_feedback(mock_bucket, db): + call_command("feedback_csv", "--weeks", "2") + + # Assert that with no feedback present, the following methods aren't called + mock_bucket.return_value.exists.assert_not_called() + mock_bucket.return_value.create.assert_not_called() + mock_bucket.return_value.write_to_bucket.assert_not_called() + + +@patch("controlpanel.cli.management.commands.feedback_csv.AWSBucket") +def test_feedback_csv_no_bucket(mock_bucket, db, feedback): + mock_bucket.return_value.exists.return_value = False + call_command("feedback_csv", "--weeks", "2") + + # Assert that with feedback present, the following methods aren't called + mock_bucket.return_value.exists.assert_called_with("test-feedback-bucket") + mock_bucket.return_value.create.assert_called_with("test-feedback-bucket") + mock_bucket.return_value.write_to_bucket.assert_called_once() + + +@patch("controlpanel.cli.management.commands.feedback_csv.AWSBucket") +def test_feedback_csv_bucket_exists(mock_bucket, db, feedback): + mock_bucket.return_value.exists.return_value = True + call_command("feedback_csv", "--weeks", "2") + + # Assert that with feedback present, the following methods aren't called + mock_bucket.return_value.exists.assert_called_with("test-feedback-bucket") + mock_bucket.return_value.create.assert_not_called() + mock_bucket.return_value.write_to_bucket.assert_called_once() diff --git a/tests/conftest.py b/tests/conftest.py index 22d2a75dd..613d0ee2f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -186,3 +186,14 @@ def retrieve_messages(sqs, queue_name=settings.DEFAULT_QUEUE): @pytest.fixture def helpers(): return Helpers + + +@pytest.fixture +def feedback(db): + feedback = baker.make( + "api.Feedback", + satisfaction_rating="5", + suggestions="Great software!", + ) + + return feedback