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

Html notification #51

Closed
wants to merge 8 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@sauthieg
Contributor

sauthieg commented Nov 5, 2012

Hi

Here is some HTML formatted email notifications for Gitblit.
I was looking for that feature since a long time and I finally decided to do it myself :)

So here is the contribution.

What's important in it:

  • Added a GitBlit.sendHtmlMail() method
  • Completely rewritten sendmail.groovy and used the Groovy MarkupBuilder to easily build an HTML page
  • Subject have been changed to something similar to the following
    [gitblit][main/repository] Guillaume Sauthier pushed 3 commits

Notice the 2 levels prefix (with repository name minus '.git'), the displayName instead of the username

The notification email supports 2 levels of details (3 could be support, but now, only 2 have been implemented):

  • commits: list commits (SHA-1, Message and Author)
  • files: within a commit, what files have been changed/deleted/added, ...
  • diff: within a file, the content diff (this part is not implemented)

The html has links to the commitdiff and blobdiff page included.

Here is a link to what it looks like in my Mail.app http://snag.gy/wQNWf.jpg

sauthieg added some commits Nov 2, 2012

Add support for Html notification messages when commits are pushed on…
… the server

* Html message is built with Groovy MarkupBuilder
* Gravatar of authors are displayed
* For each pack received, a list of commits is displayed with Author's name, Commit SHA-1 (clickable) and the Short message
Added commit's content details (added/removed/... files)
* For each commit, impacted files are now displayed as a sub-element
* Style polish
** navbar inclusion (includes bootstrap CSS)
** table's width is now 100%
* Subject now uses the user's displayName instead of username
Add links to diff pages
* Support blobdiff (per file diff)
* Support commitdiff (per commit diff)
* Removed headers of impacted files table (more compact)
@gitblit

This comment has been minimized.

Show comment
Hide comment
@gitblit

gitblit Nov 5, 2012

Owner

That looks really nice! My only complaint is that the hook re-uses the "sendmail.groovy" filename. Not everyone wants html emails.

Owner

gitblit commented Nov 5, 2012

That looks really nice! My only complaint is that the hook re-uses the "sendmail.groovy" filename. Not everyone wants html emails.

@sauthieg

This comment has been minimized.

Show comment
Hide comment
@sauthieg

sauthieg Nov 5, 2012

Contributor

Sure, we can have 2 hooks for mail notifications.

Contributor

sauthieg commented Nov 5, 2012

Sure, we can have 2 hooks for mail notifications.

@sauthieg

This comment has been minimized.

Show comment
Hide comment
@sauthieg

sauthieg Nov 5, 2012

Contributor

Do you think the subject should be the same for the 2 kinds of notifications ?

Contributor

sauthieg commented Nov 5, 2012

Do you think the subject should be the same for the 2 kinds of notifications ?

sauthieg added some commits Nov 5, 2012

Separate HTML and non-HTML notifications
* Introduce sendmail-html.groovy
* sendmail.groovy reverted to original version
* Added a dedicated Test for the html script
Cleanup only
* Updated copyright year
* Fixed script name in comment
@sauthieg

This comment has been minimized.

Show comment
Hide comment
@sauthieg

sauthieg Nov 5, 2012

Contributor

On the checklist:

  • Verify that URLs produced with mounted parameters really work (our local installation has disabled that feature)
Contributor

sauthieg commented Nov 5, 2012

On the checklist:

  • Verify that URLs produced with mounted parameters really work (our local installation has disabled that feature)
@gitblit

This comment has been minimized.

Show comment
Hide comment
@gitblit

gitblit Nov 5, 2012

Owner

Works (mostly) great for me - the trouble with html email is not all clients render it properly. I think we could probably prune some of the css (e.g. labels css because you are already pulling in Bootstrap) - or maybe you duplicated it because of the client rendering issue?

Owner

gitblit commented Nov 5, 2012

Works (mostly) great for me - the trouble with html email is not all clients render it properly. I think we could probably prune some of the css (e.g. labels css because you are already pulling in Bootstrap) - or maybe you duplicated it because of the client rendering issue?

@gitblit gitblit closed this Nov 5, 2012

@sauthieg

This comment has been minimized.

Show comment
Hide comment
@sauthieg

sauthieg Nov 5, 2012

Contributor

Argh :)
Just committed something to re-use more from Bootstrap's CSS

Contributor

sauthieg commented Nov 5, 2012

Argh :)
Just committed something to re-use more from Bootstrap's CSS

gitblit added a commit that referenced this pull request Apr 28, 2014

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