mirror of
https://github.com/saphid/frame-control.git
synced 2026-10-06 01:00:18 +02:00
Contact email: newest choice wins by rev, removal wipes the local log
Review follow-ups: - Each contact_consent event carries a rev that goes up with every change, sends are serialized, and `contacts` picks every field from the highest rev per copy, so a withdrawal can't lose to an earlier event sent in the same second or with a skewed clock. - Removing the address also replaces it with <removed> in the local sent log (earlier contact events and problem reports). - The prompt is rechecked when the Frame connects, not only at page load. - No thanks hides the bar only once the dismissal is saved. - docs/privacy.md: say that the analytics switches don't block a report or contact change the person sends deliberately. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
19a0d0af18
commit
b8ed53f2ff
5 files changed
+129
-37
No files matched your search
+13
-5
@@ -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
|
||||
|
||||
|
||||
+49
-1
@@ -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), \
|
||||
|
||||
+46
-17
@@ -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 = '<removed>', 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()
|
||||
|
||||
+7
-4
@@ -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:
|
||||
|
||||
+14
-10
@@ -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") {
|
||||
|
||||
Reference in new issue
Block a user