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

Add email notifications #42

Closed
4 tasks
riggraz opened this issue Jan 29, 2021 · 6 comments
Closed
4 tasks

Add email notifications #42

riggraz opened this issue Jan 29, 2021 · 6 comments
Labels
discussion Issues or pull requests that need discussion in order to reach an agreement feature New feature or request

Comments

@riggraz
Copy link
Collaborator

riggraz commented Jan 29, 2021

Since Astuto strives to connect a business to its users, it is essential to provide a way to keep users notified about updates. Users should be able to follow posts they are interested in and receive an email when an update for that post is published.

I took some time to think about this feature which I believe is critical to Astuto, and the following checklist contains what I've come up with:

  • Users should be able to follow (i.e. to subscribe to) a post. In particular, users that publish a post will automatically be subscribed to that post. Of course, users subscribed to a post should also be able to unsubscribe from it at any time to stop receiving notifications.
  • There should be different "degrees" of subscription to a post, in the same way there are different "degrees" of watching in a GitHub's repository. In particular, I believe that a user should be able to follow either "all updates" for a post (which means any new comment, post update or status update for that post will be emailed to the user) or "important updates" (which means just post updates and status updates will be emailed to the user). The rationale behind this choice is that subscribing to all new comments to a post may be overwhelming to users that just want to be notified when there is an actual update in the status of the post.
  • A user should be notified if there is a reply to one of their comments.
  • Users should be able to disable all notifications if they so desire. This means that, even though they are subscribed to some posts or they get a reply in a comment, they won't receive any notification at all.

I'd love to hear your opinion on the topic. I tried to reach a balance between simplicity and complexity, i.e. I didn't want to come up with a too complex notification system that would have taken years to implement but at the same time I wanted it to be flexible. Let me know what you think about it.

@Vin0uz has started working on a pull request to implement email notifications a few days ago, so I'd like to hear his opinion as well. I hope that these changes, if they are approved by the community as well, won't force him to throw away the work he's done so far.

@riggraz riggraz added feature New feature or request discussion Issues or pull requests that need discussion in order to reach an agreement labels Jan 29, 2021
@Vin0uz
Copy link
Contributor

Vin0uz commented Jan 29, 2021

👋 Thanks for this amazing issue !

I believe it's good to add things step by step instead of a huge block (because if that block turns out to be so-so useful, then it is a big loss of time). Also, I (personnally) often fall into that trap and just start piling ideas and features on one thing, and it gets so messy that nobody can understand it at the end.

On the diff I started, I already started to over-do I think...

The first step of the email notifications could be :

  • Allowing a user to globally accept/refuse notifications
  • Send a notifications when somebody answers your post

Why this is already a good step ? It means setting up the mailer, its config, test, and see how it can be triggered in controllers and so on. Once we get this tested and working, it can be taken to the next level : sending notifications if somebody answers your comment, turn on/off notifications for a specific post, notify when a post is updated etc..

@riggraz
Copy link
Collaborator Author

riggraz commented Jan 30, 2021

Do you think that we should decompose this issue into multiple smaller issues? The problem I see with this approach is that some issues may depend on others (e.g. "Setting up mailer" must be implemented before than "Send notification when somebody answers your post"), and with multipe issues there is no way to say "This issue depends on this other issue" (at least to my knowledge). This is the reason why I decided to document the email notification feature in just one big issue.

Anyway, I completely agree that work must be carried out step by step. This is what I do when I'm programming: I do small changes and commit. But I don't know exactly what you mean by small steps: making small PRs like "Setting up mailer", merge it, and then working on another small feature? Or would you be okay with a bigger PR with the feature as a whole but with a lot of commits? Of course, in each case we should run and test every change and discuss along the way where to bring the feature from there (in order to avoid writing a so-so useful block, as you call it).

All in all, I think we're primarly discussing over how to organize issues/PRs and work in general, and that's important to clarify.

@Vin0uz
Copy link
Contributor

Vin0uz commented Feb 2, 2021

Do you think that we should decompose this issue into multiple smaller issues? The problem I see with this approach is that some issues may depend on others (e.g. "Setting up mailer" must be implemented before than "Send notification when somebody answers your post"), and with multipe issues there is no way to say "This issue depends on this other issue" (at least to my knowledge). This is the reason why I decided to document the email notification feature in just one big issue.

Anyway, I completely agree that work must be carried out step by step. This is what I do when I'm programming: I do small changes and commit. But I don't know exactly what you mean by small steps: making small PRs like "Setting up mailer", merge it, and then working on another small feature? Or would you be okay with a bigger PR with the feature as a whole but with a lot of commits? Of course, in each case we should run and test every change and discuss along the way where to bring the feature from there (in order to avoid writing a so-so useful block, as you call it).

All in all, I think we're primarly discussing over how to organize issues/PRs and work in general, and that's important to clarify.

Commits should be (as much as possible) independents from one another I think, to keep clear diffs and ease the review pain (and historical retrieval of information). About the size of a PR, there is no real limits, but moving too much stuff at once is not good I believe (though to explore for ppl).

I completey agree with you when you say that a issue/pr should not contain "half a feature" ! (like setting up something but not using it)

What I was trying to tell is that how a feature is defined should be relatively contained. If I shorten your issue in a sentence, the feature is :
Setting up: user receives notifications, degree of notifications, subscription or not to specific post, global refusal of notifications

That would mean a quite big diff, and lots of change in the tool. If users complain about all those changes, how to know which part bother them the most? Which part to adapt? Also, it may be very troubling for people already using the tool, and if it breaks, lots of maintenance to do.
Doing just a first release with : User receives notifications on all posts depending if his parameter is on/off would already bring lots of feedback and potential breaks, and would still a cool release to do, wdyt?

Maybe you were just proposing your vision, and several diffs would implement it, then 🙌 I am all in :)

@riggraz
Copy link
Collaborator Author

riggraz commented Feb 4, 2021

Yes, I get what you mean.

Doing just a first release with : User receives notifications on all posts depending if his parameter is on/off would already bring lots of feedback and potential breaks, and would still a cool release to do, wdyt?

Yes, I agree. Let's go with this feature first! I'll open a new issue for it, and reference the PR your currently working on.

Anyway, what do you think about keeping this issue (maybe changing its name) to provide a long-term vision for the notification feature? And maybe encourage people to contribute by commenting and discussing it. Then we would, from time to time, decide from this discussion which small feature to implement next and open a specific issue for it (as we're doing now for the feature: User receives notifications on all posts depending if his parameter is on/off). Or do you think it'd be better to just close this issue?

@Vin0uz
Copy link
Contributor

Vin0uz commented Feb 4, 2021

If it is identified that we go in this direction for sure, let's keep it and we just check boxes when needed, but then it makes no sense to create another "small issue" dedicated to it. Creating an issue for the sake of creating an issue is a bit pointless, moreover if the PR is already opened 👀

The risk with keeping a big one, is that it gets lost in history of plenty of issues, but for now, the situation is very much ok with keeping a relatively big one I believe.

@riggraz
Copy link
Collaborator Author

riggraz commented Feb 8, 2021

This is not the definitive direction, it was just my vision but I would like to discuss it. I'm closing this issue right now, but I'll think about a way to encourage discussions about important and long-term topics like this.

Anyway, thanks a lot of this conversation! And see you on your notifications PR 😁

@riggraz riggraz closed this as completed Feb 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Issues or pull requests that need discussion in order to reach an agreement feature New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants