diff --git a/backend/cases/kb_access.py b/backend/cases/kb_access.py index 81eeaa4f..2542d392 100644 --- a/backend/cases/kb_access.py +++ b/backend/cases/kb_access.py @@ -23,9 +23,13 @@ hypothetical shape of bug for this codebase. `read` is deliberately absent: every member of the org may read every article. A knowledge base whose articles are hidden from the agents answering the -tickets is not a knowledge base. `is_published` is not an access rule either. -It is the customer-visibility switch, and the customer-facing site it gates was -explicitly cut (see `kb_views`), so today it gates the agent suggester only. +tickets is not a knowledge base. + +`is_published` is not an access rule for agents either. It is the +customer-visibility switch, and it now switches something: a published, +approved article is readable by any signed-in portal contact in the org, via +`PortalBaseView._published_articles`. That is why `release` is the narrowest +rule here. Approving is the moment an article stops being an internal note. """ from django.core.exceptions import ValidationError as DjangoValidationError diff --git a/backend/cases/kb_text.py b/backend/cases/kb_text.py new file mode 100644 index 00000000..2c764bc0 --- /dev/null +++ b/backend/cases/kb_text.py @@ -0,0 +1,36 @@ +"""Text handling shared by the two knowledge-base suggesters. + +There are two of them now: the agent-facing one in `kb_views` and the +customer-facing one in `portal_views`. They answer to different visibility +rules and return different shapes, but they read the same articles, so the +matching and the truncation live here rather than being written twice and +drifting apart. + +What is deliberately NOT shared is the queryset. Each caller builds its own, +because deciding who may see which article is the security boundary and it +should be written out at each call site rather than inherited from a helper. +""" + +from django.db.models import Q + +SNIPPET_MAX = 200 + + +def snippet(text): + """A single-line preview of an article body.""" + if not text: + return "" + text = text.strip() + if len(text) <= SNIPPET_MAX: + return text + return text[: SNIPPET_MAX - 1].rstrip() + "…" + + +def text_match(term): + """Match `term` against an article's title or body. + + Substring matching, not full-text search. At the size of one org's + knowledge base that is the right trade, and it keeps the two suggesters + returning the same articles for the same words. + """ + return Q(title__icontains=term) | Q(description__icontains=term) diff --git a/backend/cases/kb_views.py b/backend/cases/kb_views.py index 17e00e4c..8d6d0288 100644 --- a/backend/cases/kb_views.py +++ b/backend/cases/kb_views.py @@ -1,9 +1,12 @@ """Agent-facing knowledge-base helpers. -The full kb-frontend spec calls for a customer-facing KB site too. We -ship only the agent-side suggester here. See -docs/cases/tier2/IMPLEMENTATION_STATUS.md "kb-frontend" section for the -deliberate cut. The endpoint feeds the comment composer's typeahead. +This is the suggester behind the comment composer's typeahead. Its +customer-facing counterpart lives in `portal_views` and answers to a stricter +rule: this endpoint shows an agent anything published, while the portal also +requires `approved`. Text matching and truncation are shared, in `kb_text`. + +Remaining pieces of the module are tracked on issue #589: categories, +versioning, view counts, helpfulness feedback, slugs. """ from __future__ import annotations @@ -14,23 +17,14 @@ from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from rest_framework.views import APIView +from cases.kb_text import snippet, text_match from cases.models import Case, Solution from common.permissions import HasOrgContext -_SNIPPET_MAX = 200 _DEFAULT_LIMIT = 5 _MAX_LIMIT = 20 -def _snippet(text: str | None) -> str: - if not text: - return "" - text = text.strip() - if len(text) <= _SNIPPET_MAX: - return text - return text[: _SNIPPET_MAX - 1].rstrip() + "…" - - def _seed_terms(case: Case) -> list[str]: """Seed terms when q is empty: pull a handful of meaningful words from the case name + description so the agent gets useful suggestions on @@ -85,9 +79,7 @@ class SolutionSuggestionsView(APIView): published = Solution.objects.filter(org=org, is_published=True) if q: - results = published.filter( - Q(title__icontains=q) | Q(description__icontains=q) - ).order_by("-updated_at")[:limit] + results = published.filter(text_match(q)).order_by("-updated_at")[:limit] else: # Seed from the case so the panel is useful on first focus. If # there are no seed terms, or the seed-term filter produces zero @@ -98,9 +90,7 @@ class SolutionSuggestionsView(APIView): if terms: seed_filter = Q() for term in terms: - seed_filter |= Q(title__icontains=term) | Q( - description__icontains=term - ) + seed_filter |= text_match(term) results = list( published.filter(seed_filter).order_by("-updated_at")[:limit] ) @@ -111,7 +101,7 @@ class SolutionSuggestionsView(APIView): { "id": str(s.id), "title": s.title, - "snippet": _snippet(s.description), + "snippet": snippet(s.description), # Full body included so the picker can paste without a # second roundtrip. Keeps the widget snappy on a slow link. "body": s.description or "", diff --git a/backend/cases/migrations/0029_solution_tags.py b/backend/cases/migrations/0029_solution_tags.py new file mode 100644 index 00000000..24bd9495 --- /dev/null +++ b/backend/cases/migrations/0029_solution_tags.py @@ -0,0 +1,20 @@ +# Generated by Django 6.0.7 on 2026-08-16 00:48 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("cases", "0028_inboundmailbox_webhook_secret_help_text"), + ("common", "0040_comment_max_length"), + ] + + operations = [ + migrations.AddField( + model_name="solution", + name="tags", + field=models.ManyToManyField( + blank=True, related_name="solution_tags", to="common.tags" + ), + ), + ] diff --git a/backend/cases/models.py b/backend/cases/models.py index f4b96228..734059fe 100644 --- a/backend/cases/models.py +++ b/backend/cases/models.py @@ -461,6 +461,11 @@ class Solution(BaseModel): # Cases that use this solution cases = models.ManyToManyField(Case, related_name="solutions", blank=True) + # The agents' own vocabulary, shared with leads, deals and tickets. Written + # for internal filing, not for customers, so tags stay off every portal + # serializer and the portal uses them only to find related articles. + tags = models.ManyToManyField(Tags, related_name="solution_tags", blank=True) + class Meta: verbose_name = "Solution" verbose_name_plural = "Solutions" diff --git a/backend/cases/portal_serializers.py b/backend/cases/portal_serializers.py index 20ced0bf..6a60fc01 100644 --- a/backend/cases/portal_serializers.py +++ b/backend/cases/portal_serializers.py @@ -9,7 +9,7 @@ future edit to a shared serializer, and cannot be mass-assigned on the way in. from rest_framework import serializers -from cases.models import Case +from cases.models import Case, Solution from common.models import COMMENT_MAX_LENGTH, Comment @@ -57,6 +57,33 @@ class PortalCaseDetailSerializer(serializers.ModelSerializer): ) +class PortalSolutionSerializer(serializers.ModelSerializer): + """A knowledge base article as a customer sees it in a list. + + `status` and `is_published` are absent deliberately, and not because they + would embarrass anybody. They are the editorial state of the article, and a + customer who can read one can tell an approved-but-unpublished article + exists. The queryset has already decided the customer may see this row, so + repeating the decision in the payload only creates something to leak. + + `created_by` is absent for the reason `PortalCommentSerializer.get_author` + gives: which colleague wrote the answer is internal. `linked_cases` is + absent because it would hand this customer other customers' case ids. + """ + + class Meta: + model = Solution + fields = ("id", "title", "updated_at") + + +class PortalSolutionDetailSerializer(serializers.ModelSerializer): + """The article body, and nothing the list projection already withholds.""" + + class Meta: + model = Solution + fields = ("id", "title", "description", "updated_at") + + class PortalCaseCreateSerializer(serializers.ModelSerializer): """Three fields. Everything else about a new case is the server's to decide. diff --git a/backend/cases/portal_views.py b/backend/cases/portal_views.py index 37dd1750..aea697e0 100644 --- a/backend/cases/portal_views.py +++ b/backend/cases/portal_views.py @@ -7,18 +7,22 @@ one edit rather than an audit of every view. """ from django.contrib.contenttypes.models import ContentType +from django.db.models import Q from rest_framework import status from rest_framework.pagination import LimitOffsetPagination from rest_framework.response import Response from rest_framework.views import APIView -from cases.models import Case +from cases.kb_text import snippet, text_match +from cases.models import Case, Solution from cases.portal_serializers import ( PortalCaseCreateSerializer, PortalCaseDetailSerializer, PortalCaseSerializer, PortalCommentCreateSerializer, PortalCommentSerializer, + PortalSolutionDetailSerializer, + PortalSolutionSerializer, ) from common.models import Comment from common.portal_auth import IsPortalContact, PortalContactAuthentication @@ -26,6 +30,15 @@ from common.utils import STATUS_CHOICE VALID_STATUSES = {choice[0] for choice in STATUS_CHOICE} +# Three, and not configurable. A deflection panel is a nudge beside a form, so +# the useful question is "is the answer already here", not "how many can I +# scroll". The agent picker is the surface that wants a tunable limit. +SUGGEST_LIMIT = 3 + +# Same reasoning as SUGGEST_LIMIT: a short list under an article people are +# already reading, not a second index of the knowledge base. +RELATED_LIMIT = 3 + class PortalBaseView(APIView): # Declared here rather than inherited from settings, on purpose. The portal @@ -45,6 +58,19 @@ class PortalBaseView(APIView): org=request.org, contacts=request.portal_contact, is_active=True ) + def _published_articles(self, request): + """The only definition of "an article a customer may read". + + Both conditions, not just `is_published`. The pairing is enforced on + write by `SolutionSerializer.validate`, but that rule arrived after the + model did, so rows created before it can be published drafts. Requiring + `approved` here means such a row is never shown to a customer, and the + first edit to it repairs the pair anyway. + """ + return Solution.objects.filter( + org=request.org, is_published=True, status="approved" + ) + def _case_or_none(self, request, pk): return self._my_cases(request).filter(pk=pk).first() @@ -126,6 +152,106 @@ class PortalCaseDetailView(PortalBaseView): ) +class PortalArticleListView(PortalBaseView, LimitOffsetPagination): + def get(self, request): + queryset = self._published_articles(request) + + # Narrowed from the visible set, never applied to Solution.objects. A + # search box that reaches rows the list cannot is the same bug as a + # missing org filter, just harder to notice. + search = request.query_params.get("search") + if search: + queryset = queryset.filter( + Q(title__icontains=search) | Q(description__icontains=search) + ) + + queryset = queryset.order_by("title") + page = self.paginate_queryset(queryset, request, view=self) + return Response( + { + "articles": PortalSolutionSerializer(page, many=True).data, + "articles_count": self.count, + } + ) + + +class PortalArticleDetailView(PortalBaseView): + def get(self, request, pk): + article = self._published_articles(request).filter(pk=pk).first() + if article is None: + # Same response for "no such article" and "not published yet", for + # the reason `_not_found` gives about cases: telling them apart + # confirms that a draft with this id exists. + return Response( + {"error": "Article not found"}, status=status.HTTP_404_NOT_FOUND + ) + return Response( + { + "article": PortalSolutionDetailSerializer(article).data, + "related": self._related(request, article), + } + ) + + def _related(self, request, article): + """Other articles the agents filed under the same tags. + + The tag vocabulary is shared with leads and deals and reads like + "At Risk" and "VIP", so it is used to *find* these rows and never + appears in the response. Ids and titles only. + + Built from `_published_articles`, so a shared tag cannot reach a draft + or another org's article: relatedness narrows the visible set, it never + widens it. + """ + tag_ids = list(article.tags.values_list("id", flat=True)) + if not tag_ids: + return [] + + siblings = ( + self._published_articles(request) + .filter(tags__id__in=tag_ids) + .exclude(pk=article.pk) + .distinct() + .order_by("-updated_at")[:RELATED_LIMIT] + ) + return [{"id": str(s.id), "title": s.title} for s in siblings] + + +class PortalArticleSuggestView(PortalBaseView): + """Deflection, at the moment the customer is about to file a request. + + Query-driven rather than case-driven: the agent suggester hangs off an + existing case, and here there is not one yet. That is the whole reason this + is a separate endpoint rather than a reuse of `SolutionSuggestionsView`. + """ + + def get(self, request): + q = (request.query_params.get("q") or "").strip() + if not q: + # No seeding from recent articles, unlike the agent side. An agent + # scanning the newest answers is doing their job; a customer shown + # three unrelated articles just learns to ignore the panel. + return Response({"articles": []}) + + results = ( + self._published_articles(request) + .filter(text_match(q)) + .order_by("-updated_at")[:SUGGEST_LIMIT] + ) + return Response( + { + "articles": [ + { + "id": str(article.id), + "title": article.title, + "snippet": snippet(article.description), + } + for article in results + ] + } + ) + + class PortalCaseCommentView(PortalBaseView): def post(self, request, pk): case = self._case_or_none(request, pk) diff --git a/backend/cases/solution_serializers.py b/backend/cases/solution_serializers.py index 2e94a101..9685bfce 100644 --- a/backend/cases/solution_serializers.py +++ b/backend/cases/solution_serializers.py @@ -6,7 +6,7 @@ from drf_spectacular.utils import extend_schema_field from rest_framework import serializers from cases.models import Solution -from common.serializer import OrganizationSerializer +from common.serializer import OrganizationSerializer, TagsSerializer def _author_name(obj): @@ -29,6 +29,9 @@ class SolutionSerializer(serializers.ModelSerializer): org = OrganizationSerializer(read_only=True) case_count = serializers.SerializerMethodField() author = serializers.SerializerMethodField() + # Read-only here and written in the view, so the org filter on the tag ids + # cannot be skipped. See `_apply_tags` in `solution_views`. + tags = TagsSerializer(read_only=True, many=True) class Meta: model = Solution @@ -39,6 +42,7 @@ class SolutionSerializer(serializers.ModelSerializer): "status", "is_published", "org", + "tags", "case_count", "author", "created_at", @@ -156,6 +160,7 @@ class SolutionDetailSerializer(serializers.ModelSerializer): linked_cases = serializers.SerializerMethodField() case_count = serializers.SerializerMethodField() author = serializers.SerializerMethodField() + tags = TagsSerializer(read_only=True, many=True) class Meta: model = Solution @@ -166,6 +171,7 @@ class SolutionDetailSerializer(serializers.ModelSerializer): "status", "is_published", "org", + "tags", "linked_cases", "case_count", "author", diff --git a/backend/cases/solution_views.py b/backend/cases/solution_views.py index 59dc9cc0..aea39d8e 100644 --- a/backend/cases/solution_views.py +++ b/backend/cases/solution_views.py @@ -2,6 +2,8 @@ Solution (Knowledge Base) Views """ +import json + from django.db.models import Count, Q from drf_spectacular.utils import OpenApiParameter, extend_schema, inline_serializer from rest_framework import serializers, status @@ -22,7 +24,9 @@ from cases.solution_serializers import ( SolutionDetailSerializer, SolutionSerializer, ) +from common.models import Tags from common.permissions import HasOrgContext +from common.validators import uuid_list_param def _wants_release(data, instance=None): @@ -53,6 +57,31 @@ def _as_bool(value): return str(value).strip().lower() in ("true", "1", "yes", "on") +def _apply_tags(solution, data, org): + """Set an article's tags from a request body, org-safely. + + Set here rather than on `SolutionCreateUpdateSerializer` so the org filter + is unavoidable: the ids arrive from the client, and resolving them against + `org` means a tag belonging to another org is simply not found, rather than + being attached and creating a cross-tenant link. + + Absent means unchanged, which is the difference between this and the case + endpoints. `cases` clears and re-adds whenever it runs, and + `AccountDetailView.put` clears tags outright; both mean a PATCH that never + mentions tags silently drops them. An explicit `"tags": []` still clears. + """ + if "tags" not in data: + return + + ids = data.get("tags") or [] + if isinstance(ids, str): + ids = json.loads(ids) + # The web form posts ids; some clients post whole tag objects. + ids = [item.get("id") if isinstance(item, dict) else item for item in ids] + + solution.tags.set(Tags.objects.filter(id__in=ids, org=org, is_active=True)) + + class SolutionListView(APIView, LimitOffsetPagination): """ List and create solutions (Knowledge Base) @@ -73,6 +102,9 @@ class SolutionListView(APIView, LimitOffsetPagination): OpenApiParameter( "search", str, description="Search in title and description" ), + OpenApiParameter( + "tags", str, description="Comma-separated tag ids; matches any of them" + ), ], responses={ 200: inline_serializer( @@ -96,6 +128,7 @@ class SolutionListView(APIView, LimitOffsetPagination): # serializer used to compute a row at a time. queryset = ( base.select_related("org", "created_by") + .prefetch_related("tags") .annotate(linked_case_count=Count("cases", distinct=True)) .order_by("-updated_at", "-id") ) @@ -119,6 +152,12 @@ class SolutionListView(APIView, LimitOffsetPagination): Q(title__icontains=search) | Q(description__icontains=search) ) + # `distinct()` because an article matching two of the requested tags + # would otherwise be returned twice, which also breaks the page count. + tag_ids = uuid_list_param(request.query_params, "tags") + if tag_ids: + queryset = queryset.filter(tags__id__in=tag_ids).distinct() + # Counted over the whole knowledge base, not over the filtered page. # These four are a partition of the KB, "12 total, 3 of them drafts" # , so recomputing them inside a `?status=draft` filter would leave @@ -160,6 +199,7 @@ class SolutionListView(APIView, LimitOffsetPagination): if serializer.is_valid(): solution = serializer.save(org=request.profile.org, created_by=request.user) + _apply_tags(solution, request.data, request.profile.org) response_serializer = SolutionSerializer(solution) return Response(response_serializer.data, status=status.HTTP_201_CREATED) @@ -220,6 +260,7 @@ class SolutionDetailView(APIView): ) if serializer.is_valid(): solution = serializer.save(updated_by=request.user) + _apply_tags(solution, request.data, request.profile.org) return Response(SolutionSerializer(solution).data) return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) diff --git a/backend/cases/tests/test_portal_articles.py b/backend/cases/tests/test_portal_articles.py new file mode 100644 index 00000000..4e05a60d --- /dev/null +++ b/backend/cases/tests/test_portal_articles.py @@ -0,0 +1,301 @@ +"""What a portal customer may read of the knowledge base, and everything they +may not. + +The agent-side rules live in `test_solution_access.py`. These are the customer +rules, and they are stricter: an article reaches a customer only once an admin +has both approved and released it. +""" + +import pytest +from rest_framework.test import APIClient + +from cases.models import Solution +from common.models import Tags +from common.portal_auth import mint_portal_token +from contacts.models import Contact + +LIST_URL = "/api/portal/articles/" +SUGGEST_URL = "/api/portal/articles/suggest/" + + +def _client(contact): + client = APIClient() + client.credentials(HTTP_AUTHORIZATION=f"Bearer {mint_portal_token(contact)}") + return client + + +def _article(org, title="Resetting your password", **kwargs): + """A published, approved article unless a test says otherwise.""" + fields = { + "description": "Open settings, choose security, then reset.", + "status": "approved", + "is_published": True, + } + fields.update(kwargs) + return Solution.objects.create(org=org, title=title, **fields) + + +@pytest.fixture +def billing(org_a): + return Tags.objects.create(org=org_a, name="Billing") + + +@pytest.fixture +def pat(org_a): + return Contact.objects.create( + org=org_a, first_name="Pat", last_name="Smith", email="pat@example.com" + ) + + +@pytest.mark.django_db +class TestPortalArticleList: + def test_lists_a_published_article(self, pat, org_a): + article = _article(org_a) + response = _client(pat).get(LIST_URL) + assert response.status_code == 200 + assert [a["id"] for a in response.json()["articles"]] == [str(article.id)] + + def test_hides_an_unpublished_article(self, pat, org_a): + _article(org_a, title="Not released", is_published=False) + assert _client(pat).get(LIST_URL).json()["articles"] == [] + + def test_hides_a_draft(self, pat, org_a): + _article(org_a, title="Half written", status="draft", is_published=False) + assert _client(pat).get(LIST_URL).json()["articles"] == [] + + def test_hides_a_legacy_published_draft(self, pat, org_a): + """A row from before the published-implies-approved rule existed. + + `SolutionSerializer.validate` refuses to create one now, so it is built + here the only way it can still arise: straight through the ORM. If the + portal trusted `is_published` alone this would be customer-visible. + """ + _article(org_a, title="Published by accident", status="draft") + assert _client(pat).get(LIST_URL).json()["articles"] == [] + + def test_hides_another_orgs_article(self, pat, org_a, org_b): + mine = _article(org_a) + _article(org_b, title="Someone else's answer") + ids = [a["id"] for a in _client(pat).get(LIST_URL).json()["articles"]] + assert ids == [str(mine.id)] + + def test_search_matches_the_title(self, pat, org_a): + wanted = _article(org_a, title="Resetting your password") + _article(org_a, title="Exporting invoices") + ids = [ + a["id"] + for a in _client(pat).get(LIST_URL + "?search=password").json()["articles"] + ] + assert ids == [str(wanted.id)] + + def test_search_matches_the_body(self, pat, org_a): + wanted = _article( + org_a, title="Billing", description="Your VAT number lives in settings." + ) + _article(org_a, title="Exporting invoices", description="Use the export tab.") + ids = [ + a["id"] + for a in _client(pat).get(LIST_URL + "?search=VAT").json()["articles"] + ] + assert ids == [str(wanted.id)] + + def test_search_does_not_reach_past_the_visibility_rule(self, pat, org_a, org_b): + """Searching must not become a second, wider way in.""" + _article(org_a, title="Password draft", status="draft", is_published=False) + _article(org_b, title="Password reset for another org") + assert _client(pat).get(LIST_URL + "?search=password").json()["articles"] == [] + + def test_anonymous_is_refused(self, org_a): + _article(org_a) + assert APIClient().get(LIST_URL).status_code in (401, 403) + + +@pytest.mark.django_db +class TestPortalArticleDetail: + def test_reads_a_published_article(self, pat, org_a): + article = _article(org_a, description="Open settings, then reset.") + response = _client(pat).get(f"{LIST_URL}{article.id}/") + assert response.status_code == 200 + assert response.json()["article"]["description"] == "Open settings, then reset." + + def test_a_draft_is_not_found(self, pat, org_a): + article = _article(org_a, status="draft", is_published=False) + assert _client(pat).get(f"{LIST_URL}{article.id}/").status_code == 404 + + def test_an_unpublished_article_is_not_found(self, pat, org_a): + article = _article(org_a, is_published=False) + assert _client(pat).get(f"{LIST_URL}{article.id}/").status_code == 404 + + def test_another_orgs_article_is_not_found(self, pat, org_b): + article = _article(org_b) + assert _client(pat).get(f"{LIST_URL}{article.id}/").status_code == 404 + + def test_anonymous_is_refused(self, org_a): + article = _article(org_a) + assert APIClient().get(f"{LIST_URL}{article.id}/").status_code in (401, 403) + + def test_the_projection_withholds_everything_internal(self, pat, org_a, case_a): + """The leak that would matter, pinned field by field. + + Adding any of these names to `PortalSolutionDetailSerializer.Meta.fields` + is the production change that breaks this test, which is the point: + the projection is the security control, so it needs a guard. + """ + article = _article(org_a) + article.cases.add(case_a) + + body = _client(pat).get(f"{LIST_URL}{article.id}/").json()["article"] + + assert set(body) == {"id", "title", "description", "updated_at"} + for withheld in ( + "status", + "is_published", + "org", + "created_by", + "author", + "linked_cases", + "cases", + ): + assert withheld not in body + + def test_a_linked_case_id_never_reaches_the_customer(self, pat, org_a, case_a): + """A published article can be linked to somebody else's ticket.""" + article = _article(org_a) + article.cases.add(case_a) + rendered = str(_client(pat).get(f"{LIST_URL}{article.id}/").json()) + assert str(case_a.id) not in rendered + + +@pytest.mark.django_db +class TestPortalRelatedArticles: + """Related articles come from the agents' tags, without exposing them. + + The tag vocabulary is shared with leads and deals and reads like "At Risk" + and "VIP", so it computes the list and never appears in it. + """ + + def test_relates_two_articles_sharing_a_tag(self, pat, org_a, billing): + one = _article(org_a, title="Understanding your invoice") + two = _article(org_a, title="Changing your billing address") + one.tags.add(billing) + two.tags.add(billing) + + body = _client(pat).get(f"{LIST_URL}{one.id}/").json() + assert [r["title"] for r in body["related"]] == [ + "Changing your billing address" + ] + + def test_never_relates_an_article_to_itself(self, pat, org_a, billing): + one = _article(org_a) + one.tags.add(billing) + body = _client(pat).get(f"{LIST_URL}{one.id}/").json() + assert body["related"] == [] + + def test_an_untagged_article_relates_to_nothing(self, pat, org_a, billing): + one = _article(org_a, title="Untagged") + two = _article(org_a, title="Tagged") + two.tags.add(billing) + body = _client(pat).get(f"{LIST_URL}{one.id}/").json() + assert body["related"] == [] + + def test_related_obeys_the_visibility_rule(self, pat, org_a, billing): + """A shared tag is not a way past `_published_articles`.""" + one = _article(org_a, title="Visible") + draft = _article(org_a, title="Draft", status="draft", is_published=False) + unreleased = _article(org_a, title="Unreleased", is_published=False) + for article in (one, draft, unreleased): + article.tags.add(billing) + + body = _client(pat).get(f"{LIST_URL}{one.id}/").json() + assert body["related"] == [] + + def test_related_never_crosses_an_org(self, pat, org_a, org_b, billing): + """Same tag *name* in two orgs is two different rows, but prove it.""" + one = _article(org_a, title="Mine") + theirs = _article(org_b, title="Theirs") + their_tag = Tags.objects.create(org=org_b, name="Billing") + one.tags.add(billing) + theirs.tags.add(their_tag) + + body = _client(pat).get(f"{LIST_URL}{one.id}/").json() + assert body["related"] == [] + + def test_related_is_capped(self, pat, org_a, billing): + one = _article(org_a, title="Anchor") + one.tags.add(billing) + for i in range(6): + sibling = _article(org_a, title=f"Sibling {i}") + sibling.tags.add(billing) + + body = _client(pat).get(f"{LIST_URL}{one.id}/").json() + assert len(body["related"]) == 3 + + def test_related_carries_no_tag_names(self, pat, org_a, billing): + one = _article(org_a, title="Anchor") + two = _article(org_a, title="Sibling") + one.tags.add(billing) + two.tags.add(billing) + + rendered = str(_client(pat).get(f"{LIST_URL}{one.id}/").json()) + assert "Billing" not in rendered + assert set(_client(pat).get(f"{LIST_URL}{one.id}/").json()["related"][0]) == { + "id", + "title", + } + + +@pytest.mark.django_db +class TestPortalArticleSuggest: + """Deflection: what the customer is shown while typing a new request.""" + + def test_suggests_a_matching_article(self, pat, org_a): + wanted = _article(org_a, title="Resetting your password") + _article(org_a, title="Exporting invoices") + response = _client(pat).get(SUGGEST_URL + "?q=password") + assert response.status_code == 200 + assert [a["id"] for a in response.json()["articles"]] == [str(wanted.id)] + + def test_suggest_is_not_swallowed_by_the_detail_route(self, pat, org_a): + """`` matches any segment and rejects in `to_python`. + + So this path only reaches the suggester because the resolver falls + through. Pinned here because reordering the two routes would break it + silently, returning "Article not found" for every suggestion. + """ + response = _client(pat).get(SUGGEST_URL + "?q=anything") + assert response.status_code == 200 + assert "articles" in response.json() + + def test_a_blank_query_suggests_nothing(self, pat, org_a): + """No guessing. An empty box is not a request for a random article.""" + _article(org_a) + assert _client(pat).get(SUGGEST_URL + "?q=").json()["articles"] == [] + + def test_a_missing_query_suggests_nothing(self, pat, org_a): + _article(org_a) + assert _client(pat).get(SUGGEST_URL).json()["articles"] == [] + + def test_obeys_the_visibility_rule(self, pat, org_a, org_b): + _article(org_a, title="Password draft", status="draft", is_published=False) + _article(org_a, title="Password unreleased", is_published=False) + _article(org_b, title="Password reset elsewhere") + assert _client(pat).get(SUGGEST_URL + "?q=password").json()["articles"] == [] + + def test_caps_the_number_returned(self, pat, org_a): + for i in range(10): + _article(org_a, title=f"Password article {i}") + body = _client(pat).get(SUGGEST_URL + "?q=password").json() + assert len(body["articles"]) == 3 + + def test_carries_a_snippet_not_the_whole_body(self, pat, org_a): + _article(org_a, title="Password", description="x" * 500) + snippet = ( + _client(pat) + .get(SUGGEST_URL + "?q=password") + .json()["articles"][0]["snippet"] + ) + assert len(snippet) < 500 + + def test_anonymous_is_refused(self, org_a): + _article(org_a) + assert APIClient().get(SUGGEST_URL + "?q=password").status_code in (401, 403) diff --git a/backend/cases/tests/test_solution_tags.py b/backend/cases/tests/test_solution_tags.py new file mode 100644 index 00000000..7a087ad6 --- /dev/null +++ b/backend/cases/tests/test_solution_tags.py @@ -0,0 +1,151 @@ +"""Tagging a knowledge base article. + +Tags are the agents' own vocabulary and are shared with leads, deals and +tickets, so this org's list holds things like "At Risk" and "VIP". None of it +is written for customers to read, which is why tags stay on the agent +serializers and the portal only ever uses them to compute related articles. +The projection guard in `test_portal_articles.py` is what enforces that. +""" + +import pytest +from django.db import connection + +from cases.models import Solution +from common.models import Tags + +SOLUTIONS_LIST_URL = "/api/cases/solutions/" + + +def _detail_url(pk): + return f"/api/cases/solutions/{pk}/" + + +def _set_rls(org): + """Set PostgreSQL RLS context so direct ORM writes are allowed. + No-op on SQLite (used in tests). + """ + if connection.vendor != "postgresql": + return + with connection.cursor() as cursor: + cursor.execute("SELECT set_config('app.current_org', %s, false)", [str(org.id)]) + + +@pytest.fixture +def billing_tag(org_a): + _set_rls(org_a) + return Tags.objects.create(org=org_a, name="Billing") + + +@pytest.fixture +def access_tag(org_a): + _set_rls(org_a) + return Tags.objects.create(org=org_a, name="Access") + + +@pytest.fixture +def article(org_a, admin_user): + _set_rls(org_a) + return Solution.objects.create( + org=org_a, + title="Resetting your password", + description="Open settings, then reset.", + status="draft", + created_by=admin_user, + ) + + +@pytest.mark.django_db +class TestTaggingAnArticle: + def test_creates_an_article_with_tags(self, admin_client, billing_tag): + response = admin_client.post( + SOLUTIONS_LIST_URL, + { + "title": "Understanding your invoice", + "description": "Each line is one seat for one month.", + "tags": [str(billing_tag.id)], + }, + format="json", + ) + assert response.status_code == 201 + article = Solution.objects.get(title="Understanding your invoice") + assert list(article.tags.all()) == [billing_tag] + + def test_cannot_attach_another_orgs_tag(self, admin_client, article, org_b): + """The ids come from the client, so the org filter is the whole defence.""" + _set_rls(org_b) + theirs = Tags.objects.create(org=org_b, name="Their internal label") + _set_rls(article.org) + + response = admin_client.patch( + _detail_url(article.pk), {"tags": [str(theirs.id)]}, format="json" + ) + assert response.status_code == 200 + assert list(article.tags.all()) == [] + + def test_cannot_attach_an_archived_tag(self, admin_client, article, org_a): + _set_rls(org_a) + retired = Tags.objects.create(org=org_a, name="Retired", is_active=False) + admin_client.patch( + _detail_url(article.pk), {"tags": [str(retired.id)]}, format="json" + ) + assert list(article.tags.all()) == [] + + def test_replaces_the_whole_set( + self, admin_client, article, billing_tag, access_tag + ): + article.tags.add(billing_tag) + admin_client.patch( + _detail_url(article.pk), {"tags": [str(access_tag.id)]}, format="json" + ) + assert list(article.tags.all()) == [access_tag] + + def test_a_patch_that_omits_tags_keeps_them( + self, admin_client, article, billing_tag + ): + """The bug this endpoint deliberately does not copy. + + `cases` clears and re-adds, and `AccountDetailView.put` clears outright, + so editing a title there drops every tag. Absent means unchanged here. + """ + article.tags.add(billing_tag) + admin_client.patch(_detail_url(article.pk), {"title": "Renamed"}, format="json") + assert list(article.tags.all()) == [billing_tag] + + def test_an_explicit_empty_list_clears_them( + self, admin_client, article, billing_tag + ): + article.tags.add(billing_tag) + admin_client.patch(_detail_url(article.pk), {"tags": []}, format="json") + assert list(article.tags.all()) == [] + + +@pytest.mark.django_db +class TestAgentsSeeTags: + def test_the_detail_payload_names_the_tags( + self, admin_client, article, billing_tag + ): + article.tags.add(billing_tag) + body = admin_client.get(_detail_url(article.pk)).json() + assert [t["name"] for t in body["tags"]] == ["Billing"] + + def test_the_list_payload_names_the_tags(self, admin_client, article, billing_tag): + article.tags.add(billing_tag) + row = next( + r + for r in admin_client.get(SOLUTIONS_LIST_URL).json()["results"] + if r["id"] == str(article.id) + ) + assert [t["name"] for t in row["tags"]] == ["Billing"] + + def test_filters_the_list_by_tag( + self, admin_client, article, billing_tag, access_tag, org_a, admin_user + ): + article.tags.add(billing_tag) + _set_rls(org_a) + other = Solution.objects.create( + org=org_a, title="Signing in", description="...", created_by=admin_user + ) + other.tags.add(access_tag) + + body = admin_client.get(f"{SOLUTIONS_LIST_URL}?tags={access_tag.id}").json() + assert [r["id"] for r in body["results"]] == [str(other.id)] diff --git a/backend/common/portal_urls.py b/backend/common/portal_urls.py index 911db25d..084cdce8 100644 --- a/backend/common/portal_urls.py +++ b/backend/common/portal_urls.py @@ -26,6 +26,24 @@ urlpatterns = [ portal_auth_views.PortalLoginVerifyView.as_view(), name="login_verify", ), + path( + "articles/", portal_views.PortalArticleListView.as_view(), name="article_list" + ), + # Before `articles//`. The `uid` converter's regex is `[^/]+`, so it + # matches the literal "suggest" and only declines it later, in `to_python`. + # The resolver does fall through to the next pattern, so both orderings + # happen to work, but relying on that is a trap for whoever adds the next + # non-uuid segment here. + path( + "articles/suggest/", + portal_views.PortalArticleSuggestView.as_view(), + name="article_suggest", + ), + path( + "articles//", + portal_views.PortalArticleDetailView.as_view(), + name="article_detail", + ), path("cases/", portal_views.PortalCaseListView.as_view(), name="case_list"), path( "cases//", diff --git a/backend/common/tests/test_tags.py b/backend/common/tests/test_tags.py index cc3c907a..cb6a2c86 100644 --- a/backend/common/tests/test_tags.py +++ b/backend/common/tests/test_tags.py @@ -8,7 +8,7 @@ import pytest from rest_framework import status from accounts.models import Account -from cases.models import Case +from cases.models import Case, Solution from common.models import APISettings, Tags from contacts.models import Contact from leads.models import Lead @@ -193,6 +193,7 @@ class TestTagsUsageAndTotals: "contacts": 1, "tasks": 1, "api_settings": 0, + "solutions": 0, } def test_unused_tag_reports_zero_usage(self, admin_client, org_a): @@ -206,9 +207,10 @@ class TestTagsUsageAndTotals: "contacts": 0, "tasks": 0, "api_settings": 0, + "solutions": 0, } - @pytest.mark.parametrize("kind", ["contacts", "tasks", "api_settings"]) + @pytest.mark.parametrize("kind", ["contacts", "tasks", "api_settings", "solutions"]) def test_a_tag_used_only_off_the_headline_models_is_not_unused( self, admin_client, org_a, kind ): @@ -227,6 +229,10 @@ class TestTagsUsageAndTotals: Task.objects.create( org=org_a, title="Follow up", status="New", priority="Medium" ).tags.add(tag) + elif kind == "solutions": + Solution.objects.create( + org=org_a, title="Resetting your password", description="..." + ).tags.add(tag) else: APISettings.objects.create( org=org_a, title="Site", website="https://example.com" diff --git a/backend/common/views/tags_views.py b/backend/common/views/tags_views.py index 712103bc..731e8f0a 100644 --- a/backend/common/views/tags_views.py +++ b/backend/common/views/tags_views.py @@ -10,7 +10,7 @@ from rest_framework.response import Response from rest_framework.views import APIView from accounts.models import Account -from cases.models import Case +from cases.models import Case, Solution from common import swagger_params from common.lookups import get_scoped_or_404 from common.models import APISettings, Tags @@ -43,6 +43,7 @@ _TAGGABLE = ( ("contacts", Contact), ("tasks", Task), ("api_settings", APISettings), + ("solutions", Solution), ) diff --git a/backend/pyproject.toml b/backend/pyproject.toml index 31941f6b..4e2b1c5d 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "django-crm" -version = "1.4.0" +version = "1.5.0" # The one-line summary PyPI shows under the project title and in search results, # so it carries the terms people actually search for ("open source CRM", # "self-hosted CRM", "Django CRM") and names the modules rather than the diff --git a/backend/uv.lock b/backend/uv.lock index 6dc95932..638db6e0 100644 --- a/backend/uv.lock +++ b/backend/uv.lock @@ -583,7 +583,7 @@ wheels = [ [[package]] name = "django-crm" -version = "1.4.0" +version = "1.5.0" source = { editable = "." } dependencies = [ { name = "cairocffi" }, diff --git a/docs/api/cases.md b/docs/api/cases.md index cb57d2c4..9ed9c9c2 100644 --- a/docs/api/cases.md +++ b/docs/api/cases.md @@ -313,30 +313,49 @@ delete author · admin `release` is deliberately narrower than `write`: the review workflow (`draft → reviewed → approved`, plus a separate `is_published` flag) only means something if the person who approves an article is not its own author. `assert_solution_release_access` takes no article argument at all. Being the author is -the one thing that must not grant it (`cases/kb_access.py:72-81`). +the one thing that must not grant it (`cases/kb_access.py:76-86`). -`GET /api/cases/solutions/` (`SolutionListView.get`, `cases/solution_views.py:88-142`) is the one +`GET /api/cases/solutions/` (`SolutionListView.get`, `cases/solution_views.py:120-180`) is the one endpoint in this API that calls DRF's `get_paginated_response()` and returns its standard `count`/`next`/`previous`/`results` envelope, [Conventions](conventions.md#pagination) names this exact endpoint as the sole example. It adds one extra top-level key, `totals` (`count`/`published`/`draft`/`reviewed`/`approved_unpublished`), computed over the *whole* knowledge base rather than the current filter, so the four status cards stay meaningful under a `?status=` -filter (`:122-134`). Filters: `?status=`, `?is_published=` (accepts `true`/`1`/`yes`/`on`, not just the -literal string `"true"`), `?search=` (title or description, contains). +filter (`:165-177`). Filters: `?status=`, `?is_published=` (accepts `true`/`1`/`yes`/`on`, not just the +literal string `"true"`), `?search=` (title or description, contains), and `?tags=` (comma-separated +tag ids, matching any of them, `distinct()` so an article carrying two of them is not returned twice). -`POST /api/cases/solutions/` (`:150-167`) is open to any member for an ordinary draft, but is gated by +`POST /api/cases/solutions/` (`:186-208`) is open to any member for an ordinary draft, but is gated by `assert_solution_release_access` the moment the payload would create an already-`approved` or -already-`is_published` article in one request (`_wants_release`, `:28-46`): without this, an author +already-`is_published` article in one request (`_wants_release`, `:32-50`): without this, an author could write and publish an article in a single `POST`, bypassing the review workflow entirely. -`GET/PUT/PATCH/DELETE /api/cases/solutions/{id}/` (`SolutionDetailView`, `:170-237`): `GET` is open to +`GET/PUT/PATCH/DELETE /api/cases/solutions/{id}/` (`SolutionDetailView`, `:210-278`): `GET` is open to any member; `PUT`/`PATCH` require `assert_solution_write_access` (author or admin) plus, again, `assert_solution_release_access` if the request would move the article to `approved` or flip `is_published`; `DELETE` requires the same write rule (not a separate, narrower one. Deleting your own draft is ordinary tidying). +**Tags.** Articles carry the same org-scoped `Tags` as leads, deals and tickets. They are read on +the list and detail payloads as `tags` and written by passing `tags` (a list of tag ids) to `POST` +or `PATCH`. The write is handled in the view rather than on the serializer, so the org filter is +unavoidable: ids are resolved against `Tags.objects.filter(org=…, is_active=True)`, and an id +belonging to another org is not found rather than attached. + +Absent means unchanged, and only an explicit `"tags": []` clears them. This differs from the case +endpoints on purpose, where the tag set is cleared and rebuilt whenever the key is handled, so a +`PATCH` that never mentions tags drops them. + +Tag names are **agent-facing only**. They drive `related` on the portal article endpoint but never +appear in a portal payload, because the vocabulary is shared with deals and reads like "At Risk" +and "VIP". See [Customer portal](customer-portal.md#help-articles). + +**Publishing reaches customers.** An article that is both `approved` and `is_published` is readable +by any signed-in portal contact in the org and is suggested to them while they file a request, as +well as feeding the agent suggester. Unpublishing withdraws both. + `POST /api/cases/solutions/{id}/publish/` and `POST /api/cases/solutions/{id}/unpublish/` -(`SolutionPublishView`/`SolutionUnpublishView`, `:240-295`) both require +(`SolutionPublishView`/`SolutionUnpublishView`, `:281-336`) both require `assert_solution_release_access`. Publishing additionally requires `status == "approved"` (`400` otherwise); the comment on the unpublish view is explicit that pulling a bad answer down is the *same* switch as giving it to customers and therefore gated the same way, even though the urgency argument diff --git a/docs/api/conventions.md b/docs/api/conventions.md index 1893306a..aec8d877 100644 --- a/docs/api/conventions.md +++ b/docs/api/conventions.md @@ -101,7 +101,7 @@ context["open_leads"] = { (`backend/leads/views/lead_views.py:180-184`.) But `get_paginated_response()`, and with it, DRF's standard `count`/`next`/`previous`/`results` shape, does appear: `GET /api/cases/solutions/` -(`SolutionListView`, `backend/cases/solution_views.py:56`, routed at `backend/cases/urls.py:59`) +(`SolutionListView`, `backend/cases/solution_views.py:85`, routed at `backend/cases/urls.py:59`) calls it directly and adds one extra key on top: ```python @@ -110,7 +110,7 @@ response.data["totals"] = counts return response ``` -(`backend/cases/solution_views.py:140-142`.) So both shapes are real. Treat the exact pagination +(`backend/cases/solution_views.py:178-180`.) So both shapes are real. Treat the exact pagination field names, and whether the standard DRF envelope or a hand-rolled one is used, as per-endpoint: `limit`/`offset` as query parameters are consistent, but where the resulting count, next-offset and results end up in the response body is not; check the endpoint you're calling. diff --git a/docs/api/customer-portal.md b/docs/api/customer-portal.md index 6cda3aa3..aec01337 100644 --- a/docs/api/customer-portal.md +++ b/docs/api/customer-portal.md @@ -8,7 +8,8 @@ for why the separation is enforced in three independent places rather than one. A portal caller is a `Contact` row, not a `User` and not a `Profile`. They have no password, no Google account, no role, and no presence in your org's member list. What they can reach is the -support cases they are named on, and nothing else. +support cases they are named on, plus the help articles their org has approved and published, and +nothing else. !!! warning "Two different things are called "portal" in this codebase" The **invoice and estimate portal** (`/api/public/…`) is a tokenised link emailed to a @@ -203,6 +204,96 @@ On the agent side the reply arrives as an ordinary comment with `commented_by` n (see [Cases](cases.md)). It counts as a customer reply everywhere that distinction matters: first-response SLA stamping, and auto-reopen on a closed ticket. +## Help articles + +The customer-readable half of the knowledge base. Three endpoints, all read-only, all requiring a +portal token. + +**What a customer can see is narrower than what an agent can see.** The agent suggester +(`GET /api/cases/{id}/solution-suggestions/`, see [Cases](cases.md)) shows anything published; these +endpoints require the article to be **both `is_published` and `status="approved"`**, and to belong +to the token's org. `PortalBaseView._published_articles` is the single place that decides this, the +way `_my_cases` is for cases. + +Requiring both is not belt and braces for its own sake. The published-implies-approved rule is +enforced on write by `SolutionSerializer.validate`, but it arrived after the model did, so rows +created before it can be published drafts. Those stay invisible to customers, and the first edit to +such a row repairs the pair. + +### List + +`GET /api/portal/articles/` + +```json +{ + "articles": [ + { + "id": "", + "title": "Resetting your password", + "updated_at": "2026-08-16T09:12:00Z" + } + ], + "articles_count": 1 +} +``` + +Ordered by title, `LimitOffsetPagination`. Optional `?search=` matches the title or the body, +substring, case-insensitive. The search is narrowed from the same visible set as the list, never +applied to `Solution.objects`, so it cannot become a second and wider way in. + +### Detail + +`GET /api/portal/articles/{id}/` + +```json +{ + "article": { + "id": "", + "title": "Resetting your password", + "description": "Open Settings, choose Security, then Reset password.", + "updated_at": "2026-08-16T09:12:00Z" + }, + "related": [ + {"id": "", "title": "Changing your billing address"} + ] +} +``` + +Four fields, and that is the whole projection. Absent on purpose: `status` and `is_published` (the +queryset has already decided the customer may read this, so repeating the decision in the payload +only creates something to leak), `created_by` (which colleague wrote the answer is internal, the +same rule that renders every agent comment as `"Support"`), and `linked_cases`, which would hand +this customer other customers' case ids. + +`related` is computed from the **agents' tags** and never contains them. The tag vocabulary is +shared with leads and deals and reads like "At Risk" and "VIP", so it selects the rows and does not +appear in the response: ids and titles only, capped at 3, excluding the article itself. It is built +from `_published_articles`, so a shared tag cannot reach a draft or another org's article. +Relatedness narrows the visible set; it never widens it. + +A draft, an unpublished article, and another org's article all return the same `404` +`{"error": "Article not found"}` as an id that never existed. + +### Suggestions + +`GET /api/portal/articles/suggest/?q=` + +```json +{ + "articles": [ + {"id": "", "title": "Resetting your password", "snippet": "Open Settings, choose…"} + ] +} +``` + +Deflection for the new-request form: the customer types a summary, and this answers with up to +three matching articles before the request is sent. Query-driven rather than case-driven, because +at that moment no case exists yet, which is why it is a separate endpoint from the agent suggester. + +A blank or missing `q` returns an empty list. There is no seeding from recent articles, unlike the +agent side: an agent scanning the newest answers is doing their job, while a customer shown three +unrelated articles just learns to ignore the panel. + ## Notifications When an agent posts a public reply, or a case changes status, every contact named on that case who diff --git a/docs/api/endpoint-index.md b/docs/api/endpoint-index.md index 408fd607..0a307566 100644 --- a/docs/api/endpoint-index.md +++ b/docs/api/endpoint-index.md @@ -188,6 +188,9 @@ Interactive versions of the same schema are served by a running backend at | `/api/packs/` | GET | | `/api/packs/sample-data/` | DELETE | | `/api/packs/{pack_id}/apply/` | POST | +| `/api/portal/articles/` | GET | +| `/api/portal/articles/suggest/` | GET | +| `/api/portal/articles/{id}/` | GET | | `/api/portal/cases/` | GET, POST | | `/api/portal/cases/{id}/` | GET | | `/api/portal/cases/{id}/comment/` | POST | diff --git a/frontend/src/lib/server/portal.js b/frontend/src/lib/server/portal.js index 741b4384..4a3157d7 100644 --- a/frontend/src/lib/server/portal.js +++ b/frontend/src/lib/server/portal.js @@ -76,6 +76,14 @@ export const createCase = (token, body) => call('/cases/', { method: 'POST', tok export const postReply = (token, id, comment) => call(`/cases/${id}/comment/`, { method: 'POST', token, body: { comment } }); +export const listArticles = (token, search) => + call(`/articles/${search ? `?search=${encodeURIComponent(search)}` : ''}`, { token }); + +export const getArticle = (token, id) => call(`/articles/${id}/`, { token }); + +export const suggestArticles = (token, q) => + call(`/articles/suggest/?q=${encodeURIComponent(q)}`, { token }); + const UUID = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; /** diff --git a/frontend/src/lib/server/v2/solutions.js b/frontend/src/lib/server/v2/solutions.js index ce03d7fb..c940066f 100644 --- a/frontend/src/lib/server/v2/solutions.js +++ b/frontend/src/lib/server/v2/solutions.js @@ -22,12 +22,12 @@ * - `author` is new on the wire. `created_by` serialized as a bare `User` * UUID, so the Author column in the mock was showing a name the endpoint * could not have supplied. - * - **"Related articles" is gone.** The mock's detail page listed three other - * articles under that heading with no explanation of the relation, and - * there is no endpoint that computes one, no tags, no embeddings, no - * "customers also read". In its place the page lists the **tickets this - * article is filed against**, which is a relation the database actually - * holds, and which closes the loop: the ticket already links to the article. + * - **"Related articles" was gone, and is back with a defined relation.** The + * mock listed three articles under that heading with no explanation of what + * related meant, and nothing computed it. Articles now carry tags, so the + * relation is "filed under the same tag", and the portal article page shows + * it. This agent page still lists the **tickets the article is filed + * against** instead, which is the relation an agent is actually chasing. * - The mock's new-article form said an article could be published while still * a draft; "that is allowed, and sometimes right". It is not: the model's * `publish()` refuses it and the serializer now refuses it on every path. @@ -51,7 +51,7 @@ export const SOLUTION_STATUSES = ['draft', 'reviewed', 'approved']; * param the page's own load translates to `is_published` before the query * ever reaches this module, the same way contacts translates `inactive`. */ -export const FILTER_FIELDS = ['status']; +export const FILTER_FIELDS = ['status', 'tags']; /** * The subset of a Django solution the v2 pages read. @@ -69,6 +69,9 @@ function toRow(row) { // the total. See `getArticle` for why that can exceed the rows shown. use_count: row.case_count ?? 0, author: row.author ?? '', + // Agent-side only. The portal serializers do not carry tags at all, on + // purpose: the vocabulary is shared with deals and reads like "At Risk". + tags: (row.tags ?? []).map((/** @type {any} */ t) => ({ id: t.id, name: t.name })), created_at: row.created_at, updated_at: row.updated_at ?? row.created_at, // Derived here rather than in three templates: approved and unreleased is @@ -154,8 +157,15 @@ export async function getArticle({ cookies }, id) { }; } -/** Scalar fields the article forms own. Everything else is server-derived. */ -export const EDITABLE_FIELDS = ['title', 'description', 'status', 'is_published']; +/** + * Fields the article forms own. Everything else is server-derived. + * + * `tags` is the one non-scalar. It carries tag ids, and the API resolves them + * against the caller's own org before attaching any, so an id from elsewhere + * is dropped rather than linked. Sending it at all is optional: the API treats + * an absent `tags` as "leave them alone" and only an explicit `[]` as "clear". + */ +export const EDITABLE_FIELDS = ['title', 'description', 'status', 'is_published', 'tags']; /** * Turn form values into a request body. @@ -216,11 +226,11 @@ export async function createArticle({ cookies }, values) { } /** - * Release an article into the ticket reply panel, or take it back out. + * Release an article to customers, or take it back. * - * Not to customers: `is_published` gates the agent suggester in - * `cases/kb_views.py:85` and nothing else, because the customer-facing site it - * was meant to gate was cut. See `cases/kb_access.py:26-28`. + * `is_published` gates two surfaces: the agent suggester in `cases/kb_views.py` + * and, for an article that is also approved, the customer portal + * (`PortalBaseView._published_articles`). Unpublishing withdraws both. * * Both are admin-only and the API says so with a 403. The UI hides the * buttons from everybody else, but that is a courtesy and not the control. diff --git a/frontend/src/lib/v2/filters.js b/frontend/src/lib/v2/filters.js index 0f645749..90778d9e 100644 --- a/frontend/src/lib/v2/filters.js +++ b/frontend/src/lib/v2/filters.js @@ -235,7 +235,8 @@ export const FILTERS = { type: 'select', options: SOLUTION_STATUS, labelFor: (v) => SOLUTION_STATUS_LABEL[v] ?? v - } + }, + { key: 'tags', label: 'Tag', type: 'tag' } ] }, diff --git a/frontend/src/routes/(app)/solutions/+page.server.js b/frontend/src/routes/(app)/solutions/+page.server.js index 6221ba53..6ca81197 100644 --- a/frontend/src/routes/(app)/solutions/+page.server.js +++ b/frontend/src/routes/(app)/solutions/+page.server.js @@ -1,6 +1,7 @@ import { fail } from '@sveltejs/kit'; import { listArticles, setPublished, FILTER_FIELDS } from '$lib/server/v2/solutions.js'; import { readFilters, buildFilterQuery } from '$lib/server/v2/filter-params.js'; +import { getTags } from '$lib/server/v2/tags.js'; import { readableError } from '$lib/server/v2/form-errors.js'; /** @@ -21,11 +22,17 @@ export async function load({ cookies, locals, url }) { if (visibility === 'published') params.set('is_published', 'true'); if (visibility === 'internal') params.set('is_published', 'false'); - const { results, totals } = await listArticles({ cookies }, params); + const [{ results, totals }, tagList] = await Promise.all([ + listArticles({ cookies }, params), + // Same degradation as the ticket queue: losing the tag list should cost + // the Tag dropdown, not the whole knowledge base. + getTags({ cookies }).catch(() => ({ tags: [] })) + ]); return { articles: results, totals, + tags: tagList.tags ?? [], search: params.get('search') ?? '', status: params.get('status') ?? '', visibility, diff --git a/frontend/src/routes/(app)/solutions/+page.svelte b/frontend/src/routes/(app)/solutions/+page.svelte index a6d9a5f9..15ecfb83 100644 --- a/frontend/src/routes/(app)/solutions/+page.svelte +++ b/frontend/src/routes/(app)/solutions/+page.svelte @@ -5,10 +5,10 @@ * An article can be approved and still not suggested on tickets; v1 showed * status alone, so those read as live and nobody published them. * - * Publishing does not reach a customer. `is_published` gates the agent - * suggester in `cases/kb_views.py:85` and nothing else, because the - * customer-facing site it was meant to gate was cut. See - * `cases/kb_access.py:26-28`. + * Publishing does reach a customer. An article that is both approved and + * published is readable in the portal by any signed-in contact in the org, + * and is suggested to them while they file a ticket. It also feeds the agent + * suggester. Unpublishing withdraws both. * * Now on the real API, which changed one thing about that claim: publishing * is admin-only, so for everybody else the approved-and-unpublished row is @@ -35,14 +35,13 @@ {#snippet sub()} - + {count(totals.count)} {totals.count === 1 ? 'article' : 'articles'} · - {count(totals.published)} suggested to agents + {count(totals.published)} live to customers {/snippet} {#snippet actions()} New article @@ -80,7 +79,8 @@ {#if form?.error} @@ -136,14 +136,14 @@ - Suggested on tickets + Live to customers {:else} - Not suggested + Internal only {/if} diff --git a/frontend/src/routes/(app)/solutions/[id]/+page.server.js b/frontend/src/routes/(app)/solutions/[id]/+page.server.js index 1e353196..7cfe4bfa 100644 --- a/frontend/src/routes/(app)/solutions/[id]/+page.server.js +++ b/frontend/src/routes/(app)/solutions/[id]/+page.server.js @@ -35,7 +35,7 @@ export const actions = { }, /** - * Put the article into the ticket reply panel, or take it back out. + * Release the article to customers, or take it back. * * The two verbs are one action with a flag rather than two nearly identical * ones, because the thing they have in common. Both are admin-only, both diff --git a/frontend/src/routes/(app)/solutions/[id]/edit/+page.server.js b/frontend/src/routes/(app)/solutions/[id]/edit/+page.server.js index 173517bb..b86368ba 100644 --- a/frontend/src/routes/(app)/solutions/[id]/edit/+page.server.js +++ b/frontend/src/routes/(app)/solutions/[id]/edit/+page.server.js @@ -1,5 +1,6 @@ import { fail, redirect } from '@sveltejs/kit'; import { getArticle, updateArticle } from '$lib/server/v2/solutions.js'; +import { getTags } from '$lib/server/v2/tags.js'; import { readableError } from '$lib/server/v2/form-errors.js'; /** @@ -13,10 +14,17 @@ export async function load({ cookies, locals, params }) { return { article, canRelease: /** @type {any} */ (locals).profile?.role === 'ADMIN', + // Archived tags are filtered out for the reason given on the create page: + // `_apply_tags` refuses them, so the checkbox would do nothing. One caveat + // worth knowing: an article already carrying a tag that was archived later + // keeps it, because this form never sends a box it did not render, and the + // API only changes what it is told about. + tags: ((await getTags({ cookies })).tags ?? []).filter((t) => t.is_active), form: { title: article.title, description: article.description, - status: article.status + status: article.status, + tags: (article.tags ?? []).map((/** @type {any} */ t) => t.id) } }; } @@ -29,7 +37,12 @@ export const actions = { /** @type {Record} */ const values = { title: form.get('title')?.toString().trim() ?? '', - description: form.get('description')?.toString().trim() ?? '' + description: form.get('description')?.toString().trim() ?? '', + // Always sent, unlike `status`. The form renders every active tag as a + // checkbox, so an unticked box is a deliberate "not this one" and an + // empty list is a deliberate "none". Tagging is not gated on a role, so + // there is no 403 to dodge by staying silent. + tags: form.getAll('tags').map(String) }; /* diff --git a/frontend/src/routes/(app)/solutions/[id]/edit/+page.svelte b/frontend/src/routes/(app)/solutions/[id]/edit/+page.svelte index fda80048..4960af26 100644 --- a/frontend/src/routes/(app)/solutions/[id]/edit/+page.svelte +++ b/frontend/src/routes/(app)/solutions/[id]/edit/+page.svelte @@ -2,7 +2,7 @@ import { resolve } from '$app/paths'; /** * Editing an article is editing its words. The two decisions about it, - * approving it, and publishing it into the ticket reply panel, are buttons + * approving it, and releasing it to customers, are buttons * on the article page, because they are things somebody does once and not * fields somebody fills in. * @@ -24,8 +24,18 @@ let title = $state(untrack(() => form?.values?.title ?? data.form.title)); let description = $state(untrack(() => form?.values?.description ?? data.form.description)); let status = $state(untrack(() => form?.values?.status ?? data.form.status)); + // An array rather than a Set: a knowledge base has a handful of tags, so + // `includes` costs nothing, and Svelte's reactivity has no special case to + // fall foul of. Seeded from the failed save first so a rejected submit comes + // back with the same chips lit, then from the article's own tags. + let picked = $state(untrack(() => (form?.values?.tags ?? data.form.tags ?? []).map(String))); let saving = $state(false); + /** @param {string} id */ + function toggleTag(id) { + picked = picked.includes(id) ? picked.filter((x) => x !== id) : [...picked, id]; + } + let statuses = $derived( data.canRelease ? SOLUTION_STATUS : SOLUTION_STATUS.filter((s) => s !== 'approved') ); @@ -83,7 +93,8 @@ {#if data.article.is_published}

