Skip to content

fix: propagate missing params in module-level wrapper functions#479

Open
dustinbyrne wants to merge 2 commits intomasterfrom
fix/propagate-missing-params-module-wrappers
Open

fix: propagate missing params in module-level wrapper functions#479
dustinbyrne wants to merge 2 commits intomasterfrom
fix/propagate-missing-params-module-wrappers

Conversation

@dustinbyrne
Copy link
Copy Markdown
Contributor

Problem

The module-level convenience functions in posthog/__init__.py (e.g. posthog.group_identify(), posthog.get_all_flags()) were missing parameters that the underlying Client methods accept:

  • group_identify: missing distinct_id — callers couldn't specify a stable distinct ID, causing a random UUID to be generated on every call. This creates a garbage person record in the backend on each invocation.
  • get_all_flags: missing flag_keys_to_evaluate — callers couldn't limit evaluation to a subset of flags.
  • get_all_flags_and_payloads: same missing flag_keys_to_evaluate.

Changes

  • Added distinct_id parameter to group_identify() wrapper and forwarded it to the client.
  • Added flag_keys_to_evaluate parameter to get_all_flags() and get_all_flags_and_payloads() wrappers and forwarded them to the client.
  • Added tests in test_module.py verifying all three params are propagated correctly (value forwarding + default-to-None).

Propagate distinct_id for group_identify and flag_keys_to_evaluate for
get_all_flags/get_all_flags_and_payloads through the module-level
wrapper functions in posthog/__init__.py.

These parameters were accepted by Client methods but not forwarded
by the convenience wrappers, making them inaccessible when using
the posthog.group_identify() etc. module-level API.
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

posthog-python Compliance Report

Date: 2026-04-06 15:53:11 UTC
Duration: 193ms

✅ All Tests Passed!

0/0 tests passed


@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 6, 2026

Comments Outside Diff (1)

  1. posthog/__init__.py, line 619-628 (link)

    P2 New flag_keys_to_evaluate param missing from docstring

    The newly added flag_keys_to_evaluate parameter is not documented in the Args section of get_all_flags's docstring. Consider adding an entry:

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: posthog/__init__.py
    Line: 619-628
    
    Comment:
    **New `flag_keys_to_evaluate` param missing from docstring**
    
    The newly added `flag_keys_to_evaluate` parameter is not documented in the `Args` section of `get_all_flags`'s docstring. Consider adding an entry:
    
    
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: posthog/test/test_module.py
Line: 71-95

Comment:
**Prefer parameterised tests for duplicate test cases**

The four tests covering `get_all_flags` and `get_all_flags_and_payloads` are structurally identical — the only difference is which function is called. The project's convention (used in `test_utils.py`, `test_client.py`, etc.) is to use `@parameterized.expand` to avoid this duplication. For example:

```python
from parameterized import parameterized

@parameterized.expand([
    ("get_all_flags", posthog.get_all_flags, "get_all_flags"),
    ("get_all_flags_and_payloads", posthog.get_all_flags_and_payloads, "get_all_flags_and_payloads"),
])
def test_flag_keys_to_evaluate_propagated(self, _name, fn, client_method_name):
    fn("user_123", flag_keys_to_evaluate=["flag-1", "flag-2"])
    call_kwargs = getattr(self.mock_client, client_method_name).call_args[1]
    self.assertEqual(call_kwargs["flag_keys_to_evaluate"], ["flag-1", "flag-2"])

@parameterized.expand([
    ("get_all_flags", posthog.get_all_flags, "get_all_flags"),
    ("get_all_flags_and_payloads", posthog.get_all_flags_and_payloads, "get_all_flags_and_payloads"),
])
def test_flag_keys_defaults_to_none(self, _name, fn, client_method_name):
    fn("user_123")
    call_kwargs = getattr(self.mock_client, client_method_name).call_args[1]
    self.assertIsNone(call_kwargs["flag_keys_to_evaluate"])
```

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

---

This is a comment left during a code review.
Path: posthog/__init__.py
Line: 619-628

Comment:
**New `flag_keys_to_evaluate` param missing from docstring**

The newly added `flag_keys_to_evaluate` parameter is not documented in the `Args` section of `get_all_flags`'s docstring. Consider adding an entry:

```suggestion
    """
    Get all flags for a given user.

    Args:
        distinct_id: The user's distinct ID
        groups: Groups mapping
        person_properties: Person properties
        group_properties: Group properties
        only_evaluate_locally: Whether to evaluate only locally
        disable_geoip: Whether to disable GeoIP lookup
        flag_keys_to_evaluate: Optional list of flag keys to limit evaluation to a subset of flags
```

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

