dimagi/commcare-hq — pull request #3909 by esoergel (2014-07-22 16:45:55+00:00)
Title: Make icon option appear when enabled in Feature Previews and get rid of is_previewer check

@dimagi/team-commcare-hq You might find this interesting. It's a really wacky bug.

http://manage.dimagi.com/default.asp?124325

This was a pretty straightforward-looking issue, I just removed the line checking `is_previewer` and it worked. Except it didn't really make sense that non-previewers weren't able to see the feature with that line there, `user.is_previewer or feature.is_enabled` should evaluate to true if it's `False or True`.

I did some digging, turns out that `is_previewer` is a callable, so this evaluated to `is_previewer_fn or True`, and because `bool(any_function) == True`, then this should always be True.

Except bool() is never explicitly called, so instead this returns the first truthy value from the or expression, or `is_previewer_fn`. This is passed around until it ends up in a [template](https://github.com/dimagi/commcare-hq/blob/master/corehq/apps/app_manager/templates/app_manager/module_view.html#L12), and because django templates will call functions for you, that function was at last being called. In effect, `user.is_previewer or feature.is_enabled` resolved to `user.is_previewer()`.

I decided to look for other times when is_previewer is used but not called, and found these other two instances. This will be tracked here: http://manage.dimagi.com/default.asp?125511 It's easy to just add `()` after it, but that means for example, that the "Case Groups" view will no longer appear in the sidebar for people who can currently see it, so Amelia will decide whether that's acceptable or not.