- This article is published. Anything saved here is what agents are offered on tickets. + This article is published, so anything saved here is what customers read in the portal and + what agents are offered on tickets.

{/if} @@ -101,6 +112,33 @@ + {#if data.tags.length > 0} +
+
Tags
+ +

+ Customers never see these names, but articles sharing one are shown to each other in the + portal. +

+
+ {#each data.tags as tag (tag.id)} + + {/each} +
+
+ {/if} +
diff --git a/frontend/src/routes/(no-layout)/portal/articles/[id]/+page.server.js b/frontend/src/routes/(no-layout)/portal/articles/[id]/+page.server.js new file mode 100644 index 00000000..322ba415 --- /dev/null +++ b/frontend/src/routes/(no-layout)/portal/articles/[id]/+page.server.js @@ -0,0 +1,45 @@ +/** + * One help article. + * + * A 404 from the API means "no such article, or not one you may read", and the + * two are deliberately indistinguishable there. This page keeps them + * indistinguishable too: both become the same 404, so guessing ids tells a + * visitor nothing about which drafts exist. + */ + +import { error, redirect } from '@sveltejs/kit'; +import { + ACCESS_COOKIE, + ORG_COOKIE, + PortalError, + clearSession, + getArticle, + loginPath +} from '$lib/server/portal'; + +/** Send an expired or missing session back to the right org's sign-in page. */ +function toLogin(cookies) { + const org = cookies.get(ORG_COOKIE); + clearSession(cookies); + throw redirect(303, loginPath(org)); +} + +/** @type {import('./$types').PageServerLoad} */ +export async function load({ cookies, params }) { + const token = cookies.get(ACCESS_COOKIE); + if (!token) toLogin(cookies); + + try { + const data = await getArticle(token, params.id); + // `related` carries ids and titles only. The API computes it from the + // agents' tags and deliberately does not send the tag names, which read + // like "At Risk" and "VIP" and are not written for customers. + return { article: data.article, related: data.related ?? [] }; + } catch (err) { + if (err instanceof PortalError) { + if (err.status === 401 || err.status === 403) toLogin(cookies); + if (err.status === 404) throw error(404, 'Article not found'); + } + throw err; + } +} diff --git a/frontend/src/routes/(no-layout)/portal/articles/[id]/+page.svelte b/frontend/src/routes/(no-layout)/portal/articles/[id]/+page.svelte new file mode 100644 index 00000000..b780eb60 --- /dev/null +++ b/frontend/src/routes/(no-layout)/portal/articles/[id]/+page.svelte @@ -0,0 +1,144 @@ + + + + {data.article.title} + + + + Back to help articles + +
+

{data.article.title}

+ {#if data.article.updated_at} +

Updated {formatDate(data.article.updated_at)}

+ {/if} +
{data.article.description}
+
+ + {#if data.related.length > 0} + + {/if} + + +
+

Still stuck?

+ Send us a request +
+
+ + diff --git a/frontend/src/routes/(no-layout)/portal/articles/suggestions/+server.js b/frontend/src/routes/(no-layout)/portal/articles/suggestions/+server.js new file mode 100644 index 00000000..e2811ce8 --- /dev/null +++ b/frontend/src/routes/(no-layout)/portal/articles/suggestions/+server.js @@ -0,0 +1,39 @@ +/** + * Typeahead deflection for the new-request form. + * + * This exists because `portal_access` is httpOnly, so the page's own script + * cannot call the API with it. Rather than relax the cookie, the browser asks + * this same-origin route and the server attaches the token. A cookie a script + * can read is a cookie an injected script can steal, and that trade is not + * worth a suggestions panel. + * + * A static segment, so it is ranked ahead of the sibling `[id]` article page + * and never resolves as an article whose id happens to be "suggestions". + */ + +import { json } from '@sveltejs/kit'; +import { ACCESS_COOKIE, PortalError, suggestArticles } from '$lib/server/portal'; + +// Below this a query matches most of the knowledge base, which is noise rather +// than a suggestion. +const MIN_QUERY = 3; + +const EMPTY = { articles: [] }; + +/** @type {import('./$types').RequestHandler} */ +export async function GET({ cookies, url }) { + const token = cookies.get(ACCESS_COOKIE); + const q = (url.searchParams.get('q') || '').trim(); + + // No session and no match both answer the same way. This route is a hint + // beside a form: it must never redirect the page or surface an error banner + // over what the customer is in the middle of typing. + if (!token || q.length < MIN_QUERY) return json(EMPTY); + + try { + return json(await suggestArticles(token, q)); + } catch (err) { + if (err instanceof PortalError) return json(EMPTY); + throw err; + } +} diff --git a/frontend/src/routes/(no-layout)/portal/cases/+page.svelte b/frontend/src/routes/(no-layout)/portal/cases/+page.svelte index ee193e40..52703a44 100644 --- a/frontend/src/routes/(no-layout)/portal/cases/+page.svelte +++ b/frontend/src/routes/(no-layout)/portal/cases/+page.svelte @@ -12,6 +12,36 @@ let composing = $state(false); + /** + * Deflection: show the answer before the request is sent. + * + * Debounced so a normal typing speed produces one request per pause rather + * than one per keystroke, and sequenced so a slow early response cannot + * overwrite the results of a later query. + */ + let summary = $state(''); + let suggestions = $state([]); + let latestQuery = 0; + let debounce; + + function findAnswers() { + clearTimeout(debounce); + const query = summary.trim(); + if (query.length < 3) { + suggestions = []; + return; + } + debounce = setTimeout(async () => { + const ticket = ++latestQuery; + const response = await fetch( + `${resolve('/portal/articles/suggestions')}?q=${encodeURIComponent(query)}` + ); + if (!response.ok) return; + const body = await response.json(); + if (ticket === latestQuery) suggestions = body.articles ?? []; + }, 300); + } + const FILTERS = [ { value: '', label: 'All' }, { value: 'New', label: 'New' }, @@ -38,15 +68,41 @@

Your requests

- +
+ Help + +
{#if composing}
- + + + {#if suggestions.length > 0} + + {/if} @@ -119,6 +175,70 @@ background: none; cursor: pointer; } + .actions { + display: flex; + align-items: center; + gap: 8px; + } + .btn { + display: inline-flex; + align-items: center; + min-height: 44px; + padding: 0 14px; + font-size: 14px; + border: 1px solid var(--v2-rule, #d1d5db); + border-radius: 8px; + text-decoration: none; + color: inherit; + } + .deflect { + margin-top: 14px; + padding: 12px 14px; + border: 1px solid var(--v2-rule, #e5e7eb); + border-radius: 10px; + background: var(--v2-rule, #f9fafb); + } + .deflect-head { + margin: 0 0 8px; + font-size: 12.5px; + font-weight: 500; + color: var(--v2-slate, #6b7280); + } + .deflect ul { + list-style: none; + margin: 0; + padding: 0; + display: grid; + gap: 6px; + } + .deflect a { + display: block; + /* Padding rather than min-height: these stack, and 44px of dead space per + row pushes the Send button off a 390px screen. */ + padding: 10px 12px; + border-radius: 8px; + background: var(--v2-paper, #fff); + text-decoration: none; + color: inherit; + } + .deflect-title { + display: block; + font-size: 14px; + font-weight: 500; + } + .deflect-snippet { + margin-top: 2px; + font-size: 12.5px; + line-height: 1.45; + color: var(--v2-slate, #6b7280); + /* Snippets run to 200 characters; two lines is the most a phone can give + them without burying the form. */ + display: -webkit-box; + -webkit-line-clamp: 2; + line-clamp: 2; + -webkit-box-orient: vertical; + overflow: hidden; + } button.primary { width: 100%; margin-top: 14px; diff --git a/mobile/lib/data/models/solution.dart b/mobile/lib/data/models/solution.dart index f9bd6c3a..58ae9558 100644 --- a/mobile/lib/data/models/solution.dart +++ b/mobile/lib/data/models/solution.dart @@ -11,6 +11,12 @@ class Solution { final bool isPublished; final int caseCount; + /// Ids of the tags this article is filed under. Agent-side only: the portal + /// serializers carry no tags at all, because the vocabulary is shared with + /// deals and reads like "At Risk". The portal uses them to pick related + /// articles and never prints the names. + final List tagIds; + /// Id of the user who wrote the article, from the detail route's /// `created_by`. Carried because editing and deleting belong to admins and /// the author, while approving and publishing belong to admins alone. @@ -25,6 +31,7 @@ class Solution { required this.status, this.isPublished = false, this.caseCount = 0, + this.tagIds = const [], this.createdById, this.createdAt, this.updatedAt, @@ -38,6 +45,7 @@ class Solution { status: SolutionStatus.fromString(json['status'] as String?), isPublished: json['is_published'] as bool? ?? false, caseCount: json['case_count'] as int? ?? 0, + tagIds: _tagIds(json['tags']), // A bare id on the detail route; a nested object would also be readable. createdById: json['created_by'] is Map ? (json['created_by'] as Map)['id']?.toString() @@ -56,6 +64,10 @@ class Solution { 'description': description, 'status': status.value, 'is_published': isPublished, + // Always sent. The form shows every active tag, so an unticked one is a + // deliberate "not this", and the API reads an explicit empty list as + // "clear" while an absent key means "leave alone". + 'tags': tagIds, }; Solution copyWith({ @@ -65,6 +77,7 @@ class Solution { SolutionStatus? status, bool? isPublished, int? caseCount, + List? tagIds, }) { return Solution( id: id ?? this.id, @@ -73,6 +86,7 @@ class Solution { status: status ?? this.status, isPublished: isPublished ?? this.isPublished, caseCount: caseCount ?? this.caseCount, + tagIds: tagIds ?? this.tagIds, createdById: createdById, createdAt: createdAt, updatedAt: updatedAt, @@ -88,6 +102,17 @@ class Solution { int get hashCode => id.hashCode; } +/// The API nests `tags` as objects, not bare ids. Mirrors the helper of the +/// same shape in `account.dart`. +List _tagIds(dynamic raw) { + if (raw is! List) return const []; + return raw + .map((e) => e is Map ? e['id']?.toString() : e?.toString()) + .whereType() + .where((s) => s.isNotEmpty) + .toList(growable: false); +} + enum SolutionStatus { draft('draft', 'Draft', AppColors.gray500), reviewed('reviewed', 'Reviewed', AppColors.warning500), diff --git a/mobile/lib/screens/solutions/solution_detail_screen.dart b/mobile/lib/screens/solutions/solution_detail_screen.dart index 3180038e..cd6e281e 100644 --- a/mobile/lib/screens/solutions/solution_detail_screen.dart +++ b/mobile/lib/screens/solutions/solution_detail_screen.dart @@ -7,6 +7,7 @@ import '../../core/permissions.dart'; import '../../core/theme/theme.dart'; import '../../data/models/solution.dart'; import '../../providers/auth_provider.dart'; +import '../../providers/lookup_provider.dart'; import '../../providers/solutions_provider.dart'; import '../../widgets/common/common.dart'; @@ -27,6 +28,7 @@ class _SolutionDetailScreenState extends ConsumerState { final _descController = TextEditingController(); SolutionStatus _status = SolutionStatus.draft; bool _isPublished = false; + List _tagIds = []; bool _isLoading = false; bool _isFetching = false; @@ -59,6 +61,7 @@ class _SolutionDetailScreenState extends ConsumerState { _descController.text = s.description; _status = s.status; _isPublished = s.isPublished; + _tagIds = List.from(s.tagIds); } }); } @@ -75,15 +78,20 @@ class _SolutionDetailScreenState extends ConsumerState { 'title': _titleController.text.trim(), 'description': _descController.text.trim(), 'status': _status.value, + // Always sent on an update. Every active tag is on screen as a chip, so + // an unselected one is a deliberate "not this", and the API reads an + // explicit empty list as "clear" while an absent key means "leave alone". + 'tags': _tagIds, }; final notifier = ref.read(solutionsProvider.notifier); final res = widget.isCreate ? await notifier.create( Solution( id: '', - title: payload['title']!, - description: payload['description']!, + title: payload['title']! as String, + description: payload['description']! as String, status: _status, + tagIds: _tagIds, ), ) : await notifier.update(widget.solutionId!, payload); @@ -179,6 +187,12 @@ class _SolutionDetailScreenState extends ConsumerState { // admin may approve or publish, and being the author deliberately does not // grant it. The screen used to offer all three to everybody. final isAdmin = ref.watch(isOrgAdminProvider); + // Already active-only: the lookup calls /tags/ without + // `include_archived`, and `TagsListView` defaults to active. That matters + // because `_apply_tags` refuses an archived tag, so a chip for one would + // silently do nothing on save. The web form filters explicitly because + // its `getTags` asks for archived ones for the settings page. + final tags = ref.watch(tagsProvider); final canWrite = widget.isCreate || isAdminOrOwner( @@ -244,6 +258,51 @@ class _SolutionDetailScreenState extends ConsumerState { ) .toList(), ), + if (tags.isNotEmpty) ...[ + const SizedBox(height: 16), + Text( + 'TAGS', + style: AppTypography.overline.copyWith( + color: AppColors.textSecondary, + ), + ), + const SizedBox(height: 4), + // Chips rather than the picker sheet the ticket form uses: + // a knowledge base has a handful of tags and they all fit, + // so the vocabulary is visible instead of hidden behind a + // tap. Same shape as the web form. + Text( + 'Internal filing. Customers never see these names, but ' + 'articles sharing one are shown to each other in the portal.', + style: AppTypography.caption.copyWith( + color: AppColors.textSecondary, + ), + ), + const SizedBox(height: 8), + Wrap( + spacing: 8, + runSpacing: 8, + children: tags + .map( + (t) => FilterChip( + label: Text(t.name), + selected: _tagIds.contains(t.id), + onSelected: canWrite + ? (on) => setState(() { + if (on) { + _tagIds = [..._tagIds, t.id]; + } else { + _tagIds = _tagIds + .where((id) => id != t.id) + .toList(); + } + }) + : null, + ), + ) + .toList(), + ), + ], if (!widget.isCreate && _existing != null && canWrite) ...[ const SizedBox(height: 16), Container( @@ -279,13 +338,16 @@ class _SolutionDetailScreenState extends ConsumerState { style: AppTypography.label, ), Text( - // Not "and customer portal": is_published - // gates the agent suggester and nothing - // else. The customer-facing site it was - // meant to gate was cut, per - // cases/kb_access.py:26-28. + // An approved, published article is readable + // by signed-in portal contacts in this org + // and is suggested to them while they file a + // ticket, per + // PortalBaseView._published_articles. The + // agent suggester is the lesser half of what + // this switch does, so the customer comes + // first in the sentence. _isPublished - ? 'Suggested to agents on cases.' + ? 'Live to customers, and suggested to agents.' : 'Only approved solutions can be published.', style: AppTypography.caption.copyWith( color: AppColors.textSecondary, diff --git a/mobile/test/screens/solutions/solution_detail_screen_test.dart b/mobile/test/screens/solutions/solution_detail_screen_test.dart index 105a7824..74c47cb5 100644 --- a/mobile/test/screens/solutions/solution_detail_screen_test.dart +++ b/mobile/test/screens/solutions/solution_detail_screen_test.dart @@ -1,7 +1,9 @@ import 'package:bottle_crm/core/theme/theme.dart'; import 'package:bottle_crm/data/models/auth_response.dart'; +import 'package:bottle_crm/data/models/lookup_models.dart'; import 'package:bottle_crm/data/models/solution.dart'; import 'package:bottle_crm/providers/auth_provider.dart'; +import 'package:bottle_crm/providers/lookup_provider.dart'; import 'package:bottle_crm/providers/solutions_provider.dart'; import 'package:bottle_crm/screens/solutions/solution_detail_screen.dart'; import 'package:flutter/material.dart'; @@ -18,6 +20,7 @@ void main() { Solution article({ String author = 'author-1', SolutionStatus status = SolutionStatus.draft, + List tagIds = const [], }) => Solution( id: 'sol-1', title: 'Parity probe solution', @@ -25,16 +28,29 @@ void main() { status: status, isPublished: false, createdById: author, + tagIds: tagIds, ); + const orgTags = [ + TagLookup( + id: 'tag-billing', + name: 'Billing', + slug: 'billing', + color: 'blue', + ), + TagLookup(id: 'tag-access', name: 'Access', slug: 'access', color: 'green'), + ]; + Widget host( Solution solution, { required String role, required String userId, + List tags = const [], }) => ProviderScope( overrides: [ authProvider.overrideWith(() => _FakeAuth(role: role, userId: userId)), solutionsProvider.overrideWith(() => _FakeSolutions(solution)), + tagsProvider.overrideWithValue(tags), ], child: MaterialApp( theme: AppTheme.light, @@ -56,6 +72,60 @@ void main() { ); }); + group('tags', () { + testWidgets('a chip is lit for each tag the article already carries', ( + tester, + ) async { + await tester.pumpWidget( + host( + article(tagIds: const ['tag-billing']), + role: 'ADMIN', + userId: 'admin-1', + tags: orgTags, + ), + ); + await tester.pumpAndSettle(); + + FilterChip chipNamed(String label) => tester.widget( + find.ancestor(of: find.text(label), matching: find.byType(FilterChip)), + ); + expect(chipNamed('Billing').selected, isTrue); + expect(chipNamed('Access').selected, isFalse); + }); + + testWidgets('the section is absent when the org has no tags', ( + tester, + ) async { + await tester.pumpWidget( + host(article(), role: 'ADMIN', userId: 'admin-1', tags: const []), + ); + await tester.pumpAndSettle(); + expect(find.text('TAGS'), findsNothing); + }); + + testWidgets('a reader who may not edit cannot change the tags', ( + tester, + ) async { + await tester.pumpWidget( + host( + article(tagIds: const ['tag-billing']), + role: 'USER', + userId: 'somebody-else', + tags: orgTags, + ), + ); + await tester.pumpAndSettle(); + + final chip = tester.widget( + find.ancestor( + of: find.text('Access'), + matching: find.byType(FilterChip), + ), + ); + expect(chip.onSelected, isNull, reason: 'the chip must be inert'); + }); + }); + group('the three access rules the server draws', () { testWidgets('a member who did not write it can read it and nothing else', ( tester,