diff --git a/backend/app/services/patreon_client.py b/backend/app/services/patreon_client.py index 11bb7da..6609047 100644 --- a/backend/app/services/patreon_client.py +++ b/backend/app/services/patreon_client.py @@ -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) diff --git a/tests/test_patreon_memberships.py b/tests/test_patreon_memberships.py index 9612c5f..e12b161 100644 --- a/tests/test_patreon_memberships.py +++ b/tests/test_patreon_memberships.py @@ -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"]