Skip to content

Commit

Permalink
Fixed removing conditions in the UI where the value is the empty string
Browse files Browse the repository at this point in the history
Fixes disqus#55. The API the frontend uses is untested, maybe some tests would have caught it.
  • Loading branch information
adamchainz committed May 13, 2016
1 parent b203c7a commit 27fc51d
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 1 deletion.
1 change: 1 addition & 0 deletions HISTORY.rst
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ Pending Release
* Made ``gargoyle.register()`` usable as a decorator
* Made ``gargoyle.unregister()`` return the boolean value of whether something
was unregistered.
* Fixed removing conditions where the value is the empty string.

1.2.5 (2016-05-09)
------------------
Expand Down
2 changes: 1 addition & 1 deletion gargoyle/nexus_modules.py
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ def remove_condition(self, request):
field_name = request.POST.get("field")
value = request.POST.get("value")

if not all([key, condition_set_id, field_name, value]):
if not all([key, condition_set_id, field_name]):
raise GargoyleException("Fields cannot be empty")

switch = gargoyle[key]
Expand Down
17 changes: 17 additions & 0 deletions tests/testapp/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,23 @@ def test_remove_condition(self):
assert not self.gargoyle.is_active('test', user5)
assert not self.gargoyle.is_active('test', user8771)

def test_add_condition_empty(self):
condition_set = 'gargoyle.builtins.UserConditionSet(auth.user)'

switch = Switch.objects.create(key='test', status=SELECTIVE)
switch = self.gargoyle['test']

user_empty = User(email='')
user_non_empty = User(email='[email protected]')
switch.add_condition(
condition_set=condition_set,
field_name='email',
condition='',
)

assert self.gargoyle.is_active('test', user_empty)
assert not self.gargoyle.is_active('test', user_non_empty)

def test_switch_defaults(self):
"""Test that defaults pulled from GARGOYLE_SWITCH_DEFAULTS.
Expand Down

0 comments on commit 27fc51d

Please sign in to comment.