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"
|
"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 {}
|
attrs = member.get("attributes") or {}
|
||||||
if "patron_status" not in attrs:
|
if "patron_status" not in attrs:
|
||||||
raise PatreonDriftError(
|
raise PatreonDriftError(
|
||||||
@@ -640,6 +660,17 @@ class PatreonClient:
|
|||||||
|
|
||||||
campaign_ids = self._related_ids(member, "campaign")
|
campaign_ids = self._related_ids(member, "campaign")
|
||||||
if not campaign_ids:
|
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(
|
raise PatreonDriftError(
|
||||||
"Patreon member resource has no campaign relationship — a "
|
"Patreon member resource has no campaign relationship — a "
|
||||||
"membership we cannot attribute to a creator is not usable"
|
"membership we cannot attribute to a creator is not usable"
|
||||||
@@ -700,7 +731,9 @@ class PatreonClient:
|
|||||||
index = self._transform(response)
|
index = self._transform(response)
|
||||||
rows = [m for m in (response.get("data") or []) if isinstance(m, dict)]
|
rows = [m for m in (response.get("data") or []) if isinstance(m, dict)]
|
||||||
for member in rows:
|
for member in rows:
|
||||||
yield self._membership(member, index)
|
membership = self._membership(member, index)
|
||||||
|
if membership is not None:
|
||||||
|
yield membership
|
||||||
|
|
||||||
seen += len(rows)
|
seen += len(rows)
|
||||||
total = int(response["meta"]["pagination"]["total"] or 0)
|
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"))
|
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 = 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
|
client._request = lambda *a, **k: mangled
|
||||||
with pytest.raises(PatreonDriftError, match="campaign"):
|
with pytest.raises(PatreonDriftError, match="campaign"):
|
||||||
list(client.iter_memberships(user_id="1"))
|
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):
|
def test_a_member_with_no_patron_status_is_drift(client, payload):
|
||||||
mangled = json.loads(json.dumps(payload))
|
mangled = json.loads(json.dumps(payload))
|
||||||
del mangled["data"][0]["attributes"]["patron_status"]
|
del mangled["data"][0]["attributes"]["patron_status"]
|
||||||
|
|||||||
Reference in New Issue
Block a user