Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Notification message via E-Mail #43

Closed
frruit opened this issue Aug 8, 2020 · 10 comments · Fixed by #127
Closed

Notification message via E-Mail #43

frruit opened this issue Aug 8, 2020 · 10 comments · Fixed by #127
Assignees
Labels
Milestone

Comments

@frruit
Copy link

frruit commented Aug 8, 2020

Notification via E-Mail
This is currently requested by my customer and i was wondering if this feature is maybe already in planning within the library itself. If not then I can maybe support. It would be great in that case to get a proposal from you how this should be done correctly

Describe the solution you'd like

  • First version shall have a settings parameter like COMMENT_NOTIFICATION = True which is by default set to False. It should use the django send_mail functionalities if they are configured and in use (EMAIL_HOST, ...)
  • By default every new comment entry should send out a mail to all people that mark the thread as a follower. Everybody who creates a new entry, a reply or a like/unlike action shall be set automatically as a follower of the thread
  • a further boolean field should be available to follow or unfollow the thread within the UI
  • When a new action occures within the thread then a pretty information mail shall be send out to the thread followers

What do you about this feature request?

Hope to hear soon from you. CYA

@frruit frruit added the feature label Aug 8, 2020
@Radi85
Copy link
Owner

Radi85 commented Aug 8, 2020

This feature is actually on my list after finishing #33 but with lesser details than you have mentioned, i.e. I didn't think of implementing thread's follower but it seems to be a nice feature alongside with email notifcation.

I would like to have this feature implemented including all the points you have mentioned and since you are aware of the whole workflow here, I will be looking forward to your PR :)

@Radi85
Copy link
Owner

Radi85 commented Nov 18, 2020

@abhiabhi94
I am working on this feature and I hope I will get enough time to finish it 😄
I have so far added a new model, I think it's called Follower to keep tracking of the subscribers of a comment. Just wanted to share the thoughts, any idea will be welcome!

@abhiabhi94
Copy link
Collaborator

What all features are you considering in the first version of this?

@Radi85
Copy link
Owner

Radi85 commented Nov 21, 2020

  • Following parent comments and the main thread.
  • Email notifications.

@abhiabhi94
Copy link
Collaborator

As this application becomes more and more complex on the front-end part, I see a reason why django-comments-xtd is using react 😆

That apart, why are you using another model for subscribers? Can't it be a many to many field with User model.
`

@Radi85
Copy link
Owner

Radi85 commented Dec 5, 2020

As this application becomes more and more complex on the front-end part, I see a reason why django-comments-xtd is using react 😆

That is a nice solution. In fact we work on 2 different views for the same functionality (Django typical view and API view) I was thinking of using the API for our frontend as well and get rid of django typical views. However, this can be for later if we see this is really needed.

That apart, why are you using another model for subscribers? Can't it be a many to many field with User model.
`

I used User model in the begnning but I took into consideration that we might extend this feature to allow anonymous to subscribe a model object, therefore I used a seperate model with email field along with content type fields.

@abhiabhi94
Copy link
Collaborator

abhiabhi94 commented Dec 5, 2020

That is a nice solution.

No, no I am not sure I wouldn't would want to go that route. Sometime ago, when I wasn't really familiar with react, it was kind of intimidating to me to see that something like comment would use this. Now that I am a part of this, I see there are so many UI actions to handle 😆 . Also, the overhead thing is an issue as well.

I used User model in the begnning but I took into consideration that we might extend this feature to allow anonymous to subscribe a model object, therefore I used a seperate model with email field along with content type fields.

Seems okay to me as of now. Also. I was thinking of updating the documentation regarding flags and some code cleaning/refactoring but will do it after this one is merged.

@abhiabhi94
Copy link
Collaborator

Hey @Radi85, one more thing, do you like pytest? Now that I have used pytest-django, I like it. In general, I have found that we can write more test cases without having to actually write that much of code.

The fact that our existing test case will still work as before is an added advantage.

@Radi85
Copy link
Owner

Radi85 commented Dec 5, 2020

I did not use pytest previously and I don't mind to use it if you think that it can increase our code quality.

@abhiabhi94
Copy link
Collaborator

it can help us in writing tests more easily. i am not too sure about the code quality though :)

one of the best things that i like about pytest-djsngo is that we have to explicitly mark tests that require database access, so not all tests have to go through setting up of database and then its teardown. i would recommend you too to give it a try if you have time.
i will try to make a new branch and see the compatibility in our case.

@Radi85 Radi85 added this to the 2.6.0 milestone Dec 5, 2020
Radi85 pushed a commit that referenced this issue Dec 23, 2020
- Add Follower model.
- Parent comment and main thread can now be subscribed.
- Follow and unfollow functionality is integrated in the UI and the API
- Email notification is sent to all thread followers when creating a
  comment in it.
- Tests clean up.
- Adjust translation tests.
Radi85 added a commit that referenced this issue Dec 23, 2020
- Add Follower model.
- Parent comment and main thread can now be subscribed.
- Follow and unfollow functionality is integrated in the UI and the API
- Email notification is sent to all thread followers when creating a
  comment in it.
- Tests clean up.
- Adjust translation tests.
Radi85 added a commit that referenced this issue Dec 23, 2020
- Add Follower model.
- Parent comment and main thread can now be subscribed.
- Follow and unfollow functionality is integrated in the UI and the API
- Email notification is sent to all thread followers when creating a
  comment in it.
- Tests clean up.
- Adjust translation tests.
Radi85 added a commit that referenced this issue Dec 25, 2020
- Add Follower model.
- Parent comment and main thread can now be subscribed.
- Follow and unfollow functionality is integrated in the UI and the API
- Email notification is sent to all thread followers when creating a
  comment in it.
- Tests clean up.
- Adjust translation tests.
Radi85 added a commit that referenced this issue Dec 27, 2020
- Add Follower model.
- Parent comment and main thread can now be subscribed.
- Follow and unfollow functionality is integrated in the UI and the API
- Email notification is sent to all thread followers when creating a
  comment in it.
- Tests clean up.
- Adjust translation tests.
- update docs for subscription
Radi85 added a commit that referenced this issue Dec 27, 2020
- Add Follower model.
- Parent comment and main thread can now be subscribed.
- Follow and unfollow functionality is integrated in the UI and the API
- Email notification is sent to all thread followers when creating a
  comment in it.
- Tests clean up.
- Adjust translation tests.
- update docs for subscription
Radi85 added a commit that referenced this issue Dec 28, 2020
- Add Follower model.
- Parent comment and main thread can now be subscribed.
- Follow and unfollow functionality is integrated in the UI and the API
- Email notification is sent to all thread followers when creating a
  comment in it.
- Tests clean up.
- Adjust translation tests.
- update docs for subscription
@Radi85 Radi85 closed this as completed Dec 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants