From 971559822e1b908aa1faa103434cb8c00054e91b Mon Sep 17 00:00:00 2001 From: James Stott <158563996+jamesstottmoj@users.noreply.github.com> Date: Tue, 10 Dec 2024 16:20:05 +0000 Subject: [PATCH 1/3] Feature/feedback form (#1401) * Added initial feedback form * Added management command to write csv to bucket. Added tests * Fixed black error * changed parameter to weeks * Fixed references to num_weeks in command * Bumped dependencies * Removed unneeded code * Feedback on feedback code * Ran migration --- controlpanel/api/admin.py | 7 +- controlpanel/api/aws.py | 7 +- controlpanel/api/migrations/0048_feedback.py | 43 ++++++++++ .../0049_alter_feedback_suggestions.py | 19 +++++ controlpanel/api/models/__init__.py | 1 + controlpanel/api/models/feedback.py | 25 ++++++ .../cli/management/commands/feedback_csv.py | 65 +++++++++++++++ controlpanel/frontend/forms.py | 10 +++ controlpanel/frontend/jinja2/base.html | 8 ++ .../frontend/jinja2/feedback-create.html | 80 +++++++++++++++++++ .../frontend/jinja2/feedback-thanks.html | 12 +++ controlpanel/frontend/mixins.py | 35 ++++++++ controlpanel/frontend/urls.py | 2 + controlpanel/frontend/views/__init__.py | 1 + controlpanel/frontend/views/app.py | 22 ++--- controlpanel/frontend/views/feedback.py | 22 +++++ controlpanel/settings/common.py | 1 + controlpanel/settings/test.py | 2 + requirements.txt | 8 +- tests/api/cli/test_feedback_csv.py | 37 +++++++++ tests/conftest.py | 11 +++ 21 files changed, 395 insertions(+), 23 deletions(-) create mode 100644 controlpanel/api/migrations/0048_feedback.py create mode 100644 controlpanel/api/migrations/0049_alter_feedback_suggestions.py create mode 100644 controlpanel/api/models/feedback.py create mode 100644 controlpanel/cli/management/commands/feedback_csv.py create mode 100644 controlpanel/frontend/jinja2/feedback-create.html create mode 100644 controlpanel/frontend/jinja2/feedback-thanks.html create mode 100644 controlpanel/frontend/views/feedback.py create mode 100644 tests/api/cli/test_feedback_csv.py 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..d18ceb80a --- /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 %} +
+ {% 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 %} +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/