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

Complete notifications system #145

Open
andreynering opened this Issue Nov 11, 2016 · 40 comments

Comments

@andreynering
Contributor

andreynering commented Nov 11, 2016

Today Gitea can send e-mail for new Issues/PRs/Comments and that's it.

I think it should have a complete notifications system, like GitHub and GitLab.

  • Notifications would be saved in a database table
  • A page to manage them. Equivalent to https://github.com/notifications
  • Mark as read/unread, pinning a notification (visible even if read)
  • Subscribe/Unsubscribe button
  • Mark all notifications as read
  • Mark all notifications as unread
  • Automatic subscribe on Open/Comment, etc
  • Also more refined watch options?
  • E-mail read means notification gone. Each user should have the ability to disable this
  • E-Mail notifications for pushes

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@lunny

This comment has been minimized.

Show comment
Hide comment
@lunny

lunny Nov 11, 2016

Member

If you watch some repository, then defaultly, you will subscribe all the issues and prs on this repository except you unsubscribe this issue.

Member

lunny commented Nov 11, 2016

If you watch some repository, then defaultly, you will subscribe all the issues and prs on this repository except you unsubscribe this issue.

@bkcsoft

This comment has been minimized.

Show comment
Hide comment
@bkcsoft

bkcsoft Nov 28, 2016

Member

Please implement notifications like GitLab does, where you actually have to press "I'm done with this" before it goes away 🙂

Member

bkcsoft commented Nov 28, 2016

Please implement notifications like GitLab does, where you actually have to press "I'm done with this" before it goes away 🙂

@stevenroose

This comment has been minimized.

Show comment
Hide comment
@stevenroose

stevenroose Nov 28, 2016

If you add an "e-mail read means notification gone" system like GitHub's, please make it opt-out :)

stevenroose commented Nov 28, 2016

If you add an "e-mail read means notification gone" system like GitHub's, please make it opt-out :)

@lunny

This comment has been minimized.

Show comment
Hide comment
@lunny

lunny Nov 28, 2016

Member

@stevenroose that is a great idea. I think I can implement it.

Member

lunny commented Nov 28, 2016

@stevenroose that is a great idea. I think I can implement it.

@lunny

This comment has been minimized.

Show comment
Hide comment
@lunny

lunny Nov 28, 2016

Member

@andreynering could you add e-mail read means notification gone to the check list?

Member

lunny commented Nov 28, 2016

@andreynering could you add e-mail read means notification gone to the check list?

@andreynering

This comment has been minimized.

Show comment
Hide comment
@andreynering

andreynering Nov 28, 2016

Contributor

@lunny @stevenroose Updated

GitHub implements that by having a 1px-1px transparent image which point to https://github.com/some-route?token=.... When the GitHub server serve that image for the first time the notification is marked as read.

Contributor

andreynering commented Nov 28, 2016

@lunny @stevenroose Updated

GitHub implements that by having a 1px-1px transparent image which point to https://github.com/some-route?token=.... When the GitHub server serve that image for the first time the notification is marked as read.

@lunny

This comment has been minimized.

Show comment
Hide comment
@lunny

lunny Nov 29, 2016

Member

Yes. A small trick. :)

Member

lunny commented Nov 29, 2016

Yes. A small trick. :)

@stevenroose

This comment has been minimized.

Show comment
Hide comment
@stevenroose

stevenroose Nov 29, 2016

@andreynering I know they do it like that. But most e-mail clients don't have a feature to block displaying emails from specific senders :) So if you don't like that feature, it can be annoying.

stevenroose commented Nov 29, 2016

@andreynering I know they do it like that. But most e-mail clients don't have a feature to block displaying emails from specific senders :) So if you don't like that feature, it can be annoying.

@tboerger

This comment has been minimized.

Show comment
Hide comment
@tboerger

tboerger Nov 29, 2016

Member

👍 for opt-out of this feature.

Member

tboerger commented Nov 29, 2016

👍 for opt-out of this feature.

@stevenroose

This comment has been minimized.

Show comment
Hide comment
@stevenroose

stevenroose Nov 30, 2016

@tboerger Don't get me wrong. I got to like the e-mail thing because now I read all my GitHub stuff from e-mail. But for someone who doesn't it be annoying. I had a lot of "ah, let me quickly check this issue in my email and look at it further tonight" and then I ended up not finding the issue and ploughing through archived e-mails to find it back :D

stevenroose commented Nov 30, 2016

