mirror of
https://github.com/saphid/frame-control.git
synced 2026-10-06 02:00:19 +02:00
Contact email: withdrawal covers earlier reports; consent is a real true
- A report with follow-up ticked carries this copy's contact id, and the inbox marks its permission withdrawn when a later choice from that copy no longer agrees to follow-up questions at that address. - The one-time prompt never appears in a visit that showed the privacy notice, even if the Frame connects just after it's dismissed. - Saving contact details isn't headset work: it can't hold up switching headsets or be refused after a switch. - Consent flags must be JSON true/false; "false" is no longer consent. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
2ca0e6924a
commit
01d5c612c0
7 files changed
+160
-20
No files matched your search
+9
-4
@@ -107,7 +107,9 @@ wrote, a short reference shown after sending, and the diagnostics below. Your
|
||||
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.
|
||||
your analytics events. With that box ticked it also carries this copy's contact
|
||||
id (`contact_id`, see below), so removing or changing the address later takes
|
||||
back the follow-up permission given with the report too.
|
||||
|
||||
With **Include diagnostics** ticked (the default), the report adds:
|
||||
|
||||
@@ -141,8 +143,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, and never while or straight after the
|
||||
first-run privacy notice is showing. **No thanks** hides it for good, and it isn't
|
||||
connected for the first time, and never in the same visit as the first-run
|
||||
privacy notice. **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.
|
||||
|
||||
@@ -164,7 +166,10 @@ 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
|
||||
works the same way for that choice. This also covers problem reports you sent
|
||||
from this copy with follow-up questions ticked: if your newest choice no longer
|
||||
agrees to follow-up questions at that address, the maintainer's inbox shows the
|
||||
permission as withdrawn and leaves the address out. If you're offline, the change waits on
|
||||
this computer and is sent when PostHog can be reached. The earlier event
|
||||
stays in PostHog until its data retention removes it; to have it deleted
|
||||
sooner, ask the maintainer (for example in a problem report).
|
||||
|
||||
@@ -59,6 +59,16 @@ class Contact(Base):
|
||||
self.assertEqual(fc.load()["email"], "")
|
||||
self.assertEqual(self.got, [])
|
||||
|
||||
def test_only_a_real_true_counts_as_consent(self):
|
||||
for wrong in ("false", "true", 1, 0, [], {}):
|
||||
with self.assertRaisesRegex(ValueError, "true or false"):
|
||||
fc.save({"email": "me@example.com", "updates": wrong, "followup": True})
|
||||
with self.assertRaisesRegex(ValueError, "true or false"):
|
||||
fr.send({**REPORT, "contact": "me@example.com", "contactFollowup": wrong})
|
||||
self.assertEqual((fc.load()["email"], self.got), ("", []))
|
||||
fc.save({"email": "me@example.com", "updates": True}) # left out is no
|
||||
self.assertEqual((fc.load()["updates"], fc.load()["followup"]), (True, False))
|
||||
|
||||
def test_each_choice_is_sent_privately_on_its_own(self):
|
||||
fc.save({"email": " me@example.com ", "updates": True})
|
||||
fc.save({"email": "me@example.com", "updates": False, "followup": True})
|
||||
@@ -248,6 +258,63 @@ class Contact(Base):
|
||||
with self.assertRaisesRegex(ValueError, "email address"):
|
||||
fr.send({**REPORT, "contact": "discord:me", "contactFollowup": True})
|
||||
|
||||
def test_a_report_with_follow_up_carries_the_kept_contact_id(self):
|
||||
fr.send({**REPORT, "contact": "me@example.com"})
|
||||
self.assertFalse(fc.FILE.exists()) # no follow-up, nothing kept or linked
|
||||
fr.send({**REPORT, "contact": "me@example.com", "contactFollowup": True})
|
||||
without, with_ = (e["properties"] for e in self.events())
|
||||
self.assertEqual(without["contact_id"], "")
|
||||
self.assertEqual(with_["contact_id"], fc.load()["id"])
|
||||
self.assertNotEqual(with_["contact_id"], tm.settings()["id"]) # not the analytics id
|
||||
fc.save({"email": "me@example.com", "updates": True})
|
||||
self.assertEqual(self.events()[-1]["distinct_id"], with_["contact_id"]) # same copy, same id
|
||||
|
||||
def report_row(self, ts, contact="me@example.com", followup=True, cid="copy"):
|
||||
return [ts, "AB12CD34", "bug", "RDP", "It never connects.", contact, "0.4.0", "Windows", "", "", followup, cid]
|
||||
|
||||
def test_a_later_withdrawal_takes_back_a_reports_follow_up_permission(self):
|
||||
reports = [self.report_row("2026-09-10T10:00:00Z"), # removed later
|
||||
self.report_row("2026-09-12T10:00:00Z", cid="other"), # another copy
|
||||
self.report_row("2026-09-20T10:00:00Z"), # after the withdrawal
|
||||
self.report_row("2026-09-11T10:00:00Z", cid="", followup=True), # sent before contact_id
|
||||
self.report_row("2026-09-15T10:00:00Z", cid="same-second")]
|
||||
consents = [["copy", "me@example.com", True, 1, "2026-09-01T10:00:00Z"],
|
||||
["copy", "", False, 2, "2026-09-14T10:00:00Z"],
|
||||
["other", "me@example.com", True, 1, "2026-09-13T10:00:00Z"], # still agrees
|
||||
["same-second", "", False, 1, "2026-09-15T10:00:00Z"], ["short"]]
|
||||
fr.mark_withdrawn(reports, consents)
|
||||
self.assertEqual([r[10] for r in reports], ["withdrawn", True, True, True, "withdrawn"])
|
||||
|
||||
def test_changing_the_address_or_unticking_follow_up_takes_it_back_too(self):
|
||||
reports = [self.report_row("2026-09-10T10:00:00Z", cid="moved"),
|
||||
self.report_row("2026-09-10T10:00:00Z", cid="news-only"),
|
||||
self.report_row("2026-09-10T10:00:00Z", contact="Me@Example.com", cid="case")]
|
||||
consents = [["moved", "new@example.com", True, 1, "2026-09-11T10:00:00Z"],
|
||||
["news-only", "me@example.com", False, 1, "2026-09-11T10:00:00Z"],
|
||||
["case", "me@example.com", True, 1, "2026-09-11T10:00:00Z"],
|
||||
# rev, not the clock, decides which later choice is newest
|
||||
["case", "", False, 2, "2026-09-11T09:59:00Z"]]
|
||||
fr.mark_withdrawn(reports, consents)
|
||||
self.assertEqual([r[10] for r in reports], ["withdrawn", "withdrawn", "withdrawn"])
|
||||
reports[2][10] = True
|
||||
fr.mark_withdrawn(reports[2:], consents[2:3])
|
||||
self.assertIs(reports[2][10], True) # same address, any case, still agrees
|
||||
|
||||
def test_the_inbox_shows_withdrawn_follow_up_without_the_address(self):
|
||||
reports = [self.report_row("2026-09-10T10:00:00Z"), ["short"]]
|
||||
consents = [["copy", "", False, 2, "2026-09-14T10:00:00Z"]]
|
||||
with mock.patch.object(db, "_posthog_query", side_effect=[{"results": reports}, {"results": consents}]) as q, \
|
||||
mock.patch.object(sys, "argv", ["frame_report.py", "inbox", "30"]), \
|
||||
mock.patch("builtins.print") as out:
|
||||
fr.main()
|
||||
self.assertIn("event = 'contact_consent'", q.call_args_list[1].args[0])
|
||||
printed = " ".join(str(c.args[0]) for c in out.call_args_list if c.args)
|
||||
self.assertIn("follow-up permission since withdrawn", printed)
|
||||
self.assertNotIn("me@example.com", printed)
|
||||
with mock.patch.object(db, "_posthog_query", return_value={"results": [self.report_row("x", followup=False)]}) as q:
|
||||
fr.inbox()
|
||||
self.assertEqual(q.call_count, 1) # nothing to reconcile, no second query
|
||||
|
||||
def test_contacts_lists_the_newest_choice_per_copy_by_consent(self):
|
||||
rows = [["a", "both@example.com", True, "true", "2026-09-01T10:00:00Z"],
|
||||
["b", "news@example.com", "true", False, "2026-09-02T10:00:00Z"],
|
||||
@@ -272,6 +339,22 @@ class Contact(Base):
|
||||
self.assertIs(server.POST["/api/contact"], fc.save)
|
||||
self.assertIs(server.POST["/api/contact/prompt"], fc.prompt)
|
||||
|
||||
def test_saving_is_not_headset_work(self):
|
||||
"""A slow send mustn't hold up switching headsets, nor be refused after a switch."""
|
||||
import io
|
||||
import server
|
||||
seen = []
|
||||
for path in ("/api/contact", "/api/contact/prompt"):
|
||||
h = server.Handler.__new__(server.Handler)
|
||||
body = b'{"prompt": "shown"}' if path.endswith("prompt") else b'{"email": "me@example.com", "updates": true}'
|
||||
h.path, h.rfile = path, io.BytesIO(body)
|
||||
h.headers = {"Content-Length": str(len(body)), "X-Frame-Device": "a-headset-switched-away-from"}
|
||||
h.local_request = lambda: True
|
||||
h.send_json = lambda obj, status=200: seen.append((status, server._work[0]))
|
||||
with mock.patch.object(fc, "_send_pending", side_effect=lambda block=True: seen.append(("send", server._work[0]))):
|
||||
h.do_POST()
|
||||
self.assertEqual(seen, [("send", 0), (200, 0), (200, 0)])
|
||||
|
||||
|
||||
# Run these once, in test_telemetry, not again through the import above.
|
||||
del Base, ReportProblem
|
||||
|
||||
@@ -422,8 +422,8 @@ class ReportProblem(Base):
|
||||
|
||||
def test_the_inbox_skips_malformed_reports(self):
|
||||
good = ["2026-09-28T09:50:00Z", "AB12CD34", "bug", "Live view stops", "It stops.", None,
|
||||
"0.4.0", "macOS", "", "", None]
|
||||
rows = [["2026-09-28T10:00:00Z", "X", "bug", "Hand-made", None, None, None, None, None, None, None],
|
||||
"0.4.0", "macOS", "", "", None, None]
|
||||
rows = [["2026-09-28T10:00:00Z", "X", "bug", "Hand-made", None, None, None, None, None, None, None, None],
|
||||
["short"], good]
|
||||
with mock.patch.object(db, "_posthog_query", return_value={"results": rows}), \
|
||||
mock.patch.object(sys, "argv", ["frame_report.py", "inbox"]), \
|
||||
|
||||
+19
-1
@@ -71,6 +71,24 @@ def valid_email(email):
|
||||
return len(email) <= EMAIL_MAX and bool(EMAIL_RE.fullmatch(email))
|
||||
|
||||
|
||||
def flag(body, key):
|
||||
"""A consent choice: true only when it really is true (not "false" or 1), left out is no."""
|
||||
v = body.get(key)
|
||||
if v is not None and not isinstance(v, bool):
|
||||
raise ValueError(f'{key} must be true or false')
|
||||
return v is True
|
||||
|
||||
|
||||
def contact_id():
|
||||
"""This copy's contact id, kept from now on. A report with follow-up consent carries it, so
|
||||
removing the address later takes back the follow-up permission given with the report too."""
|
||||
with _lock:
|
||||
s = load()
|
||||
if not FILE.exists():
|
||||
_save(s)
|
||||
return s['id']
|
||||
|
||||
|
||||
def state():
|
||||
"""What the page shows. showPrompt: the one-time prompt hasn't been shown or answered yet,
|
||||
and the Frame has connected at least once (setup worked), so it never greets a new install."""
|
||||
@@ -156,7 +174,7 @@ 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."""
|
||||
email = str(body.get('email') or '').strip()
|
||||
updates, followup = bool(body.get('updates')), bool(body.get('followup'))
|
||||
updates, followup = flag(body, 'updates'), flag(body, 'followup')
|
||||
if email and not valid_email(email):
|
||||
raise ValueError("that doesn't look like an email address")
|
||||
if email and not (updates or followup):
|
||||
|
||||
+37
-8
@@ -112,13 +112,15 @@ def send(body):
|
||||
"""Send the report to PostHog. Returns {"id", "message"}; raises ReportError."""
|
||||
kind = body.get('kind') if body.get('kind') in KINDS else 'bug'
|
||||
title, text, diag = compose(body)
|
||||
followup = bool(body.get('contactFollowup'))
|
||||
followup = frame_contact.flag(body, 'contactFollowup')
|
||||
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')
|
||||
ref = uuid.uuid4().hex[:8].upper()
|
||||
props = {**frame_telemetry.common(), 'kind': kind, 'title': title, 'message': text,
|
||||
'contact': contact, 'contact_followup': followup, 'diagnostics': diag,
|
||||
# Only with an address: so removing it later (Settings) also takes this permission back.
|
||||
'contact_id': frame_contact.contact_id() if followup else '',
|
||||
'report_id': ref, 'steamos': str(frame.get('build') or '')[:120], 'level': 'report'}
|
||||
# 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()),
|
||||
@@ -143,15 +145,43 @@ class ReportError(RuntimeError):
|
||||
|
||||
def inbox(days=30):
|
||||
"""The maintainer's recent reports from PostHog, newest first (needs the personal API key
|
||||
frame_compat_db.sync uses)."""
|
||||
frame_compat_db.sync uses). Column 10 is whether the person may be asked follow-up
|
||||
questions now: 'withdrawn' when a later choice from the same copy took it back."""
|
||||
import frame_compat_db
|
||||
days = int(days)
|
||||
res = frame_compat_db._posthog_query(
|
||||
"SELECT timestamp, properties.report_id, properties.kind, properties.title, properties.message, "
|
||||
"properties.contact, properties.app_version, properties.os, properties.steamos, properties.diagnostics, "
|
||||
"properties.contact_followup "
|
||||
f"FROM events WHERE event = 'problem_report' AND timestamp > now() - INTERVAL {int(days)} DAY "
|
||||
"properties.contact_followup, properties.contact_id "
|
||||
f"FROM events WHERE event = 'problem_report' AND timestamp > now() - INTERVAL {days} DAY "
|
||||
"ORDER BY timestamp DESC LIMIT 200")
|
||||
return res.get('results') or []
|
||||
rows = [r for r in res.get('results') or [] if isinstance(r, list) and len(r) == 12]
|
||||
if any(r[11] and _yes(r[10]) for r in rows):
|
||||
later = frame_compat_db._posthog_query(
|
||||
"SELECT distinct_id, properties.email, properties.followup, ifNull(toInt(properties.rev), 0), timestamp "
|
||||
f"FROM events WHERE event = 'contact_consent' AND timestamp > now() - INTERVAL {days} DAY LIMIT 100000")
|
||||
mark_withdrawn(rows, later.get('results') or [])
|
||||
return rows
|
||||
|
||||
|
||||
def mark_withdrawn(reports, consents):
|
||||
"""Mark reports whose follow-up permission was taken back: the newest contact choice from
|
||||
the same copy made at or after the report (same second counts, so a withdrawal wins) no
|
||||
longer agrees to follow-up questions at that address."""
|
||||
newest = {}
|
||||
for c in consents:
|
||||
if not isinstance(c, list) or len(c) != 5:
|
||||
continue
|
||||
cid, email, followup, rev, ts = c
|
||||
newest.setdefault(str(cid), []).append(((int(rev or 0), str(ts or '')), str(email or ''), followup))
|
||||
for r in reports:
|
||||
if not (r[11] and _yes(r[10])):
|
||||
continue
|
||||
after = [c for c in newest.get(str(r[11]), []) if c[0][1] >= str(r[0] or '')]
|
||||
if after:
|
||||
_, email, followup = max(after, key=lambda c: c[0])
|
||||
if not (_yes(followup) and email.strip().lower() == str(r[5] or '').strip().lower()):
|
||||
r[10] = 'withdrawn'
|
||||
|
||||
|
||||
def _yes(v):
|
||||
@@ -204,14 +234,13 @@ def main():
|
||||
if cmd != 'inbox':
|
||||
sys.exit(USAGE)
|
||||
for row in inbox(*(args[:1] or [30])):
|
||||
if not isinstance(row, list) or len(row) != 11:
|
||||
continue
|
||||
ts, ref, kind, title, text, contact, version, osname, steamos, diag = (str(v or '') for v in row[:10])
|
||||
# Reports from before contact_followup existed only carried an address given for a reply.
|
||||
reply = contact and (row[10] is None or _yes(row[10]))
|
||||
print(f"== {ts[:16].replace('T', ' ')} {ref} [{kind}] {title}")
|
||||
print(f" {version} on {osname}, SteamOS {steamos or 'unknown'}"
|
||||
f"{', may follow up at ' + contact if reply else ''}")
|
||||
f"{', may follow up at ' + contact if reply else ''}"
|
||||
f"{', follow-up permission since withdrawn' if row[10] == 'withdrawn' else ''}")
|
||||
print(' ' + text.replace('\n', '\n '))
|
||||
if diag:
|
||||
print(' --- diagnostics\n ' + diag.replace('\n', '\n '))
|
||||
|
||||
+7
-4
@@ -3963,6 +3963,7 @@ async function offerTest(m) {
|
||||
|
||||
// ---- privacy: anonymous analytics levels (ui/frame_telemetry.py, docs/privacy.md) ----
|
||||
const telemetry = { usage: false, compat: false, blocked: "not loaded" };
|
||||
let privacyNoticeShown = false; // this visit: then the contact prompt waits for another one
|
||||
function renderTelemetry(s) {
|
||||
Object.assign(telemetry, s);
|
||||
setRepHint();
|
||||
@@ -3973,6 +3974,7 @@ function renderTelemetry(s) {
|
||||
: "Nothing sent yet.";
|
||||
const showNotice = !s.blocked && !s.noticeShown && s.usage;
|
||||
$("privacyNotice").hidden = !showNotice;
|
||||
if (showNotice) privacyNoticeShown = true;
|
||||
if (showNotice) api("/api/telemetry", { noticeShown: true }).catch(() => {});
|
||||
}
|
||||
async function loadTelemetry() {
|
||||
@@ -4015,13 +4017,14 @@ async function loadContact() {
|
||||
try { renderContact(await api("/api/contact")); } catch { return; }
|
||||
checkContactPrompt();
|
||||
}
|
||||
// One time only, only once the Frame has connected, and never on top of the privacy notice or
|
||||
// straight after it (two asks in a row is nagging): checked at load and whenever the Frame connects.
|
||||
// One time only, only once the Frame has connected, and never in a visit that showed the privacy
|
||||
// notice (two asks in a row is nagging): checked at load and whenever the Frame connects.
|
||||
async function checkContactPrompt() {
|
||||
if (!$("contactNotice").hidden || !$("privacyNotice").hidden) return;
|
||||
await telemetryLoaded;
|
||||
if (privacyNoticeShown || !$("contactNotice").hidden) return;
|
||||
let s;
|
||||
try { s = await api("/api/contact"); } catch { return; }
|
||||
if (!s.showPrompt || !$("contactNotice").hidden || !$("privacyNotice").hidden) return;
|
||||
if (!s.showPrompt || privacyNoticeShown || !$("contactNotice").hidden) return;
|
||||
$("contactNotice").hidden = false;
|
||||
api("/api/contact/prompt", { prompt: "shown" }).catch(() => {});
|
||||
}
|
||||
|
||||
+3
-1
@@ -134,6 +134,7 @@ LINK = None # the connector (frame_link.Link); None on the Frame itself
|
||||
# install's clean-up) to the other headset.
|
||||
_work_lock = threading.Lock()
|
||||
_work = [0]
|
||||
NOT_HEADSET_WORK = {"/api/devices", "/api/contact", "/api/contact/prompt"}
|
||||
|
||||
|
||||
@contextlib.contextmanager
|
||||
@@ -2440,7 +2441,8 @@ class Handler(BaseHTTPRequestHandler):
|
||||
body = json.loads(self.rfile.read(length) or b"{}")
|
||||
if not isinstance(body, dict):
|
||||
raise Failure("request body must be a JSON object", 400)
|
||||
with (contextlib.nullcontext() if path == "/api/devices" else working(meant)):
|
||||
# Not headset work: switching headsets mustn't wait for (or refuse) these.
|
||||
with (contextlib.nullcontext() if path in NOT_HEADSET_WORK else working(meant)):
|
||||
result = handler(body)
|
||||
self.send_json(result)
|
||||
except Failure as e:
|
||||
|
||||
Reference in new issue
Block a user