zimka/master -- initial #2
Conversation
models.py
Outdated
| if days_shift: | ||
| kwargs["days_shift"] = days_shift | ||
| course_shift_group, created_shift = CourseShiftGroup.objects.get_or_create(**kwargs) | ||
| return course_shift_group, created and created_shift |
There was a problem hiding this comment.
нечитаемо. and в return - запутывает. return должен возвращать понятную штуку, без логики
|
Стоит логгировать в процессе стабилизации изменения и создание объектов |
models.py
Outdated
|
|
||
| def delete(self, *args, **kwargs): | ||
| group = self.course_user_group | ||
| delete = super(CourseShiftGroup, self).delete(*args, **kwargs) |
There was a problem hiding this comment.
переменная с названием delete - это нехорошо
There was a problem hiding this comment.
вообще какой-то взрыв мозга тут происходит, метод нужно читаемым сделать
There was a problem hiding this comment.
А как тут лучше? Идея была, чтоб если мы удаляем ShiftGroup, соответствующий UserGroup тоже удаляли. Автоматически джанго умеет только наоборот, если ключ(UserGroup) удалят, то и ShiftGroup тоже
models.py
Outdated
| """ | ||
| all_memberships = cls.objects.filter(user=user) | ||
| course_membership = all_memberships.filter(course_shift_group__course_user_group__course_id=course_key) | ||
| membership = course_membership.first() |
There was a problem hiding this comment.
а возможна ситуация, когда course_membership - ов несколько?
There was a problem hiding this comment.
м.б. лучше просто
course_membership = ls.objects.filter(user=user, course_shift_group__course_user_group__course_id=course_key)
?
There was a problem hiding this comment.
У юзера может быть много membership, а пара (юзер, course_key) должна иметь не более 1 membership
There was a problem hiding this comment.
я пока не понял, зачем фильтрацию в 2 этапа делать? результат же такой и будет
models.py
Outdated
| autostart_period_days = models.IntegerField( | ||
| default=28, | ||
| db_column='autostart_period_days', | ||
| help_text="Period of generated groups", |
There was a problem hiding this comment.
help_text надо поразвернутее написать
models.py
Outdated
|
|
||
| enroll_before_days = models.IntegerField( | ||
| default=14, | ||
| help_text="Days before start when student can enroll already" |
There was a problem hiding this comment.
и тут, желательно, с примерами
models.py
Outdated
| current_settings, created = cls.objects.get_or_create(course_key=course_key) | ||
| return current_settings | ||
|
|
||
| def update_shifts(self): |
There was a problem hiding this comment.
а зачем этот метод? что он делает и когда?
models.py
Outdated
| class CourseShiftPlannedRun(models.Model): | ||
| """ | ||
| Represents planned shift for course. Real plans are stored | ||
| in db and user only when new shifts are generated manually('is_autostart'=False) |
There was a problem hiding this comment.
А так?
Represents planned shift for course.
Plan can be launched, then it creates the shift and disappears.
For 'autostart' mode in settings mocked plans can be created:
they can be launched, but they are not stored in db and don't hit
it at plan deletion.
models.py
Outdated
| enroll_before_days = models.IntegerField( | ||
| default=14, | ||
| help_text="Days before shift start when student can enroll already."\ | ||
| "E.g. if shift start 20.01.2020 and value is 5 shift will be available"\ |
There was a problem hiding this comment.
Если уж мы пишем на англ, то дату стоит ставить либо 20 Jan 2020 либо 01/20/2020
models.py
Outdated
|
|
||
| class CourseShiftSettings(models.Model): | ||
| """ | ||
| Describes how should course shifts be run for |
There was a problem hiding this comment.
Course Shift settings for start and due dates in the specific course run.
models.py
Outdated
| max_length=255, | ||
| db_index=True, | ||
| unique=True, | ||
| ) |
There was a problem hiding this comment.
либо на строчку вверх, либо на 4 символа влево
models.py
Outdated
|
|
||
| is_shift_enabled = models.BooleanField( | ||
| default=False, | ||
| help_text="Is feature enabled for course" |
There was a problem hiding this comment.
True value if this feature enabled for the course run
manager.py
Outdated
| start_date=start_date, | ||
| days_shift=days_shift | ||
| ) | ||
| return shift |
No description provided.