diff --git a/docs/privacy.md b/docs/privacy.md index 4edda2e..5619075 100644 --- a/docs/privacy.md +++ b/docs/privacy.md @@ -141,7 +141,8 @@ are two separate choices, both off until you tick them: | **The maintainer may contact me with follow-up questions** | Questions about problem reports you send, mostly | You're asked once, in a bar at the top of the page, after the Frame has -connected for the first time. **No thanks** hides it for good, and it isn't +connected for the first time (never while the first-run privacy notice is +showing). **No thanks** hides it for good, and it isn't shown again even if you ignore it. **Contact email** in **Privacy & updates** is where you add, change or remove the address and either choice at any time. @@ -152,13 +153,16 @@ or `withdraw`) and the common properties above. Only the maintainer can read that project, and nothing in it is published or shared. It's sent only when you save, whatever the analytics settings are, because you chose to. It carries its own random contact id, not the analytics id, so it isn't linked -to your usage events. On this computer the address and choices are kept in +to your usage events, and a `rev` number that goes up with each change, so +the newest choice always wins. Like everything else sent, it's listed under +**Show what's been sent**. On this computer the address and choices are kept in `contact/contact.json` in Frame Control's data folder. An address is only kept with at least one choice ticked. **Removing it.** **Remove my email** (or clearing the address and saving) -deletes it from this computer and sends a `withdraw` event with no address in -it. The maintainer's list only uses the newest event from each copy, so from +deletes it from this computer, including from the **Show what's been sent** +log (in earlier contact events and problem reports), and sends a `withdraw` +event with no address in it. The maintainer's list only uses the newest event from each copy, so from then on the address isn't listed for either choice. Unticking one choice works the same way for that choice. If you're offline, the change waits on this computer and is sent when PostHog can be reached. The earlier event @@ -174,7 +178,11 @@ personal API key as `inbox`. Untick the boxes, or set `DO_NOT_TRACK=1` or `FRAME_CONTROL_TELEMETRY=0` in the environment that starts Frame Control. A copy run from a source checkout -never sends anything unless `FRAME_CONTROL_TELEMETRY=1` is set. +never sends analytics unless `FRAME_CONTROL_TELEMETRY=1` is set. + +These switches cover the analytics above. A problem report or a contact email +is only ever sent when you press its Send or Save button, so those still go +when you choose to send them; if you don't, nothing is sent. ## Update checks diff --git a/tests/test_contact.py b/tests/test_contact.py index c775541..330fb3a 100644 --- a/tests/test_contact.py +++ b/tests/test_contact.py @@ -5,6 +5,8 @@ Run: python3 -m unittest discover -s tests """ import sandbox # noqa: F401 (first: keeps tests off real data and services) import sys +import threading +import time import unittest from pathlib import Path from unittest import mock @@ -110,6 +112,51 @@ class Contact(Base): self.assertFalse(fc.state()["waiting"]) self.assertEqual([e["properties"]["action"] for e in self.events()], ["set", "withdraw"]) + def test_removing_the_address_wipes_it_from_the_sent_log_too(self): + fc.save({"email": "me@example.com", "followup": True}) + fr.send({**REPORT, "contact": "me@example.com", "contactFollowup": True}) + self.assertIn("me@example.com", tm.SENT.read_text()) + fc.save({"email": ""}) + self.assertNotIn("me@example.com", tm.SENT.read_text()) + self.assertEqual([e["properties"].get("action") for e in tm._read_lines(tm.SENT) + if e["event"] == "contact_consent"], ["set", "withdraw"]) + + def test_each_change_has_a_higher_rev_so_the_newest_wins_whatever_the_clock(self): + fc.save({"email": "me@example.com", "updates": True}) + fc.save({"email": "new@example.com", "updates": True}) + fc.save({"email": ""}) + self.assertEqual([e["properties"]["rev"] for e in self.events()], [1, 2, 3]) + + def test_a_withdrawal_during_a_send_goes_after_it(self): + started, release, order = threading.Event(), threading.Event(), [] + real = tm.post + + def slow(batch, timeout=20): + order.append(batch[0]["properties"]["action"]) + if len(order) == 1: + started.set() + release.wait(5) + real(batch, timeout) + + with mock.patch.object(tm, "post", side_effect=slow): + t = threading.Thread(target=fc.save, args=({"email": "me@example.com", "updates": True},)) + t.start() + self.assertTrue(started.wait(5)) + w = threading.Thread(target=fc.save, args=({"email": ""},)) + w.start() + for _ in range(500): # the withdrawal is saved while the first send is still out + if fc.load()["rev"] == 2: + break + time.sleep(0.01) + self.assertEqual(fc.load()["pending"]["properties"]["action"], "withdraw") + release.set() + t.join(5) + w.join(5) + self.assertEqual(order, ["set", "withdraw"]) + self.assertEqual([e["properties"]["action"] for e in self.events()], ["set", "withdraw"]) + self.assertFalse(fc.state()["waiting"]) + self.assertNotIn("me@example.com", tm.SENT.read_text()) + # ---- the one-time prompt def test_the_prompt_waits_for_a_working_setup_then_stays_dismissed(self): @@ -149,7 +196,8 @@ class Contact(Base): ["d", "not-an-address", True, True, "2026-09-03T10:00:00Z"], ["short"]] with mock.patch.object(db, "_posthog_query", return_value={"results": rows}) as q: found = fr.contacts() - self.assertIn("argMax", q.call_args.args[0]) + self.assertIn("argMax(properties.email, tuple(ifNull(toInt(properties.rev), 0), timestamp))", + q.call_args.args[0]) self.assertEqual(found, {"updates": [("both@example.com", "2026-09-01"), ("news@example.com", "2026-09-02")], "followup": [("both@example.com", "2026-09-01")]}) with mock.patch.object(fr, "contacts", return_value=found), \ diff --git a/ui/frame_contact.py b/ui/frame_contact.py index dbe313b..0a7eb87 100644 --- a/ui/frame_contact.py +++ b/ui/frame_contact.py @@ -8,9 +8,10 @@ Two separate opt-in choices, both off until ticked: The address and the choices are kept on this computer (frame_host.data_dir('contact')) and sent privately to Frame Control's PostHog project as a `contact_consent` event, the same way as problem reports (frame_report.py), so only the maintainer can read them. Every change -sends a new event under this copy's own random contact id (not the analytics id), and the -newest event for an id is the one that counts: removing the address sends a withdrawal with -no address in it. The maintainer lists who agreed to what with +sends a new event under this copy's own random contact id (not the analytics id), numbered +by `rev`, and the highest rev for an id is the one that counts, whatever the clocks say: +removing the address sends a withdrawal with no address in it, and wipes the address from +the local log of what was sent. The maintainer lists who agreed to what with `python3 ui/frame_report.py contacts`. Nothing here sends email. A change that can't be sent (offline) waits in the state file and is retried in the @@ -35,12 +36,13 @@ PROMPTS = ('new', 'shown', 'dismissed', 'answered') RETRY_EVERY = 600 _lock = threading.RLock() +_send_lock = threading.Lock() # one send at a time, so events reach PostHog in rev order _retrier = None def _defaults(): return {'id': str(uuid.uuid4()), 'email': '', 'updates': False, 'followup': False, - 'prompt': 'new', 'pending': None} + 'prompt': 'new', 'pending': None, 'rev': 0} def load(): @@ -82,30 +84,50 @@ def _event(s): 'timestamp': time.strftime('%Y-%m-%dT%H:%M:%SZ', time.gmtime()), 'properties': {**frame_telemetry.common(), 'email': email, 'updates': bool(email and s['updates']), 'followup': bool(email and s['followup']), - 'action': 'set' if email else 'withdraw', 'level': 'contact'}} + 'action': 'set' if email else 'withdraw', 'rev': s['rev'], 'level': 'contact'}} def _send_pending(): """Send the waiting change. True if nothing is left waiting.""" - with _lock: - s = load() - event = s['pending'] + with _send_lock: + with _lock: + event = load()['pending'] if event is None: return True - try: - frame_telemetry.post([event], timeout=30) - except frame_telemetry.SendError: - return False + try: + frame_telemetry.post([event], timeout=30) + except frame_telemetry.SendError: + return False + _sent(event) + return True + + +def _sent(event): with _lock: s = load() if s['pending'] and s['pending'].get('uuid') == event['uuid']: # not replaced meanwhile s['pending'] = None _save(s) - try: - frame_telemetry.record_sent([event]) - except OSError: - pass - return True + # A withdrawal, or the address still in use: not an old one removed while this was on its way. + if event['properties']['email'] in ('', s['email']): + try: + frame_telemetry.record_sent([event]) + except OSError: + pass + + +def _forget_locally(email): + """Take a removed address out of the log of what was sent (contact events and reports).""" + with frame_telemetry._lock: + rows = frame_telemetry._read_lines(frame_telemetry.SENT) + hit = False + for e in rows: + p = e.get('properties') or {} + for k in ('email', 'contact'): + if p.get(k) and str(p[k]).strip().lower() == email.lower(): + p[k], hit = '', True + if hit: + frame_telemetry._write_lines(frame_telemetry.SENT, rows) def save(body): @@ -121,6 +143,7 @@ def save(body): updates = followup = False with _lock: s = load() + old = s['email'] changed = (email, updates, followup) != (s['email'], s['updates'], s['followup']) s.update(email=email, updates=updates, followup=followup) if body.get('fromPrompt') or email: @@ -128,8 +151,14 @@ def save(body): if changed: # Only the newest choice matters, so it replaces anything still waiting. A withdrawal # is sent even for an address still waiting here: its send may already be under way. + s['rev'] += 1 s['pending'] = _event(s) _save(s) + if old and old.lower() != email.lower(): + try: + _forget_locally(old) + except OSError: + pass if changed: _send_pending() return state() diff --git a/ui/frame_report.py b/ui/frame_report.py index 9134456..a39d36d 100644 --- a/ui/frame_report.py +++ b/ui/frame_report.py @@ -158,12 +158,15 @@ def _yes(v): def contacts(): """{'updates': [(email, since)], 'followup': [...]}: the addresses whose newest contact_consent event agrees to each, oldest first. A withdrawal, or a change to another - address, replaces what came before, so withdrawn addresses are never listed.""" + address, replaces what came before, so withdrawn addresses are never listed. "Newest" is + the highest rev from that copy (then time), so every field comes from the same event + whatever order they arrived in or what the clocks said.""" import frame_compat_db + newest = "tuple(ifNull(toInt(properties.rev), 0), timestamp)" res = frame_compat_db._posthog_query( - "SELECT distinct_id, argMax(properties.email, timestamp), argMax(properties.updates, timestamp), " - "argMax(properties.followup, timestamp), max(timestamp) FROM events WHERE event = 'contact_consent' " - "GROUP BY distinct_id ORDER BY max(timestamp) LIMIT 100000") + f"SELECT distinct_id, argMax(properties.email, {newest}), argMax(properties.updates, {newest}), " + f"argMax(properties.followup, {newest}), argMax(timestamp, {newest}) FROM events " + "WHERE event = 'contact_consent' GROUP BY distinct_id ORDER BY max(timestamp) LIMIT 100000") out = {'updates': [], 'followup': []} for row in res.get('results') or []: if not isinstance(row, list) or len(row) != 5: diff --git a/ui/index.html b/ui/index.html index 09512bd..df27d06 100644 --- a/ui/index.html +++ b/ui/index.html @@ -4010,16 +4010,20 @@ async function saveContact(change) { renderContact(s); return s; } -// One time only, never on top of the privacy notice, and only once the Frame has connected. async function loadContact() { await telemetryLoaded; + try { renderContact(await api("/api/contact")); } catch { return; } + checkContactPrompt(); +} +// One time only, never on top of the privacy notice, and only once the Frame has connected: +// checked at load and again whenever the Frame connects. +async function checkContactPrompt() { + if (!$("contactNotice").hidden || !$("privacyNotice").hidden) return; let s; try { s = await api("/api/contact"); } catch { return; } - renderContact(s); - if (s.showPrompt && $("privacyNotice").hidden) { - $("contactNotice").hidden = false; - api("/api/contact/prompt", { prompt: "shown" }).catch(() => {}); - } + if (!s.showPrompt) return; + $("contactNotice").hidden = false; + api("/api/contact/prompt", { prompt: "shown" }).catch(() => {}); } $("contactNotice").onsubmit = async e => { e.preventDefault(); @@ -4032,9 +4036,9 @@ $("contactNotice").onsubmit = async e => { } catch (err) { $("cpMsg").textContent = `Couldn't save: ${err.message}`; } finally { $("cpSave").disabled = false; } }; -$("cpNo").onclick = () => { - $("contactNotice").hidden = true; - api("/api/contact/prompt", { prompt: "dismissed" }).catch(() => {}); +$("cpNo").onclick = async () => { + try { await api("/api/contact/prompt", { prompt: "dismissed" }); $("contactNotice").hidden = true; } + catch (err) { $("cpMsg").textContent = `Couldn't save that: ${err.message}. Try again.`; } }; $("contactForm").onsubmit = async e => { e.preventDefault(); @@ -4418,7 +4422,7 @@ function onConnection(s) { $("offline").hidden = true; const reload = link.reload; link.reload = false; - refresh().then(() => { if (reload) reloadAll(); }); + refresh().then(() => { if (reload) reloadAll(); checkContactPrompt(); }); } else if (s.phase === "failed" && s.error) { setOnline(false, s.error.message); } else if (s.phase === "connecting" && prev && prev.phase === "connected") {