Notifications - issue #165 #240
Conversation
| @@ -23,6 +23,10 @@ class Connection(mongoengine.EmbeddedDocument): | |||
| http_if_modified_since = mongoengine.StringField() | |||
| channel_id = mongoengine.StringField() | |||
|
|
|||
| class Notification(mongoengine.EmbeddedDocument): | |||
| read = mongoengine.BooleanField() | |||
| comment = mongoengine.IntField() | |||
There was a problem hiding this comment.
Int field? What will be stored here?
|
Please update from main repository as some things were changed in how JavaScript is included. |
| @@ -0,0 +1 @@ | |||
| Subproject commit 01201c7b14127a3cb2cb027965320ed8ec499a4e | |||
|
Please update from main repository. |
|
OK, I updated this pull request. I have a question now that entries are successfully added to user's notification list when post or comment is written. Should i write part where notification list is drawn and updated in folder "panels" like onlineusers is? |
| @@ -3,6 +3,7 @@ | |||
| from tastypie_mongoengine import fields, resources | |||
|
|
|||
| from piplmesh.account import models as account_models | |||
|
|
|||
There was a problem hiding this comment.
No need, because they are all part of the same package.
No! Notification should be a drop-down list from the header. Like Facebook has. Panels are for content. |
| @@ -32,6 +32,10 @@ class Connection(mongoengine.EmbeddedDocument): | |||
| http_if_modified_since = mongoengine.StringField() | |||
| channel_id = mongoengine.StringField() | |||
|
|
|||
| class Notification(mongoengine.EmbeddedDocument): | |||
| read = mongoengine.BooleanField() | |||
| comment = mongoengine.StringField() | |||
There was a problem hiding this comment.
String? No, why not a reference field to Comment document?
But you probably get an circular import error. :-)
There was a problem hiding this comment.
Maybe this shows that list of notifications should not be in the User document. Which is also reasonable. If you think, much better is to have a separate collections of all notifications, with reference to user and comment. And then do a query on it when you want to display notifications. Why would you get a list of ALL (past and current) notifications for the user everytime you access user object?
There was a problem hiding this comment.
No I don't get any errors with StringField. When you post comments entry is added to every user that is on superscription list. I placed notifications in user model because we said so when we were deciding what would be the best solution. But i can change it.
There was a problem hiding this comment.
You would get error, if you would do reference to the Comment document. :-) With string you do not get it, because you do not have to import Comment document. :-)
Yes, I think it would be better to move notifications to separate document.
|
So, please fix the model. Also, I am missing push updates. You have to push to the clients new notifications. Or will this come later? This is for now. Ball is yours. |
…el. Add notification to push server.
… sent to push server.
|
So, i fixed the model and other things that you weren't pleased with. Notifications are now separated from User model. I also added push updates and created notifications api. Fields in notifications api and in push updates are the same so that drawing notifications list will be easy. |
|
Now i have to write JS part so that notification list will be displaying and updating. |
|
Yes. And displaying the counter. And selecting the ones which have been read. And a separate view which shows you all notifications/whole activity history. The tricky part will probably be linking from notifications. So how to link to a comment/post where something happened. You can just provide a stub function for that for now and we will fill this in later, when we will have code for posts and comments and will then scroll the post into the focus. |
| for subscriber in self.instance.subscribers: | ||
| if subscriber != bundle.obj.author: | ||
| notification = api_models.Notification.objects.create(recipient=subscriber, post=self.instance, comment=bundle.obj.pk) | ||
| signals.notification_created.send(sender=self, notification=notification, request=request or bundle.request, bundle=bundle) |
There was a problem hiding this comment.
Yes, then bundle is not reasonable to have here, if it is not notification bundle. Remove it.
| @@ -1,3 +1,4 @@ | |||
| from django import dispatch | |||
|
|
|||
| post_created = dispatch.Signal(providing_args=('post', 'request', 'bundle')) | |||
| notification_created = dispatch.Signal(providing_args=('notification', 'request', 'bundle')) | |||
|
|
||
| $.ajax({ | ||
| type: 'POST', | ||
| url: post_url + 'comments/', |
There was a problem hiding this comment.
Mnja. Let it be. :-) But you should add TODO, that it should not be hardcoded like this. :-)
|
(And please check changes I have done and try to understand them and also reasons for them.) |
Notifications about new comments to wall posts.