@tboerger Don't get me wrong. I got to like the e-mail thing because now I read all my GitHub stuff from e-mail. But for someone who doesn't it be annoying. I had a lot of "ah, let me quickly check this issue in my email and look at it further tonight" and then I ended up not finding the issue and ploughing through archived e-mails to find it back :D

@stroucki

This comment has been minimized.

Show comment
Hide comment
@stroucki

stroucki Nov 30, 2016

Contributor

My team would like commit messages, so I'm working on implementing that for all watchers to a repository.

There is a "notifyWatchers" function on commit, but all it does is make an entry in a database table.

Contributor

stroucki commented Nov 30, 2016

My team would like commit messages, so I'm working on implementing that for all watchers to a repository.

There is a "notifyWatchers" function on commit, but all it does is make an entry in a database table.

@andreynering

This comment has been minimized.

Show comment
Hide comment
@andreynering

andreynering Nov 30, 2016

Contributor

@stroucki Did you start anything?

I just started but only had time to create the database table until you sent this message. 😃

Maybe we should create a feature branch and work together on this, so we don't have conflicting changes.

Contributor

andreynering commented Nov 30, 2016

@stroucki Did you start anything?

I just started but only had time to create the database table until you sent this message. 😃

Maybe we should create a feature branch and work together on this, so we don't have conflicting changes.

@stroucki

This comment has been minimized.

Show comment
Hide comment
@stroucki

stroucki Dec 1, 2016

Contributor

@andreynering Actually I have something that works :)

Though it is probably not acceptable for general usage, since you can't opt out. It is similar to what gogs does for mentionemails: Do a database entry that apparently is only useful for webhooks, then send out email to all watchers.

You can see it in my fork: (note I am still based on gogs)
https://github.com/stroucki/gogs/tree/commitemails

But while testing I see that it is not working for commits made via ssh yet.

Contributor

stroucki commented Dec 1, 2016

@andreynering Actually I have something that works :)

Though it is probably not acceptable for general usage, since you can't opt out. It is similar to what gogs does for mentionemails: Do a database entry that apparently is only useful for webhooks, then send out email to all watchers.

You can see it in my fork: (note I am still based on gogs)
https://github.com/stroucki/gogs/tree/commitemails

But while testing I see that it is not working for commits made via ssh yet.

@andreynering

This comment has been minimized.

Show comment
Hide comment
@andreynering

andreynering Dec 1, 2016

Contributor

@stroucki Thanks for linking your branch.

I started something on #321 (still in an early stage) for who's interested.

We will probably integrate notifications and email sending. Your patch will be useful.

Contributor

andreynering commented Dec 1, 2016

@stroucki Thanks for linking your branch.

I started something on #321 (still in an early stage) for who's interested.

We will probably integrate notifications and email sending. Your patch will be useful.

@stroucki

This comment has been minimized.

Show comment
Hide comment
@stroucki

stroucki Dec 1, 2016

Contributor

I've been looking at getting notifications working on ssh commits today.
This is pretty messy, because 1) the serve command doesn't set up any of the mailer stuff and 2) it uses "log.GitLogger" to log stuff, but the mailer initialization bits want to log, they use "log". Things don't even seem to blow up then, the git-receive-pack ends up hanging and nothing really happens.
I got to the point now where it is sending commit messages, but the command line output gets a "failed to push some refs" error (but things have committed).

Contributor

stroucki commented Dec 1, 2016

I've been looking at getting notifications working on ssh commits today.
This is pretty messy, because 1) the serve command doesn't set up any of the mailer stuff and 2) it uses "log.GitLogger" to log stuff, but the mailer initialization bits want to log, they use "log". Things don't even seem to blow up then, the git-receive-pack ends up hanging and nothing really happens.
I got to the point now where it is sending commit messages, but the command line output gets a "failed to push some refs" error (but things have committed).

@stroucki

This comment has been minimized.

Show comment
Hide comment
@stroucki

stroucki Dec 2, 2016

Contributor

OK. I think I have it. I now get commit messages on https and ssh commits.

The big thing was that console logs interfere with the git processes that gogs creates. I'm guessing that since the log used stdout, the subprocess never terminated. Besides that, mailing services needed to be initialized too.
It is too late for tonight, but tomorrow I'll update my branch.

Contributor

stroucki commented Dec 2, 2016

OK. I think I have it. I now get commit messages on https and ssh commits.

The big thing was that console logs interfere with the git processes that gogs creates. I'm guessing that since the log used stdout, the subprocess never terminated. Besides that, mailing services needed to be initialized too.
It is too late for tonight, but tomorrow I'll update my branch.

