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

Why aren't groups/permissions built-in conditions? #35

Closed AdrienLemaire closed 5 years ago

AdrienLemaire commented 5 years ago

I have never used django-flags so far. I found it interesting to build a workflow based on django groups, but was surprised to notice that neither groups or permissions are mentioned in the built-in conditions, although user and anonymous are. Is there a reason for that, like security concerns?

willbarton commented 5 years ago

@Fandekasp Hi! There aren't any particular concerns, and if you have a use for them it'd be fairly easy to implement a condition based on group-checking and permission-checking. We'd welcome the contribution back too 😃 .

AdrienLemaire commented 5 years ago

@willbarton thanks for the reply. Thinking a bit more about it, I actually can't see a use-case for it. Django always makes a {{ perms }} object available to templates, so I don't see how django-flags can bring more value for this.

django perms

{% if perms.foo.can_vote %}
{% endif %}

django flags

{% load feature_flags %}
{% flag_enabled 'CAN_VOTE' as can_vote %}

{% if can_vote %}
{% endif %}

If you think otherwise, please let me know about it. Otherwise, you can close this issue :)

willbarton commented 5 years ago

@Fandekasp that's a good point.

The nice thing that flags do is make it so that the conditions under which code can be enabled are distinct from the code that depends on them. So, in your example, CAN_VOTE could be based on permissions, yes, but also on any number of other conditions, and the implementation in the template doesn't have to change to accommodate them.

That said, I am struggling to come up with instances where I think permission-based flags would make sense — flags are meant to be ephemeral (though there's no enforcement of that), and permissions or group conditions could encourage longer-lived flags (not that our existing conditions don't do that).

But, it's an interesting question to consider! I'll go ahead and close this, and if anyone comes across a good use case, as I said, we'll happily accept contributions!