Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions app/api/helpers/utilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,7 @@ def require_relationship(resource_list, data):


def string_empty(value):
is_not_str_type = type(value) is not str
if sys.version_info[0] < 3:
is_not_str_type = is_not_str_type and type(value) is not unicode
if type(value) is not value and is_not_str_type:
return False
return not (value and value.strip() and value != u'' and value != u' ')
return isinstance(value, str) and not value.strip()


def strip_tags(html):
Expand Down
12 changes: 0 additions & 12 deletions tests/all/integration/api/helpers/test_utilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,6 @@ def test_require_relationship(self):
data = ['event']
require_relationship(['sponsor', 'event'], data)

def test_string_empty(self):
"""Method to test whether an empty string is correctly identified."""

with app.test_request_context():
self.assertTrue(string_empty(''))
self.assertTrue(string_empty(' '))
self.assertFalse(string_empty('some value'))
self.assertFalse(string_empty(' some value '))
self.assertFalse(string_empty(str))
self.assertFalse(string_empty(int))
self.assertFalse(string_empty(None))

def test_monthdelta(self):
"""Method to test difference in months result"""

Expand Down
16 changes: 15 additions & 1 deletion tests/all/unit/api/helpers/test_utilities.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import unittest
from app.api.helpers.utilities import get_filename_from_cd

from app.api.helpers.utilities import string_empty

class TestUtilitiesHelperValidation(unittest.TestCase):
def test_get_filename_from_cd(self):
Expand All @@ -17,6 +17,20 @@ def test_get_filename_from_cd(self):
self.assertEqual(expected_response_first, response_first)
self.assertEqual(expected_response_none, response_none)

def test_string_empty(self):
"""Method to test whether an empty string is correctly identified."""

self.assertTrue(string_empty(''))
self.assertTrue(string_empty(' '))
self.assertTrue(string_empty('\t'))
self.assertTrue(string_empty('\n'))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add tabs as well

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

self.assertFalse(string_empty(None))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be true IMO

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None isn't str object I believe?

Copy link
Member

@iamareebjamal iamareebjamal Jul 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of times None can be passed for a string. We need to treat it as en empty string. For example, non-existent field. If we are using string_empty for validation, then having None as non empty will pass it as a correct value

However, this needs to be checked where string_empty is used so that we can ensure the correct behaviour

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Travis was failing when I did that. What could be done?

self.assertFalse(string_empty('some value'))
self.assertFalse(string_empty(' some value '))
self.assertFalse(string_empty(0))
self.assertFalse(string_empty([]))
self.assertFalse(string_empty(False))


if __name__ == '__main__':
unittest.main()