diff --git a/backend/common/migrations/0056_org_signup_source.py b/backend/common/migrations/0056_org_signup_source.py new file mode 100644 index 00000000..6c3af1b6 --- /dev/null +++ b/backend/common/migrations/0056_org_signup_source.py @@ -0,0 +1,17 @@ +# Generated by Django 6.1.2 on 2026-10-24 03:50 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("common", "0055_org_modules"), + ] + + operations = [ + migrations.AddField( + model_name="org", + name="signup_source", + field=models.JSONField(blank=True, default=dict), + ), + ] diff --git a/backend/common/models.py b/backend/common/models.py index 9793a618..a07daac4 100644 --- a/backend/common/models.py +++ b/backend/common/models.py @@ -164,6 +164,11 @@ class Org(BaseModel): # explicit confirmation; this flag only controls the default state. auto_close_children_on_parent_close = models.BooleanField(default=False) + # Where the org came from, written once at creation by + # `common.signup_source.clean_signup_source`. Growth analytics only: + # NEVER used in a permission check or a queryset filter. + signup_source = models.JSONField(default=dict, blank=True) + # Vertical packs. Descriptive only: records which pack was applied at # signup. NEVER used in a permission check or a queryset filter. vertical = models.CharField( diff --git a/backend/common/signup_source.py b/backend/common/signup_source.py new file mode 100644 index 00000000..630e6afe --- /dev/null +++ b/backend/common/signup_source.py @@ -0,0 +1,41 @@ +"""What an org's creation records about where it came from. + +The web app keeps the first visit's campaign tags and referring host in a +cookie and sends them with the create; the API cleans them here and adds which +client created the org. A bad value is dropped rather than refused: this is +analytics, and it must never stop an org from being created. +""" + +import re + +TAG_KEYS = ("utm_source", "utm_medium", "utm_campaign") +MAX_TAG_LENGTH = 100 +_HOST = re.compile( + r"^(?=.{1,253}$)[a-z0-9](?:[a-z0-9-]*[a-z0-9])?(?:\.[a-z0-9](?:[a-z0-9-]*[a-z0-9])?)*$" +) + +# The Flutter app's user agent. Web creates go through the SvelteKit server. +PHONE_USER_AGENT_PREFIX = "Dart/" + + +def clean_signup_source(raw, user_agent): + """Return the dict to store on `Org.signup_source`. + + Always has `client` ("phone" or "web"); adds each `utm_*` tag that is a + non-blank string (printable characters only, at most MAX_TAG_LENGTH) and + `referrer` when it is a bare host name. Anything else in `raw` is ignored. + """ + phone = (user_agent or "").startswith(PHONE_USER_AGENT_PREFIX) + source = {"client": "phone" if phone else "web"} + if not isinstance(raw, dict): + return source + for key in TAG_KEYS: + value = raw.get(key) + if isinstance(value, str): + value = "".join(ch for ch in value if ch.isprintable()).strip() + if value: + source[key] = value[:MAX_TAG_LENGTH] + referrer = raw.get("referrer") + if isinstance(referrer, str) and _HOST.match(referrer.strip().lower()): + source["referrer"] = referrer.strip().lower() + return source diff --git a/backend/common/tests/test_signup_source.py b/backend/common/tests/test_signup_source.py new file mode 100644 index 00000000..fa9beb09 --- /dev/null +++ b/backend/common/tests/test_signup_source.py @@ -0,0 +1,121 @@ +"""Where an org came from: what POST /api/org/ stores in `Org.signup_source`.""" + +import pytest +from rest_framework import status + +from common.models import Org +from common.signup_source import MAX_TAG_LENGTH, clean_signup_source + +URL = "/api/org/" + + +class TestClean: + def test_keeps_tags_and_a_host(self): + raw = { + "utm_source": "newsletter", + "utm_medium": "email", + "utm_campaign": "october", + "referrer": "news.example.com", + } + assert clean_signup_source(raw, "axios/1.20.0") == {"client": "web", **raw} + + def test_the_phone_app_is_recognised_by_its_user_agent(self): + assert clean_signup_source({}, "Dart/3.12 (dart:io)") == {"client": "phone"} + + def test_a_missing_user_agent_is_web(self): + assert clean_signup_source(None, None) == {"client": "web"} + + @pytest.mark.parametrize("raw", [None, "utm_source=x", ["x"], 42]) + def test_anything_but_a_dict_records_only_the_client(self, raw): + assert clean_signup_source(raw, "") == {"client": "web"} + + def test_unknown_keys_are_dropped(self): + source = clean_signup_source( + {"utm_source": "x", "email": "a@b.com", "org": "1"}, "" + ) + assert source == {"client": "web", "utm_source": "x"} + + @pytest.mark.parametrize("value", [None, 7, ["x"], {"a": 1}, " ", "\x00\x01"]) + def test_a_tag_that_is_not_text_or_is_blank_is_dropped(self, value): + assert "utm_source" not in clean_signup_source({"utm_source": value}, "") + + def test_a_long_tag_is_cut_and_control_characters_removed(self): + source = clean_signup_source({"utm_campaign": "a\nb" + "c" * 500}, "") + assert source["utm_campaign"].startswith("abc") + assert len(source["utm_campaign"]) == MAX_TAG_LENGTH + + @pytest.mark.parametrize( + "referrer", + [ + "https://news.example.com/", + "news.example.com/path?token=secret", + "user@news.example.com", + "-bad.example.com", + "a" * 254, + "", + 7, + ], + ) + def test_a_referrer_that_is_not_a_bare_host_is_dropped(self, referrer): + # A full URL can carry a path, a query or a token, so only a host name + # is ever stored. + assert "referrer" not in clean_signup_source({"referrer": referrer}, "") + + def test_a_host_is_stored_lower_case(self): + assert clean_signup_source({"referrer": " News.Example.COM "}, "")[ + "referrer" + ] == ("news.example.com") + + +@pytest.mark.django_db +class TestCreate: + def test_the_source_is_stored_on_the_new_org(self, admin_client): + response = admin_client.post( + URL, + { + "name": "Sourced Org", + "signup_source": {"utm_source": "site", "referrer": "example.org"}, + }, + format="json", + ) + assert response.status_code == status.HTTP_201_CREATED + org = Org.objects.get(name="Sourced Org") + assert org.signup_source == { + "client": "web", + "utm_source": "site", + "referrer": "example.org", + } + # Analytics only: the response does not echo it. + assert "signup_source" not in response.data["org"] + + def test_an_org_created_without_one_still_records_the_client(self, admin_client): + response = admin_client.post( + URL, + {"name": "Plain Org"}, + format="json", + HTTP_USER_AGENT="Dart/3.12 (dart:io)", + ) + assert response.status_code == status.HTTP_201_CREATED + assert Org.objects.get(name="Plain Org").signup_source == {"client": "phone"} + + def test_a_malformed_source_never_blocks_the_create(self, admin_client): + response = admin_client.post( + URL, + { + "name": "Junk Org", + "signup_source": {"referrer": "https://x.example/?t=1", "x": 1}, + }, + format="json", + ) + assert response.status_code == status.HTTP_201_CREATED + assert Org.objects.get(name="Junk Org").signup_source == {"client": "web"} + + def test_a_refused_create_stores_nothing(self, admin_client): + before = Org.objects.count() + response = admin_client.post( + URL, + {"name": "Bad & Name", "signup_source": {"utm_source": "x"}}, + format="json", + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert Org.objects.count() == before diff --git a/backend/common/views/organization_views.py b/backend/common/views/organization_views.py index 55e70edf..e731a686 100644 --- a/backend/common/views/organization_views.py +++ b/backend/common/views/organization_views.py @@ -18,6 +18,7 @@ from common.serializer import ( ProfileSerializer, ShowOrganizationListSerializer, ) +from common.signup_source import clean_signup_source class OrgProfileCreateView(APIView): @@ -42,7 +43,12 @@ class OrgProfileCreateView(APIView): # auditing where org keys come from read the wrong line. serializer = self.serializer_class(data=request.data) if serializer.is_valid(): - org_obj = serializer.save() + org_obj = serializer.save( + signup_source=clean_signup_source( + request.data.get("signup_source"), + request.headers.get("User-Agent", ""), + ) + ) # now creating the profile profile_obj = self.model2.objects.create(user=request.user, org=org_obj) diff --git a/backend/opportunity/tasks.py b/backend/opportunity/tasks.py index 66b395af..6baf03a8 100644 --- a/backend/opportunity/tasks.py +++ b/backend/opportunity/tasks.py @@ -1,5 +1,6 @@ import logging from collections import defaultdict +from datetime import timedelta from celery import shared_task from django.conf import settings @@ -9,7 +10,7 @@ from django.template.loader import render_to_string from django.utils import timezone from common.links import frontend_url -from common.models import Org, Profile +from common.models import Activity, Org, Profile from common.modules import module_is_available, module_on_since from common.org_time import activate_org_timezone, org_timezone from opportunity.models import Opportunity, SalesGoal @@ -51,6 +52,23 @@ def send_email_to_assigned_user(recipients, opportunity_id, org_id): msg.send() +# Stale-deal alerts go only to orgs someone used lately. A dormant org's admin +# would otherwise get the same email every day about deals nobody works. +STALE_ALERT_ACTIVE_DAYS = 30 + + +def _used_recently(org): + """Whether anyone worked in the org, or signed in to it, in the last + STALE_ALERT_ACTIVE_DAYS days. Runs under the org's RLS context.""" + since = timezone.now() - timedelta(days=STALE_ALERT_ACTIVE_DAYS) + return ( + Activity.objects.filter(org=org, created_at__gte=since).exists() + or Profile.objects.filter( + org=org, is_active=True, user__last_login__gte=since + ).exists() + ) + + @shared_task def check_stale_opportunities(): """Daily task: find rotten deals across all orgs and send email alerts.""" @@ -63,6 +81,8 @@ def check_stale_opportunities(): continue try: _set_rls_context_safe(str(org.id)) + if not _used_recently(org): + continue stages = stage_index(org.id) rotten_q = aging_q(stages.values(), "red") @@ -71,8 +91,9 @@ def check_stale_opportunities(): # Already rotten when Pipeline came back on: it went stale # while nobody could see it, so it is never alerted. rotten_q &= ~aging_q(stages.values(), "red", since) + # A pack's sample deals are nobody's work, so they never alert. rotten = Opportunity.objects.filter( - rotten_q, org=org, is_active=True + rotten_q, org=org, is_active=True, is_sample=False ).select_related("org") stale_opps = [] diff --git a/backend/opportunity/tests/test_deal_aging.py b/backend/opportunity/tests/test_deal_aging.py index b3e84a0c..e265f544 100644 --- a/backend/opportunity/tests/test_deal_aging.py +++ b/backend/opportunity/tests/test_deal_aging.py @@ -270,6 +270,9 @@ class TestStaleDealsTask: stage_changed_at=timezone.now() - timedelta(days=30), ) rotten.assigned_to.add(admin_profile) + # Alerts reach only orgs used in the last 30 days. + admin_user.last_login = timezone.now() + admin_user.save(update_fields=["last_login"]) from opportunity.tasks import check_stale_opportunities @@ -303,3 +306,81 @@ class TestStaleDealsTask: # Alert should not be called since no deals are stale mock_alert.assert_not_called() + + +class TestStaleDealsReachOnlyRealWork: + """Alerts skip a pack's sample deals and orgs nobody has used lately.""" + + def _rotten(self, org, user, profile, **fields): + deal = Opportunity.objects.create( + name=fields.pop("name", "Stale"), + stage="PROSPECTING", + org=org, + created_by=user, + stage_changed_at=timezone.now() - timedelta(days=30), + **fields, + ) + deal.assigned_to.add(profile) + return deal + + def _go_dormant(self, org, user): + from common.models import Activity + + Activity.objects.filter(org=org).update( + created_at=timezone.now() - timedelta(days=60) + ) + user.last_login = None + user.save(update_fields=["last_login"]) + + @patch("opportunity.tasks.send_stale_deals_alert") + def test_sample_deals_never_alert( + self, mock_alert, org_a, admin_user, admin_profile + ): + from opportunity.tasks import check_stale_opportunities + + self._rotten(org_a, admin_user, admin_profile, is_sample=True) + check_stale_opportunities() + mock_alert.assert_not_called() + + @patch("opportunity.tasks.send_stale_deals_alert") + def test_an_org_nobody_used_for_30_days_is_not_alerted( + self, mock_alert, org_a, admin_user, admin_profile + ): + from opportunity.tasks import check_stale_opportunities + + self._rotten(org_a, admin_user, admin_profile) + self._go_dormant(org_a, admin_user) + check_stale_opportunities() + mock_alert.assert_not_called() + + @patch("opportunity.tasks.send_stale_deals_alert") + def test_recent_work_counts_as_use( + self, mock_alert, org_a, admin_user, admin_profile + ): + from common.models import Activity + from opportunity.tasks import check_stale_opportunities + + deal = self._rotten(org_a, admin_user, admin_profile) + self._go_dormant(org_a, admin_user) + Activity.objects.create( + user=admin_profile, + org=org_a, + action="UPDATE", + entity_type="Opportunity", + entity_id=deal.id, + ) + check_stale_opportunities() + assert mock_alert.called + + @patch("opportunity.tasks.send_stale_deals_alert") + def test_a_recent_sign_in_counts_as_use( + self, mock_alert, org_a, admin_user, admin_profile + ): + from opportunity.tasks import check_stale_opportunities + + self._rotten(org_a, admin_user, admin_profile) + self._go_dormant(org_a, admin_user) + admin_user.last_login = timezone.now() - timedelta(days=3) + admin_user.save(update_fields=["last_login"]) + check_stale_opportunities() + assert mock_alert.called diff --git a/backend/opportunity/tests/test_module_off_jobs.py b/backend/opportunity/tests/test_module_off_jobs.py index b37fef1e..8d93b64e 100644 --- a/backend/opportunity/tests/test_module_off_jobs.py +++ b/backend/opportunity/tests/test_module_off_jobs.py @@ -38,6 +38,12 @@ def clock(): yield c +def _signed_in(user): + """A sign-in now, so the stale-deal job counts the user's org as in use.""" + user.last_login = timezone.now() + user.save(update_fields=["last_login"]) + + def _switch(org, actor, *off): """Leave exactly ``off`` switched off, through the one writer.""" assert change_modules(org, actor, {"off": list(off), "order": []}) == {} @@ -106,12 +112,14 @@ def _won(org, user, profile, amount): @patch("opportunity.tasks.send_stale_deals_alert") def test_stale_deal_alert_skips_an_org_with_deals_off( - mock_alert, org_a, org_b, admin_user, admin_profile, user_b + mock_alert, org_a, org_b, admin_user, admin_profile, user_b, profile_b ): _switch(org_a, admin_profile, "deals") _deal(org_a, admin_user, "Ours", 30) with rls_org(org_b): _deal(org_b, user_b, "Theirs", 30) + _signed_in(admin_user) + _signed_in(user_b) check_stale_opportunities() @@ -127,6 +135,7 @@ def test_deals_that_went_stale_while_off_are_not_alerted( _deal(org_a, admin_user, "Went stale while off", 30) _deal(org_a, admin_user, "Turns stale later", 15) _switch(org_a, admin_profile) + _signed_in(admin_user) check_stale_opportunities() assert _alerted(mock_alert) == [] diff --git a/backend/pyproject.toml b/backend/pyproject.toml index 064a24dc..943fed51 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "django-crm" -version = "1.24.0" +version = "1.25.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 4b9f2a97..bde1e175 100644 --- a/backend/uv.lock +++ b/backend/uv.lock @@ -704,7 +704,7 @@ wheels = [ [[package]] name = "django-crm" -version = "1.24.0" +version = "1.25.0" source = { editable = "." } dependencies = [ { name = "cairocffi" }, diff --git a/docs/api/opportunities.md b/docs/api/opportunities.md index 462c5b22..b687dff3 100644 --- a/docs/api/opportunities.md +++ b/docs/api/opportunities.md @@ -128,6 +128,12 @@ an open stage, and `red` (stalled, rotting) at `ceil(expected_days * 1.5)` (`agi `workflow.py`). `?rotten=true` and the `stalled_count` in `totals` use the same thresholds as `Opportunity.get_aging_status()`, as queryset cutoffs (`opportunity/stages.py`, `aging_q`). +The daily stale-deal email (`check_stale_opportunities` in `opportunity/tasks.py`, run by Celery +beat) uses the same `red` threshold. It goes to each stalled deal's assignees, or to the org's +admins when nobody is assigned. It skips a pack's sample deals (`is_sample`), and it skips an org +nobody has used in the last 30 days: no activity and no member sign-in, so a dormant org is not +sent the same email every day. + ### Pipeline administration Reads are open to every member of the org; every write is admin-only (`403` otherwise). An id from diff --git a/docs/architecture/data-model.md b/docs/architecture/data-model.md index 1f04e669..f3265112 100644 --- a/docs/architecture/data-model.md +++ b/docs/architecture/data-model.md @@ -127,3 +127,20 @@ directly in the serializer instead of relying on `full_clean()`, `opportunity/se own comment. If you add a `clean()` method to a model, verify whether that model's `save()` calls `full_clean()` before assuming the rule is enforced anywhere outside a unit test that calls `full_clean()` directly. + +## Where an org came from + +`Org.signup_source` records, once, how the org's creator arrived, for growth analytics. It is never +read by a permission check or a queryset filter. + +- The web app's sign-in page stores the first visit's `utm_source`, `utm_medium` and + `utm_campaign` tags and the referring site's host name in an httpOnly `signup_source` cookie + (30 days). An existing cookie is kept, so the first visit wins. Google's redirect back to the + sign-in page and the app's own pages are never taken as a source. +- Creating an org sends that value with `POST /api/org/` as `signup_source`, and the cookie is + then deleted. +- The API keeps only those four keys, as short printable text and a bare host name (never a full + URL, whose path or query can carry tokens), and adds `client`: `phone` for the mobile app, + `web` otherwise. A malformed value is dropped and never blocks the create. + +Nothing personal is stored: no address, no email and no full referring URL. diff --git a/docs/getting-started/demo-data.md b/docs/getting-started/demo-data.md index a7859ebc..a5638f91 100644 --- a/docs/getting-started/demo-data.md +++ b/docs/getting-started/demo-data.md @@ -60,6 +60,10 @@ through: - **The UI**: the Settings → Organization page (for an existing org) and the organization creation page (for a brand-new org) both offer a pack picker, calling the same backend endpoint. + Both exist in the web app and the mobile app. On creation the picker asks "What kind of + business is this?" and defaults to "Skip for now". It is hidden when the pack list cannot be + loaded. A pack that fails to apply never undoes the new org; the mobile app says so and points + to Settings → Organization, where the pack can be applied again. - **The API**: `POST /api/packs//apply/`, restricted to an organization admin (`request.profile.role == "ADMIN"`); any authenticated member can list available packs via `GET /api/packs/`, but only an admin can apply one. diff --git a/frontend/src/lib/server/signup-source-routes.test.js b/frontend/src/lib/server/signup-source-routes.test.js new file mode 100644 index 00000000..04ab8762 --- /dev/null +++ b/frontend/src/lib/server/signup-source-routes.test.js @@ -0,0 +1,137 @@ +/** + * The sign-in page remembers where a visitor came from, and creating an org + * sends it once. Route modules are imported by path, as in + * relay-callers.test.js. + */ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { SIGNUP_SOURCE_COOKIE } from './signup-source.js'; + +vi.mock('axios', () => ({ default: { post: vi.fn(), get: vi.fn() } })); +vi.mock('#lib/server/packs.js', () => ({ listPacks: vi.fn(), applyPack: vi.fn() })); + +const axios = (await import('axios')).default; +const login = await import('../../routes/(no-layout)/login/+page.server.js'); +const newOrg = await import('../../routes/(no-layout)/org/new/+page.server.js'); + +/** @param {Record} initial */ +function jar(initial = {}) { + /** @type {Record} */ + const store = { ...initial }; + return { + store, + get: vi.fn((/** @type {string} */ name) => store[name]), + set: vi.fn((/** @type {string} */ name, /** @type {string} */ value) => { + store[name] = value; + }), + delete: vi.fn((/** @type {string} */ name) => { + delete store[name]; + }) + }; +} + +/** @param {() => unknown} run */ +async function ignoringRedirect(run) { + try { + await run(); + } catch (/** @type {any} */ err) { + if (!err?.status || err.status < 300 || err.status >= 400) throw err; + } +} + +/** @param {string} url @param {Record} headers */ +function loadLogin(url, headers, cookies) { + return ignoringRedirect(() => + login.load( + /** @type {any} */ ({ + url: new URL(url), + request: new Request(url, { headers }), + cookies, + getClientAddress: () => '198.51.100.7' + }) + ) + ); +} + +function createOrg(cookies) { + const body = new FormData(); + body.set('org_name', 'Acme'); + return ignoringRedirect(() => + newOrg.actions.default( + /** @type {any} */ ({ + request: new Request('http://app.test/org/new', { method: 'POST', body }), + cookies, + locals: { user: { id: 'u' } }, + getClientAddress: () => '198.51.100.7' + }) + ) + ); +} + +const orgCreateBody = () => + vi.mocked(axios.post).mock.calls.find(([url]) => String(url).endsWith('/api/org/'))?.[1]; + +beforeEach(() => { + vi.mocked(axios.post).mockReset(); + vi.mocked(axios.post).mockResolvedValue({ data: { org: { id: 'org-1' } } }); +}); + +describe('the sign-in page', () => { + it('remembers the tags and the referring host of a first visit', async () => { + const cookies = jar(); + await loadLogin( + 'http://app.test/login?utm_source=site&utm_campaign=pricing', + { + referer: 'https://www.example.org/pricing' + }, + cookies + ); + expect(JSON.parse(cookies.store[SIGNUP_SOURCE_COOKIE])).toEqual({ + utm_source: 'site', + utm_campaign: 'pricing', + referrer: 'www.example.org' + }); + }); + + it("does not take Google's redirect back for a source", async () => { + const cookies = jar(); + await loadLogin( + 'http://app.test/login?code=c&state=s', + { + referer: 'https://accounts.google.com/' + }, + cookies + ); + expect(cookies.store[SIGNUP_SOURCE_COOKIE]).toBeUndefined(); + }); +}); + +describe('creating an org', () => { + it('sends the remembered source once, then forgets it', async () => { + const cookies = jar({ + jwt_access: 'token', + [SIGNUP_SOURCE_COOKIE]: '{"utm_source":"site","referrer":"example.org"}' + }); + await createOrg(cookies); + expect(orgCreateBody()).toEqual({ + name: 'Acme', + signup_source: { utm_source: 'site', referrer: 'example.org' } + }); + expect(cookies.store[SIGNUP_SOURCE_COOKIE]).toBeUndefined(); + }); + + it('sends no source when none was remembered', async () => { + await createOrg(jar({ jwt_access: 'token' })); + expect(orgCreateBody()).toEqual({ name: 'Acme' }); + }); + + it('keeps the source when the create is refused', async () => { + vi.mocked(axios.post).mockRejectedValueOnce( + Object.assign(new Error('Bad Request'), { + response: { status: 400, data: { errors: { name: ['taken'] } } } + }) + ); + const cookies = jar({ jwt_access: 'token', [SIGNUP_SOURCE_COOKIE]: '{"utm_source":"site"}' }); + await createOrg(cookies); + expect(cookies.store[SIGNUP_SOURCE_COOKIE]).toBe('{"utm_source":"site"}'); + }); +}); diff --git a/frontend/src/lib/server/signup-source.js b/frontend/src/lib/server/signup-source.js new file mode 100644 index 00000000..5061caf1 --- /dev/null +++ b/frontend/src/lib/server/signup-source.js @@ -0,0 +1,70 @@ +/** + * Where a new org came from: the first visit's campaign tags and the host that + * referred it, kept in a cookie until the org is created, then sent with the + * create. The API cleans the value again; nothing here is a check. + */ + +export const SIGNUP_SOURCE_COOKIE = 'signup_source'; + +const TAGS = ['utm_source', 'utm_medium', 'utm_campaign']; +const MAX_LENGTH = 100; +const THIRTY_DAYS = 60 * 60 * 24 * 30; + +/** + * The tags on this URL, and the referring host unless it is this app's own. + * Only the host of a referrer is kept: its path or query can carry tokens. + * @param {URL} url + * @param {string | null} referer + * @returns {Record} + */ +export function sourceOfVisit(url, referer) { + /** @type {Record} */ + const source = {}; + for (const tag of TAGS) { + const value = url.searchParams.get(tag)?.trim(); + if (value) source[tag] = value.slice(0, MAX_LENGTH); + } + if (referer) { + try { + const host = new URL(referer).hostname.toLowerCase(); + if (host && host !== url.hostname.toLowerCase()) source.referrer = host; + } catch { + // A malformed header names no source. + } + } + return source; +} + +/** + * Remember this visit's source unless an earlier visit already did: the first + * visit is the one that brought the person here. + * @param {{ url: URL, request: Request, cookies: import('@sveltejs/kit').Cookies }} event + * @param {boolean} secure + */ +export function rememberSignupSource({ url, request, cookies }, secure) { + if (cookies.get(SIGNUP_SOURCE_COOKIE) !== undefined) return; + const source = sourceOfVisit(url, request.headers.get('referer')); + cookies.set(SIGNUP_SOURCE_COOKIE, JSON.stringify(source), { + path: '/', + httpOnly: true, + sameSite: 'lax', + secure, + maxAge: THIRTY_DAYS + }); +} + +/** + * The remembered source, or undefined when there is none or it will not parse. + * @param {{ get: (name: string) => string | undefined }} cookies + * @returns {Record | undefined} + */ +export function readSignupSource(cookies) { + const raw = cookies.get(SIGNUP_SOURCE_COOKIE); + if (!raw) return undefined; + try { + const value = JSON.parse(raw); + return value && typeof value === 'object' && !Array.isArray(value) ? value : undefined; + } catch { + return undefined; + } +} diff --git a/frontend/src/lib/server/signup-source.test.js b/frontend/src/lib/server/signup-source.test.js new file mode 100644 index 00000000..e55d97d4 --- /dev/null +++ b/frontend/src/lib/server/signup-source.test.js @@ -0,0 +1,110 @@ +import { describe, expect, it } from 'vitest'; +import { + SIGNUP_SOURCE_COOKIE, + readSignupSource, + rememberSignupSource, + sourceOfVisit +} from './signup-source.js'; + +const APP = 'https://crm.example.com/login'; + +function fakeCookies(initial = {}) { + /** @type {Record} */ + const jar = { ...initial }; + /** @type {any[]} */ + const sets = []; + return { + jar, + sets, + get: (/** @type {string} */ name) => jar[name], + set: (/** @type {string} */ name, /** @type {string} */ value, /** @type {any} */ opts) => { + jar[name] = value; + sets.push({ name, value, opts }); + } + }; +} + +/** @param {string} url @param {string | null} referer */ +function visit(url, referer = null) { + const headers = new Headers(); + if (referer) headers.set('referer', referer); + return { url: new URL(url), request: new Request(url, { headers }) }; +} + +describe('sourceOfVisit', () => { + it('keeps the campaign tags and the referring host', () => { + const url = new URL(`${APP}?utm_source=site&utm_medium=web&utm_campaign=pricing&x=1`); + expect(sourceOfVisit(url, 'https://News.Example.org/some/path?token=abc')).toEqual({ + utm_source: 'site', + utm_medium: 'web', + utm_campaign: 'pricing', + referrer: 'news.example.org' + }); + }); + + it("ignores the app's own pages as a referrer", () => { + expect(sourceOfVisit(new URL(APP), 'https://crm.example.com/logout')).toEqual({}); + }); + + it('ignores a malformed referrer and blank tags', () => { + expect(sourceOfVisit(new URL(`${APP}?utm_source=%20`), 'not a url')).toEqual({}); + }); + + it('cuts a long tag', () => { + const url = new URL(`${APP}?utm_campaign=${'c'.repeat(300)}`); + expect(sourceOfVisit(url, null).utm_campaign).toHaveLength(100); + }); +}); + +describe('rememberSignupSource', () => { + it('stores the first visit in an httpOnly cookie', () => { + const cookies = fakeCookies(); + rememberSignupSource( + { + ...visit(`${APP}?utm_source=site`, 'https://example.org/'), + cookies: /** @type {any} */ (cookies) + }, + true + ); + expect(JSON.parse(cookies.jar[SIGNUP_SOURCE_COOKIE])).toEqual({ + utm_source: 'site', + referrer: 'example.org' + }); + expect(cookies.sets[0].opts).toMatchObject({ + httpOnly: true, + sameSite: 'lax', + secure: true, + path: '/' + }); + }); + + it('records a direct visit too, so a later one does not replace it', () => { + const cookies = fakeCookies(); + rememberSignupSource({ ...visit(APP), cookies: /** @type {any} */ (cookies) }, false); + expect(cookies.jar[SIGNUP_SOURCE_COOKIE]).toBe('{}'); + }); + + it('keeps an earlier visit', () => { + const cookies = fakeCookies({ [SIGNUP_SOURCE_COOKIE]: '{"referrer":"first.example"}' }); + rememberSignupSource( + { ...visit(`${APP}?utm_source=later`), cookies: /** @type {any} */ (cookies) }, + false + ); + expect(cookies.sets).toHaveLength(0); + }); +}); + +describe('readSignupSource', () => { + it('returns the stored object', () => { + const cookies = fakeCookies({ [SIGNUP_SOURCE_COOKIE]: '{"utm_source":"site"}' }); + expect(readSignupSource(cookies)).toEqual({ utm_source: 'site' }); + }); + + it.each([undefined, '', 'not json', '[1,2]', '"text"', 'null'])( + 'returns undefined for %j', + (raw) => { + const cookies = fakeCookies(raw === undefined ? {} : { [SIGNUP_SOURCE_COOKIE]: raw }); + expect(readSignupSource(cookies)).toBeUndefined(); + } + ); +}); diff --git a/frontend/src/routes/(no-layout)/login/+page.server.js b/frontend/src/routes/(no-layout)/login/+page.server.js index 99592ce7..5e87aee7 100644 --- a/frontend/src/routes/(no-layout)/login/+page.server.js +++ b/frontend/src/routes/(no-layout)/login/+page.server.js @@ -17,6 +17,7 @@ import * as publicEnv from '$app/env/public'; import { generateCodeVerifier, generateCodeChallenge, generateState } from '#lib/utils/pkce.js'; import { describeError } from '#lib/server/log-safe.js'; import { relayHeaders } from '#lib/server/relay.js'; +import { rememberSignupSource } from '#lib/server/signup-source.js'; const GOOGLE_AUTH_URL = 'https://accounts.google.com/o/oauth2/v2/auth'; const GOOGLE_SCOPES = ['openid', 'email', 'profile'].join(' '); @@ -62,6 +63,10 @@ export async function load({ url, cookies, getClientAddress, request }) { return handleOAuthCallback(code, returnedState, cookies, { getClientAddress, request }); } + // After the error and callback returns: Google's redirect back is not where + // the person came from. + rememberSignupSource({ url, request, cookies }, env.NODE_ENV === 'production'); + // Check if user is already authenticated const jwtAccess = cookies.get('jwt_access'); if (jwtAccess) { diff --git a/frontend/src/routes/(no-layout)/org/new/+page.server.js b/frontend/src/routes/(no-layout)/org/new/+page.server.js index b1023dea..2ef7bb4b 100644 --- a/frontend/src/routes/(no-layout)/org/new/+page.server.js +++ b/frontend/src/routes/(no-layout)/org/new/+page.server.js @@ -9,6 +9,7 @@ import axios from 'axios'; import { describeError } from '#lib/server/log-safe.js'; import { relayHeaders } from '#lib/server/relay.js'; import { listPacks, applyPack } from '#lib/server/packs.js'; +import { SIGNUP_SOURCE_COOKIE, readSignupSource } from '#lib/server/signup-source.js'; import { listTimezones } from '#lib/server/v2/organization.js'; /** @type {import('./$types').PageServerLoad} */ @@ -79,9 +80,14 @@ export const actions = { // Create organization and profile via Django API // Django's OrgProfileCreateView creates both org and profile + const signupSource = readSignupSource(cookies); const response = await axios.post( `${apiUrl}/api/org/`, - timezone ? { name: orgName.trim(), timezone } : { name: orgName.trim() }, + { + name: orgName.trim(), + ...(timezone ? { timezone } : {}), + ...(signupSource ? { signup_source: signupSource } : {}) + }, { headers: { Authorization: `Bearer ${jwtAccess}`, @@ -93,6 +99,9 @@ export const actions = { // Response should contain the created org const newOrg = response.data.org || response.data; + // Spent: a second org this person creates did not come from that visit. + cookies.delete(SIGNUP_SOURCE_COOKIE, { path: '/' }); + // Set org cookie for the newly created org await cookies.set('org', newOrg.id, { path: '/', diff --git a/mobile/lib/screens/auth/org_create_screen.dart b/mobile/lib/screens/auth/org_create_screen.dart index 732ecbc1..42cd3aff 100644 --- a/mobile/lib/screens/auth/org_create_screen.dart +++ b/mobile/lib/screens/auth/org_create_screen.dart @@ -3,7 +3,9 @@ import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:go_router/go_router.dart'; import '../../config/api_config.dart'; +import '../../data/models/org_settings.dart'; import '../../providers/auth_provider.dart'; +import '../../providers/settings_provider.dart'; import '../../routes/app_router.dart'; import '../../services/api_service.dart'; import '../../widgets/forms/unsaved_changes.dart'; @@ -31,6 +33,9 @@ class _OrgCreateScreenState extends ConsumerState { /// different spelling of the same zone. List<_Zone> _zones = const []; String _timezone = 'UTC'; + + /// The chosen pack id; empty is "Skip for now". + String _vertical = ''; bool _loadingZones = true; bool _submitting = false; @@ -93,26 +98,61 @@ class _OrgCreateScreenState extends ConsumerState { .createOrganization(_nameController.text.trim(), timezone: _timezone); if (!mounted) return; - setState(() => _submitting = false); - if (failure != null) { + setState(() => _submitting = false); ScaffoldMessenger.of(context).showSnackBar( SnackBar(content: Text(failure), backgroundColor: Colors.red), ); return; } + + // The org already exists and is selected, so a pack that will not apply + // must not undo it, as on the web. Unlike the web, say so: Settings > + // Organization can apply it again. + String? packFailure; + if (_vertical.isNotEmpty) { + final result = await ref + .read(orgSettingsProvider.notifier) + .applyPack(_vertical); + packFailure = result.error; + } + + if (!mounted) return; + setState(() => _submitting = false); + if (packFailure != null) { + ScaffoldMessenger.of(context).showSnackBar( + SnackBar( + content: Text( + 'Your organization is ready, but the industry setup did not ' + 'apply ($packFailure). You can apply it again from Organization ' + 'settings.', + ), + ), + ); + } // A new workspace is empty, so the first thing offered is bringing in // the people from a spreadsheet, with a skip to the dashboard. The web's // org/new goes to /welcome the same way. context.go(AppRoutes.importWelcome); } + Widget _packOption(String id, String name, String description) => + RadioListTile( + value: id, + enabled: !_submitting, + contentPadding: EdgeInsets.zero, + title: Text(name, style: const TextStyle(fontWeight: FontWeight.w600)), + subtitle: description.isEmpty ? null : Text(description), + ); + /// The timezone is a guess the screen made, not something the user typed, /// so only a name counts as work worth keeping. bool get _hasUnsavedChanges => _nameController.text.trim().isNotEmpty; @override Widget build(BuildContext context) { + // A list that fails to load hides the question, as the web does. + final packs = ref.watch(orgPacksProvider).value ?? const []; return UnsavedChangesGuard( hasUnsavedChanges: () => _hasUnsavedChanges, isSaving: _submitting, @@ -195,6 +235,39 @@ class _OrgCreateScreenState extends ConsumerState { 'Settings.', style: TextStyle(fontSize: 12, color: Colors.grey[600]), ), + if (packs.isNotEmpty) ...[ + const SizedBox(height: 24), + const Text( + 'What kind of business is this?', + style: TextStyle( + fontSize: 15, + fontWeight: FontWeight.w600, + ), + ), + const SizedBox(height: 4), + Text( + 'Sets up a starter pipeline, tags and fields for your ' + 'industry. You can change everything later.', + style: TextStyle(fontSize: 12, color: Colors.grey[600]), + ), + RadioGroup( + groupValue: _vertical, + onChanged: (value) { + if (value != null) setState(() => _vertical = value); + }, + child: Column( + children: [ + _packOption( + '', + 'Skip for now', + 'Start with a blank workspace.', + ), + for (final pack in packs) + _packOption(pack.id, pack.name, pack.description), + ], + ), + ), + ], const SizedBox(height: 24), SizedBox( width: double.infinity, diff --git a/mobile/pubspec.yaml b/mobile/pubspec.yaml index 6451136c..27b265c1 100644 --- a/mobile/pubspec.yaml +++ b/mobile/pubspec.yaml @@ -4,7 +4,7 @@ description: "CRM for AI and People." # pub.dev using `flutter pub publish`. This is preferred for private packages. publish_to: "none" # Remove this line if you wish to publish to pub.dev -version: 1.11.0+15 +version: 1.12.0+16 environment: sdk: ^3.8.1 diff --git a/mobile/test/screens/auth/org_create_screen_test.dart b/mobile/test/screens/auth/org_create_screen_test.dart index 73532c64..b8556548 100644 --- a/mobile/test/screens/auth/org_create_screen_test.dart +++ b/mobile/test/screens/auth/org_create_screen_test.dart @@ -1,5 +1,7 @@ import 'package:bottle_crm/data/models/auth_response.dart'; +import 'package:bottle_crm/data/models/org_settings.dart'; import 'package:bottle_crm/providers/auth_provider.dart'; +import 'package:bottle_crm/providers/settings_provider.dart'; import 'package:bottle_crm/routes/app_router.dart'; import 'package:bottle_crm/screens/auth/org_create_screen.dart'; import 'package:bottle_crm/screens/auth/org_selection_screen.dart'; @@ -149,14 +151,165 @@ void main() { ); }); }); + + group('the industry choice', () { + testWidgets('offers the packs the server lists, and a way to skip', ( + tester, + ) async { + await tester.pumpWidget(_creating(org: _FakeOrgSettings())); + await tester.pumpAndSettle(); + + expect(find.text('What kind of business is this?'), findsOneWidget); + expect(find.text('Skip for now'), findsOneWidget); + expect(find.text('Professional services'), findsOneWidget); + expect(find.text('Real estate'), findsOneWidget); + }); + + testWidgets('is left out when the pack list is empty', (tester) async { + await tester.pumpWidget( + _creating(org: _FakeOrgSettings(), packs: () async => const []), + ); + await tester.pumpAndSettle(); + + expect(find.text('What kind of business is this?'), findsNothing); + }); + + testWidgets('is left out when the pack list fails to load', (tester) async { + await tester.pumpWidget( + _creating( + org: _FakeOrgSettings(), + packs: () async => throw Exception('offline'), + ), + ); + await tester.pumpAndSettle(); + + expect(find.text('What kind of business is this?'), findsNothing); + // The form itself still works without it. + expect(find.text('Create organization'), findsOneWidget); + }); + + testWidgets('the chosen pack is applied to the new organization', ( + tester, + ) async { + final org = _FakeOrgSettings(); + await tester.pumpWidget(_creating(org: org)); + await tester.pumpAndSettle(); + + await _create(tester, pick: 'Real estate'); + + expect(org.applied, ['real-estate']); + expect(find.text('the welcome screen'), findsOneWidget); + }); + + testWidgets('skipping applies nothing', (tester) async { + final org = _FakeOrgSettings(); + await tester.pumpWidget(_creating(org: org)); + await tester.pumpAndSettle(); + + await _create(tester); + + expect(org.applied, isEmpty); + expect(find.text('the welcome screen'), findsOneWidget); + }); + + testWidgets('a pack that will not apply still leaves the organization ' + 'created, and says so', (tester) async { + final org = _FakeOrgSettings(error: 'Only admins can apply a pack.'); + await tester.pumpWidget(_creating(org: org)); + await tester.pumpAndSettle(); + + await _create(tester, pick: 'Professional services'); + + expect(org.applied, ['professional-services']); + expect(find.text('the welcome screen'), findsOneWidget); + expect( + find.textContaining('the industry setup did not apply'), + findsOneWidget, + ); + }); + + testWidgets('a create the server refuses applies no pack', (tester) async { + final org = _FakeOrgSettings(); + await tester.pumpWidget(_creating(org: org, createFails: true)); + await tester.pumpAndSettle(); + + await _create(tester, pick: 'Real estate'); + + expect(org.applied, isEmpty); + expect(find.text('That name is taken.'), findsOneWidget); + expect(find.text('the welcome screen'), findsNothing); + }); + }); } Widget _host(Widget screen, {List orgs = const []}) => ProviderScope( - overrides: [authProvider.overrideWith(() => _FakeAuth(orgs))], + overrides: [ + authProvider.overrideWith(() => _FakeAuth(orgs)), + orgPacksProvider.overrideWith((ref) async => const []), + ], child: MaterialApp(home: screen), ); +const _packs = [ + VerticalPack( + id: 'professional-services', + name: 'Professional services', + description: 'Agencies, consultancies and accounting firms.', + ), + VerticalPack(id: 'real-estate', name: 'Real estate'), +]; + +/// The create screen under a router, so a finished create lands somewhere +/// assertable. [org] records which packs were applied. +Widget _creating({ + required _FakeOrgSettings org, + Future> Function()? packs, + bool createFails = false, +}) { + final router = GoRouter( + initialLocation: AppRoutes.orgCreate, + routes: [ + GoRoute( + path: AppRoutes.orgCreate, + builder: (_, _) => const OrgCreateScreen(), + ), + // A Scaffold like the real welcome screen's, so a snackbar raised on + // the way there has somewhere to show. + GoRoute( + path: AppRoutes.importWelcome, + builder: (_, _) => const Scaffold(body: Text('the welcome screen')), + ), + ], + ); + return ProviderScope( + overrides: [ + authProvider.overrideWith( + createFails ? _RefusingAuth.new : _CreatingAuth.new, + ), + orgPacksProvider.overrideWith( + (ref) => packs == null ? Future.value(_packs) : packs(), + ), + orgSettingsProvider.overrideWith(() => org), + ], + child: MaterialApp.router(routerConfig: router), + ); +} + +Future _create(WidgetTester tester, {String? pick}) async { + await tester.enterText(find.byType(TextFormField), 'Acme Inc.'); + if (pick != null) { + await tester.ensureVisible(find.text(pick)); + await tester.pumpAndSettle(); + await tester.tap(find.text(pick)); + await tester.pump(); + } + await tester.ensureVisible(find.text('Create organization')); + await tester.pumpAndSettle(); + await tester.tap(find.text('Create organization')); + await tester.pumpAndSettle(); +} + class _FakeAuth extends AuthNotifier { _FakeAuth(this.orgs); @@ -180,3 +333,32 @@ class _CreatingAuth extends _FakeAuth { Future createOrganization(String name, {String? timezone}) async => null; } + +class _RefusingAuth extends _FakeAuth { + _RefusingAuth() : super(const []); + + @override + Future createOrganization(String name, {String? timezone}) async => + 'That name is taken.'; +} + +/// Records each pack applied. [error] makes every apply fail with it. +class _FakeOrgSettings extends OrgSettingsNotifier { + _FakeOrgSettings({this.error}); + + final String? error; + final applied = []; + + @override + Future build() async => + OrgSettings.fromJson(const {'id': 'org-1', 'name': 'Acme Inc.'}); + + @override + Future<({String? error, PackApplyReport? report})> applyPack( + String packId, + ) async { + applied.add(packId); + if (error != null) return (error: error, report: null); + return (error: null, report: const PackApplyReport(created: 3)); + } +} diff --git a/mobile/test/screens/phone_viewport_test.dart b/mobile/test/screens/phone_viewport_test.dart index 8ca11585..b183410e 100644 --- a/mobile/test/screens/phone_viewport_test.dart +++ b/mobile/test/screens/phone_viewport_test.dart @@ -56,6 +56,7 @@ import 'package:bottle_crm/screens/settings/macros_screen.dart'; import 'package:bottle_crm/screens/settings/mailbox_form_sheet.dart'; import 'package:bottle_crm/screens/settings/organization_edit_screen.dart'; import 'package:bottle_crm/screens/settings/organization_screen.dart'; +import 'package:bottle_crm/screens/auth/org_create_screen.dart'; import 'package:bottle_crm/screens/tickets/close_with_children_dialog.dart'; import 'package:bottle_crm/screens/tickets/macro_picker_sheet.dart'; import 'package:bottle_crm/screens/settings/settings_hub_screen.dart'; @@ -3011,6 +3012,58 @@ void main() { }); }); + group('new organization at 390px', () { + // Every pack listed, the longest description included, so the radio rows + // are at their tallest. + Widget orgCreateApp() => ProviderScope( + overrides: [ + orgPacksProvider.overrideWith( + (ref) async => const [ + VerticalPack( + id: 'professional-services', + name: 'Professional services', + description: + 'Agencies, consultancies and accounting firms selling ' + 'billable work.', + ), + VerticalPack( + id: 'education-admissions', + name: 'Education admissions', + description: + 'Coaching centres, training institutes and colleges ' + 'managing admissions.', + ), + VerticalPack( + id: 'events-weddings', + name: 'Events & weddings', + description: + 'Wedding planners, event agencies and venues taking ' + 'enquiries through to a delivered event.', + ), + VerticalPack( + id: 'real-estate', + name: 'Real estate', + description: + 'Brokers, agencies and developers selling residential and ' + 'commercial property.', + ), + ], + ), + ], + child: const MaterialApp(home: OrgCreateScreen()), + ); + + for (final scale in [1.0, 1.5]) { + testWidgets('the industry choice fits at text scale $scale', ( + tester, + ) async { + await pump(tester, orgCreateApp(), textScale: scale); + expect(find.text('What kind of business is this?'), findsOneWidget); + expect(tester.takeException(), isNull); + }); + } + }); + group('organization at 390px', () { testWidgets('renders without overflowing', (tester) async { await pump(tester, organizationApp(isAdmin: true));