I have a Django model User
class Notification(TimeStampedModel):
user = models.ForeignKey(settings.AUTH_USER_MODEL, verbose_name=_('user'),
related_name='notifications')
viewed = models.BooleanField(verbose_name=_('has viewed'), default=False)
def make_viewed(self):
if self.viewed:
return
self.viewed = True
User.objects.filter(id=self.user.id).update(not_viewed_notifications=F('not_viewed_notifications') - Value(1))
self.save()
So I think that order methods in model is bad idea and actually
it's anti-pattern, so next one solution was like this:
class Notification(TimeStampedModel):
user = models.ForeignKey(settings.AUTH_USER_MODEL, verbose_name=_('user'),
related_name='notifications')
_viewed = models.BooleanField(verbose_name=_('has viewed'), default=False)
@property
def viewed(self):
return self._viewed
@viewed.setter
def viewed(self, value):
if self._viewed:
return
self._viewed = True
self.user.not_viewed_notifications = F('not_viewed_notifications') - Value(1)
self.user.save(commit=False)
self.save()
It looks better, but we have a couple of problems:
1) Model have methods yet
2) Property care side-effect, it doesn't obviously
3) That action require access to instance, so we can't move it to custom field.
What are you thinking about?
1 Answer 1
I would go with a third approach so you don't have to manually update your User
model:
class Notification(TimeStampedModel):
user = models.ForeignKey(settings.AUTH_USER_MODEL, verbose_name=_('user'),
related_name='notifications')
viewed = models.BooleanField(verbose_name=_('has viewed'), default=False)
from django.contrib.auth.models import User as DjangoUser
class User(DjangoUser):
@property
def not_viewed_notifications(self):
return self.notifications.filter(viewed=False).count()
This is twofold:
- You don't need special logic in
Notification
and handle it in your view very simply (you can also have your users "unsee" some notifications); - You can never be out-of-sync between the number of not seen notifications for a user and the state of the notifications for said user.
This approach also have the advantage of not requiring anything from the selected User
model (settings.AUTH_USER_MODEL
). If using the proposed one, you will have access to the "not viewed" count from the property, if using the base User model from Django (or any other, for that matter), nothing will break and you'll still have access to the reverse relationship. Whereas, in your current implementation, you'll get into troubles when trying to access a non-existent not_viewed_notifications
.
-
\$\begingroup\$ Your proposal is great, but I choiced current solution for not build my bicycle, it's just because .count() is very expensive solution, so I have to denormalize my database model. \$\endgroup\$Denny– Denny2018年02月01日 16:58:22 +00:00Commented Feb 1, 2018 at 16:58
notification.viewed = False
and it will setnotification._viewed
toTrue
and remove anot_viewed_notifications
from the user? \$\endgroup\$