Skip to content
This repository has been archived by the owner on Sep 10, 2020. It is now read-only.

[utils] Add error handling in csvfile validation #252

Merged
merged 3 commits into from
Nov 21, 2019
Merged

[utils] Add error handling in csvfile validation #252

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0530d86
[utils] Add error handling in csvfile validation #251
Aviral14 Nov 19, 2019
f4699ff
[utils] Improved CSV format validation error message
nemesifier Nov 21, 2019
d48d1fc
[utils] Updated format validation test
nemesifier Nov 21, 2019
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
29 changes: 19 additions & 10 deletions django_freeradius/tests/base/test_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,26 @@
class BaseTestUtils(object):
def test_find_available_username(self):
User = get_user_model()
User.objects.create(username='rohith', password='password')
self.assertEqual(find_available_username('rohith', []), 'rohith1')
User.objects.create(username='rohith1', password='password')
self.assertEqual(find_available_username('rohith', []), 'rohith2')
User.objects.create(username="rohith", password="password")
self.assertEqual(find_available_username("rohith", []), "rohith1")
User.objects.create(username="rohith1", password="password")
self.assertEqual(find_available_username("rohith", []), "rohith2")

def test_validate_file_format(self):
invalid_format_path = self._get_path("static/test_batch_invalid_format.pdf")
Copy link
Member

Choose a reason for hiding this comment

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

I think this test method is becoming too big, can you please write a new test method with the assertions you have added?

Another suggestion: please keep the changes to minimum, do not edit lines that are not bugged, right now I see you're changing a lot of single quotes to double quotes, which even if it's good for consistency, adds noise and makes the work of the reviewers harder. Those kind of clean up changes should be done separately if you really want to do it (clean up is welcome in my opinion).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nemesisdesign ok I will try to write a completely new method for testing the validation of csv files.
Actually,my formatter was automatically doing these cleanup changes you mentioned.I will remember not to do any kind of edits in non-buggy lines!

with self.assertRaises(ValidationError) as error:
validate_csvfile(open(invalid_format_path, "rb"))
self.assertTrue(
"Unrecognized file format, the supplied file does not look like a CSV file."
in error.exception.message
)

def test_validate_csvfile(self):
invalid_csv_path = self._get_path('static/test_batch_invalid.csv')
improper_csv_path = self._get_path('static/test_batch_improper.csv')
invalid_csv_path = self._get_path("static/test_batch_invalid.csv")
improper_csv_path = self._get_path("static/test_batch_improper.csv")
with self.assertRaises(ValidationError) as error:
validate_csvfile(open(invalid_csv_path, 'rt'))
self.assertTrue('Enter a valid email address' in error.exception.message)
validate_csvfile(open(invalid_csv_path, "rt"))
self.assertTrue("Enter a valid email address" in error.exception.message)
with self.assertRaises(ValidationError) as error:
validate_csvfile(open(improper_csv_path, 'rt'))
self.assertTrue('Improper CSV format' in error.exception.message)
validate_csvfile(open(improper_csv_path, "rt"))
self.assertTrue("Improper CSV format" in error.exception.message)
Binary file added django_freeradius/tests/static/test_batch_invalid_format.pdf
View file
Open in desktop
Binary file not shown.
32 changes: 20 additions & 12 deletions django_freeradius/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,24 @@
def find_available_username(username, users_list, prefix=False):
User = get_user_model()
suffix = 1
tmp = '{}{}'.format(username, suffix) if prefix else username
tmp = "{}{}".format(username, suffix) if prefix else username
names_list = map(lambda x: x.username, users_list)
while User.objects.filter(username=tmp).exists() or tmp in names_list:
suffix += 1 if prefix else 0
tmp = '{}{}'.format(username, suffix)
tmp = "{}{}".format(username, suffix)
suffix += 1 if not prefix else 0
return tmp


def validate_csvfile(csvfile):
csv_data = csvfile.read()
csv_data = csv_data.decode('utf-8') if isinstance(csv_data, bytes) else csv_data
reader = csv.reader(StringIO(csv_data), delimiter=',')
try:
csv_data = csv_data.decode("utf-8") if isinstance(csv_data, bytes) else csv_data
except UnicodeDecodeError:
raise ValidationError(
_("Unrecognized file format, the supplied file does not look like a CSV file.")
)
reader = csv.reader(StringIO(csv_data), delimiter=",")
error_message = "The CSV contains a line with invalid data,\
line number {} triggered the following error: {}"
row_count = 1
Expand All @@ -40,10 +45,14 @@ def validate_csvfile(csvfile):
try:
validate_email(email)
except ValidationError as e:
raise ValidationError(_(error_message.format(str(row_count), e.message)))
raise ValidationError(
_(error_message.format(str(row_count), e.message))
)
row_count += 1
elif len(row) > 0:
raise ValidationError(_(error_message.format(str(row_count), "Improper CSV format.")))
raise ValidationError(
_(error_message.format(str(row_count), "Improper CSV format."))
)
csvfile.seek(0)


Expand All @@ -64,20 +73,19 @@ def prefix_generate_users(prefix, n, password_length):
def generate_pdf(prefix, data):
template = get_template(BATCH_PDF_TEMPLATE)
html = template.render(data)
f = open('{}/{}.pdf'.format(settings.MEDIA_ROOT, prefix), 'w+b')
pisa.CreatePDF(html.encode('utf-8'), dest=f, encoding='utf-8')
f = open("{}/{}.pdf".format(settings.MEDIA_ROOT, prefix), "w+b")
pisa.CreatePDF(html.encode("utf-8"), dest=f, encoding="utf-8")
f.seek(0)
return File(f)


def set_default_group(sender, instance, created, **kwargs):
if created:
RadiusGroup = swapper.load_model('django_freeradius', 'RadiusGroup')
RadiusUserGroup = swapper.load_model('django_freeradius', 'RadiusUserGroup')
RadiusGroup = swapper.load_model("django_freeradius", "RadiusGroup")
RadiusUserGroup = swapper.load_model("django_freeradius", "RadiusUserGroup")
queryset = RadiusGroup.objects.filter(default=True)
if queryset.exists():
ug = RadiusUserGroup(user=instance,
group=queryset.first())
ug = RadiusUserGroup(user=instance, group=queryset.first())
ug.full_clean()
ug.save()

Expand Down