# HG changeset patch # User Marcin Kuzminski # Date 2016-12-19 16:38:22 # Node ID 952cdf08325725cb6185ae47c5cea9253310b680 # Parent 2ae8ce4684eaf40dd758d7c885f63e354108c320 pull-requests: expose version browsing of pull requests. - show nav for browsing each version of pr - show detailed changes for each of version - update comment template to indicate the changes were due to update of a pr diff --git a/rhodecode/controllers/pullrequests.py b/rhodecode/controllers/pullrequests.py --- a/rhodecode/controllers/pullrequests.py +++ b/rhodecode/controllers/pullrequests.py @@ -21,11 +21,13 @@ """ pull requests controller for rhodecode for initializing pull requests """ +import types import peppercorn import formencode import logging + from webob.exc import HTTPNotFound, HTTPForbidden, HTTPBadRequest from pylons import request, tmpl_context as c, url from pylons.controllers.util import redirect @@ -46,8 +48,9 @@ from rhodecode.lib.channelstream import from rhodecode.lib.compat import OrderedDict from rhodecode.lib.utils import jsonify from rhodecode.lib.utils2 import ( - safe_int, safe_str, str2bool, safe_unicode, StrictAttributeDict) -from rhodecode.lib.vcs.backends.base import EmptyCommit, UpdateFailureReason + safe_int, safe_str, str2bool, safe_unicode) +from rhodecode.lib.vcs.backends.base import ( + EmptyCommit, UpdateFailureReason, EmptyRepository) from rhodecode.lib.vcs.exceptions import ( EmptyRepositoryError, CommitDoesNotExistError, RepositoryRequirementError, NodeDoesNotExistError) @@ -680,7 +683,13 @@ class PullrequestsController(BaseRepoCon def _get_pr_version(self, pull_request_id, version=None): pull_request_id = safe_int(pull_request_id) at_version = None - if version: + + if version and version == 'latest': + pull_request_ver = PullRequest.get(pull_request_id) + pull_request_obj = pull_request_ver + _org_pull_request_obj = pull_request_obj + at_version = 'latest' + elif version: pull_request_ver = PullRequestVersion.get_or_404(version) pull_request_obj = pull_request_ver _org_pull_request_obj = pull_request_ver.pull_request @@ -688,57 +697,58 @@ class PullrequestsController(BaseRepoCon else: _org_pull_request_obj = pull_request_obj = PullRequest.get_or_404(pull_request_id) - class PullRequestDisplay(object): - """ - Special object wrapper for showing PullRequest data via Versions - It mimics PR object as close as possible. This is read only object - just for display - """ - def __init__(self, attrs): - self.attrs = attrs - # internal have priority over the given ones via attrs - self.internal = ['versions'] - - def __getattr__(self, item): - if item in self.internal: - return getattr(self, item) - try: - return self.attrs[item] - except KeyError: - raise AttributeError( - '%s object has no attribute %s' % (self, item)) - - def versions(self): - return pull_request_obj.versions.order_by( - PullRequestVersion.pull_request_version_id).all() - - def is_closed(self): - return pull_request_obj.is_closed() - - attrs = StrictAttributeDict(pull_request_obj.get_api_data()) - - attrs.author = StrictAttributeDict( - pull_request_obj.author.get_api_data()) - if pull_request_obj.target_repo: - attrs.target_repo = StrictAttributeDict( - pull_request_obj.target_repo.get_api_data()) - attrs.target_repo.clone_url = pull_request_obj.target_repo.clone_url - - if pull_request_obj.source_repo: - attrs.source_repo = StrictAttributeDict( - pull_request_obj.source_repo.get_api_data()) - attrs.source_repo.clone_url = pull_request_obj.source_repo.clone_url - - attrs.source_ref_parts = pull_request_obj.source_ref_parts - attrs.target_ref_parts = pull_request_obj.target_ref_parts - - attrs.shadow_merge_ref = _org_pull_request_obj.shadow_merge_ref - - pull_request_display_obj = PullRequestDisplay(attrs) - + pull_request_display_obj = PullRequest.get_pr_display_object( + pull_request_obj, _org_pull_request_obj) return _org_pull_request_obj, pull_request_obj, \ pull_request_display_obj, at_version + def _get_pr_version_changes(self, version, pull_request_latest): + """ + Generate changes commits, and diff data based on the current pr version + """ + + #TODO(marcink): save those changes as JSON metadata for chaching later. + + # fake the version to add the "initial" state object + pull_request_initial = PullRequest.get_pr_display_object( + pull_request_latest, pull_request_latest, + internal_methods=['get_commit', 'versions']) + pull_request_initial.revisions = [] + pull_request_initial.source_repo.get_commit = types.MethodType( + lambda *a, **k: EmptyCommit(), pull_request_initial) + pull_request_initial.source_repo.scm_instance = types.MethodType( + lambda *a, **k: EmptyRepository(), pull_request_initial) + + _changes_versions = [pull_request_latest] + \ + list(reversed(c.versions)) + \ + [pull_request_initial] + + if version == 'latest': + index = 0 + else: + for pos, prver in enumerate(_changes_versions): + ver = getattr(prver, 'pull_request_version_id', -1) + if ver == safe_int(version): + index = pos + break + else: + index = 0 + + cur_obj = _changes_versions[index] + prev_obj = _changes_versions[index + 1] + + old_commit_ids = set(prev_obj.revisions) + new_commit_ids = set(cur_obj.revisions) + + changes = PullRequestModel()._calculate_commit_id_changes( + old_commit_ids, new_commit_ids) + + old_diff_data, new_diff_data = PullRequestModel()._generate_update_diffs( + cur_obj, prev_obj) + file_changes = PullRequestModel()._calculate_file_changes( + old_diff_data, new_diff_data) + return changes, file_changes + @LoginRequired() @HasRepoPermissionAnyDecorator('repository.read', 'repository.write', 'repository.admin') @@ -763,7 +773,7 @@ class PullrequestsController(BaseRepoCon pull_request_at_ver) pr_closed = pull_request_latest.is_closed() - if at_version: + if at_version and not at_version == 'latest': c.allowed_to_change_status = False c.allowed_to_update = False c.allowed_to_merge = False @@ -840,11 +850,21 @@ class PullrequestsController(BaseRepoCon statuses = ChangesetStatus.STATUSES c.commit_statuses = statuses - c.ancestor = None # TODO: add ancestor here + c.ancestor = None # TODO: add ancestor here c.pull_request = pull_request_display_obj c.pull_request_latest = pull_request_latest c.at_version = at_version + c.versions = pull_request_display_obj.versions() + c.changes = None + c.file_changes = None + + c.show_version_changes = 1 + + if at_version and c.show_version_changes: + c.changes, c.file_changes = self._get_pr_version_changes( + version, pull_request_latest) + return render('/pullrequests/pullrequest_show.html') @LoginRequired() diff --git a/rhodecode/lib/utils2.py b/rhodecode/lib/utils2.py --- a/rhodecode/lib/utils2.py +++ b/rhodecode/lib/utils2.py @@ -665,7 +665,8 @@ class StrictAttributeDict(dict): try: return self[attr] except KeyError: - raise AttributeError('%s object has no attribute %s' % (self, attr)) + raise AttributeError('%s object has no attribute %s' % ( + self.__class__, attr)) __setattr__ = dict.__setitem__ __delattr__ = dict.__delitem__ diff --git a/rhodecode/lib/vcs/backends/base.py b/rhodecode/lib/vcs/backends/base.py --- a/rhodecode/lib/vcs/backends/base.py +++ b/rhodecode/lib/vcs/backends/base.py @@ -1442,6 +1442,15 @@ class EmptyChangeset(EmptyCommit): self.idx = value +class EmptyRepository(BaseRepository): + def __init__(self, repo_path=None, config=None, create=False, **kwargs): + pass + + def get_diff(self, *args, **kwargs): + from rhodecode.lib.vcs.backends.git.diff import GitDiff + return GitDiff('') + + class CollectionGenerator(object): def __init__(self, repo, commit_ids, collection_size=None, pre_load=None): diff --git a/rhodecode/model/db.py b/rhodecode/model/db.py --- a/rhodecode/model/db.py +++ b/rhodecode/model/db.py @@ -53,7 +53,7 @@ from rhodecode.lib.vcs.backends.base imp from rhodecode.lib.utils2 import ( str2bool, safe_str, get_commit_safe, safe_unicode, md5_safe, time_to_datetime, aslist, Optional, safe_int, get_clone_url, AttributeDict, - glob2re) + glob2re, StrictAttributeDict) from rhodecode.lib.jsonalchemy import MutationObj, MutationList, JsonType from rhodecode.lib.ext_json import json from rhodecode.lib.caching_query import FromCache @@ -3213,6 +3213,64 @@ class PullRequest(Base, _PullRequestBase cascade="all, delete, delete-orphan", lazy='dynamic') + + @classmethod + def get_pr_display_object(cls, pull_request_obj, org_pull_request_obj, + internal_methods=None): + + class PullRequestDisplay(object): + """ + Special object wrapper for showing PullRequest data via Versions + It mimics PR object as close as possible. This is read only object + just for display + """ + + def __init__(self, attrs, internal=None): + self.attrs = attrs + # internal have priority over the given ones via attrs + self.internal = internal or ['versions'] + + def __getattr__(self, item): + if item in self.internal: + return getattr(self, item) + try: + return self.attrs[item] + except KeyError: + raise AttributeError( + '%s object has no attribute %s' % (self, item)) + + def __repr__(self): + return '' % self.attrs.get('pull_request_id') + + def versions(self): + return pull_request_obj.versions.order_by( + PullRequestVersion.pull_request_version_id).all() + + def is_closed(self): + return pull_request_obj.is_closed() + + attrs = StrictAttributeDict(pull_request_obj.get_api_data()) + + attrs.author = StrictAttributeDict( + pull_request_obj.author.get_api_data()) + if pull_request_obj.target_repo: + attrs.target_repo = StrictAttributeDict( + pull_request_obj.target_repo.get_api_data()) + attrs.target_repo.clone_url = pull_request_obj.target_repo.clone_url + + if pull_request_obj.source_repo: + attrs.source_repo = StrictAttributeDict( + pull_request_obj.source_repo.get_api_data()) + attrs.source_repo.clone_url = pull_request_obj.source_repo.clone_url + + attrs.source_ref_parts = pull_request_obj.source_ref_parts + attrs.target_ref_parts = pull_request_obj.target_ref_parts + attrs.revisions = pull_request_obj.revisions + + attrs.shadow_merge_ref = org_pull_request_obj.shadow_merge_ref + + return PullRequestDisplay(attrs, internal=internal_methods) + def is_closed(self): return self.status == self.STATUS_CLOSED diff --git a/rhodecode/public/css/main.less b/rhodecode/public/css/main.less --- a/rhodecode/public/css/main.less +++ b/rhodecode/public/css/main.less @@ -1382,6 +1382,16 @@ table.integrations { } } +.compare_view_commits_title { + .disabled { + cursor: inherit; + &:hover{ + background-color: inherit; + color: inherit; + } + } +} + // new entry in group_members .td-author-new-entry { background-color: rgba(red(@alert1), green(@alert1), blue(@alert1), 0.3); diff --git a/rhodecode/templates/pullrequests/pullrequest_show.html b/rhodecode/templates/pullrequests/pullrequest_show.html --- a/rhodecode/templates/pullrequests/pullrequest_show.html +++ b/rhodecode/templates/pullrequests/pullrequest_show.html @@ -45,7 +45,7 @@
<%summary = lambda n:{False:'summary-short'}.get(n)%>
- ${_('Pull request #%s') % c.pull_request.pull_request_id} ${_('From')} ${h.format_date(c.pull_request.created_on)} + ${_('Pull request #%s') % c.pull_request.pull_request_id} ${_('From')} ${h.format_date(c.pull_request.created_on)} %if c.allowed_to_update:
% if c.allowed_to_delete: @@ -112,22 +112,26 @@
## Link to the shadow repository. - %if not c.pull_request.is_closed() and c.pull_request.shadow_merge_ref: -
-
- +
+
+ +
+
+ % if not c.pull_request.is_closed() and c.pull_request.shadow_merge_ref: +
+ %if h.is_hg(c.pull_request.target_repo): + + %elif h.is_git(c.pull_request.target_repo): + + %endif
-
-
- %if h.is_hg(c.pull_request.target_repo): - - %elif h.is_git(c.pull_request.target_repo): - - %endif -
+ % else: +
+ ${_('Shadow repository data not available')}.
+ % endif
- %endif +
@@ -187,21 +191,23 @@
- +
+
+ % if c.show_version_changes: - + - + % for ver in reversed(c.pull_request.versions()): @@ -214,10 +220,36 @@ - + % endfor
- % if c.at_version == None: + % if c.at_version in [None, 'latest']: % endif latestlatest ${c.pull_request_latest.source_ref_parts.commit_id[:6]} ${_('created')} ${h.age_component(c.pull_request.created_on)}${_('created')} ${h.age_component(c.pull_request_latest.updated_on)}
${ver.source_ref_parts.commit_id[:6]} ${_('created')} ${h.age_component(ver.created_on)}${_('created')} ${h.age_component(ver.updated_on)}
+ + % if c.at_version: +
+  Changed commits:
+    * added: ${len(c.changes.added)}
+    * removed: ${len(c.changes.removed)}
+
+  % if not (c.file_changes.added+c.file_changes.modified+c.file_changes.removed):
+  No file changes found
+  % else:
+  Changed files:
+    %for file_name in c.file_changes.added:
+    * A ${file_name}
+    %endfor
+    %for file_name in c.file_changes.modified:
+    * M ${file_name}
+    %endfor
+    %for file_name in c.file_changes.removed:
+    * R ${file_name}
+    %endfor
+  % endif
+
+ % endif + % else: + ${_('Pull request versions not available')}. + % endif
@@ -329,9 +361,9 @@ % endif
% if c.allowed_to_update and not c.pull_request.is_closed(): - + ${_('Update commits')} % else: - + ${_('Update commits')} % endif % if len(c.commit_ranges):

${ungettext('Compare View: %s commit','Compare View: %s commits', len(c.commit_ranges)) % len(c.commit_ranges)}

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 @@ -1,5 +1,5 @@ ## -*- coding: utf-8 -*- -Auto status change to |under_review| +Pull request updated. Auto status change to |under_review| .. role:: added .. role:: removed 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 @@ -95,7 +95,7 @@ def test_rst_xss_raw_directive(): def test_render_rst_template_without_files(): expected = u'''\ -Auto status change to |under_review| +Pull request updated. Auto status change to |under_review| .. role:: added .. role:: removed @@ -125,7 +125,7 @@ Auto status change to |under_review| def test_render_rst_template_with_files(): expected = u'''\ -Auto status change to |under_review| +Pull request updated. Auto status change to |under_review| .. role:: added .. role:: removed 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 @@ -722,7 +722,7 @@ def test_update_adds_a_comment_to_the_pu # Expect to find a new comment about the change expected_message = textwrap.dedent( """\ - Auto status change to |under_review| + Pull request updated. Auto status change to |under_review| .. role:: added .. role:: removed