search

cfpb / django-flags

Feature flags for Django projects
https://cfpb.github.io/django-flags/
Creative Commons Zero v1.0 Universal
260 stars 32 forks source link

GET parameter condition is case sensitive #30

Closed contolini closed 5 years ago

contolini commented 5 years ago

Currently, a GET parameter will only trigger a flag if it equals True. For example, given:

FLAGS = {
    'THERE_IS_AN_OUTAGE': [
        {'condition': 'parameter', 'value': 'outage'}
    ]
}

The flag will be enabled with /foo?outage=True.

It won't be enabled with /foo?outage=true, /foo?outage=today or /foo?outage.

I'm unsure if the above functionality is by design but IMHO it should at least accept /foo?outage=true if not the other options as well.

BTW A+ Django app 10/10 five stars would download again 💯

willbarton commented 5 years ago

I'm unsure if the above functionality is by design but IMHO it should at least accept /foo?outage=true if not the other options as well.

@contolini That's a good question — the git history and the documentation are somewhat ambiguous, and I suspect the person responsible cough doesn't recall either.

What would be the desired behavior here?

It won't be enabled with /foo?outage=true, /foo?outage=today or /foo?outage.

I kinda think it should be enabled for any of these, personally — that the parameter exists should be enough. I'll open a PR to fix this and #27 together, but I'd like thoughts on this question specifically.

contolini commented 5 years ago

I kinda think it should be enabled for any of these

👍 I agree, I think the existence of the parameter should enable the flag.