First sketch meetup model #6
Conversation
|
meetup/models.py
Outdated
| location = models.CharField(max_length=255, null=True) | ||
| description = models.TextField(null=True) | ||
| latitude = models.FloatField() | ||
| longitude = models.FloatField() |
There was a problem hiding this comment.
Use PointField or DecimalField instead of FloatField.
FloatField has not enough precision to use lat, lng.
meetup/models.py
Outdated
| @@ -1,3 +1,62 @@ | |||
| from uuid import uuid4 | |||
|
|
|||
| from django.contrib.auth.models import User | |||
There was a problem hiding this comment.
from django.contrib.auth import get_user_model
User = get_user_model()get_user_model() provide more flexibility.
There was a problem hiding this comment.
| """This model is for that lecture or event by speaker(s)""" | ||
| # There should be difficulty and language information in Program? | ||
| title = models.CharField(max_length=255) | ||
| brief = models.TextField(null=True, verbose_name='simple information') |
There was a problem hiding this comment.
from django.utils.translation import ugettext as _
verbose_name=_('simple information')Please use gettext to description text translation.
There was a problem hiding this comment.
On my plan, I will translate whole words later at once.
But I think, you are not agree with me. are you?
There was a problem hiding this comment.
I agree with you.
I think gettext is still right choice if your plan include i18n.
There was a problem hiding this comment.
Thank you, I want to use the i18n and L10n on this project and PyCon Korea Website, for Best Practice :)
meetup/models.py
Outdated
| description = models.TextField(null=True) | ||
| speakers = models.ManyToManyField(Speaker) | ||
| category = models.ForeignKey(ProgramCategory, verbose_name='The category which this program is belonged') | ||
| slide_url = models.CharField(max_length=255, null=True, verbose_name='A slide url') |
There was a problem hiding this comment.
URLField is more fit here I think. (https://docs.djangoproject.com/ko/1.11/ref/models/fields/#urlfield)
meetup/models.py
Outdated
|
|
||
| class Profile(models.Model): | ||
| """This model is for which profile for user""" | ||
| user = models.ForeignKey(User) |
There was a problem hiding this comment.
meetup/models.py
Outdated
| """This model is for which profile for user""" | ||
| user = models.ForeignKey(User) | ||
| name = models.CharField(max_length=50) | ||
| slug = models.SlugField(max_length=50, unique=True, verbose_name='nickname') |
There was a problem hiding this comment.
Common use case of slug is make url for specific object. That is reason why slugfield cannot use space. But 'nickname' is nickname. Nickname need space, Korean character and special character also.
So make new field for nickname is more make sense to me.
There was a problem hiding this comment.
Then, can I use that nickname as the slug like that Noh Seho to Noh-Seho?
or the slug variable should be changed the name to username? The username what I think means that the username is not allowed space.
There was a problem hiding this comment.
If you have plan to provide url for each profiles then keep this slug field and just change verbose_name 😄 .
and also you can use nickname field as slug. But that is not a good idea in performance reason.
meetup/models.py
Outdated
| created_at = models.DateTimeField(auto_now_add=True) | ||
|
|
||
| def save(self, *args, **kwargs): | ||
| self.token = str(uuid4()) |
There was a problem hiding this comment.
Remove this and use UUIDfield instead. (https://docs.djangoproject.com/en/1.11/ref/models/fields/#django.db.models.UUIDField)
| @@ -1,3 +1,4 @@ | |||
| django==1.11.4 | |||
| psycopg2==2.7.3 | |||
| uWSGI==2.0.15 | |||
| Pillow==4.2.1 | |||
There was a problem hiding this comment.
I have two different opinion of this.
- Use pillow-simd (https://github.com/uploadcare/pillow-simd)
pillow-simd is highly optimized version of pillow. You can get more than 2 times performance benefit with tiny effort. But it just opinion. I cannot strongly recommend it. - Add this package when it need.
I cannot found any image proccessing code yet. so.. why we need this?
There was a problem hiding this comment.
- OK
- When I run the
python manage.py sqlmigrateor something about django command, django framework has verifed about thatImageField required the Pillow dependencyhttps://docs.djangoproject.com/en/1.11/ref/models/fields/#imagefield
and I will not spend much time with huge effort about image processing before that would be needed.
| """This test class is for proving that the relationship among declared django models are organized successfully""" | ||
|
|
||
| def test_user_has_the_profile(self): | ||
| """This test is for proving that a user has a profile""" |
There was a problem hiding this comment.
I think we can add attribute exist check.
you can use model meta before creating instance
There was a problem hiding this comment.
That means, verifing the models' attributes between model what I made on models.py and model what I declared in test?
There was a problem hiding this comment.
@NohSeho yes. IMHO declaration should be tested. In my case, I start to test from import
There was a problem hiding this comment.
I see, I will add that test code soon
|
LGTM and we can go |
This pull request is what I think about the seminar website's basic feature, not a perfect modeling.
So feel free to feedback me and let's discuss about this.