# HG changeset patch # User Marcin Kuzminski # Date 2019-12-04 19:58:23 # Node ID 7cd93c2b20ed1421374c91023e880eab87e8b442 # Parent 938f856a631422ee032531b8fac59f7017fd2ba2 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 diff --git a/rhodecode/api/views/pull_request_api.py b/rhodecode/api/views/pull_request_api.py --- a/rhodecode/api/views/pull_request_api.py +++ b/rhodecode/api/views/pull_request_api.py @@ -895,7 +895,9 @@ 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() diff --git a/rhodecode/apps/admin/views/settings.py b/rhodecode/apps/admin/views/settings.py --- a/rhodecode/apps/admin/views/settings.py +++ b/rhodecode/apps/admin/views/settings.py @@ -573,8 +573,7 @@ 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, diff --git a/rhodecode/apps/debug_style/views.py b/rhodecode/apps/debug_style/views.py --- a/rhodecode/apps/debug_style/views.py +++ b/rhodecode/apps/debug_style/views.py @@ -78,7 +78,16 @@ 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 @@ 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 @@ 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 @@ users: description edit fixes 'pull_request_comment+file': {}, 'pull_request_comment+status': {}, + + 'pull_request_update': {}, } c.email_types.update(EmailNotificationModel.email_types) diff --git a/rhodecode/apps/repository/views/repo_pull_requests.py b/rhodecode/apps/repository/views/repo_pull_requests.py --- a/rhodecode/apps/repository/views/repo_pull_requests.py +++ b/rhodecode/apps/repository/views/repo_pull_requests.py @@ -1117,7 +1117,8 @@ 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: diff --git a/rhodecode/lib/helpers.py b/rhodecode/lib/helpers.py --- a/rhodecode/lib/helpers.py +++ b/rhodecode/lib/helpers.py @@ -821,7 +821,7 @@ 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 ` """ @@ -1015,10 +1015,11 @@ 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) #============================================================================== diff --git a/rhodecode/model/db.py b/rhodecode/model/db.py --- a/rhodecode/model/db.py +++ b/rhodecode/model/db.py @@ -4401,6 +4401,7 @@ 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) diff --git a/rhodecode/model/notification.py b/rhodecode/model/notification.py --- a/rhodecode/model/notification.py +++ b/rhodecode/model/notification.py @@ -293,6 +293,7 @@ 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 @@ 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 @@ 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, diff --git a/rhodecode/model/pull_request.py b/rhodecode/model/pull_request.py --- a/rhodecode/model/pull_request.py +++ b/rhodecode/model/pull_request.py @@ -65,9 +65,19 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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) diff --git a/rhodecode/templates/email_templates/pull_request_comment.mako b/rhodecode/templates/email_templates/pull_request_comment.mako --- a/rhodecode/templates/email_templates/pull_request_comment.mako +++ b/rhodecode/templates/email_templates/pull_request_comment.mako @@ -115,17 +115,17 @@ data = {

-
- @${h.person(user.username)} +
+ @${h.person(user.username)} + ${_('left a')} + + % if comment_file: + ${_('{comment_type} on file `{comment_file}` in pull request.').format(**data)} + % else: + ${_('{comment_type} on pull request.').format(**data) |n} + % endif +
- ${_('left a')} - - % if comment_file: - ${_('{comment_type} on file `{comment_file}` in pull request.').format(**data)} - % else: - ${_('{comment_type} on pull request.').format(**data) |n} - % endif -
${_('Pull request')} !${data['pr_id']}: ${data['pr_title']}

diff --git a/rhodecode/templates/email_templates/pull_request_review.mako b/rhodecode/templates/email_templates/pull_request_review.mako --- a/rhodecode/templates/email_templates/pull_request_review.mako +++ b/rhodecode/templates/email_templates/pull_request_review.mako @@ -78,13 +78,13 @@ data = {

-
- @${h.person(user.username)} +
+ @${h.person(user.username)} + ${_('requested a')} + + ${_('pull request review.').format(**data) } +
- ${_('requested a')} - - ${_('pull request review.').format(**data) } -
${_('Pull request')} !${data['pr_id']}: ${data['pr_title']}

diff --git a/rhodecode/templates/email_templates/pull_request_update.mako b/rhodecode/templates/email_templates/pull_request_update.mako new file mode 100644 --- /dev/null +++ b/rhodecode/templates/email_templates/pull_request_update.mako @@ -0,0 +1,164 @@ +## -*- 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} + + +## 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()} + +<% +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), +} +%> + + + + + + +
+ +

+
+ @${h.person(updating_user.username)} + ${_('updated')} + + ${_('pull request.').format(**data) } + +
+
+ ${_('Pull request')} !${data['pr_id']}: ${data['pr_title']} +

+ +
+ + + ## spacing def + + + + + + + + + + + + + + + + + + + + + + + + +
${_('Pull request')}: + + !${pull_request.pull_request_id} + +
${_('Commit Flow')}: + ${'{}:{}'.format(data['source_ref_type'], pull_request.source_ref_parts.name)} ${_('of')} ${data['source_repo_url']} + → + ${'{}:{}'.format(data['target_ref_type'], pull_request.target_ref_parts.name)} ${_('of')} ${data['target_repo_url']} +
${_('Description')}:${pull_request.description | trim}
${_('Changes')}:\ + 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 +
diff --git a/rhodecode/templates/rst_templates/pull_request_update.mako b/rhodecode/templates/rst_templates/pull_request_update.mako --- a/rhodecode/templates/rst_templates/pull_request_update.mako +++ b/rhodecode/templates/rst_templates/pull_request_update.mako @@ -14,13 +14,13 @@ 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 diff --git a/rhodecode/tests/lib/test_mako_emails.py b/rhodecode/tests/lib/test_mako_emails.py --- a/rhodecode/tests/lib/test_mako_emails.py +++ b/rhodecode/tests/lib/test_mako_emails.py @@ -87,6 +87,59 @@ 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 diff --git a/rhodecode/tests/lib/test_markup_renderer.py b/rhodecode/tests/lib/test_markup_renderer.py --- a/rhodecode/tests/lib/test_markup_renderer.py +++ b/rhodecode/tests/lib/test_markup_renderer.py @@ -538,6 +538,7 @@ 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 @@ 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 @@ 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) diff --git a/rhodecode/tests/models/test_changeset_status.py b/rhodecode/tests/models/test_changeset_status.py --- a/rhodecode/tests/models/test_changeset_status.py +++ b/rhodecode/tests/models/test_changeset_status.py @@ -81,7 +81,7 @@ 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 @@ 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 @@ 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( diff --git a/rhodecode/tests/models/test_pullrequest.py b/rhodecode/tests/models/test_pullrequest.py --- a/rhodecode/tests/models/test_pullrequest.py +++ b/rhodecode/tests/models/test_pullrequest.py @@ -814,7 +814,7 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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] diff --git a/rhodecode/tests/plugin.py b/rhodecode/tests/plugin.py --- a/rhodecode/tests/plugin.py +++ b/rhodecode/tests/plugin.py @@ -1047,7 +1047,7 @@ 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 @@ 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