Skip to content

feat(stripe-mock): Add subscription_metadata and charge_metadata config#53596

Merged
arthurdedeus merged 2 commits intomasterfrom
test/support-per-object-metadata
Apr 8, 2026
Merged

feat(stripe-mock): Add subscription_metadata and charge_metadata config#53596
arthurdedeus merged 2 commits intomasterfrom
test/support-per-object-metadata

Conversation

@arthurdedeus
Copy link
Copy Markdown
Contributor

@arthurdedeus arthurdedeus commented Apr 7, 2026

Problem

The stripe-mock service applied customer_metadata to all object types (customer, subscription, charge), making it impossible
to test scenarios where posthog_person_distinct_id is set on subscriptions or charges but not on the customer.

Part of #52270

Changes

  • Added subscription_metadata and charge_metadata config fields to stripe-mock, each injected only into their respective
    object type
  • Updated default config comments with examples

How did you test this code?

Verified stripe-mock generates correct per-object metadata via local dev testing.

Publish to changelog?

No

Docs update

no

Copy link
Copy Markdown
Contributor Author

arthurdedeus commented Apr 7, 2026

@arthurdedeus arthurdedeus changed the base branch from feat/coalesce-distinct-id-from-child-objects to graphite-base/53596 April 7, 2026 18:01
@arthurdedeus arthurdedeus force-pushed the test/support-per-object-metadata branch from f3277ef to 2eec6e4 Compare April 7, 2026 18:10
@arthurdedeus arthurdedeus marked this pull request as ready for review April 7, 2026 18:11
@arthurdedeus arthurdedeus requested review from a team as code owners April 7, 2026 18:11
@assign-reviewers-posthog assign-reviewers-posthog bot requested review from a team April 7, 2026 18:12
@arthurdedeus arthurdedeus removed request for a team April 7, 2026 18:12
Allows setting metadata separately per object type, so you can test
the person resolution feature by putting posthog_person_distinct_id
on subscriptions/charges without it being on the customer.
@arthurdedeus arthurdedeus force-pushed the test/support-per-object-metadata branch from 2eec6e4 to 910110e Compare April 7, 2026 18:13
@arthurdedeus arthurdedeus changed the base branch from graphite-base/53596 to master April 7, 2026 18:14
@arthurdedeus arthurdedeus self-assigned this Apr 7, 2026
@arthurdedeus arthurdedeus added the stamphog Request AI review from stamphog label Apr 7, 2026
Copy link
Copy Markdown

@stamphog stamphog bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gates denied this PR because it touches billing-related code (stripe-mock), which is on the deny-list and requires human review.

@stamphog stamphog bot removed the stamphog Request AI review from stamphog label Apr 7, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 7, 2026

Comments Outside Diff (1)

  1. services/stripe-mock/tests/test_scenarios.py, line 143-167 (link)

    P2 Missing test coverage for subscription_metadata and charge_metadata

    The existing test_custom_metadata_injected test only verifies that customer_metadata is applied to customers. The new subscription_metadata and charge_metadata fields — which are the core feature of this PR — have no test coverage at all. There is also no test verifying the isolation guarantee (e.g. that subscription_metadata does NOT appear on customers or charges).

    Per the project's preference for parameterised tests, consider expanding this into something like:

    @pytest.mark.parametrize(
        "field,collection",
        [
            ("customer_metadata", "customers"),
            ("subscription_metadata", "subscriptions"),
            ("charge_metadata", "charges"),
        ],
    )
    def test_metadata_injected_per_object_type(self, field, collection):
        cfg = MockConfig(
            **{field: {"env": "test"}},
            customer_types={
                "loyalists_monthly": 1,
                "loyalists_annual": 0,
                "churners": 0,
                "resubscribers": 0,
                "upgraders": 0,
                "downgraders": 0,
                "interval_switchers": 0,
                "coupon_users": 0,
                "multi_currency_eur": 0,
                "multi_currency_gbp": 0,
                "multi_currency_jpy": 0,
                "refund_recipients": 0,
                "trial_users": 0,
                "late_joiners": 0,
                "edge_combos": 0,
            },
        )
        collections = build_from_config(cfg)
        assert collections[collection][0]["metadata"]["env"] == "test"
    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: services/stripe-mock/tests/test_scenarios.py
    Line: 143-167
    
    Comment:
    **Missing test coverage for `subscription_metadata` and `charge_metadata`**
    
    The existing `test_custom_metadata_injected` test only verifies that `customer_metadata` is applied to customers. The new `subscription_metadata` and `charge_metadata` fields — which are the core feature of this PR — have no test coverage at all. There is also no test verifying the isolation guarantee (e.g. that `subscription_metadata` does NOT appear on customers or charges).
    
    Per the project's preference for parameterised tests, consider expanding this into something like:
    
    ```python
    @pytest.mark.parametrize(
        "field,collection",
        [
            ("customer_metadata", "customers"),
            ("subscription_metadata", "subscriptions"),
            ("charge_metadata", "charges"),
        ],
    )
    def test_metadata_injected_per_object_type(self, field, collection):
        cfg = MockConfig(
            **{field: {"env": "test"}},
            customer_types={
                "loyalists_monthly": 1,
                "loyalists_annual": 0,
                "churners": 0,
                "resubscribers": 0,
                "upgraders": 0,
                "downgraders": 0,
                "interval_switchers": 0,
                "coupon_users": 0,
                "multi_currency_eur": 0,
                "multi_currency_gbp": 0,
                "multi_currency_jpy": 0,
                "refund_recipients": 0,
                "trial_users": 0,
                "late_joiners": 0,
                "edge_combos": 0,
            },
        )
        collections = build_from_config(cfg)
        assert collections[collection][0]["metadata"]["env"] == "test"
    ```
    
    How can I resolve this? If you propose a fix, please make it concise.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix All With AI
