Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for Case Sensitivity Issue in Python Library When Retrieving Feature Flag Payload #123

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

reihtw
Copy link

@reihtw reihtw commented May 15, 2024

When trying to retrieve the feature flag payload, it is not possible to do so using uppercase characters because the code always converts the provided feature flag key to lowercase.

This issue occurs only in the Python library. I tried retrieving the payload using other languages, and it works normally.

@neilkakkar neilkakkar requested a review from a team September 5, 2024 11:14
@dmarticus
Copy link
Contributor

dmarticus commented Sep 10, 2024

Hi @reihtw, sorry for the delay in reviewing this, I was OOO for the last week.

I reproduced this issue locally and confirmed that your fix works. Thanks for the contribution! Do you mind adding a test to test/test_feature_flags.py that verifies this behavior? It couldn't hurt to encode these types of bugs into our tests. Something like this

    @mock.patch("posthog.client.decide")
    def test_boolean_feature_flag_payload_for_capitalized_feature_flags_decide(
        self, patch_decide
    ):
        patch_decide.return_value = {"featureFlagPayloads": {"PERSON-FLAG": 300}}
        self.assertEqual(
            self.client.get_feature_flag_payload(
                "PERSON-FLAG", "some-distinct-id", person_properties={"region": "USA"}
            ),
            300,
        )

        self.assertEqual(
            self.client.get_feature_flag_payload(
                "PERSON-FLAG",
                "some-distinct-id",
                match_value=True,
                person_properties={"region": "USA"},
            ),
            300,
        )
        self.assertEqual(patch_decide.call_count, 2)

and maybe another one for the local evaluation path is all we would need :)

Thank you!

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