[qa] Move ReadOnlyAdmin to openwisp-utils #28
Conversation
tests/test_project/models.py
Outdated
| @@ -30,3 +30,135 @@ def __str__(self): | |||
|
|
|||
| class Meta: | |||
| abstract = False | |||
|
|
|||
|
|
|||
| class AbstractRadiusAccounting(models.Model): | |||
There was a problem hiding this comment.
name it RadiusAccounting and simplify the fields, we don't need to have all the real fields, show just 3 or 4
There was a problem hiding this comment.
Yes, I know it I feel that I understand ReadOnlyAdmin class well.
BTW you are really fast 😳
tests/test_project/models.py
Outdated
| def save(self, *args, **kwargs): | ||
| if not self.start_time: | ||
| self.start_time = now() | ||
| super(AbstractRadiusAccounting, self).save(*args, **kwargs) |
There was a problem hiding this comment.
Know it too I sent PR because in my local machine I was getting weird error:
from openwisp_utils.admin import (MultitenantAdminMixin, MultitenantOrgFilter,
ImportError: cannot import name 'ReadOnlyAdmin'
There was a problem hiding this comment.
that's probably because of the modifications you have made in your previous task, check that
There was a problem hiding this comment.
@nemesisdesign For this task I am using unmarged version of repo (without my changes from previous task)
tests/test_project/models.py
Outdated
| self.start_time = now() | ||
| super(AbstractRadiusAccounting, self).save(*args, **kwargs) | ||
|
|
||
| class Meta: |
There was a problem hiding this comment.
remove this Meta definition, it's not needed here
There was a problem hiding this comment.
I know it too as I said before I sent PR only because I wanted to check is above error occuring in travis too
There was a problem hiding this comment.
can you now remove this please?
tests/test_project/models.py
Outdated
| abstract = True | ||
|
|
||
| def __str__(self): | ||
| return self.unique_id |
|
Can somebody check |
|
|
||
| def test_radiusaccounting_change(self): | ||
| options = dict(username='bobby', session_id='1') | ||
| obj = self._create_radius_accounting(**options) |
There was a problem hiding this comment.
you are getting redirected in the test to the login form because you have not logged-in as an admin, check out how other tests do this in this or other modules, is quite common
tests/test_project/models.py
Outdated
| self.start_time = now() | ||
| super(AbstractRadiusAccounting, self).save(*args, **kwargs) | ||
|
|
||
| class Meta: |
There was a problem hiding this comment.
can you now remove this please?
strang1ato
changed the title
| user = User.objects.create_superuser(username='administrator', | ||
| email='test@test.com', | ||
| password='admin') | ||
| self.client.force_login(user) |
There was a problem hiding this comment.
never duplicate code, it will set a bad precedent for future contributors, they will also start duplicating code if they see this, put these 2 lines in the setUp method of the test class, do you know what is that? If not, google it and read about it in the python docs, you will have learnt a new thing
There was a problem hiding this comment.
I haven't used setUp function because with that other tests were failing
There was a problem hiding this comment.
in these case you can override it, eg:
def setUp(self):
super().setup()
# your own additional logic
Fixes #25