@bkcsoft

This comment has been minimized.

Show comment
Hide comment
@bkcsoft

bkcsoft Dec 2, 2016

Member

@andreynering I also think that Action-table holds most of the data already. Might be worth looking into before making a new table 🙂

Member

bkcsoft commented Dec 2, 2016

@andreynering I also think that Action-table holds most of the data already. Might be worth looking into before making a new table 🙂

@lunny

This comment has been minimized.

Show comment
Hide comment
@lunny

lunny Dec 2, 2016

Member

^

Member

lunny commented Dec 2, 2016

^

@andreynering

This comment has been minimized.

Show comment
Hide comment
@andreynering

andreynering Dec 2, 2016

Contributor

@bkcsoft @lunny I will take a look at that, but I think we need another table. An action is created every time an issue changes, but we should have only one notification per issue/PR.

Also, an user may be subscribed to an issue but don't watch the repo, so won't have an action for him, etc. Situations like this.

Contributor

andreynering commented Dec 2, 2016

@bkcsoft @lunny I will take a look at that, but I think we need another table. An action is created every time an issue changes, but we should have only one notification per issue/PR.

Also, an user may be subscribed to an issue but don't watch the repo, so won't have an action for him, etc. Situations like this.

@stroucki

This comment has been minimized.

Show comment
Hide comment
@stroucki

stroucki Dec 2, 2016

Contributor

Is the action table actually used for anything?

Contributor

stroucki commented Dec 2, 2016

Is the action table actually used for anything?

@stroucki

This comment has been minimized.

Show comment
Hide comment
@stroucki

stroucki Dec 2, 2016

Contributor

I committed changes to my branch that now let commit messages go out to watchers on https and ssh commits.

Contributor

stroucki commented Dec 2, 2016

I committed changes to my branch that now let commit messages go out to watchers on https and ssh commits.

@andreynering

This comment has been minimized.

Show comment
Hide comment
@andreynering

andreynering Dec 2, 2016

Contributor

@stroucki

Is the action table actually used for anything?

It's only used by the feed on the dashboard.

I committed changes to my branch that now let commit messages go out to watchers on https and ssh commits

I'll look into integrate that after I'm done with my implementation

Contributor

andreynering commented Dec 2, 2016

@stroucki

Is the action table actually used for anything?

It's only used by the feed on the dashboard.

I committed changes to my branch that now let commit messages go out to watchers on https and ssh commits

I'll look into integrate that after I'm done with my implementation

@andreynering andreynering referenced this issue Dec 20, 2016

Closed

Notifications - Step 1 #429

0 of 2 tasks complete

@andreynering andreynering referenced this issue Dec 28, 2016

Merged

Notification - Step 1 #523

1 of 2 tasks complete

@lunny lunny closed this in #523 Dec 30, 2016

@bkcsoft

This comment has been minimized.

Show comment
Hide comment
@bkcsoft

bkcsoft Dec 30, 2016

Member

@lunny this isn't done yet ,only part 1 of 2 ;)

Member

bkcsoft commented Dec 30, 2016

@lunny this isn't done yet ,only part 1 of 2 ;)

@bkcsoft bkcsoft reopened this Dec 30, 2016

@bkcsoft

This comment has been minimized.

Show comment
Hide comment
@bkcsoft

bkcsoft Dec 30, 2016

Member

ooh, it was #523 that had the text "resolve #number" in it 😆

Member

bkcsoft commented Dec 30, 2016

ooh, it was #523 that had the text "resolve #number" in it 😆

@lunny

This comment has been minimized.

Show comment
Hide comment
@lunny

lunny Dec 31, 2016

Member

So not me? Or maybe we have to review and change the title when merging it.

Member

lunny commented Dec 31, 2016

So not me? Or maybe we have to review and change the title when merging it.

@bkcsoft

This comment has been minimized.

Show comment
Hide comment
@bkcsoft

bkcsoft Dec 31, 2016

Member

@lunny no need to change the title, just the body 🙁

Member

bkcsoft commented Dec 31, 2016

@lunny no need to change the title, just the body 🙁

@strk

This comment has been minimized.

Show comment
Hide comment
@strk

strk Jan 5, 2017

Member

Is the roadmap in the original submission updated after part 2 ?
I've tested notifications on try.gitea.io but doesn't seem to be working: https://try.gitea.io/gitea/gitea/issues/1

Member

strk commented Jan 5, 2017

Is the roadmap in the original submission updated after part 2 ?
I've tested notifications on try.gitea.io but doesn't seem to be working: https://try.gitea.io/gitea/gitea/issues/1

