diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 0c1f5f3d0..dcc6905f0 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -39,6 +39,11 @@ jobs: - name: Install Poetry uses: snok/install-poetry@a783c322200f0519c7926aa6faa857c4e23e9263 + - name: Set up Node + uses: actions/setup-node@v7 + with: + node-version: "20" + - name: Setup test infrastructure run: | cd integration_tests diff --git a/Makefile b/Makefile index 37ca79618..1348ffd0b 100644 --- a/Makefile +++ b/Makefile @@ -58,7 +58,10 @@ build-django-static: ## Build django-app static files test-integration: docker compose down opensearch db sso minio docker compose up -d --wait opensearch db sso minio - cd django_app && \ + cd django_app/frontend && \ + npm ci && \ + npm run build && \ + cd .. && \ poetry install && \ poetry run playwright install --with-deps chromium && \ DJANGO_ALLOW_ASYNC_UNSAFE=1 poetry run pytest tests/playwright -rP --browser chromium --tracing retain-on-failure --video on --screenshot on diff --git a/django_app/frontend/src/interaction_design_system/ids/components/_index.scss b/django_app/frontend/src/interaction_design_system/ids/components/_index.scss index f4d7a0ec5..f29365f4b 100644 --- a/django_app/frontend/src/interaction_design_system/ids/components/_index.scss +++ b/django_app/frontend/src/interaction_design_system/ids/components/_index.scss @@ -8,6 +8,7 @@ @forward './collapsible-menu.scss'; @forward './divider.scss'; @forward './editable-text.scss'; +@forward './feedback.scss'; @forward './header.scss'; @forward './list-row.scss'; @forward './loading-message.scss'; diff --git a/django_app/frontend/src/interaction_design_system/ids/components/chat-message.scss b/django_app/frontend/src/interaction_design_system/ids/components/chat-message.scss index e8336345b..4d299a59f 100644 --- a/django_app/frontend/src/interaction_design_system/ids/components/chat-message.scss +++ b/django_app/frontend/src/interaction_design_system/ids/components/chat-message.scss @@ -32,3 +32,15 @@ ids-chat-message { .ids-chat-message__container { gap: var(--chat-message-gap); } + +.ids-chat-message__post_message_actions_container { + display: flex; + justify-content: space-between; + border-bottom: 1px; + border-color: var(--gds-mid-grey); + border-bottom-style: solid; +} + +.ids-chat-message__copy-text-container { + min-width: fit-content; +} diff --git a/django_app/frontend/src/interaction_design_system/ids/components/feedback.scss b/django_app/frontend/src/interaction_design_system/ids/components/feedback.scss new file mode 100644 index 000000000..81e78e3a8 --- /dev/null +++ b/django_app/frontend/src/interaction_design_system/ids/components/feedback.scss @@ -0,0 +1,23 @@ +.feedback-form-container { + width: 100%; + // min height prevents layout shift as feedback buttons are loaded + min-height: 4.5rem; +} + +.feedback-button-container { + display: flex; + gap: 10px; + align-items: center; + flex-wrap: wrap; +} + +.feedback-actions { + display: flex; + gap: 10px; + flex-wrap: nowrap; +} + +.feedback-button { + width: auto; + white-space: nowrap; +} \ No newline at end of file diff --git a/django_app/frontend/src/interaction_design_system/ids/styles/chat-styles.scss b/django_app/frontend/src/interaction_design_system/ids/styles/chat-styles.scss index b7c2ffd06..d58acb7b2 100644 --- a/django_app/frontend/src/interaction_design_system/ids/styles/chat-styles.scss +++ b/django_app/frontend/src/interaction_design_system/ids/styles/chat-styles.scss @@ -43,7 +43,7 @@ chat-history[data-initialised] .rb-chat-history__actions-button { flex-wrap: wrap; border: none; gap: 15px; - padding: 0px;; + padding: 0px; } .feedback__heading { @@ -61,7 +61,7 @@ chat-history[data-initialised] .rb-chat-history__actions-button { } .feedback__text-area { - margin-top: 0.5rem; /* Adjust as needed */ + margin-top: 0.5rem; } .feedback__text-input { display: block; @@ -298,12 +298,6 @@ main:has(.iai-chat-bubble[data-role="ai"]) .exit-feedback { gap: 18px; } - -.feedback-button-container { - display: flex; - gap: 8px; -} - .ids-message-container { display: flex; flex-direction: column; diff --git a/django_app/frontend/src/js/web-components/chats/chat-controller.js b/django_app/frontend/src/js/web-components/chats/chat-controller.js index cc8e69ec9..c68196e2f 100644 --- a/django_app/frontend/src/js/web-components/chats/chat-controller.js +++ b/django_app/frontend/src/js/web-components/chats/chat-controller.js @@ -3,6 +3,8 @@ import { emitEvent, Events, listenEvent } from "../../../interaction_design_system/ids/events"; import { getActiveToolId, sanitizeHtml } from "../../utils"; import { ChatMessage } from "./chat-message"; +import htmx from "htmx.org"; + const STATE = { EMPTY: "empty", @@ -280,12 +282,17 @@ export class ChatController extends HTMLElement { if (!this.messageContainer) return console.error("Missing message container"); if (!this.currentStream) return console.error("No active stream"); - const html = sanitizeHtml(response.html); - this.messageContainer.insertAdjacentHTML("beforeend", html); + // Sanitising here strips the feedback hx-* attributes. + // LLM output is sanitised at its own boundary in StreamedContent. + this.messageContainer.insertAdjacentHTML("beforeend", response.html); + + const message = this.getMessage(response.chat_message_id); + + if (message) htmx.process(message); if (response.chat_message_role === "ai") { this.currentStream.messageId = response.chat_message_id; - this.getMessage(response.chat_message_id)?.focus(); + message?.focus(); } } @@ -308,8 +315,14 @@ export class ChatController extends HTMLElement { * @param {MessageCompleteResponse} response */ handleMessageComplete(response) { - this.getMessage(response.chat_message_id)?.complete(response.html); + const message = this.getMessage(response.chat_message_id); + message?.complete(response.html); if (this.currentStream) this.currentStream.title = response.title; + console.log('complete id:', response.chat_message_id, 'found:', message); // <-- add this + + // Feedback chrome in the message shell waits on this event before + // firing its hx-get (see _feedback_container.html hx-trigger). + message?.dispatchEvent(new CustomEvent("streaming-complete", { bubbles: true })); } diff --git a/django_app/redbox_app/jinja2.py b/django_app/redbox_app/jinja2.py index ec0f184d9..730096751 100644 --- a/django_app/redbox_app/jinja2.py +++ b/django_app/redbox_app/jinja2.py @@ -12,7 +12,7 @@ from django.urls import reverse from django.utils.timezone import template_localtime from markdown_it import MarkdownIt -from waffle import flag_is_active +from waffle import flag_is_active, switch_is_active from redbox_app.redbox_core import flags from redbox_app.redbox_core.types import APPROVED_FILE_EXTENSIONS @@ -164,6 +164,7 @@ def environment(**options): "google_analytics_iframe_src": settings.GOOGLE_ANALYTICS_IFRAME_SRC, "get_messages": messages.get_messages, "flag_is_active": flag_is_active, + "switch_is_active": switch_is_active, "flags": flags, "get_menu_items": get_menu_items, "product_name": get_product_name, diff --git a/django_app/redbox_app/redbox_core/flags.py b/django_app/redbox_app/redbox_core/flags.py index 81d51bb58..55ee6615f 100644 --- a/django_app/redbox_app/redbox_core/flags.py +++ b/django_app/redbox_app/redbox_core/flags.py @@ -9,3 +9,4 @@ ENABLE_INVEST_LENS = "enable_invest_lens" RESET_SSO_SYNC_SESSION = "reset_sso_sync_session" ENABLE_CHATS_REDESIGN = "enable_chats_redesign" +ENABLE_FEEDBACK_REDESIGN = "enable_feedback_redesign" diff --git a/django_app/redbox_app/redbox_core/forms.py b/django_app/redbox_app/redbox_core/forms.py index 0d9c2155f..a0727428f 100644 --- a/django_app/redbox_app/redbox_core/forms.py +++ b/django_app/redbox_app/redbox_core/forms.py @@ -4,7 +4,7 @@ from django import forms from django.contrib.auth import get_user_model -from redbox_app.redbox_core.models import Tool, ToolAccessRule, UserTool +from redbox_app.redbox_core.models import ChatMessageFeedback, Tool, ToolAccessRule, UserTool from redbox_app.redbox_core.services import url as url_service User = get_user_model() @@ -392,3 +392,30 @@ def clean(self): self.cleaned_data["user_ids"] = raw_user_ids return cleaned_data + + +class ChatMessageFeedbackForm(forms.ModelForm): + is_positive = forms.BooleanField(required=False) + + reason = forms.MultipleChoiceField( + choices=ChatMessageFeedback.Reason.choices, + required=False, + widget=forms.CheckboxSelectMultiple, + ) + + class Meta: + model = ChatMessageFeedback + fields = ("is_positive", "reason", "detail") + widgets: ClassVar[Mapping[str, forms.Widget]] = { + "detail": forms.Textarea(attrs={"rows": 3}), + } + + def clean(self): + cleaned = super().clean() + is_positive = cleaned.get("is_positive") + if is_positive: + if cleaned.get("reason"): + self.add_error("reason", "Positive feedback cannot have reasons.") + if cleaned.get("detail"): + self.add_error("detail", "Positive feedback cannot have detail.") + return cleaned diff --git a/django_app/redbox_app/redbox_core/services/chats.py b/django_app/redbox_app/redbox_core/services/chats.py index e752b87e8..bbbbd52d4 100644 --- a/django_app/redbox_app/redbox_core/services/chats.py +++ b/django_app/redbox_app/redbox_core/services/chats.py @@ -76,6 +76,7 @@ def get_context(request: HttpRequest, chat_id: UUID | None = None, slug: str | N "redbox_api_key": settings.REDBOX_API_KEY, "enable_dictation_flag_is_active": flag_is_active(request, flags.ENABLE_DICTATION), "enable_chats_redesign": flag_is_active(request, flags.ENABLE_CHATS_REDESIGN), + "enable_feedback_redesign": flag_is_active(request, flags.ENABLE_FEEDBACK_REDESIGN), **file_context, "urls": urls, "errors": {"upload_doc": []}, diff --git a/django_app/redbox_app/redbox_core/views/__init__.py b/django_app/redbox_app/redbox_core/views/__init__.py index 393451a6b..dbd5fb0f8 100644 --- a/django_app/redbox_app/redbox_core/views/__init__.py +++ b/django_app/redbox_app/redbox_core/views/__init__.py @@ -23,6 +23,7 @@ remove_doc_view, upload_document, ) +from redbox_app.redbox_core.views.feedback_views import chat_message_feedback, get_feedback_buttons from redbox_app.redbox_core.views.file_views import ( file_icon_view, file_ingest_errors_view, @@ -106,6 +107,7 @@ "add_team_member_row_view", "add_team_member_view", "aws_credentials_api", + "chat_message_feedback", "create_team_view", "delete_document", "delete_team_member_row_view", @@ -120,6 +122,7 @@ "file_icon_view", "file_ingest_errors_view", "file_status_api_view", + "get_feedback_buttons", "health", "homepage_view", "message_view_pre_alpha", diff --git a/django_app/redbox_app/redbox_core/views/feedback_views.py b/django_app/redbox_app/redbox_core/views/feedback_views.py new file mode 100644 index 000000000..1e5147b24 --- /dev/null +++ b/django_app/redbox_app/redbox_core/views/feedback_views.py @@ -0,0 +1,77 @@ +from django.contrib.auth.decorators import login_required +from django.http import Http404 +from django.shortcuts import get_object_or_404, redirect, render +from django.urls import reverse +from django.views.decorators.http import require_http_methods +from waffle import switch_is_active + +from redbox_app.redbox_core import flags +from redbox_app.redbox_core.forms import ChatMessageFeedbackForm +from redbox_app.redbox_core.models import ChatMessage, ChatMessageFeedback + +FEEDBACK_SWITCH = flags.ENABLE_FEEDBACK_REDESIGN +FORM_TEMPLATE = "chat/message/feedback/_feedback-form.html" +BUTTONS_TEMPLATE = "chat/message/feedback/_feedback_buttons.html" +THANKS_TEMPLATE = "chat/message/feedback/_feedback-thanks.html" + + +@login_required +@require_http_methods(["GET"]) +def get_feedback_buttons(request, message_id): + if not switch_is_active(FEEDBACK_SWITCH): + raise Http404 + + message = get_object_or_404( + ChatMessage.objects.filter(chat__user=request.user), + id=message_id, + ) + + instance = ChatMessageFeedback.objects.filter(message=message).first() + context = {"message_id": message.id, "message": message} + + if instance is None: + return render(request, BUTTONS_TEMPLATE, context) + + return render(request, THANKS_TEMPLATE, context) + + +@login_required +@require_http_methods(["POST", "DELETE"]) +def chat_message_feedback(request, message_id): + if not switch_is_active(FEEDBACK_SWITCH): + raise Http404 + + message = get_object_or_404( + ChatMessage.objects.filter(chat__user=request.user), + id=message_id, + ) + + instance = ChatMessageFeedback.objects.filter(message=message).first() + context = {"message_id": message.id, "message": message} + + if request.method == "DELETE": + if instance is not None: + instance.delete() + return render(request, BUTTONS_TEMPLATE, context) + + form = ChatMessageFeedbackForm(request.POST, instance=instance) + if not form.is_valid(): + response = render(request, FORM_TEMPLATE, {**context, "form": form}, status=422) + response["HX-Reswap"] = "innerHTML" + return response + + feedback, _ = ChatMessageFeedback.objects.update_or_create( + message=message, + defaults={ + "is_positive": form.cleaned_data["is_positive"], + "reason": form.cleaned_data["reason"], + "detail": form.cleaned_data["detail"], + }, + ) + + show_form = request.GET.get("show_form") == "true" + if show_form: + form = ChatMessageFeedbackForm(instance=feedback) + return render(request, FORM_TEMPLATE, {**context, "form": form}) + + return redirect(reverse("chat-message-feedback-buttons", kwargs={"message_id": message_id})) diff --git a/django_app/redbox_app/templates/chat/chat_feed.html b/django_app/redbox_app/templates/chat/chat_feed.html index 451778331..d07045b1b 100644 --- a/django_app/redbox_app/templates/chat/chat_feed.html +++ b/django_app/redbox_app/templates/chat/chat_feed.html @@ -37,6 +37,7 @@

New chat for {{ tool if tool else product_name {# SSR messages #}
    + {% for message in messages %} {{ message_box( role=message.role, @@ -44,7 +45,8 @@

    New chat for {{ tool if tool else product_name message_id=message.id, selected_files=message.unique_selected_files(), resources=message.resources, - route=message.route + route=message.route, + initial_page_load=True ) }} {% endfor %}

diff --git a/django_app/redbox_app/templates/chat/message/feedback.html b/django_app/redbox_app/templates/chat/message/feedback.html deleted file mode 100644 index 4830a4970..000000000 --- a/django_app/redbox_app/templates/chat/message/feedback.html +++ /dev/null @@ -1,6 +0,0 @@ -{% if role == "ai" %} -
- - -
-{% endif %} diff --git a/django_app/redbox_app/templates/chat/message/feedback/_feedback-form.html b/django_app/redbox_app/templates/chat/message/feedback/_feedback-form.html new file mode 100644 index 000000000..b99d99381 --- /dev/null +++ b/django_app/redbox_app/templates/chat/message/feedback/_feedback-form.html @@ -0,0 +1,118 @@ + + diff --git a/django_app/redbox_app/templates/chat/message/feedback/_feedback-thanks.html b/django_app/redbox_app/templates/chat/message/feedback/_feedback-thanks.html new file mode 100644 index 000000000..5b715eea9 --- /dev/null +++ b/django_app/redbox_app/templates/chat/message/feedback/_feedback-thanks.html @@ -0,0 +1,21 @@ + diff --git a/django_app/redbox_app/templates/chat/message/feedback/_feedback_buttons.html b/django_app/redbox_app/templates/chat/message/feedback/_feedback_buttons.html new file mode 100644 index 000000000..8734b7514 --- /dev/null +++ b/django_app/redbox_app/templates/chat/message/feedback/_feedback_buttons.html @@ -0,0 +1,27 @@ + \ No newline at end of file diff --git a/django_app/redbox_app/templates/chat/message/feedback/feedback.html b/django_app/redbox_app/templates/chat/message/feedback/feedback.html new file mode 100644 index 000000000..900522e9e --- /dev/null +++ b/django_app/redbox_app/templates/chat/message/feedback/feedback.html @@ -0,0 +1,16 @@ +{% if role == "ai" %} + +{% endif %} diff --git a/django_app/redbox_app/templates/chat/message/macros/message-box.html b/django_app/redbox_app/templates/chat/message/macros/message-box.html index 74f5e1ec5..8b1982807 100644 --- a/django_app/redbox_app/templates/chat/message/macros/message-box.html +++ b/django_app/redbox_app/templates/chat/message/macros/message-box.html @@ -6,7 +6,9 @@ message_id=None, selected_files=None, resources=None, - route=None + route=None, + enable_feedback_redesign=None, + initial_page_load=False ) %} {% set role_text = "System response" %} @@ -31,6 +33,9 @@ {% include "chat/message/message-content.html" %} + {% if switch_is_active(flags.ENABLE_FEEDBACK_REDESIGN) %} + {% include "chat/message/feedback/feedback.html" %} + {% endif %} {% include "chat/message/loading-message.html" %}
diff --git a/django_app/redbox_app/templates/chat/message/message-content.html b/django_app/redbox_app/templates/chat/message/message-content.html index 37e8ae057..288762746 100644 --- a/django_app/redbox_app/templates/chat/message/message-content.html +++ b/django_app/redbox_app/templates/chat/message/message-content.html @@ -4,5 +4,16 @@ {% include "chat/message/selected-files.html" %} {% include "chat/message/citations/resources.html" %} {{ govukErrorSummary(hidden=True) }} -{% include "chat/message/route-display.html" %} -{% include "chat/message/feedback.html" %} +{% if switch_is_active(flags.ENABLE_FEEDBACK_REDESIGN) %} + {% if role=='ai' %} +
+ {% include "chat/message/route-display.html" %} +
+ +
+
+ {% endif %} +{% else %} + {% include "chat/message/route-display.html" %} + {% include "chat/message/feedback/feedback.html" %} +{% endif %} diff --git a/django_app/redbox_app/templates/chat/message/route-display.html b/django_app/redbox_app/templates/chat/message/route-display.html index fd03c3960..4c527c114 100644 --- a/django_app/redbox_app/templates/chat/message/route-display.html +++ b/django_app/redbox_app/templates/chat/message/route-display.html @@ -1,6 +1,6 @@ {% set productName = product_name(request) %} -
+
How {{ productName }} generated this response diff --git a/django_app/redbox_app/urls.py b/django_app/redbox_app/urls.py index 1a373b1f7..06850b7dd 100644 --- a/django_app/redbox_app/urls.py +++ b/django_app/redbox_app/urls.py @@ -164,6 +164,19 @@ path("api/v0/aws-credentials", views.aws_credentials_api, name="aws-credentials"), ] +feedback_url_patterns = [ + path( + "chat-message//feedback/", + views.chat_message_feedback, + name="chat-message-feedback", + ), + path( + "chat-message//buttons/", + views.get_feedback_buttons, + name="chat-message-feedback-buttons", + ), +] + urlpatterns = ( info_urlpatterns + other_urlpatterns @@ -175,6 +188,7 @@ + tools_urlpatterns + admin_urlpatterns + api_url_patterns + + feedback_url_patterns ) if settings.DEBUG: diff --git a/django_app/tests/playwright/conftest.py b/django_app/tests/playwright/conftest.py index 7b874c447..59d62cf73 100644 --- a/django_app/tests/playwright/conftest.py +++ b/django_app/tests/playwright/conftest.py @@ -2,8 +2,11 @@ import boto3 import pytest +from django.contrib.staticfiles.testing import StaticLiveServerTestCase from moto import mock_aws +from redbox_app.redbox_core.models import Chat, ChatMessage + @pytest.fixture(autouse=True) def s3_client(): @@ -58,3 +61,36 @@ def pytest_runtest_setup(item): # if name found, test has failed for the combination of class name & test name if test_name is not None: pytest.xfail(f"previous test failed ({test_name})") + + +VYVYAN_USERNAME = os.environ["MOCK_SSO_USERNAME"] + + +@pytest.fixture +def vyvyan(create_user): + return create_user(username=VYVYAN_USERNAME) + + +@pytest.fixture +def vyvyan_chat(vyvyan) -> Chat: + return Chat.objects.create(user=vyvyan, name="A chat") + + +@pytest.fixture +def vyvyan_ai_message(vyvyan_chat: Chat) -> ChatMessage: + ChatMessage.objects.create(chat=vyvyan_chat, text="A question?", role=ChatMessage.Role.user) + return ChatMessage.objects.create( + chat=vyvyan_chat, + text="An answer with citation.", + role=ChatMessage.Role.ai, + route="chat", + ) + + +@pytest.fixture(scope="class") +def live_server_url(): + """Provide live server URL to test class.""" + server = StaticLiveServerTestCase + server.setUpClass() + yield server.live_server_url + server.tearDownClass() diff --git a/django_app/tests/playwright/pages.py b/django_app/tests/playwright/pages.py index b53ed2767..62d0509e8 100644 --- a/django_app/tests/playwright/pages.py +++ b/django_app/tests/playwright/pages.py @@ -280,6 +280,9 @@ def from_element(cls, element: Locator, page: "ChatsPage") -> "ChatMessage": sources = element.locator("sources-list").get_by_role("listitem").all_inner_texts() return cls(status=status, text=text, sources=sources, element=element, chats_page=page) + def feedback(self) -> "FeedbackComponent": + return FeedbackComponent.for_message(self) + class ChatsPage(SignedInBasePage): @override @@ -289,7 +292,10 @@ def check_a11y(self, **kwargs): @property def expected_page_title(self) -> str: - return "New chat - Chats - Assist at DBT" + if self.url.path.rstrip("/") == "/chats": + return "New chat - Chats - Assist at DBT" + chat_name = self.page.locator(".ids-chat-title__heading").inner_text() + return f"{chat_name} - Chats - Assist at DBT" @property def selected_llm(self) -> str: @@ -433,3 +439,88 @@ class SupportPage(BasePage): @property def expected_page_title(self) -> str: return "Support - Assist at DBT" + + +@dataclass +class FeedbackComponent: + """The redesigned feedback component that hangs off a single AI chat message. + + Wraps the #feedback-{message_id} container and the buttons/form/thanks states + that htmx swaps into it. + """ + + container: Locator = field(repr=False) + page: Page = field(repr=False) + + @classmethod + def for_message(cls, message: "ChatMessage") -> "FeedbackComponent": + container = message.element.locator("[id^='feedback-']") + return cls(container=container, page=message.chats_page.page) + + # --- state inspection --- + + @property + def not_quite_button(self) -> Locator: + return self.container.get_by_role("button", name="Not quite") + + @property + def yes_button(self) -> Locator: + return self.container.get_by_role("button", name="Yes") + + @property + def form(self) -> Locator: + return self.container.locator(".feedback-form") + + @property + def id_prefer_to_not_say_button(self) -> Locator: + return self.container.get_by_role("button", name="I'd prefer not to say") + + @property + def send_feedback_button(self) -> Locator: + return self.container.get_by_role("button", name="Send feedback") + + @property + def change_feedback_button(self) -> Locator: + return self.container.get_by_role("button", name="Change your answers") + + @property + def detail(self) -> str: + return self.container.locator("textarea[name='detail']").input_value() + + # --- actions --- + def wait_for_feedback_ready(self) -> "FeedbackComponent": + "allow htmx to process before proceeding" + + self.page.wait_for_load_state("networkidle") + return self + + def click_yes(self) -> "FeedbackComponent": + self.yes_button.click() + return self + + def click_change_feedback(self) -> "FeedbackComponent": + self.change_feedback_button.click() + return self + + def click_not_quite(self) -> "FeedbackComponent": + self.not_quite_button.click() + return self + + def click_id_prefer_to_not_say(self) -> "FeedbackComponent": + self.id_prefer_to_not_say_button.click() + return self + + def click_send_feedback(self) -> "FeedbackComponent": + self.send_feedback_button.click() + return self + + def select_reasons(self, reasons: Collection[str]) -> None: + for reason in reasons: + self.container.get_by_label(reason).check() + + def input_detail(self, text: str): + self.container.locator("textarea[name='detail']").fill(text) + + @property + def reason_errors(self) -> Sequence[str]: + return self.container.locator(".govuk-error-message").all_inner_texts() diff --git a/django_app/tests/playwright/test_feedback.py b/django_app/tests/playwright/test_feedback.py new file mode 100644 index 000000000..5224fdde9 --- /dev/null +++ b/django_app/tests/playwright/test_feedback.py @@ -0,0 +1,166 @@ +import pytest +from pages import FeedbackComponent, LandingPage +from playwright.sync_api import expect +from waffle.testutils import override_switch + +from redbox_app.redbox_core import flags +from redbox_app.redbox_core.models import ChatMessageFeedback + + +@pytest.fixture +def feedback_switch_active(): + with override_switch(flags.ENABLE_FEEDBACK_REDESIGN, active=True): + yield + + +@pytest.mark.django_db(transaction=True) +@pytest.mark.usefixtures("feedback_switch_active") +def test_positive_feedback_journey(page, live_server_url, vyvyan_ai_message): + landing_page = LandingPage(page, live_server_url) + + # Sign in + chats_page = landing_page.sign_in() + + existing_chat_page = chats_page.navigate_to_titled_chat(vyvyan_ai_message.chat.name) + message = next(m for m in existing_chat_page.all_messages if m.element.locator("[id^='feedback-']").count()) + feedback_component = FeedbackComponent.for_message(message) + feedback_component.wait_for_feedback_ready() + + expect(feedback_component.not_quite_button).to_be_visible() + expect(feedback_component.yes_button).to_be_visible() + + initial_feedback = ChatMessageFeedback.objects.filter(message=vyvyan_ai_message) + assert len(initial_feedback) == 0 + + # click yes + feedback_component_with_positive_feedback = feedback_component.click_yes() + feedback_component_with_positive_feedback.wait_for_feedback_ready() + expect(feedback_component_with_positive_feedback.change_feedback_button).to_be_visible() + + saved_positive_feedback = ChatMessageFeedback.objects.filter(message=vyvyan_ai_message) + assert len(saved_positive_feedback) == 1 + assert saved_positive_feedback[0].is_positive is True + assert saved_positive_feedback[0].detail == "" + assert saved_positive_feedback[0].reason == [] + + # Change feedback + feedback_component_with_changed_feedback = feedback_component_with_positive_feedback.click_change_feedback() + feedback_component_with_changed_feedback.wait_for_feedback_ready() + + expect(feedback_component.not_quite_button).to_be_visible() + expect(feedback_component.yes_button).to_be_visible() + + deleted_feedback = ChatMessageFeedback.objects.filter(message=vyvyan_ai_message) + assert len(deleted_feedback) == 0 + + +@pytest.mark.django_db(transaction=True) +@pytest.mark.usefixtures("feedback_switch_active") +def test_negative_feedback_journey_no_details(page, live_server_url, vyvyan_ai_message): + landing_page = LandingPage(page, live_server_url) + + # Sign in + chats_page = landing_page.sign_in() + + existing_chat_page = chats_page.navigate_to_titled_chat(vyvyan_ai_message.chat.name) + message = next(m for m in existing_chat_page.all_messages if m.element.locator("[id^='feedback-']").count()) + feedback_component = FeedbackComponent.for_message(message) + feedback_component.wait_for_feedback_ready() + + expect(feedback_component.not_quite_button).to_be_visible() + expect(feedback_component.yes_button).to_be_visible() + + initial_feedback = ChatMessageFeedback.objects.filter(message=vyvyan_ai_message) + assert len(initial_feedback) == 0 + + # click not quite + feedback_component.click_not_quite() + feedback_component.wait_for_feedback_ready() + + expect(feedback_component.form).to_be_visible() + expect(feedback_component.id_prefer_to_not_say_button).to_be_visible() + expect(feedback_component.send_feedback_button).to_be_visible() + + saved_negative_feedback = ChatMessageFeedback.objects.filter(message=vyvyan_ai_message) + assert len(saved_negative_feedback) == 1 + assert saved_negative_feedback[0].is_positive is False + assert saved_negative_feedback[0].detail == "" + assert saved_negative_feedback[0].reason == [] + + # click I'd prefer not to say + feedback_component.click_id_prefer_to_not_say() + feedback_component.wait_for_feedback_ready() + + expect(feedback_component.change_feedback_button).to_be_visible() + + saved_negative_feedback = ChatMessageFeedback.objects.filter(message=vyvyan_ai_message) + assert len(saved_negative_feedback) == 1 + assert saved_negative_feedback[0].is_positive is False + assert saved_negative_feedback[0].detail == "" + assert saved_negative_feedback[0].reason == [] + + # Change feedback + feedback_component.click_change_feedback() + feedback_component.wait_for_feedback_ready() + + expect(feedback_component.not_quite_button).to_be_visible() + expect(feedback_component.yes_button).to_be_visible() + + deleted_feedback = ChatMessageFeedback.objects.filter(message=vyvyan_ai_message) + assert len(deleted_feedback) == 0 + + +@pytest.mark.django_db(transaction=True) +@pytest.mark.usefixtures("feedback_switch_active") +def test_negative_feedback_journey_with_details(page, live_server_url, vyvyan_ai_message): + landing_page = LandingPage(page, live_server_url) + + # Sign in + chats_page = landing_page.sign_in() + + existing_chat_page = chats_page.navigate_to_titled_chat(vyvyan_ai_message.chat.name) + message = next(m for m in existing_chat_page.all_messages if m.element.locator("[id^='feedback-']").count()) + feedback_component = FeedbackComponent.for_message(message) + feedback_component.wait_for_feedback_ready() + + expect(feedback_component.not_quite_button).to_be_visible() + expect(feedback_component.yes_button).to_be_visible() + + initial_feedback = ChatMessageFeedback.objects.filter(message=vyvyan_ai_message) + assert len(initial_feedback) == 0 + + # click not quite + feedback_component.click_not_quite() + feedback_component.wait_for_feedback_ready() + + expect(feedback_component.form).to_be_visible() + + saved_negative_feedback = ChatMessageFeedback.objects.filter(message=vyvyan_ai_message) + assert len(saved_negative_feedback) == 1 + assert saved_negative_feedback[0].is_positive is False + assert saved_negative_feedback[0].detail == "" + assert saved_negative_feedback[0].reason == [] + + # fill out and submit form + feedback_component.select_reasons(["It was inaccurate", "It wasn't what I asked for"]) + feedback_component.input_detail(text="test 1") + feedback_component.click_send_feedback() + feedback_component.wait_for_feedback_ready() + + expect(feedback_component.change_feedback_button).to_be_visible() + + saved_negative_feedback = ChatMessageFeedback.objects.filter(message=vyvyan_ai_message) + assert len(saved_negative_feedback) == 1 + assert saved_negative_feedback[0].is_positive is False + assert saved_negative_feedback[0].detail == "test 1" + assert saved_negative_feedback[0].reason == ["INACCURATE", "UNASKED"] + + # Change feedback + feedback_component.click_change_feedback() + feedback_component.wait_for_feedback_ready() + + expect(feedback_component.not_quite_button).to_be_visible() + expect(feedback_component.yes_button).to_be_visible() + + deleted_feedback = ChatMessageFeedback.objects.filter(message=vyvyan_ai_message) + assert len(deleted_feedback) == 0 diff --git a/django_app/tests/playwright/test_journey.py b/django_app/tests/playwright/test_journey.py index f54035624..e9ab1cfca 100644 --- a/django_app/tests/playwright/test_journey.py +++ b/django_app/tests/playwright/test_journey.py @@ -2,7 +2,6 @@ import os import pytest -from django.contrib.staticfiles.testing import StaticLiveServerTestCase from pages import LandingPage from playwright.sync_api import Page @@ -10,15 +9,6 @@ logger = logging.getLogger(__name__) -@pytest.fixture(scope="class") -def live_server_url(): - """Provide live server URL to test class.""" - server = StaticLiveServerTestCase - server.setUpClass() - yield server.live_server_url - server.tearDownClass() - - @pytest.mark.django_db(transaction=True) def test_user_journey(page: Page, live_server_url): """End to end user journey test. diff --git a/django_app/tests/views/test_feedback_views.py b/django_app/tests/views/test_feedback_views.py new file mode 100644 index 000000000..97d4cc45a --- /dev/null +++ b/django_app/tests/views/test_feedback_views.py @@ -0,0 +1,181 @@ +from http import HTTPStatus + +import pytest +from bs4 import BeautifulSoup +from django.test import Client +from django.urls import reverse +from waffle.testutils import override_switch + +from redbox_app.redbox_core.models import ChatMessage, ChatMessageFeedback + +FEEDBACK_FLAG = "enable_feedback_redesign" + + +# --- get_feedback_buttons --- + + +@pytest.mark.django_db +@override_switch(FEEDBACK_FLAG, active=False) +def test_get_buttons_404_when_flag_inactive(alice, chat_message: ChatMessage, client: Client): + client.force_login(alice) + url = reverse("chat-message-feedback-buttons", kwargs={"message_id": chat_message.id}) + + response = client.get(url) + + assert response.status_code == HTTPStatus.NOT_FOUND + + +@pytest.mark.django_db +@override_switch(FEEDBACK_FLAG, active=True) +def test_get_buttons_renders_buttons_when_no_feedback(alice, chat_message: ChatMessage, client: Client): + client.force_login(alice) + url = reverse("chat-message-feedback-buttons", kwargs={"message_id": chat_message.id}) + + response = client.get(url) + + assert response.status_code == HTTPStatus.OK + soup = BeautifulSoup(response.content, "html.parser") + heading = soup.find("legend", class_="feedback__heading") + assert heading is not None + assert heading.get_text(strip=True) == "Did you get what you wanted from this response?" + buttons = [b.get_text(strip=True) for b in soup.find_all("button")] + assert "Yes" in buttons + assert "Not quite" in buttons + + +@pytest.mark.django_db +@override_switch(FEEDBACK_FLAG, active=True) +def test_get_buttons_renders_thanks_when_feedback_exists(alice, chat_message: ChatMessage, client: Client): + ChatMessageFeedback.objects.create(message=chat_message, is_positive=True) + client.force_login(alice) + url = reverse("chat-message-feedback-buttons", kwargs={"message_id": chat_message.id}) + + response = client.get(url) + + assert response.status_code == HTTPStatus.OK + soup = BeautifulSoup(response.content, "html.parser") + heading = soup.find("legend", class_="feedback__heading") + assert heading is not None + assert heading.get_text(strip=True) == "Thanks for your feedback" + button = soup.find("button") + assert button is not None + assert button.get_text(strip=True) == "Change feedback" + + +@pytest.mark.django_db +@override_switch(FEEDBACK_FLAG, active=True) +def test_get_buttons_404_for_other_users_message(bob, chat_message: ChatMessage, client: Client): + # chat_message belongs to alice; bob must not see it + client.force_login(bob) + url = reverse("chat-message-feedback-buttons", kwargs={"message_id": chat_message.id}) + + response = client.get(url) + + assert response.status_code == HTTPStatus.NOT_FOUND + + +# --- chat_message_feedback --- + + +@pytest.mark.django_db +@override_switch(FEEDBACK_FLAG, active=False) +def test_feedback_404_when_flag_inactive(alice, chat_message: ChatMessage, client: Client): + client.force_login(alice) + url = reverse("chat-message-feedback", kwargs={"message_id": chat_message.id}) + + response = client.post(url, data={"is_positive": True}) + + assert response.status_code == HTTPStatus.NOT_FOUND + + +@pytest.mark.django_db +@override_switch(FEEDBACK_FLAG, active=True) +def test_feedback_post_creates_and_redirects(alice, chat_message: ChatMessage, client: Client): + client.force_login(alice) + url = reverse("chat-message-feedback", kwargs={"message_id": chat_message.id}) + + response = client.post(url, data={"is_positive": True}) + + assert response.status_code == HTTPStatus.FOUND + assert response.url == reverse("chat-message-feedback-buttons", kwargs={"message_id": chat_message.id}) + feedback = ChatMessageFeedback.objects.get(message=chat_message) + assert feedback.is_positive is True + + +@pytest.mark.django_db +@override_switch(FEEDBACK_FLAG, active=True) +def test_feedback_post_updates_existing(alice, chat_message: ChatMessage, client: Client): + ChatMessageFeedback.objects.create(message=chat_message, is_positive=True) + client.force_login(alice) + url = reverse("chat-message-feedback", kwargs={"message_id": chat_message.id}) + + response = client.post(url, data={"is_positive": False, "reason": "INACCURATE", "detail": "test 1"}) + + assert response.status_code == HTTPStatus.FOUND + assert ChatMessageFeedback.objects.filter(message=chat_message).count() == 1 + feedback = ChatMessageFeedback.objects.get(message=chat_message) + assert feedback.is_positive is False + assert feedback.reason == ["INACCURATE"] + assert feedback.detail == "test 1" + + +@pytest.mark.django_db +@override_switch(FEEDBACK_FLAG, active=True) +def test_feedback_post_show_form_returns_form(alice, chat_message: ChatMessage, client: Client): + client.force_login(alice) + url = reverse("chat-message-feedback", kwargs={"message_id": chat_message.id}) + + response = client.post(f"{url}?show_form=true", data={"is_positive": True}) + + assert response.status_code == HTTPStatus.OK + soup = BeautifulSoup(response.content, "html.parser") + assert soup.find("div", class_="feedback-form") is not None + submit = soup.find("button", attrs={"type": "submit"}) + assert submit is not None + assert submit.get_text(strip=True) == "Send feedback" + + +@pytest.mark.django_db +@override_switch(FEEDBACK_FLAG, active=True) +def test_feedback_post_invalid_returns_422(alice, chat_message: ChatMessage, client: Client): + client.force_login(alice) + url = reverse("chat-message-feedback", kwargs={"message_id": chat_message.id}) + + response = client.post(url, data={"reason": ["not-a-valid-choice"]}) + + assert response.status_code == 422 + assert response["HX-Reswap"] == "innerHTML" + soup = BeautifulSoup(response.content, "html.parser") + assert soup.find("div", class_="feedback-form") is not None + + +@pytest.mark.django_db +@override_switch(FEEDBACK_FLAG, active=True) +def test_feedback_delete_removes_and_renders_buttons(alice, chat_message: ChatMessage, client: Client): + ChatMessageFeedback.objects.create(message=chat_message, is_positive=True) + client.force_login(alice) + url = reverse("chat-message-feedback", kwargs={"message_id": chat_message.id}) + + response = client.delete(url) + + assert response.status_code == HTTPStatus.OK + assert not ChatMessageFeedback.objects.filter(message=chat_message).exists() + soup = BeautifulSoup(response.content, "html.parser") + heading = soup.find("legend", class_="feedback__heading") + assert heading is not None + assert heading.get_text(strip=True) == "Did you get what you wanted from this response?" + + +@pytest.mark.django_db +@override_switch(FEEDBACK_FLAG, active=True) +def test_feedback_delete_no_instance_is_safe(alice, chat_message: ChatMessage, client: Client): + client.force_login(alice) + url = reverse("chat-message-feedback", kwargs={"message_id": chat_message.id}) + + response = client.delete(url) + + assert response.status_code == HTTPStatus.OK + soup = BeautifulSoup(response.content, "html.parser") + heading = soup.find("legend", class_="feedback__heading") + assert heading is not None + assert heading.get_text(strip=True) == "Did you get what you wanted from this response?"