[utils] Add error handling in csvfile validation #252
Conversation
Aviral14
force-pushed
the
issues/251
branch
2 times, most recently
from
782122a
to
0c4baf3
Compare
atb00ker
moved this from To do (general)
to Needs review
in OpenWISP Contributor's Board
|
@atb00ker All checks have passed now! |
| 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") | ||
| invalid_format_path = self._get_path("static/test_batch_invalid_format.pdf") |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
@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!
|
PS: please also rebase your branch on the current master. |
|
@Aviral14 ping me when ready for review |
Fixes openwisp#251 1.Add try except blocks to catch the error 2.Add test for the same 3.Use ugettext_lazy 4.Fix Formatting in utils file 5.Create separate method to validate file format
|
@nemesisdesign Please Review! |
There was a problem hiding this comment.
Thanks 👍
PS: I edited the error message, "Try again" is not a good suggestion in this case because it may be misinterpreted. You should use "Try again" in an error message only if there's a temporary server or network issue, because the user may understand that they have to try again repeating the same operation, which in this case won't work.
Fixes #251