Reviews (1): Last reviewed commit: "fix: propagate missing params in module-..." | Re-trigger Greptile

Comment on lines +71 to +95
def test_get_all_flags_propagates_flag_keys_to_evaluate(self):
posthog.get_all_flags(
"user_123",
flag_keys_to_evaluate=["flag-1", "flag-2"],
)
call_kwargs = self.mock_client.get_all_flags.call_args[1]
self.assertEqual(call_kwargs["flag_keys_to_evaluate"], ["flag-1", "flag-2"])

def test_get_all_flags_flag_keys_defaults_to_none(self):
posthog.get_all_flags("user_123")
call_kwargs = self.mock_client.get_all_flags.call_args[1]
self.assertIsNone(call_kwargs["flag_keys_to_evaluate"])

def test_get_all_flags_and_payloads_propagates_flag_keys_to_evaluate(self):
posthog.get_all_flags_and_payloads(
"user_123",
flag_keys_to_evaluate=["flag-1", "flag-2"],
)
call_kwargs = self.mock_client.get_all_flags_and_payloads.call_args[1]
self.assertEqual(call_kwargs["flag_keys_to_evaluate"], ["flag-1", "flag-2"])

def test_get_all_flags_and_payloads_flag_keys_defaults_to_none(self):
posthog.get_all_flags_and_payloads("user_123")
call_kwargs = self.mock_client.get_all_flags_and_payloads.call_args[1]
self.assertIsNone(call_kwargs["flag_keys_to_evaluate"])
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 Prefer parameterised tests for duplicate test cases

The four tests covering get_all_flags and get_all_flags_and_payloads are structurally identical — the only difference is which function is called. The project's convention (used in test_utils.py, test_client.py, etc.) is to use @parameterized.expand to avoid this duplication. For example:

from parameterized import parameterized

@parameterized.expand([
    ("get_all_flags", posthog.get_all_flags, "get_all_flags"),
    ("get_all_flags_and_payloads", posthog.get_all_flags_and_payloads, "get_all_flags_and_payloads"),
])
def test_flag_keys_to_evaluate_propagated(self, _name, fn, client_method_name):
    fn("user_123", flag_keys_to_evaluate=["flag-1", "flag-2"])
    call_kwargs = getattr(self.mock_client, client_method_name).call_args[1]
    self.assertEqual(call_kwargs["flag_keys_to_evaluate"], ["flag-1", "flag-2"])

@parameterized.expand([
    ("get_all_flags", posthog.get_all_flags, "get_all_flags"),
    ("get_all_flags_and_payloads", posthog.get_all_flags_and_payloads, "get_all_flags_and_payloads"),
])
def test_flag_keys_defaults_to_none(self, _name, fn, client_method_name):
    fn("user_123")
    call_kwargs = getattr(self.mock_client, client_method_name).call_args[1]
    self.assertIsNone(call_kwargs["flag_keys_to_evaluate"])
Prompt To Fix With AI
This is a comment left during a code review.
Path: posthog/test/test_module.py
Line: 71-95

Comment:
**Prefer parameterised tests for duplicate test cases**

The four tests covering `get_all_flags` and `get_all_flags_and_payloads` are structurally identical — the only difference is which function is called. The project's convention (used in `test_utils.py`, `test_client.py`, etc.) is to use `@parameterized.expand` to avoid this duplication. For example:

```python
from parameterized import parameterized

@parameterized.expand([
    ("get_all_flags", posthog.get_all_flags, "get_all_flags"),
    ("get_all_flags_and_payloads", posthog.get_all_flags_and_payloads, "get_all_flags_and_payloads"),
])
def test_flag_keys_to_evaluate_propagated(self, _name, fn, client_method_name):
    fn("user_123", flag_keys_to_evaluate=["flag-1", "flag-2"])
    call_kwargs = getattr(self.mock_client, client_method_name).call_args[1]
    self.assertEqual(call_kwargs["flag_keys_to_evaluate"], ["flag-1", "flag-2"])

@parameterized.expand([
    ("get_all_flags", posthog.get_all_flags, "get_all_flags"),
    ("get_all_flags_and_payloads", posthog.get_all_flags_and_payloads, "get_all_flags_and_payloads"),
])
def test_flag_keys_defaults_to_none(self, _name, fn, client_method_name):
    fn("user_123")
    call_kwargs = getattr(self.mock_client, client_method_name).call_args[1]
    self.assertIsNone(call_kwargs["flag_keys_to_evaluate"])
```

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant