notifications changes

- closing pull requests will give notification about it
- more detailed notifications about commenting on changesets and pull requests

rhodecode/controllers/changeset.py

@@ -329,7 +329,7 b' class ChangesetController(BaseRepoContro'
             text = text or (_('Status change -> %s')
                             % ChangesetStatus.get_status_lbl(status))
 
-        comm = ChangesetCommentsModel().create(
+        c.co = comm = ChangesetCommentsModel().create(
             text=text,
             repo=c.rhodecode_db_repo.repo_id,
             user=c.rhodecode_user.user_id,
@@ -371,12 +371,11 b' class ChangesetController(BaseRepoContro'
         if not request.environ.get('HTTP_X_PARTIAL_XHR'):
             return redirect(h.url('changeset_home', repo_name=repo_name,
                                   revision=revision))
-
+        #only ajax below
         data = {
            'target_id': h.safeid(h.safe_unicode(request.POST.get('f_path'))),
         }
         if comm:
-            c.co = comm
             data.update(comm.get_dict())
             data.update({'rendered_text':
                          render('changeset/changeset_comment_block.html')})

rhodecode/controllers/pullrequests.py

@@ -403,11 +403,15 b' class PullrequestsController(BaseRepoCon'
         status = request.POST.get('changeset_status')
         change_status = request.POST.get('change_changeset_status')
         text = request.POST.get('text')
+        close_pr = request.POST.get('save_close')
 
         allowed_to_change_status = self._get_is_allowed_change_status(pull_request)
         if status and change_status and allowed_to_change_status:
-            text = text or (_('Status change -> %s')
+            _def = (_('status change -> %s')
                             % ChangesetStatus.get_status_lbl(status))
+            if close_pr:
+                _def = _('Closing with') + ' ' + _def
+            text = text or _def
         comm = ChangesetCommentsModel().create(
             text=text,
             repo=c.rhodecode_db_repo.repo_id,
@@ -416,7 +420,9 b' class PullrequestsController(BaseRepoCon'
             f_path=request.POST.get('f_path'),
             line_no=request.POST.get('line'),
             status_change=(ChangesetStatus.get_status_lbl(status)
-            if status and change_status and allowed_to_change_status else None)
+                           if status and change_status
+                           and allowed_to_change_status else None),
+            closing_pr=close_pr
         )
 
         action_logger(self.rhodecode_user,
@@ -434,7 +440,7 b' class PullrequestsController(BaseRepoCon'
                     pull_request=pull_request_id
                 )
 
-            if request.POST.get('save_close'):
+            if close_pr:
                 if status in ['rejected', 'approved']:
                     PullRequestModel().close_pull_request(pull_request_id)
                     action_logger(self.rhodecode_user,

rhodecode/model/comment.py

@@ -35,6 +35,7 b' from rhodecode.model import BaseModel'
 from rhodecode.model.db import ChangesetComment, User, Repository, \
     Notification, PullRequest
 from rhodecode.model.notification import NotificationModel
+from rhodecode.model.meta import Session
 
 log = logging.getLogger(__name__)
 
@@ -57,8 +58,103 b' class ChangesetCommentsModel(BaseModel):'
                 user_objects.append(user_obj)
         return user_objects
 
+    def _get_notification_data(self, repo, comment, user, comment_text,
+                               line_no=None, revision=None, pull_request=None,
+                               status_change=None, closing_pr=False):
+        """
+        Get notification data
+
+        :param comment_text:
+        :param line:
+        :returns: tuple (subj,body,recipients,notification_type,email_kwargs)
+        """
+        # make notification
+        body = comment_text  # text of the comment
+        line = ''
+        if line_no:
+            line = _('on line %s') % line_no
+
+        #changeset
+        if revision:
+            notification_type = Notification.TYPE_CHANGESET_COMMENT
+            cs = repo.scm_instance.get_changeset(revision)
+            desc = "%s" % (cs.short_id)
+
+            _url = h.url('changeset_home',
+                repo_name=repo.repo_name,
+                revision=revision,
+                anchor='comment-%s' % comment.comment_id,
+                qualified=True,
+            )
+            subj = safe_unicode(
+                h.link_to('Re changeset: %(desc)s %(line)s' % \
+                          {'desc': desc, 'line': line},
+                          _url)
+            )
+            email_subject = 'User %s commented on changeset %s' % \
+                (user.username, h.short_id(revision))
+            # get the current participants of this changeset
+            recipients = ChangesetComment.get_users(revision=revision)
+            # add changeset author if it's in rhodecode system
+            cs_author = User.get_from_cs_author(cs.author)
+            if not cs_author:
+                #use repo owner if we cannot extract the author correctly
+                cs_author = repo.user
+            recipients += [cs_author]
+            email_kwargs = {
+                'status_change': status_change,
+                'cs_comment_user': h.person(user.email),
+                'cs_target_repo': h.url('summary_home', repo_name=repo.repo_name,
+                                        qualified=True),
+                'cs_comment_url': _url,
+                'raw_id': revision,
+                'message': cs.message
+            }
+        #pull request
+        elif pull_request:
+            notification_type = Notification.TYPE_PULL_REQUEST_COMMENT
+            desc = comment.pull_request.title
+            _url = h.url('pullrequest_show',
+                repo_name=pull_request.other_repo.repo_name,
+                pull_request_id=pull_request.pull_request_id,
+                anchor='comment-%s' % comment.comment_id,
+                qualified=True,
+            )
+            subj = safe_unicode(
+                h.link_to('Re pull request #%(pr_id)s: %(desc)s %(line)s' % \
+                          {'desc': desc,
+                           'pr_id': comment.pull_request.pull_request_id,
+                           'line': line},
+                          _url)
+            )
+            email_subject = 'User %s commented on pull request #%s' % \
+                    (user.username, comment.pull_request.pull_request_id)
+            # get the current participants of this pull request
+            recipients = ChangesetComment.get_users(pull_request_id=
+                                                pull_request.pull_request_id)
+            # add pull request author
+            recipients += [pull_request.author]
+
+            # add the reviewers to notification
+            recipients += [x.user for x in pull_request.reviewers]
+
+            #set some variables for email notification
+            email_kwargs = {
+                'pr_id': pull_request.pull_request_id,
+                'status_change': status_change,
+                'closing_pr': closing_pr,
+                'pr_comment_url': _url,
+                'pr_comment_user': h.person(user.email),
+                'pr_target_repo': h.url('summary_home',
+                                   repo_name=pull_request.other_repo.repo_name,
+                                   qualified=True)
+            }
+
+        return subj, body, recipients, notification_type, email_kwargs, email_subject
+
     def create(self, text, repo, user, revision=None, pull_request=None,
-               f_path=None, line_no=None, status_change=None, send_email=True):
+               f_path=None, line_no=None, status_change=None, closing_pr=False,
+               send_email=True):
         """
         Creates new comment for changeset or pull request.
         IF status_change is not none this comment is associated with a
@@ -72,9 +168,11 b' class ChangesetCommentsModel(BaseModel):'
         :param f_path:
         :param line_no:
         :param status_change:
+        :param closing_pr:
         :param send_email:
         """
         if not text:
+            log.warning('Missing text for comment, skipping...')
             return
 
         repo = self._get_repo(repo)
@@ -87,8 +185,6 b' class ChangesetCommentsModel(BaseModel):'
         comment.line_no = line_no
 
         if revision:
-            cs = repo.scm_instance.get_changeset(revision)
-            desc = "%s - %s" % (cs.short_id, h.shorter(cs.message, 256))
             comment.revision = revision
         elif pull_request:
             pull_request = self.__get_pull_request(pull_request)
@@ -96,82 +192,24 b' class ChangesetCommentsModel(BaseModel):'
         else:
             raise Exception('Please specify revision or pull_request_id')
 
-        self.sa.add(comment)
-        self.sa.flush()
-
-        # make notification
-        line = ''
-        body = text
-
-        #changeset
-        if revision:
-            if line_no:
-                line = _('on line %s') % line_no
-            subj = safe_unicode(
-                h.link_to('Re commit: %(desc)s %(line)s' % \
-                          {'desc': desc, 'line': line},
-                          h.url('changeset_home', repo_name=repo.repo_name,
-                                revision=revision,
-                                anchor='comment-%s' % comment.comment_id,
-                                qualified=True,
-                          )
-                )
-            )
-            notification_type = Notification.TYPE_CHANGESET_COMMENT
-            # get the current participants of this changeset
-            recipients = ChangesetComment.get_users(revision=revision)
-            # add changeset author if it's in rhodecode system
-            cs_author = User.get_from_cs_author(cs.author)
-            if not cs_author:
-                #use repo owner if we cannot extract the author correctly
-                cs_author = repo.user
-            recipients += [cs_author]
-            email_kwargs = {
-                'status_change': status_change,
-            }
-        #pull request
-        elif pull_request:
-            _url = h.url('pullrequest_show',
-                repo_name=pull_request.other_repo.repo_name,
-                pull_request_id=pull_request.pull_request_id,
-                anchor='comment-%s' % comment.comment_id,
-                qualified=True,
-            )
-            subj = safe_unicode(
-                h.link_to('Re pull request #%(pr_id)s: %(desc)s %(line)s' % \
-                          {'desc': comment.pull_request.title,
-                           'pr_id': comment.pull_request.pull_request_id,
-                           'line': line},
-                          _url)
-            )
-
-            notification_type = Notification.TYPE_PULL_REQUEST_COMMENT
-            # get the current participants of this pull request
-            recipients = ChangesetComment.get_users(pull_request_id=
-                                                pull_request.pull_request_id)
-            # add pull request author
-            recipients += [pull_request.author]
-
-            # add the reviewers to notification
-            recipients += [x.user for x in pull_request.reviewers]
-
-            #set some variables for email notification
-            email_kwargs = {
-                'pr_id': pull_request.pull_request_id,
-                'status_change': status_change,
-                'pr_comment_url': _url,
-                'pr_comment_user': h.person(user.email),
-                'pr_target_repo': h.url('summary_home',
-                                   repo_name=pull_request.other_repo.repo_name,
-                                   qualified=True)
-            }
+        Session().add(comment)
+        Session().flush()
 
         if send_email:
+            (subj, body, recipients, notification_type,
+             email_kwargs, email_subject) = self._get_notification_data(
+                                repo, comment, user,
+                                comment_text=text,
+                                line_no=line_no,
+                                revision=revision,
+                                pull_request=pull_request,
+                                status_change=status_change,
+                                closing_pr=closing_pr)
             # create notification objects, and emails
             NotificationModel().create(
                 created_by=user, subject=subj, body=body,
                 recipients=recipients, type_=notification_type,
-                email_kwargs=email_kwargs
+                email_kwargs=email_kwargs, email_subject=email_subject
             )
 
             mention_recipients = set(self._extract_mentions(body))\
@@ -195,7 +233,7 b' class ChangesetCommentsModel(BaseModel):'
         :param comment_id:
         """
         comment = self.__get_changeset_comment(comment)
-        self.sa.delete(comment)
+        Session().delete(comment)
 
         return comment
 
@@ -204,11 +242,8 b' class ChangesetCommentsModel(BaseModel):'
         Get's main comments based on revision or pull_request_id
 
         :param repo_id:
-        :type repo_id:
         :param revision:
-        :type revision:
         :param pull_request:
-        :type pull_request:
         """
 
         q = ChangesetComment.query()\
@@ -226,7 +261,7 b' class ChangesetCommentsModel(BaseModel):'
         return q.all()
 
     def get_inline_comments(self, repo_id, revision=None, pull_request=None):
-        q = self.sa.query(ChangesetComment)\
+        q = Session().query(ChangesetComment)\
             .filter(ChangesetComment.repo_id == repo_id)\
             .filter(ChangesetComment.line_no != None)\
             .filter(ChangesetComment.f_path != None)\

rhodecode/model/notification.py

@@ -35,6 +35,7 b' import rhodecode'
 from rhodecode.lib import helpers as h
 from rhodecode.model import BaseModel
 from rhodecode.model.db import Notification, User, UserNotification
+from rhodecode.model.meta import Session
 
 log = logging.getLogger(__name__)
 
@@ -55,7 +56,7 b' class NotificationModel(BaseModel):'
 
     def create(self, created_by, subject, body, recipients=None,
                type_=Notification.TYPE_MESSAGE, with_email=True,
-               email_kwargs={}):
+               email_kwargs={}, email_subject=None):
         """
 
         Creates notification of given type
@@ -69,6 +70,7 b' class NotificationModel(BaseModel):'
         :param type_: type of notification
         :param with_email: send email with this notification
         :param email_kwargs: additional dict to pass as args to email template
+        :param email_subject: use given subject as email subject
         """
         from rhodecode.lib.celerylib import tasks, run_task
 
@@ -106,7 +108,8 b' class NotificationModel(BaseModel):'
 
         # send email with notification to all other participants
         for rec in rec_objs:
-            email_subject = NotificationModel().make_description(notif, False)
+            if not email_subject:
+                email_subject = NotificationModel().make_description(notif, show_age=False)
             type_ = type_
             email_body = body
             ## this is passed into template
@@ -131,7 +134,7 b' class NotificationModel(BaseModel):'
                         .filter(UserNotification.notification
                                 == notification)\
                         .one()
-                self.sa.delete(obj)
+                Session().delete(obj)
                 return True
         except Exception:
             log.error(traceback.format_exc())
@@ -142,7 +145,6 b' class NotificationModel(BaseModel):'
         Get mentions for given user, filter them if filter dict is given
 
         :param user:
-        :type user:
         :param filter:
         """
         user = self._get_user(user)
@@ -168,7 +170,7 b' class NotificationModel(BaseModel):'
                                 == notification)\
                         .one()
                 obj.read = True
-                self.sa.add(obj)
+                Session().add(obj)
                 return True
         except Exception:
             log.error(traceback.format_exc())
@@ -188,7 +190,7 b' class NotificationModel(BaseModel):'
         # update on joined tables :(
         for obj in q.all():
             obj.read = True
-            self.sa.add(obj)
+            Session().add(obj)
 
     def get_unread_cnt_for_user(self, user):
         user = self._get_user(user)
@@ -218,7 +220,7 b' class NotificationModel(BaseModel):'
         #alias
         _n = notification
         _map = {
-            _n.TYPE_CHANGESET_COMMENT: _('commented on commit at %(when)s'),
+            _n.TYPE_CHANGESET_COMMENT: _('commented on changeset at %(when)s'),
             _n.TYPE_MESSAGE: _('sent message at %(when)s'),
             _n.TYPE_MENTION: _('mentioned you at %(when)s'),
             _n.TYPE_REGISTRATION: _('registered in RhodeCode at %(when)s'),

rhodecode/model/pull_request.py

@@ -75,13 +75,13 b' class PullRequestModel(BaseModel):'
         new.title = title
         new.description = description
         new.author = created_by_user
-        self.sa.add(new)
+        Session().add(new)
         Session().flush()
         #members
         for member in set(reviewers):
             _usr = self._get_user(member)
             reviewer = PullRequestReviewers(_usr, new)
-            self.sa.add(reviewer)
+            Session().add(reviewer)
 
         #reset state to under-review
         ChangesetStatusModel().set_status(
@@ -90,7 +90,8 b' class PullRequestModel(BaseModel):'
             user=created_by_user,
             pull_request=new
         )
-
+        revision_data = [(x.raw_id, x.message)
+                         for x in map(org_repo.get_changeset, revisions)]
         #notification to reviewers
         notif = NotificationModel()
 
@@ -114,7 +115,7 b' class PullRequestModel(BaseModel):'
             'pr_repo_url': h.url('summary_home', repo_name=other_repo.repo_name,
                                  qualified=True,),
             'pr_url': pr_url,
-            'pr_revisions': revisions
+            'pr_revisions': revision_data
         }
 
         notif.create(created_by=created_by_user, subject=subject, body=body,
@@ -140,7 +141,7 b' class PullRequestModel(BaseModel):'
         for uid in to_add:
             _usr = self._get_user(uid)
             reviewer = PullRequestReviewers(_usr, pull_request)
-            self.sa.add(reviewer)
+            Session().add(reviewer)
 
         for uid in to_remove:
             reviewer = PullRequestReviewers.query()\
@@ -148,7 +149,7 b' class PullRequestModel(BaseModel):'
                             PullRequestReviewers.pull_request==pull_request)\
                     .scalar()
             if reviewer:
-                self.sa.delete(reviewer)
+                Session().delete(reviewer)
 
     def delete(self, pull_request):
         pull_request = self.__get_pull_request(pull_request)
@@ -158,7 +159,7 b' class PullRequestModel(BaseModel):'
         pull_request = self.__get_pull_request(pull_request)
         pull_request.status = PullRequest.STATUS_CLOSED
         pull_request.updated_on = datetime.datetime.now()
-        self.sa.add(pull_request)
+        Session().add(pull_request)
 
     def _get_changesets(self, alias, org_repo, org_ref, other_repo, other_ref):
         """

rhodecode/templates/email_templates/changeset_comment.html

@@ -1,12 +1,17 b''
 ## -*- coding: utf-8 -*-
 <%inherit file="main.html"/>
-
-<h4>${subject}</h4>
-
+##message from user goes here
+<p>
+${cs_comment_user}: <br/>
 ${body}
+</p>
+%if status_change:
+    <span>${_('New status')} -&gt; ${status_change}</span>
+%endif
+<div>${_('View this comment here')}: ${cs_comment_url}</div>
 
-% if status_change is not None:
-<div>
-    ${_('New status')} -&gt; ${status_change}
-</div>
-% endif
+<pre>
+${_('Repo')}: ${cs_target_repo}
+${_('Changeset')}: ${h.short_id(raw_id)}
+${_('desc')}: ${h.shorter(message, 256)}
+</pre>

rhodecode/templates/email_templates/pull_request.html

@@ -10,8 +10,10 b''
 </p>
 
 <div>${_('revisions for reviewing')}</div>
-<ul>
-%for r in pr_revisions:
-    <li>${r}</li>
+<pre>
+%for r,r_msg in pr_revisions:
+${h.short_id(r)}:
+    ${h.shorter(r_msg, 256)}
+
 %endfor
-</ul>
+</pre>

rhodecode/templates/email_templates/pull_request_comment.html

@@ -1,13 +1,18 b''
 ## -*- coding: utf-8 -*-
 <%inherit file="main.html"/>
-
-${_('User %s commented on pull request #%s for repository %s') % ('<b>%s</b>' % pr_comment_user, pr_id, pr_target_repo) |n}
+${_('Pull request #%s for repository %s') % (pr_id, pr_target_repo) |n}
+##message from user goes here
+<p>
+${pr_comment_user}: <br/>
+${body}
+</p>
 <div>${_('View this comment here')}: ${pr_comment_url}</div>
 
-<p>
-${body}
-
 %if status_change:
-    <span>${_('New status')} -&gt; ${status_change}</span>
+	%if closing_pr:
+	   <span>${_('Closing pull request with status')} -&gt; ${status_change}</span>
+	%else:
+	   <span>${_('New status')} -&gt; ${status_change}</span>
+	%endif
 %endif
 </p>

rhodecode/templates/pullrequests/pullrequest.html

@@ -30,7 +30,7 b''
                 <span style="font-size: 20px">
                 ${h.select('org_repo','',c.org_repos,class_='refs')}:${h.select('org_ref',c.default_org_ref,c.org_refs,class_='refs')}
                 </span>
-                 <div style="padding:5px 3px 3px 42px;">${c.rhodecode_db_repo.description}</div>
+                 <div style="padding:5px 3px 3px 20px;">${c.rhodecode_db_repo.description}</div>
             </div>
             <div style="clear:both;padding-top: 10px"></div>
         </div>
@@ -44,7 +44,7 b''
                 <span style="font-size: 20px">
                 ${h.select('other_repo',c.default_other_repo,c.other_repos,class_='refs')}:${h.select('other_ref',c.default_other_ref,c.default_other_refs,class_='refs')}
                 </span>
-                 <div id="other_repo_desc" style="padding:5px 3px 3px 42px;"></div>
+                 <div id="other_repo_desc" style="padding:5px 3px 3px 20px;"></div>
             </div>
             <div style="clear:both;padding-top: 10px"></div>
         </div>