fix: one lapsed membership to a deleted creator no longer fails the Patreon roster
CI / lint (push) Successful in 3s
CI / extension-version (push) Successful in 3s
Build images / sign-extension (push) Successful in 5s
Build images / build-agent (push) Successful in 7s
CI / backend-lint-and-test (push) Failing after 35s
CI / frontend-build (push) Successful in 32s
Build images / build-web (push) Successful in 1m20s
Build images / smoke-web (push) Skipped
Build images / build-ml (push) Successful in 2m17s
Build images / promote (push) Skipped
CI / integration (push) Successful in 2m26s
CI / lint (push) Successful in 3s
CI / extension-version (push) Successful in 3s
Build images / sign-extension (push) Successful in 5s
Build images / build-agent (push) Successful in 7s
CI / backend-lint-and-test (push) Failing after 35s
CI / frontend-build (push) Successful in 32s
Build images / build-web (push) Successful in 1m20s
Build images / smoke-web (push) Skipped
Build images / build-ml (push) Successful in 2m17s
Build images / promote (push) Skipped
CI / integration (push) Successful in 2m26s
The first live sweep failed with "Patreon member resource has no campaign relationship". The operator ran FC's exact /api/members request in a browser and shared the response. - It has 104 rows. The browser's settings page shows 43, because FC sends no membership-type filter and so also gets lapses back to 2016. - Exactly one row has no `campaign` key at all: a former_patron whose membership ended in 2017. Its included reward has no campaign link either, so the creator's page is gone. - Every other row, including the 4 is_active=false campaigns, has a campaign. _membership returns None for a campaign-less row only when has_paid_access says it is definitely not paying (a known lapsed status, or a free member), and iter_memberships skips it. That changes no conclusion. A lapsed membership and an absent one both mean "not paying", and no Source can match a campaign with no id. An active or unrecognised membership without a campaign still raises, because dropping a membership that might be paid would read downstream as a cancellation. Paging still counts the rows the server sent, not the rows kept. A test pins that, so a skip can't re-read an offset or stop a page short. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SHQB1YukL3VyvMK8rcbmV9
This commit is contained in:
@@ -631,7 +631,27 @@ class PatreonClient:
|
||||
"cannot tell a complete roster from a truncated one"
|
||||
)
|
||||
|
||||
def _membership(self, member: dict, index: dict) -> Membership:
|
||||
def _membership(self, member: dict, index: dict) -> Membership | None:
|
||||
"""One member row as a Membership, or None for a row the roster can skip.
|
||||
|
||||
The one skippable row is a LAPSED membership whose creator no longer
|
||||
exists. The live roster (note #3886, CORRECTION 3) returned 104 rows,
|
||||
because FC sends no membership-type filter and so gets lapses going back
|
||||
years. One of them, a membership that ended in 2017, carried no
|
||||
`campaign` relationship at all: the key is absent, not null, and its
|
||||
reward names no campaign either. The creator's page is gone.
|
||||
|
||||
Raising on that row made the whole roster unusable over one membership
|
||||
nobody can act on. Skipping it changes no conclusion. A lapsed
|
||||
membership already means "not paying", absence means the same, and no
|
||||
Source can be matched to a campaign that no longer has an id.
|
||||
|
||||
The refusal stays for every other row. An active or unrecognised
|
||||
membership without a creator is something FC cannot vouch for, and
|
||||
dropping it would read downstream as a cancellation.
|
||||
"""
|
||||
from .membership_roster import has_paid_access
|
||||
|
||||
attrs = member.get("attributes") or {}
|
||||
if "patron_status" not in attrs:
|
||||
raise PatreonDriftError(
|
||||
@@ -640,6 +660,17 @@ class PatreonClient:
|
||||
|
||||
campaign_ids = self._related_ids(member, "campaign")
|
||||
if not campaign_ids:
|
||||
paid = has_paid_access(
|
||||
"patreon", attrs.get("patron_status"),
|
||||
is_free_member=bool(attrs.get("is_free_member")),
|
||||
)
|
||||
if paid is False:
|
||||
log.info(
|
||||
"Patreon roster: skipping a lapsed membership with no campaign "
|
||||
"(creator deleted); status=%s access_expires_at=%s",
|
||||
attrs.get("patron_status"), attrs.get("access_expires_at"),
|
||||
)
|
||||
return None
|
||||
raise PatreonDriftError(
|
||||
"Patreon member resource has no campaign relationship — a "
|
||||
"membership we cannot attribute to a creator is not usable"
|
||||
@@ -700,7 +731,9 @@ class PatreonClient:
|
||||
index = self._transform(response)
|
||||
rows = [m for m in (response.get("data") or []) if isinstance(m, dict)]
|
||||
for member in rows:
|
||||
yield self._membership(member, index)
|
||||
membership = self._membership(member, index)
|
||||
if membership is not None:
|
||||
yield membership
|
||||
|
||||
seen += len(rows)
|
||||
total = int(response["meta"]["pagination"]["total"] or 0)
|
||||
|
||||
@@ -239,14 +239,72 @@ def test_a_missing_data_list_is_drift(client, payload):
|
||||
list(client.iter_memberships(user_id="1"))
|
||||
|
||||
|
||||
def test_a_member_with_no_campaign_is_drift(client, payload):
|
||||
def _index_of(payload, status):
|
||||
return next(
|
||||
i for i, m in enumerate(payload["data"])
|
||||
if m["attributes"]["patron_status"] == status
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("shape", ["null", "absent"])
|
||||
def test_an_active_member_with_no_campaign_is_drift(client, payload, shape):
|
||||
"""A PAID membership FC cannot attribute must refuse the roster. Dropping it
|
||||
would read downstream as the operator having cancelled it."""
|
||||
mangled = json.loads(json.dumps(payload))
|
||||
mangled["data"][0]["relationships"]["campaign"] = {"data": None}
|
||||
rels = mangled["data"][_index_of(mangled, "active_patron")]["relationships"]
|
||||
if shape == "null":
|
||||
rels["campaign"] = {"data": None}
|
||||
else:
|
||||
del rels["campaign"]
|
||||
client._request = lambda *a, **k: mangled
|
||||
with pytest.raises(PatreonDriftError, match="campaign"):
|
||||
list(client.iter_memberships(user_id="1"))
|
||||
|
||||
|
||||
def test_an_unrecognised_status_with_no_campaign_is_drift(client, payload):
|
||||
"""Only a status KNOWN to mean "not paying" may be skipped. An unknown word
|
||||
could be a paid membership (has_paid_access returns None for it)."""
|
||||
mangled = json.loads(json.dumps(payload))
|
||||
row = mangled["data"][_index_of(mangled, "active_patron")]
|
||||
row["attributes"]["patron_status"] = "some_new_status"
|
||||
del row["relationships"]["campaign"]
|
||||
client._request = lambda *a, **k: mangled
|
||||
with pytest.raises(PatreonDriftError, match="campaign"):
|
||||
list(client.iter_memberships(user_id="1"))
|
||||
|
||||
|
||||
def test_a_lapsed_member_whose_creator_is_gone_is_skipped(client, payload):
|
||||
"""The live roster's shape (note #3886, CORRECTION 3). A membership that
|
||||
lapsed in 2017 came back with no `campaign` key at all, because the creator's
|
||||
page no longer exists. It used to fail the whole roster. Now it is the only
|
||||
row missing, and every other row still arrives."""
|
||||
mangled = json.loads(json.dumps(payload))
|
||||
lapsed = _index_of(mangled, "former_patron")
|
||||
del mangled["data"][lapsed]["relationships"]["campaign"]
|
||||
client._request = lambda *a, **k: mangled
|
||||
rows = list(client.iter_memberships(user_id="1"))
|
||||
assert len(rows) == len(payload["data"]) - 1
|
||||
assert all(r.campaign_id for r in rows)
|
||||
assert all(r.status == "active_patron" for r in rows)
|
||||
|
||||
|
||||
def test_skipping_does_not_end_paging_early(client, payload):
|
||||
"""Paging counts the rows the SERVER sent, not the ones kept. Counting kept
|
||||
rows would re-request an offset that was already read, or stop one page
|
||||
short, whenever a row is skipped."""
|
||||
first = json.loads(json.dumps(payload))
|
||||
del first["data"][_index_of(first, "former_patron")]["relationships"]["campaign"]
|
||||
total = 2 * len(payload["data"])
|
||||
first["meta"]["pagination"]["total"] = total
|
||||
second = json.loads(json.dumps(payload))
|
||||
second["meta"]["pagination"]["total"] = total
|
||||
calls = _serve(client, first, second)
|
||||
rows = list(client.iter_memberships(user_id="1"))
|
||||
assert len(calls) == 2
|
||||
assert calls[1][1]["page[offset]"] == str(len(payload["data"]))
|
||||
assert len(rows) == total - 1
|
||||
|
||||
|
||||
def test_a_member_with_no_patron_status_is_drift(client, payload):
|
||||
mangled = json.loads(json.dumps(payload))
|
||||
del mangled["data"][0]["attributes"]["patron_status"]
|
||||
|
||||
Reference in New Issue
Block a user