@strk

This comment has been minimized.

Show comment
Hide comment
@strk

strk Jan 5, 2017

Member

Step 2, merged, was #539

Member

strk commented Jan 5, 2017

Step 2, merged, was #539

@andreynering

This comment has been minimized.

Show comment
Hide comment
@andreynering

andreynering Jan 6, 2017

Contributor

@strk You are not watching the repo, so you won't get any notifications:

https://try.gitea.io/gitea/gitea/watchers

Contributor

andreynering commented Jan 6, 2017

@strk You are not watching the repo, so you won't get any notifications:

https://try.gitea.io/gitea/gitea/watchers

@strk

This comment has been minimized.

Show comment
Hide comment
@strk

strk Jan 6, 2017

Member
Member

strk commented Jan 6, 2017

@andreynering

This comment has been minimized.

Show comment
Hide comment
@andreynering

andreynering Jan 6, 2017

Contributor

I thought one of the goals of the notification system was allowing for per-issue watchers?

Yes, but it was not implemented yet. I'm doing it in parts. By now it's only notifying watchers. I hope we get it 100℅ on v1.1.0

Contributor

andreynering commented Jan 6, 2017

I thought one of the goals of the notification system was allowing for per-issue watchers?

Yes, but it was not implemented yet. I'm doing it in parts. By now it's only notifying watchers. I hope we get it 100℅ on v1.1.0

ethantkoenig pushed a commit to ethantkoenig/gitea that referenced this issue Feb 15, 2017

Merge pull request #145 from ethantkoenig/ufw_develop
Display as text if YAML parse fails

@lunny lunny referenced this issue Feb 23, 2017

Open

Gitea hosted Gitea #1029

6 of 8 tasks complete
@gayprogrammer

This comment has been minimized.

Show comment
Hide comment
@gayprogrammer

gayprogrammer Jun 8, 2017

I would like to note that email notifications currently do not say which user commented on the issue. It only displays the content of the comment.

Can we add the name of who commented to the email somehow?

gayprogrammer commented Jun 8, 2017

I would like to note that email notifications currently do not say which user commented on the issue. It only displays the content of the comment.

Can we add the name of who commented to the email somehow?

@tboerger

This comment has been minimized.

Show comment
Hide comment
@tboerger

tboerger Jun 8, 2017

Member

Than the current mail template is missing that information

Member

tboerger commented Jun 8, 2017

Than the current mail template is missing that information

@mxmehl

This comment has been minimized.

Show comment
Hide comment
@mxmehl

mxmehl Jun 23, 2017

It would be good to have a button to mark all notifications as read. (I hope this is the right issue for this feature request)

mxmehl commented Jun 23, 2017

It would be good to have a button to mark all notifications as read. (I hope this is the right issue for this feature request)

@andreynering

This comment has been minimized.

Show comment
Hide comment
@andreynering

andreynering Jun 23, 2017

Contributor

@mxmehl Yes, that's reasonable.

Contributor

andreynering commented Jun 23, 2017

@mxmehl Yes, that's reasonable.

@mxmehl

This comment has been minimized.

Show comment
Hide comment
@mxmehl

mxmehl Jun 23, 2017

@mxmehl Yes, that's reasonable.

Cool! Could you please add that to the list in the top post?

mxmehl commented Jun 23, 2017

@mxmehl Yes, that's reasonable.

Cool! Could you please add that to the list in the top post?

@andreynering

This comment has been minimized.

Show comment
Hide comment
@andreynering

andreynering Jun 23, 2017

Contributor

@mxmehl Done!

Contributor

andreynering commented Jun 23, 2017

@mxmehl Done!

@McLive

This comment has been minimized.

Show comment
Hide comment
@McLive

McLive Feb 22, 2018

Any update on this? I'd like to see E-Mail notifications for pushes.

gogs/gogs#1441

McLive commented Feb 22, 2018

Any update on this? I'd like to see E-Mail notifications for pushes.

gogs/gogs#1441

@lunny

This comment has been minimized.

Show comment
Hide comment
@lunny

lunny Feb 23, 2018

Member

@McLive added

Member

lunny commented Feb 23, 2018

@McLive added

@hasufell

This comment has been minimized.

Show comment
Hide comment
@hasufell

hasufell Jun 28, 2018

What I find missing here is optional admin notifications, like when a user has registered.

hasufell commented Jun 28, 2018

What I find missing here is optional admin notifications, like when a user has registered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment