From 989962aecc70a4b312c828843df5b08e8c384c9d Mon Sep 17 00:00:00 2001 From: saphid <4596216+saphid@users.noreply.github.com> Date: Thu, 1 Oct 2026 21:25:22 +1000 Subject: [PATCH] Contact email: a report's rev is its own change; another address starts fresh - from_report applies its change and reads the id and rev together, so a removal made while that change is sending is newer than the report; the report's redaction window now starts before the address is saved. - A report with a different address replaces the saved one with follow-up questions only: update notices aren't carried over to an address nobody agreed them for, and the form says so before sending. - Settings refreshes after every report send, whatever the box shows by then. - privacy.md: a report with follow-up ticked also saves and sends the address. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/privacy.md | 11 ++++++++--- tests/test_contact.py | 30 ++++++++++++++++++++++++++++-- ui/frame_contact.py | 32 ++++++++++++++++++++++---------- ui/frame_report.py | 2 +- ui/index.html | 14 +++++++++++++- 5 files changed, 72 insertions(+), 17 deletions(-) diff --git a/docs/privacy.md b/docs/privacy.md index e866419..726105e 100644 --- a/docs/privacy.md +++ b/docs/privacy.md @@ -108,8 +108,10 @@ email address goes with it only if you tick **The maintainer may contact me with follow-up questions** (the report then carries `contact_followup: true`); it's filled in from **Contact email** below when you've agreed there. It has its own random id, so it isn't linked to your analytics events. With that box ticked, the address also becomes your -**Contact email** below with follow-up questions ticked (your update choice -stays as it was), so you remove it there like any other. The report then also +**Contact email** below with follow-up questions ticked, so you remove it there +like any other. If it's a different address from the one saved there, it +replaces it, and update notices stop until you turn them on again (they were +agreed for the old address); the form says so before you send. The report then also carries this copy's contact id and change number (`contact_id`, `contact_rev`, see below), so removing or changing the address later takes back the follow-up permission given with the report too. @@ -156,7 +158,10 @@ Frame Control's PostHog project, the same place as problem reports, as a `contact_consent` event with `email`, `updates`, `followup`, `action` (`set` 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 +you save, or when you send a problem report with follow-up questions ticked, +whatever the analytics settings are, because you chose to. With a report, the +address is saved and sent first, so it stands even if the report itself then +fails to send. It carries its own random contact id, not the analytics id, so it isn't linked 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 diff --git a/tests/test_contact.py b/tests/test_contact.py index 7a00f03..882a596 100644 --- a/tests/test_contact.py +++ b/tests/test_contact.py @@ -281,12 +281,38 @@ class Contact(Base): logged = [e["properties"].get("contact") for e in tm._read_lines(tm.SENT) if e["event"] == "problem_report"] self.assertEqual(logged, ["", ""]) - def test_a_report_to_another_address_keeps_the_update_choice(self): + def test_a_report_to_another_address_replaces_it_with_follow_up_only(self): + """Update notices were agreed for the old address, not the new one (the form says so).""" fc.save({"email": "old@example.com", "updates": True}) fr.send({**REPORT, "contact": "new@example.com", "contactFollowup": True}) s = fc.state() - self.assertEqual((s["email"], s["updates"], s["followup"]), ("new@example.com", True, True)) + self.assertEqual((s["email"], s["updates"], s["followup"]), ("new@example.com", False, True)) self.assertEqual(self.reports()[0]["contact_rev"], 2) + fc.save({"email": "new@example.com", "updates": True, "followup": False}) + fr.send({**REPORT, "contact": "NEW@example.com", "contactFollowup": True}) # same address: kept + s = fc.state() + self.assertEqual((s["email"], s["updates"], s["followup"]), ("new@example.com", True, True)) + + def test_a_removal_while_the_report_saves_its_address_still_counts(self): + """Removed while the report's own consent is on its way: the report keeps that consent's + rev (so the removal is newer) and is logged without the address.""" + post, removed = tm.post, [] + + def slow_post(events, **kw): + post(events, **kw) + if not removed and events[0]["event"] == "contact_consent": + removed.append(fc.save({"email": ""})) # Remove my email, mid-send + with mock.patch.object(tm, "post", side_effect=slow_post): + fr.send({**REPORT, "contact": "me@example.com", "contactFollowup": True}) + report = self.reports()[0] + self.assertEqual((report["contact_rev"], fc.load()["rev"], fc.state()["email"]), (1, 2, "")) + consents = [[e["distinct_id"], e["properties"]["email"], e["properties"]["followup"], e["properties"]["rev"]] + for e in self.events() if e["event"] == "contact_consent"] + row = self.report_row(cid=report["contact_id"], rev=report["contact_rev"]) + fr.mark_withdrawn([row], consents) + self.assertEqual(row[10], "withdrawn") + logged = [e["properties"]["contact"] for e in tm._read_lines(tm.SENT) if e["event"] == "problem_report"] + self.assertEqual(logged, [""]) def report_row(self, contact="me@example.com", followup=True, cid="copy", rev=1): return ["2026-09-10T10:00:00Z", "AB12CD34", "bug", "RDP", "It never connects.", contact, diff --git a/ui/frame_contact.py b/ui/frame_contact.py index 5ba3d36..8dca387 100644 --- a/ui/frame_contact.py +++ b/ui/frame_contact.py @@ -81,16 +81,19 @@ def flag(body, key): def from_report(email): """Follow-up questions agreed to with a problem report: the address becomes the contact - email with that choice ticked (the update choice stays as it was), so it shows in Settings - and is removed the same way. Returns (contact id, rev) for the report to carry: a later - change from this copy has a higher rev, and the newest such change decides whether the - report's follow-up permission still stands, whatever the clocks say.""" - s = load() - if s['email'].lower() != email.lower() or not s['followup']: - save({'email': s['email'] if s['email'].lower() == email.lower() else email, - 'updates': s['updates'], 'followup': True}) + email with that choice ticked, so it shows in Settings and is removed the same way. Update + notices stay on only for the same address: a different one replaces the old address with + follow-up questions only (the report form says so before sending). Returns (contact id, + rev) for the report to carry, read together with the change itself: a later change from + this copy has a higher rev, and the newest such change decides whether the report's + follow-up permission still stands, whatever the clocks say.""" + with _lock: s = load() - return s['id'], s['rev'] + same = s['email'].lower() == email.lower() + changed, cid, rev = _apply({'email': s['email'] if same else email, + 'updates': s['updates'] and same, 'followup': True}) + _deliver(changed) + return cid, rev def state(): @@ -177,6 +180,12 @@ def redact_removed(event, started): def save(body): """Set, change or remove the address and the two choices. An address needs at least one choice ticked; an empty address (or neither ticked) removes it and withdraws both.""" + _deliver(_apply(body)[0]) + return state() + + +def _apply(body): + """save()'s change, kept here and waiting to send. Returns (changed, contact id, rev).""" email = str(body.get('email') or '').strip() updates, followup = flag(body, 'updates'), flag(body, 'followup') if email and not valid_email(email): @@ -203,9 +212,12 @@ def save(body): _forget_locally(old) except OSError: pass + return changed, s['id'], s['rev'] + + +def _deliver(changed): if changed and not _send_pending(block=False): _wake.set() # offline, or a send under way that will take this change with it - return state() def prompt(body): diff --git a/ui/frame_report.py b/ui/frame_report.py index c04289a..1073e29 100644 --- a/ui/frame_report.py +++ b/ui/frame_report.py @@ -116,6 +116,7 @@ def send(body): contact = str(body.get('contact') or '').strip() if followup else '' if followup and not frame_contact.valid_email(contact): raise ValueError('add your email address for follow-up questions, or untick that box') + started = time.time() # a removal from now on (even while saving the address) is redacted from the log # It becomes the contact email in Settings, where it's changed or removed like any other. contact_id, contact_rev = frame_contact.from_report(contact) if followup else ('', 0) ref = uuid.uuid4().hex[:8].upper() @@ -127,7 +128,6 @@ def send(body): # Its own random id: a report can carry contact details, so it isn't linked to this copy's analytics. event = {'event': 'problem_report', 'distinct_id': str(uuid.uuid4()), 'uuid': str(uuid.uuid4()), 'timestamp': time.strftime('%Y-%m-%dT%H:%M:%SZ', time.gmtime()), 'properties': props} - started = time.time() try: frame_telemetry.post([event], timeout=30) except frame_telemetry.SendError as e: diff --git a/ui/index.html b/ui/index.html index ce97499..9809512 100644 --- a/ui/index.html +++ b/ui/index.html @@ -1264,6 +1264,7 @@ +