This is a comment left during a code review.
Path: services/stripe-mock/src/stripe_mock/data/scenarios.py
Line: 288

Comment:
**`charge_metadata` computed inside loop**

Unlike `customer_metadata` and `subscription_metadata` (which are built once before the loop on lines 148–149), `charge_metadata` is recomputed on every monthly iteration even though neither `self.cfg.charge_metadata` nor `persona_metadata` changes during the loop. For consistency, hoist it alongside the other two metadata variables before the loop starts:

```suggestion
            charge_metadata = {**self.cfg.charge_metadata, **(persona_metadata or {})}
```

(Add this line after line 149, and remove it from inside the loop.)

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: services/stripe-mock/tests/test_scenarios.py
Line: 143-167

Comment:
**Missing test coverage for `subscription_metadata` and `charge_metadata`**

The existing `test_custom_metadata_injected` test only verifies that `customer_metadata` is applied to customers. The new `subscription_metadata` and `charge_metadata` fields — which are the core feature of this PR — have no test coverage at all. There is also no test verifying the isolation guarantee (e.g. that `subscription_metadata` does NOT appear on customers or charges).

Per the project's preference for parameterised tests, consider expanding this into something like:

```python
@pytest.mark.parametrize(
    "field,collection",
    [
        ("customer_metadata", "customers"),
        ("subscription_metadata", "subscriptions"),
        ("charge_metadata", "charges"),
    ],
)
def test_metadata_injected_per_object_type(self, field, collection):
    cfg = MockConfig(
        **{field: {"env": "test"}},
        customer_types={
            "loyalists_monthly": 1,
            "loyalists_annual": 0,
            "churners": 0,
            "resubscribers": 0,
            "upgraders": 0,
            "downgraders": 0,
            "interval_switchers": 0,
            "coupon_users": 0,
            "multi_currency_eur": 0,
            "multi_currency_gbp": 0,
            "multi_currency_jpy": 0,
            "refund_recipients": 0,
            "trial_users": 0,
            "late_joiners": 0,
            "edge_combos": 0,
        },
    )
    collections = build_from_config(cfg)
    assert collections[collection][0]["metadata"]["env"] == "test"
```

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "feat(stripe-mock): Add subscription_meta..." | Re-trigger Greptile

sub["latest_invoice"] = invoice["id"]

ch_idx = _next_id("ch")
charge_metadata = {**self.cfg.charge_metadata, **(persona_metadata or {})}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 charge_metadata computed inside loop

Unlike customer_metadata and subscription_metadata (which are built once before the loop on lines 148–149), charge_metadata is recomputed on every monthly iteration even though neither self.cfg.charge_metadata nor persona_metadata changes during the loop. For consistency, hoist it alongside the other two metadata variables before the loop starts:

Suggested change
charge_metadata = {**self.cfg.charge_metadata, **(persona_metadata or {})}
charge_metadata = {**self.cfg.charge_metadata, **(persona_metadata or {})}

(Add this line after line 149, and remove it from inside the loop.)

Prompt To Fix With AI
This is a comment left during a code review.
Path: services/stripe-mock/src/stripe_mock/data/scenarios.py
Line: 288

Comment:
**`charge_metadata` computed inside loop**

Unlike `customer_metadata` and `subscription_metadata` (which are built once before the loop on lines 148–149), `charge_metadata` is recomputed on every monthly iteration even though neither `self.cfg.charge_metadata` nor `persona_metadata` changes during the loop. For consistency, hoist it alongside the other two metadata variables before the loop starts:

```suggestion
            charge_metadata = {**self.cfg.charge_metadata, **(persona_metadata or {})}
```

(Add this line after line 149, and remove it from inside the loop.)

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Copy Markdown
Contributor

@vdekrijger vdekrijger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The greptile comment seems fair, but since it's a mock I guess it's not that big of an issue 😄

@arthurdedeus arthurdedeus merged commit 6583432 into master Apr 8, 2026
136 of 137 checks passed
@arthurdedeus arthurdedeus deleted the test/support-per-object-metadata branch April 8, 2026 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants