pull-requests: added update pull-requests email+notifications

- fixed generated update message comments + invalid anchor linkes generated to the changed files
- unified headers on all email messages for pull requests

rhodecode/templates/email_templates/pull_request_update.mako

@@ -0,0 +1,164 b''
+## -*- coding: utf-8 -*-
+<%inherit file="base.mako"/>
+<%namespace name="base" file="base.mako"/>
+
+## EMAIL SUBJECT
+<%def name="subject()" filter="n,trim,whitespace_filter">
+<%
+data = {
+    'updating_user': '@'+h.person(updating_user),
+    'pr_id': pull_request.pull_request_id,
+    'pr_title': pull_request.title,
+}
+%>
+
+${_('{updating_user} updated pull request. !{pr_id}: "{pr_title}"').format(**data) |n}
+</%def>
+
+## PLAINTEXT VERSION OF BODY
+<%def name="body_plaintext()" filter="n,trim">
+<%
+data = {
+    'updating_user': h.person(updating_user),
+    'pr_id': pull_request.pull_request_id,
+    'pr_title': pull_request.title,
+    'source_ref_type': pull_request.source_ref_parts.type,
+    'source_ref_name': pull_request.source_ref_parts.name,
+    'target_ref_type': pull_request.target_ref_parts.type,
+    'target_ref_name': pull_request.target_ref_parts.name,
+    'repo_url': pull_request_source_repo_url,
+    'source_repo': pull_request_source_repo.repo_name,
+    'target_repo': pull_request_target_repo.repo_name,
+    'source_repo_url': pull_request_source_repo_url,
+    'target_repo_url': pull_request_target_repo_url,
+}
+%>
+
+* ${_('Pull Request link')}: ${pull_request_url}
+
+* ${h.literal(_('Commit flow: {source_ref_type}:{source_ref_name} of {source_repo_url} into {target_ref_type}:{target_ref_name} of {target_repo_url}').format(**data))}
+
+* ${_('Title')}: ${pull_request.title}
+
+* ${_('Description')}:
+
+${pull_request.description | trim}
+
+* Changed commits:
+
+ - Added: ${len(added_commits)}
+ - Removed: ${len(removed_commits)}
+
+* Changed files:
+
+%if not changed_files:
+ No file changes found
+%else:
+%for file_name in added_files:
+ - A `${file_name}`
+%endfor
+%for file_name in modified_files:
+ - M `${file_name}`
+%endfor
+%for file_name in removed_files:
+ - R `${file_name}`
+%endfor
+%endif
+
+---
+${self.plaintext_footer()}
+</%def>
+<%
+data = {
+    'updating_user': h.person(updating_user),
+    'pr_id': pull_request.pull_request_id,
+    'pr_title': pull_request.title,
+    'source_ref_type': pull_request.source_ref_parts.type,
+    'source_ref_name': pull_request.source_ref_parts.name,
+    'target_ref_type': pull_request.target_ref_parts.type,
+    'target_ref_name': pull_request.target_ref_parts.name,
+    'repo_url': pull_request_source_repo_url,
+    'source_repo': pull_request_source_repo.repo_name,
+    'target_repo': pull_request_target_repo.repo_name,
+    'source_repo_url': h.link_to(pull_request_source_repo.repo_name, pull_request_source_repo_url),
+    'target_repo_url': h.link_to(pull_request_target_repo.repo_name, pull_request_target_repo_url),
+}
+%>
+
+<table style="text-align:left;vertical-align:middle;width: 100%">
+    <tr>
+    <td style="width:100%;border-bottom:1px solid #dbd9da;">
+
+        <h4 style="margin: 0">
+            <div style="margin-bottom: 4px">
+                <span style="color:#7E7F7F">@${h.person(updating_user.username)}</span>
+                ${_('updated')}
+                <a href="${pull_request_url}" style="${base.link_css()}">
+                ${_('pull request.').format(**data) }
+                </a>
+            </div>
+            <div style="margin-top: 10px"></div>
+            ${_('Pull request')} <code>!${data['pr_id']}: ${data['pr_title']}</code>
+        </h4>
+
+    </td>
+    </tr>
+
+</table>
+
+<table style="text-align:left;vertical-align:middle;width: 100%">
+    ## spacing def
+    <tr>
+        <td style="width: 130px"></td>
+        <td></td>
+    </tr>
+
+    <tr>
+        <td style="padding-right:20px;">${_('Pull request')}:</td>
+        <td>
+            <a href="${pull_request_url}" style="${base.link_css()}">
+            !${pull_request.pull_request_id}
+            </a>
+        </td>
+    </tr>
+
+    <tr>
+        <td style="padding-right:20px;line-height:20px;">${_('Commit Flow')}:</td>
+        <td style="line-height:20px;">
+            <code>${'{}:{}'.format(data['source_ref_type'], pull_request.source_ref_parts.name)}</code> ${_('of')} ${data['source_repo_url']}
+            &rarr;
+            <code>${'{}:{}'.format(data['target_ref_type'], pull_request.target_ref_parts.name)}</code> ${_('of')}  ${data['target_repo_url']}
+        </td>
+    </tr>
+
+    <tr>
+        <td style="padding-right:20px;">${_('Description')}:</td>
+        <td style="white-space:pre-wrap"><code>${pull_request.description | trim}</code></td>
+    </tr>
+    <tr>
+        <td style="padding-right:20px;">${_('Changes')}:</td>
+        <td style="white-space:pre-line">\
+            <strong>Changed commits:</strong>
+
+             - Added: ${len(added_commits)}
+             - Removed: ${len(removed_commits)}
+
+            <strong>Changed files:</strong>
+
+            %if not changed_files:
+             No file changes found
+            %else:
+            %for file_name in added_files:
+             - A <a href="${pull_request_url + '#a_' + h.FID(ancestor_commit_id, file_name)}">${file_name}</a>
+            %endfor
+            %for file_name in modified_files:
+             - M <a href="${pull_request_url + '#a_' + h.FID(ancestor_commit_id, file_name)}">${file_name}</a>
+            %endfor
+            %for file_name in removed_files:
+             - R <a href="${pull_request_url + '#a_' + h.FID(ancestor_commit_id, file_name)}">${file_name}</a>
+            %endfor
+            %endif
+        </td>
+    </tr>
+
+</table>

rhodecode/api/views/pull_request_api.py

@@ -895,7 +895,9 b' def update_pull_request('
 
         with pull_request.set_state(PullRequest.STATE_UPDATING):
             if PullRequestModel().has_valid_update_type(pull_request):
-                update_response = PullRequestModel().update_commits(pull_request)
+                db_user = apiuser.get_instance()
+                update_response = PullRequestModel().update_commits(
+                    pull_request, db_user)
                 commit_changes = update_response.changes or commit_changes
             Session().commit()
 

rhodecode/apps/admin/views/settings.py

@@ -573,8 +573,7 b' class AdminSettingsView(BaseAppView):'
 
         email_kwargs = {
             'date': datetime.datetime.now(),
-            'user': c.rhodecode_user,
-            'rhodecode_version': c.rhodecode_version
+            'user': c.rhodecode_user
         }
 
         (subject, headers, email_body,

rhodecode/apps/debug_style/views.py

@@ -78,7 +78,16 b' Check if we should use full-topic or min'
         target_repo = AttributeDict(repo_name='repo_group/target_repo')
         source_repo = AttributeDict(repo_name='repo_group/source_repo')
         user = User.get_by_username(self.request.GET.get('user')) or self._rhodecode_db_user
-
+        # file/commit changes for PR update
+        commit_changes = AttributeDict({
+            'added': ['aaaaaaabbbbb', 'cccccccddddddd'],
+            'removed': ['eeeeeeeeeee'],
+        })
+        file_changes = AttributeDict({
+            'added': ['a/file1.md', 'file2.py'],
+            'modified': ['b/modified_file.rst'],
+            'removed': ['.idea'],
+        })
         email_kwargs = {
             'test': {},
             'message': {
@@ -87,7 +96,6 b' Check if we should use full-topic or min'
             'email_test': {
                 'user': user,
                 'date': datetime.datetime.now(),
-                'rhodecode_version': c.rhodecode_version
             },
             'password_reset': {
                 'password_reset_url': 'http://example.com/reset-rhodecode-password/token',
@@ -215,6 +223,35 b' This should work better !'
 
             },
 
+            'pull_request_update': {
+                'updating_user': user,
+
+                'status_change': None,
+                'status_change_type': None,
+
+                'pull_request': pr,
+                'pull_request_commits': [],
+
+                'pull_request_target_repo': target_repo,
+                'pull_request_target_repo_url': 'http://target-repo/url',
+
+                'pull_request_source_repo': source_repo,
+                'pull_request_source_repo_url': 'http://source-repo/url',
+
+                'pull_request_url': 'http://localhost/pr1',
+
+                # update comment links
+                'pr_comment_url': 'http://comment-url',
+                'pr_comment_reply_url': 'http://comment-url#reply',
+                'ancestor_commit_id': 'f39bd443',
+                'added_commits': commit_changes.added,
+                'removed_commits': commit_changes.removed,
+                'changed_files': (file_changes.added + file_changes.modified + file_changes.removed),
+                'added_files': file_changes.added,
+                'modified_files': file_changes.modified,
+                'removed_files': file_changes.removed,
+            },
+
             'cs_comment': {
                 'user': user,
                 'commit': AttributeDict(idx=123, raw_id='a'*40, message='Commit message'),
@@ -338,6 +375,8 b' users: description edit fixes'
 
             'pull_request_comment+file': {},
             'pull_request_comment+status': {},
+
+            'pull_request_update': {},
         }
         c.email_types.update(EmailNotificationModel.email_types)
 

rhodecode/apps/repository/views/repo_pull_requests.py

@@ -1117,7 +1117,8 b' class RepoPullRequestsView(RepoAppView, '
         _ = self.request.translate
 
         with pull_request.set_state(PullRequest.STATE_UPDATING):
-            resp = PullRequestModel().update_commits(pull_request)
+            resp = PullRequestModel().update_commits(
+                pull_request, self._rhodecode_db_user)
 
         if resp.executed:
 

rhodecode/lib/helpers.py

@@ -821,7 +821,7 b' def is_svn_without_proxy(repository):'
 
 def discover_user(author):
     """
-    Tries to discover RhodeCode User based on the autho string. Author string
+    Tries to discover RhodeCode User based on the author string. Author string
     is typically `FirstName LastName <[e-mail]>`
     """
 
@@ -1015,10 +1015,11 b' def bool2icon(value, show_at_false=True)'
 #==============================================================================
 # PERMS
 #==============================================================================
-from rhodecode.lib.auth import HasPermissionAny, HasPermissionAll, \
-HasRepoPermissionAny, HasRepoPermissionAll, HasRepoGroupPermissionAll, \
-HasRepoGroupPermissionAny, HasRepoPermissionAnyApi, get_csrf_token, \
-csrf_token_key
+from rhodecode.lib.auth import (
+    HasPermissionAny, HasPermissionAll,
+    HasRepoPermissionAny, HasRepoPermissionAll, HasRepoGroupPermissionAll,
+    HasRepoGroupPermissionAny, HasRepoPermissionAnyApi, get_csrf_token,
+    csrf_token_key, AuthUser)
 
 
 #==============================================================================

rhodecode/model/db.py

@@ -4401,6 +4401,7 b' class Notification(Base, BaseModel):'
     TYPE_REGISTRATION = u'registration'
     TYPE_PULL_REQUEST = u'pull_request'
     TYPE_PULL_REQUEST_COMMENT = u'pull_request_comment'
+    TYPE_PULL_REQUEST_UPDATE = u'pull_request_update'
 
     notification_id = Column('notification_id', Integer(), nullable=False, primary_key=True)
     subject = Column('subject', Unicode(512), nullable=True)

rhodecode/model/notification.py

@@ -293,6 +293,7 b' class EmailNotificationModel(BaseModel):'
     TYPE_REGISTRATION = Notification.TYPE_REGISTRATION
     TYPE_PULL_REQUEST = Notification.TYPE_PULL_REQUEST
     TYPE_PULL_REQUEST_COMMENT = Notification.TYPE_PULL_REQUEST_COMMENT
+    TYPE_PULL_REQUEST_UPDATE = Notification.TYPE_PULL_REQUEST_UPDATE
     TYPE_MAIN = Notification.TYPE_MESSAGE
 
     TYPE_PASSWORD_RESET = 'password_reset'
@@ -319,6 +320,8 b' class EmailNotificationModel(BaseModel):'
             'rhodecode:templates/email_templates/pull_request_review.mako',
         TYPE_PULL_REQUEST_COMMENT:
             'rhodecode:templates/email_templates/pull_request_comment.mako',
+        TYPE_PULL_REQUEST_UPDATE:
+            'rhodecode:templates/email_templates/pull_request_update.mako',
     }
 
     def __init__(self):
@@ -341,6 +344,7 b' class EmailNotificationModel(BaseModel):'
         """
 
         kwargs['rhodecode_instance_name'] = self.rhodecode_instance_name
+        kwargs['rhodecode_version'] = rhodecode.__version__
         instance_url = h.route_url('home')
         _kwargs = {
             'instance_url': instance_url,

rhodecode/model/pull_request.py

@@ -65,9 +65,19 b' log = logging.getLogger(__name__)'
 
 # Data structure to hold the response data when updating commits during a pull
 # request update.
-UpdateResponse = collections.namedtuple('UpdateResponse', [
-    'executed', 'reason', 'new', 'old', 'changes',
-    'source_changed', 'target_changed'])
+class UpdateResponse(object):
+
+    def __init__(self, executed, reason, new, old, common_ancestor_id,
+                 commit_changes, source_changed, target_changed):
+
+        self.executed = executed
+        self.reason = reason
+        self.new = new
+        self.old = old
+        self.common_ancestor_id = common_ancestor_id
+        self.changes = commit_changes
+        self.source_changed = source_changed
+        self.target_changed = target_changed
 
 
 class PullRequestModel(BaseModel):
@@ -672,11 +682,13 b' class PullRequestModel(BaseModel):'
         source_ref_type = pull_request.source_ref_parts.type
         return source_ref_type in self.REF_TYPES
 
-    def update_commits(self, pull_request):
+    def update_commits(self, pull_request, updating_user):
         """
         Get the updated list of commits for the pull request
         and return the new pull request version and the list
         of commits processed by this update action
+
+        updating_user is the user_object who triggered the update
         """
         pull_request = self.__get_pull_request(pull_request)
         source_ref_type = pull_request.source_ref_parts.type
@@ -693,7 +705,7 b' class PullRequestModel(BaseModel):'
             return UpdateResponse(
                 executed=False,
                 reason=UpdateFailureReason.WRONG_REF_TYPE,
-                old=pull_request, new=None, changes=None,
+                old=pull_request, new=None, common_ancestor_id=None, commit_changes=None,
                 source_changed=False, target_changed=False)
 
         # source repo
@@ -705,7 +717,7 b' class PullRequestModel(BaseModel):'
             return UpdateResponse(
                 executed=False,
                 reason=UpdateFailureReason.MISSING_SOURCE_REF,
-                old=pull_request, new=None, changes=None,
+                old=pull_request, new=None, common_ancestor_id=None, commit_changes=None,
                 source_changed=False, target_changed=False)
 
         source_changed = source_ref_id != source_commit.raw_id
@@ -719,7 +731,7 b' class PullRequestModel(BaseModel):'
             return UpdateResponse(
                 executed=False,
                 reason=UpdateFailureReason.MISSING_TARGET_REF,
-                old=pull_request, new=None, changes=None,
+                old=pull_request, new=None, common_ancestor_id=None, commit_changes=None,
                 source_changed=False, target_changed=False)
         target_changed = target_ref_id != target_commit.raw_id
 
@@ -728,7 +740,7 b' class PullRequestModel(BaseModel):'
             return UpdateResponse(
                 executed=False,
                 reason=UpdateFailureReason.NO_CHANGE,
-                old=pull_request, new=None, changes=None,
+                old=pull_request, new=None, common_ancestor_id=None, commit_changes=None,
                 source_changed=target_changed, target_changed=source_changed)
 
         change_in_found = 'target repo' if target_changed else 'source repo'
@@ -759,7 +771,7 b' class PullRequestModel(BaseModel):'
             return UpdateResponse(
                 executed=False,
                 reason=UpdateFailureReason.MISSING_TARGET_REF,
-                old=pull_request, new=None, changes=None,
+                old=pull_request, new=None, common_ancestor_id=None, commit_changes=None,
                 source_changed=source_changed, target_changed=target_changed)
 
         # re-compute commit ids
@@ -769,13 +781,13 b' class PullRequestModel(BaseModel):'
             target_commit.raw_id, source_commit.raw_id, source_repo, merge=True,
             pre_load=pre_load)
 
-        ancestor = source_repo.get_common_ancestor(
+        ancestor_commit_id = source_repo.get_common_ancestor(
             source_commit.raw_id, target_commit.raw_id, target_repo)
 
         pull_request.source_ref = '%s:%s:%s' % (
             source_ref_type, source_ref_name, source_commit.raw_id)
         pull_request.target_ref = '%s:%s:%s' % (
-            target_ref_type, target_ref_name, ancestor)
+            target_ref_type, target_ref_name, ancestor_commit_id)
 
         pull_request.revisions = [
             commit.raw_id for commit in reversed(commit_ranges)]
@@ -787,7 +799,7 b' class PullRequestModel(BaseModel):'
             pull_request, pull_request_version)
 
         # calculate commit and file changes
-        changes = self._calculate_commit_id_changes(
+        commit_changes = self._calculate_commit_id_changes(
             old_commit_ids, new_commit_ids)
         file_changes = self._calculate_file_changes(
             old_diff_data, new_diff_data)
@@ -797,23 +809,23 b' class PullRequestModel(BaseModel):'
             pull_request, old_diff_data=old_diff_data,
             new_diff_data=new_diff_data)
 
-        commit_changes = (changes.added or changes.removed)
+        valid_commit_changes = (commit_changes.added or commit_changes.removed)
         file_node_changes = (
             file_changes.added or file_changes.modified or file_changes.removed)
-        pr_has_changes = commit_changes or file_node_changes
+        pr_has_changes = valid_commit_changes or file_node_changes
 
         # Add an automatic comment to the pull request, in case
         # anything has changed
         if pr_has_changes:
             update_comment = CommentsModel().create(
-                text=self._render_update_message(changes, file_changes),
+                text=self._render_update_message(ancestor_commit_id, commit_changes, file_changes),
                 repo=pull_request.target_repo,
                 user=pull_request.author,
                 pull_request=pull_request,
                 send_email=False, renderer=DEFAULT_COMMENTS_RENDERER)
 
             # Update status to "Under Review" for added commits
-            for commit_id in changes.added:
+            for commit_id in commit_changes.added:
                 ChangesetStatusModel().set_status(
                     repo=pull_request.source_repo,
                     status=ChangesetStatus.STATUS_UNDER_REVIEW,
@@ -822,10 +834,19 b' class PullRequestModel(BaseModel):'
                     pull_request=pull_request,
                     revision=commit_id)
 
+            # send update email to users
+            try:
+                self.notify_users(pull_request=pull_request, updating_user=updating_user,
+                                  ancestor_commit_id=ancestor_commit_id,
+                                  commit_changes=commit_changes,
+                                  file_changes=file_changes)
+            except Exception:
+                log.exception('Failed to send email notification to users')
+
         log.debug(
             'Updated pull request %s, added_ids: %s, common_ids: %s, '
             'removed_ids: %s', pull_request.pull_request_id,
-            changes.added, changes.common, changes.removed)
+            commit_changes.added, commit_changes.common, commit_changes.removed)
         log.debug(
             'Updated pull request with the following file changes: %s',
             file_changes)
@@ -841,7 +862,8 b' class PullRequestModel(BaseModel):'
 
         return UpdateResponse(
             executed=True, reason=UpdateFailureReason.NONE,
-            old=pull_request, new=pull_request_version, changes=changes,
+            old=pull_request, new=pull_request_version,
+            common_ancestor_id=ancestor_commit_id, commit_changes=commit_changes,
             source_changed=source_changed, target_changed=target_changed)
 
     def _create_version_from_snapshot(self, pull_request):
@@ -963,12 +985,13 b' class PullRequestModel(BaseModel):'
 
         return FileChangeTuple(added_files, modified_files, removed_files)
 
-    def _render_update_message(self, changes, file_changes):
+    def _render_update_message(self, ancestor_commit_id, changes, file_changes):
         """
         render the message using DEFAULT_COMMENTS_RENDERER (RST renderer),
         so it's always looking the same disregarding on which default
         renderer system is using.
 
+        :param ancestor_commit_id: ancestor raw_id
         :param changes: changes named tuple
         :param file_changes: file changes named tuple
 
@@ -987,6 +1010,7 b' class PullRequestModel(BaseModel):'
             'added_files': file_changes.added,
             'modified_files': file_changes.modified,
             'removed_files': file_changes.removed,
+            'ancestor_commit_id': ancestor_commit_id
         }
         renderer = RstTemplateRenderer()
         return renderer.render('pull_request_update.mako', **params)
@@ -1170,6 +1194,75 b' class PullRequestModel(BaseModel):'
             email_kwargs=kwargs,
         )
 
+    def notify_users(self, pull_request, updating_user, ancestor_commit_id,
+                     commit_changes, file_changes):
+
+        updating_user_id = updating_user.user_id
+        reviewers = set([x.user.user_id for x in pull_request.reviewers])
+        # NOTE(marcink): send notification to all other users except to
+        # person who updated the PR
+        recipients = reviewers.difference(set([updating_user_id]))
+
+        log.debug('Notify following recipients about pull-request update %s', recipients)
+
+        pull_request_obj = pull_request
+
+        # send email about the update
+        changed_files = (
+                file_changes.added + file_changes.modified + file_changes.removed)
+
+        pr_source_repo = pull_request_obj.source_repo
+        pr_target_repo = pull_request_obj.target_repo
+
+        pr_url = h.route_url('pullrequest_show',
+                             repo_name=pr_target_repo.repo_name,
+                             pull_request_id=pull_request_obj.pull_request_id,)
+
+        # set some variables for email notification
+        pr_target_repo_url = h.route_url(
+            'repo_summary', repo_name=pr_target_repo.repo_name)
+
+        pr_source_repo_url = h.route_url(
+            'repo_summary', repo_name=pr_source_repo.repo_name)
+
+        email_kwargs = {
+            'date': datetime.datetime.now(),
+            'updating_user': updating_user,
+
+            'pull_request': pull_request_obj,
+
+            'pull_request_target_repo': pr_target_repo,
+            'pull_request_target_repo_url': pr_target_repo_url,
+
+            'pull_request_source_repo': pr_source_repo,
+            'pull_request_source_repo_url': pr_source_repo_url,
+
+            'pull_request_url': pr_url,
+
+            'ancestor_commit_id': ancestor_commit_id,
+            'added_commits': commit_changes.added,
+            'removed_commits': commit_changes.removed,
+            'changed_files': changed_files,
+            'added_files': file_changes.added,
+            'modified_files': file_changes.modified,
+            'removed_files': file_changes.removed,
+        }
+
+        (subject,
+         _h, _e,  # we don't care about those
+         body_plaintext) = EmailNotificationModel().render_email(
+            EmailNotificationModel.TYPE_PULL_REQUEST_UPDATE, **email_kwargs)
+
+        # create notification objects, and emails
+        NotificationModel().create(
+            created_by=updating_user,
+            notification_subject=subject,
+            notification_body=body_plaintext,
+            notification_type=EmailNotificationModel.TYPE_PULL_REQUEST_UPDATE,
+            recipients=recipients,
+            email_kwargs=email_kwargs,
+        )
+
     def delete(self, pull_request, user):
         pull_request = self.__get_pull_request(pull_request)
         old_data = pull_request.get_api_data(with_merge_state=False)

rhodecode/templates/email_templates/pull_request_comment.mako

@@ -115,17 +115,17 b' data = {'
     <td style="width:100%;border-bottom:1px solid #dbd9da;">
 
         <h4 style="margin: 0">
-            <div style="margin-bottom: 4px; color:#7E7F7F">
-                @${h.person(user.username)}
+            <div style="margin-bottom: 4px">
+                <span style="color:#7E7F7F">@${h.person(user.username)}</span>
+                ${_('left a')}
+                <a href="${pr_comment_url}" style="${base.link_css()}">
+                    % if comment_file:
+                        ${_('{comment_type} on file `{comment_file}` in pull request.').format(**data)}
+                    % else:
+                        ${_('{comment_type} on pull request.').format(**data) |n}
+                    % endif
+                </a>
             </div>
-            ${_('left a')}
-            <a href="${pr_comment_url}" style="${base.link_css()}">
-                % if comment_file:
-                    ${_('{comment_type} on file `{comment_file}` in pull request.').format(**data)}
-                % else:
-                    ${_('{comment_type} on pull request.').format(**data) |n}
-                % endif
-            </a>
             <div style="margin-top: 10px"></div>
             ${_('Pull request')} <code>!${data['pr_id']}: ${data['pr_title']}</code>
         </h4>

rhodecode/templates/email_templates/pull_request_review.mako

@@ -78,13 +78,13 b' data = {'
     <td style="width:100%;border-bottom:1px solid #dbd9da;">
 
         <h4 style="margin: 0">
-            <div style="margin-bottom: 4px; color:#7E7F7F">
-                @${h.person(user.username)}
+            <div style="margin-bottom: 4px">
+                <span style="color:#7E7F7F">@${h.person(user.username)}</span>
+                ${_('requested a')}
+                <a href="${pull_request_url}" style="${base.link_css()}">
+                ${_('pull request review.').format(**data) }
+                </a>
             </div>
-            ${_('requested a')}
-            <a href="${pull_request_url}" style="${base.link_css()}">
-            ${_('pull request review.').format(**data) }
-            </a>
             <div style="margin-top: 10px"></div>
             ${_('Pull request')} <code>!${data['pr_id']}: ${data['pr_title']}</code>
         </h4>

rhodecode/templates/rst_templates/pull_request_update.mako

@@ -14,13 +14,13 b' Pull request updated. Auto status change'
   %else:
   Changed files:
     %for file_name in added_files:
-    * `A ${file_name} <#${'a_' + h.FID('', file_name)}>`_
+    * `A ${file_name} <#${'a_' + h.FID(ancestor_commit_id, file_name)}>`_
     %endfor
     %for file_name in modified_files:
-    * `M ${file_name} <#${'a_' + h.FID('', file_name)}>`_
+    * `M ${file_name} <#${'a_' + h.FID(ancestor_commit_id, file_name)}>`_
     %endfor
     %for file_name in removed_files:
-    * R ${file_name}
+    * `R ${file_name}`
     %endfor
   %endif
 

rhodecode/tests/lib/test_mako_emails.py

@@ -87,6 +87,59 b' def test_render_pr_email(app, user_admin'
     assert subject == '@test_admin (RhodeCode Admin) requested a pull request review. !200: "Example Pull Request"'
 
 
+def test_render_pr_update_email(app, user_admin):
+    ref = collections.namedtuple(
+        'Ref', 'name, type')('fxies123', 'book')
+
+    pr = collections.namedtuple('PullRequest',
+        'pull_request_id, title, description, source_ref_parts, source_ref_name, target_ref_parts, target_ref_name')(
+        200, 'Example Pull Request', 'Desc of PR', ref, 'bookmark', ref, 'Branch')
+
+    source_repo = target_repo = collections.namedtuple(
+        'Repo', 'type, repo_name')('hg', 'pull_request_1')
+
+    commit_changes = AttributeDict({
+        'added': ['aaaaaaabbbbb', 'cccccccddddddd'],
+        'removed': ['eeeeeeeeeee'],
+    })
+    file_changes = AttributeDict({
+        'added': ['a/file1.md', 'file2.py'],
+        'modified': ['b/modified_file.rst'],
+        'removed': ['.idea'],
+    })
+
+    kwargs = {
+        'updating_user': User.get_first_super_admin(),
+
+        'pull_request': pr,
+        'pull_request_commits': [],
+
+        'pull_request_target_repo': target_repo,
+        'pull_request_target_repo_url': 'x',
+
+        'pull_request_source_repo': source_repo,
+        'pull_request_source_repo_url': 'x',
+
+        'pull_request_url': 'http://localhost/pr1',
+
+        'pr_comment_url': 'http://comment-url',
+        'pr_comment_reply_url': 'http://comment-url#reply',
+        'ancestor_commit_id': 'f39bd443',
+        'added_commits': commit_changes.added,
+        'removed_commits': commit_changes.removed,
+        'changed_files': (file_changes.added + file_changes.modified + file_changes.removed),
+        'added_files': file_changes.added,
+        'modified_files': file_changes.modified,
+        'removed_files': file_changes.removed,
+    }
+
+    subject, headers, body, body_plaintext = EmailNotificationModel().render_email(
+        EmailNotificationModel.TYPE_PULL_REQUEST_UPDATE, **kwargs)
+
+    # subject
+    assert subject == '@test_admin (RhodeCode Admin) updated pull request. !200: "Example Pull Request"'
+
+
 @pytest.mark.parametrize('mention', [
     True,
     False

rhodecode/tests/lib/test_markup_renderer.py

@@ -538,6 +538,7 b' Pull request updated. Auto status change'
         'added_files': [],
         'modified_files': [],
         'removed_files': [],
+        'ancestor_commit_id': 'aaabbbcccdddeee',
     }
     renderer = RstTemplateRenderer()
     rendered = renderer.render('pull_request_update.mako', **params)
@@ -557,11 +558,11 b' Pull request updated. Auto status change'
     * :removed:`3 removed`
 
   Changed files:
-    * `A /path/a.py <#a_c--68ed34923b68>`_
-    * `A /path/b.js <#a_c--64f90608b607>`_
-    * `M /path/d.js <#a_c--85842bf30c6e>`_
-    * `M /path/ę.py <#a_c--d713adf009cd>`_
-    * R /path/ź.py
+    * `A /path/a.py <#a_c-aaabbbcccddd-68ed34923b68>`_
+    * `A /path/b.js <#a_c-aaabbbcccddd-64f90608b607>`_
+    * `M /path/d.js <#a_c-aaabbbcccddd-85842bf30c6e>`_
+    * `M /path/ę.py <#a_c-aaabbbcccddd-d713adf009cd>`_
+    * `R /path/ź.py`
 
 .. |under_review| replace:: *"NEW STATUS"*'''
 
@@ -577,6 +578,7 b' Pull request updated. Auto status change'
         'added_files': added,
         'modified_files': modified,
         'removed_files': removed,
+        'ancestor_commit_id': 'aaabbbcccdddeee',
     }
     renderer = RstTemplateRenderer()
     rendered = renderer.render('pull_request_update.mako', **params)

rhodecode/tests/models/test_changeset_status.py

@@ -81,7 +81,7 b' def test_pull_request_stays_if_update_wi'
         voted_status, *pull_request.reviewers)
 
     # Update, without change
-    PullRequestModel().update_commits(pull_request)
+    PullRequestModel().update_commits(pull_request, pull_request.author)
 
     # Expect that review status is the voted_status
     expected_review_status = voted_status
@@ -100,7 +100,7 b' def test_pull_request_under_review_if_up'
 
     # Update, with change
     pr_util.update_source_repository()
-    PullRequestModel().update_commits(pull_request)
+    PullRequestModel().update_commits(pull_request, pull_request.author)
 
     # Expect that review status is the voted_status
     expected_review_status = db.ChangesetStatus.STATUS_UNDER_REVIEW
@@ -171,7 +171,7 b' def test_commit_keeps_status_if_unchange'
     commit_id = pull_request.revisions[-1]
     pr_util.create_status_votes(voted_status, pull_request.reviewers[0])
     pr_util.update_source_repository()
-    PullRequestModel().update_commits(pull_request)
+    PullRequestModel().update_commits(pull_request, pull_request.author)
     assert pull_request.revisions[-1] == commit_id
 
     status = ChangesetStatusModel().get_status(

rhodecode/tests/models/test_pullrequest.py

@@ -814,7 +814,7 b' def test_update_writes_snapshot_into_pul'
     pull_request = pr_util.create_pull_request()
     pr_util.update_source_repository()
 
-    model.update_commits(pull_request)
+    model.update_commits(pull_request, pull_request.author)
 
     # Expect that it has a version entry now
     assert len(model.get_versions(pull_request)) == 1
@@ -823,7 +823,7 b' def test_update_writes_snapshot_into_pul'
 def test_update_skips_new_version_if_unchanged(pr_util, config_stub):
     pull_request = pr_util.create_pull_request()
     model = PullRequestModel()
-    model.update_commits(pull_request)
+    model.update_commits(pull_request, pull_request.author)
 
     # Expect that it still has no versions
     assert len(model.get_versions(pull_request)) == 0
@@ -835,7 +835,7 b' def test_update_assigns_comments_to_the_'
     comment = pr_util.create_comment()
     pr_util.update_source_repository()
 
-    model.update_commits(pull_request)
+    model.update_commits(pull_request, pull_request.author)
 
     # Expect that the comment is linked to the pr version now
     assert comment.pull_request_version == model.get_versions(pull_request)[0]
@@ -847,8 +847,9 b' def test_update_adds_a_comment_to_the_pu'
     pr_util.update_source_repository()
     pr_util.update_source_repository()
 
-    model.update_commits(pull_request)
+    update_response = model.update_commits(pull_request, pull_request.author)
 
+    commit_id = update_response.common_ancestor_id
     # Expect to find a new comment about the change
     expected_message = textwrap.dedent(
         """\
@@ -863,10 +864,10 b' def test_update_adds_a_comment_to_the_pu'
             * :removed:`0 removed`
 
           Changed files:
-            * `A file_2 <#a_c--92ed3b5f07b4>`_
+            * `A file_2 <#a_c-{}-92ed3b5f07b4>`_
 
         .. |under_review| replace:: *"Under Review"*"""
-    )
+    ).format(commit_id[:12])
     pull_request_comments = sorted(
         pull_request.comments, key=lambda c: c.modified_at)
     update_comment = pull_request_comments[-1]

rhodecode/tests/plugin.py

@@ -1047,7 +1047,7 b' class PRTestUtility(object):'
     def add_one_commit(self, head=None):
         self.update_source_repository(head=head)
         old_commit_ids = set(self.pull_request.revisions)
-        PullRequestModel().update_commits(self.pull_request)
+        PullRequestModel().update_commits(self.pull_request, self.pull_request.author)
         commit_ids = set(self.pull_request.revisions)
         new_commit_ids = commit_ids - old_commit_ids
         assert len(new_commit_ids) == 1
@@ -1066,7 +1066,7 b' class PRTestUtility(object):'
             kwargs = {}
         source_vcs.strip(removed_commit_id, **kwargs)
 
-        PullRequestModel().update_commits(self.pull_request)
+        PullRequestModel().update_commits(self.pull_request, self.pull_request.author)
         assert len(self.pull_request.revisions) == 1
         return removed_commit_id