openwisp
[fix] Fixed 500 error when adding OrganizationUser and assigning RadiusGroup together
In this PR, I added a try-except block to ensure that whenever a new user is created, and edited by simultaneously adding organization user with "default" org & "default-users" RadiusGroup, an error is logged as warning instead of the server raising 500 error.
Fixes #507
@pandafy could you provide me with some sample user data to add in the failing test case?
coverage: 98.669% (-0.05%) from 98.723% when pulling e13a3fef5573c638cd03d7093f677c2e1ff1bdc1 on Shiva953:issues/507-organizationuser-radiusgroup-error into 67fb11273444fe73018d7da40f139e787b26dee3 on openwisp:master.
Looks better, please add a failing test case and don't forget to always follow the commit message style guidelines to allow the build to pass.
In which file under the tests directory should the test case be added for this function? would test_checks.py be an apt one?
@Shiva953, you need to add a test in https://github.com/openwisp/openwisp-radius/blob/master/openwisp_radius/tests/test_users_integration.py.
It was mentioned in the issue description.
sure, test_default_group_handler can be a suitable name(to be added as method of TestUsersIntegration class)? Also, should we create another class for adding the test method?
@Shiva953, you need to add a test in https://github.com/openwisp/openwisp-radius/blob/master/openwisp_radius/tests/test_users_integration.py. It was mentioned in the issue description.
sure,
test_default_group_handlercan be a suitable name(to be added as method ofTestUsersIntegrationclass)? Also, should we create another class for adding the test method?
The test name sounds good to me. No, we don't need a new class for this test. You can add this to the existing class. I would also add a docstring to the text hinting why there was a need for such a test case. Basically, summarise the issue description in 1-2 lines.
