diff --git a/rhodecode/__init__.py b/rhodecode/__init__.py --- a/rhodecode/__init__.py +++ b/rhodecode/__init__.py @@ -48,7 +48,7 @@ PYRAMID_SETTINGS = {} EXTENSIONS = {} __version__ = ('.'.join((str(each) for each in VERSION[:3]))) -__dbversion__ = 106 # defines current db version for migrations +__dbversion__ = 107 # defines current db version for migrations __platform__ = platform.system() __license__ = 'AGPLv3, and Commercial License' __author__ = 'RhodeCode GmbH' 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 @@ -686,28 +686,9 @@ def create_pull_request( full_source_ref = resolve_ref_or_error(source_ref, source_db_repo) full_target_ref = resolve_ref_or_error(target_ref, target_db_repo) - source_scm = source_db_repo.scm_instance() - target_scm = target_db_repo.scm_instance() - source_commit = get_commit_or_error(full_source_ref, source_db_repo) target_commit = get_commit_or_error(full_target_ref, target_db_repo) - ancestor = source_scm.get_common_ancestor( - source_commit.raw_id, target_commit.raw_id, target_scm) - if not ancestor: - raise JSONRPCError('no common ancestor found') - - # recalculate target ref based on ancestor - target_ref_type, target_ref_name, __ = full_target_ref.split(':') - full_target_ref = ':'.join((target_ref_type, target_ref_name, ancestor)) - - commit_ranges = target_scm.compare( - target_commit.raw_id, source_commit.raw_id, source_scm, - merge=True, pre_load=[]) - - if not commit_ranges: - raise JSONRPCError('no commits found') - reviewer_objects = Optional.extract(reviewers) or [] # serialize and validate passed in given reviewers @@ -727,16 +708,16 @@ def create_pull_request( PullRequestModel().get_reviewer_functions() # recalculate reviewers logic, to make sure we can validate this - reviewer_rules = get_default_reviewers_data( + default_reviewers_data = get_default_reviewers_data( owner, source_db_repo, source_commit, target_db_repo, target_commit) # now MERGE our given with the calculated - reviewer_objects = reviewer_rules['reviewers'] + reviewer_objects + reviewer_objects = default_reviewers_data['reviewers'] + reviewer_objects try: reviewers = validate_default_reviewers( - reviewer_objects, reviewer_rules) + reviewer_objects, default_reviewers_data) except ValueError as e: raise JSONRPCError('Reviewers Validation: {}'.format(e)) @@ -748,6 +729,24 @@ def create_pull_request( source_ref=title_source_ref, target=target_repo ) + + diff_info = default_reviewers_data['diff_info'] + common_ancestor_id = diff_info['ancestor'] + commits = diff_info['commits'] + + if not common_ancestor_id: + raise JSONRPCError('no common ancestor found') + + if not commits: + raise JSONRPCError('no commits found') + + # NOTE(marcink): reversed is consistent with how we open it in the WEB interface + revisions = [commit.raw_id for commit in reversed(commits)] + + # recalculate target ref based on ancestor + target_ref_type, target_ref_name, __ = full_target_ref.split(':') + full_target_ref = ':'.join((target_ref_type, target_ref_name, common_ancestor_id)) + # fetch renderer, if set fallback to plain in case of PR rc_config = SettingsModel().get_all_settings() default_system_renderer = rc_config.get('rhodecode_markup_renderer', 'plain') @@ -760,12 +759,13 @@ def create_pull_request( source_ref=full_source_ref, target_repo=target_repo, target_ref=full_target_ref, - revisions=[commit.raw_id for commit in reversed(commit_ranges)], + common_ancestor_id=common_ancestor_id, + revisions=revisions, reviewers=reviewers, title=title, description=description, description_renderer=description_renderer, - reviewer_data=reviewer_rules, + reviewer_data=default_reviewers_data, auth_user=apiuser ) diff --git a/rhodecode/apps/repository/tests/test_repo_compare.py b/rhodecode/apps/repository/tests/test_repo_compare.py --- a/rhodecode/apps/repository/tests/test_repo_compare.py +++ b/rhodecode/apps/repository/tests/test_repo_compare.py @@ -484,7 +484,7 @@ class TestCompareView(object): # outgoing commits between those commits compare_page = ComparePage(response) - compare_page.contains_commits(commits=[commit1], ancestors=[commit0]) + compare_page.contains_commits(commits=[commit1]) def test_errors_when_comparing_unknown_source_repo(self, backend): repo = backend.repo @@ -641,6 +641,7 @@ class ComparePage(AssertResponse): self.contains_one_link( 'r%s:%s' % (commit.idx, commit.short_id), self._commit_url(commit)) + if ancestors: response.mustcontain('Ancestor') for ancestor in ancestors: diff --git a/rhodecode/apps/repository/utils.py b/rhodecode/apps/repository/utils.py --- a/rhodecode/apps/repository/utils.py +++ b/rhodecode/apps/repository/utils.py @@ -20,6 +20,9 @@ from rhodecode.lib import helpers as h from rhodecode.lib.utils2 import safe_int +from rhodecode.model.pull_request import get_diff_info + +REVIEWER_API_VERSION = 'V3' def reviewer_as_json(user, reasons=None, mandatory=False, rules=None, user_group=None): @@ -47,15 +50,20 @@ def reviewer_as_json(user, reasons=None, def get_default_reviewers_data( current_user, source_repo, source_commit, target_repo, target_commit): + """ + Return json for default reviewers of a repository + """ - """ Return json for default reviewers of a repository """ + diff_info = get_diff_info( + source_repo, source_commit.raw_id, target_repo, target_commit.raw_id) reasons = ['Default reviewer', 'Repository owner'] json_reviewers = [reviewer_as_json( user=target_repo.user, reasons=reasons, mandatory=False, rules=None)] return { - 'api_ver': 'v1', # define version for later possible schema upgrade + 'api_ver': REVIEWER_API_VERSION, # define version for later possible schema upgrade + 'diff_info': diff_info, 'reviewers': json_reviewers, 'rules': {}, 'rules_data': {}, 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 @@ -40,12 +40,12 @@ from rhodecode.lib.auth import ( NotAnonymous, CSRFRequired) from rhodecode.lib.utils2 import str2bool, safe_str, safe_unicode from rhodecode.lib.vcs.backends.base import EmptyCommit, UpdateFailureReason -from rhodecode.lib.vcs.exceptions import (CommitDoesNotExistError, - RepositoryRequirementError, EmptyRepositoryError) +from rhodecode.lib.vcs.exceptions import ( + CommitDoesNotExistError, RepositoryRequirementError, EmptyRepositoryError) from rhodecode.model.changeset_status import ChangesetStatusModel from rhodecode.model.comment import CommentsModel -from rhodecode.model.db import (func, or_, PullRequest, PullRequestVersion, - ChangesetComment, ChangesetStatus, Repository) +from rhodecode.model.db import ( + func, or_, PullRequest, ChangesetComment, ChangesetStatus, Repository) from rhodecode.model.forms import PullRequestForm from rhodecode.model.meta import Session from rhodecode.model.pull_request import PullRequestModel, MergeCheck @@ -210,10 +210,12 @@ class RepoPullRequestsView(RepoAppView, return caching_enabled def _get_diffset(self, source_repo_name, source_repo, + ancestor_commit, source_ref_id, target_ref_id, target_commit, source_commit, diff_limit, file_limit, fulldiff, hide_whitespace_changes, diff_context): + target_ref_id = ancestor_commit.raw_id vcs_diff = PullRequestModel().get_diff( source_repo, source_ref_id, target_ref_id, hide_whitespace_changes, diff_context) @@ -278,6 +280,7 @@ class RepoPullRequestsView(RepoAppView, _new_state = { 'created': PullRequest.STATE_CREATED, }.get(self.request.GET.get('force_state')) + if c.is_super_admin and _new_state: with pull_request.set_state(PullRequest.STATE_UPDATING, final_state=_new_state): h.flash( @@ -557,7 +560,8 @@ class RepoPullRequestsView(RepoAppView, source_scm, target_commit, target_ref_id, - target_scm, maybe_unreachable=maybe_unreachable) + target_scm, + maybe_unreachable=maybe_unreachable) # register our commit range for comm in commit_cache.values(): @@ -591,11 +595,10 @@ class RepoPullRequestsView(RepoAppView, has_proper_diff_cache = cached_diff and cached_diff.get('commits') if not force_recache and has_proper_diff_cache: c.diffset = cached_diff['diff'] - (ancestor_commit, commit_cache, missing_requirements, - source_commit, target_commit) = cached_diff['commits'] else: c.diffset = self._get_diffset( c.source_repo.repo_name, commits_source_repo, + c.ancestor_commit, source_ref_id, target_ref_id, target_commit, source_commit, diff_limit, file_limit, c.fulldiff, @@ -675,8 +678,10 @@ class RepoPullRequestsView(RepoAppView, # calculate the diff for commits between versions c.commit_changes = [] - mark = lambda cs, fw: list( - h.itertools.izip_longest([], cs, fillvalue=fw)) + + def mark(cs, fw): + return list(h.itertools.izip_longest([], cs, fillvalue=fw)) + for c_type, raw_id in mark(commit_changes.added, 'a') \ + mark(commit_changes.removed, 'r') \ + mark(commit_changes.common, 'c'): @@ -739,13 +744,14 @@ class RepoPullRequestsView(RepoAppView, except RepositoryRequirementError: log.warning('Failed to get all required data from repo', exc_info=True) missing_requirements = True - ancestor_commit = None + + pr_ancestor_id = pull_request_at_ver.common_ancestor_id + try: - ancestor_id = source_scm.get_common_ancestor( - source_commit.raw_id, target_commit.raw_id, target_scm) - ancestor_commit = source_scm.get_commit(ancestor_id) + ancestor_commit = source_scm.get_commit(pr_ancestor_id) except Exception: ancestor_commit = None + return ancestor_commit, commit_cache, missing_requirements, source_commit, target_commit def assure_not_empty_repo(self): @@ -949,6 +955,7 @@ class RepoPullRequestsView(RepoAppView, target_repo = _form['target_repo'] target_ref = _form['target_ref'] commit_ids = _form['revisions'][::-1] + common_ancestor_id = _form['common_ancestor'] # find the ancestor for this pr source_db_repo = Repository.get_by_repo_name(_form['source_repo']) @@ -1035,6 +1042,7 @@ class RepoPullRequestsView(RepoAppView, target_repo=target_repo, target_ref=target_ref, revisions=commit_ids, + common_ancestor_id=common_ancestor_id, reviewers=reviewers, title=pullrequest_title, description=description, diff --git a/rhodecode/apps/repository/views/repo_review_rules.py b/rhodecode/apps/repository/views/repo_review_rules.py --- a/rhodecode/apps/repository/views/repo_review_rules.py +++ b/rhodecode/apps/repository/views/repo_review_rules.py @@ -54,8 +54,20 @@ class RepoReviewRulesView(RepoAppView): renderer='json_ext') def repo_default_reviewers_data(self): self.load_default_context() - target_repo_name = self.request.GET.get('target_repo', self.db_repo.repo_name) + + request = self.request + source_repo = self.db_repo + source_repo_name = source_repo.repo_name + target_repo_name = request.GET.get('target_repo', source_repo_name) target_repo = Repository.get_by_repo_name(target_repo_name) + + source_ref = request.GET['source_ref'] + target_ref = request.GET['target_ref'] + source_commit = source_repo.get_commit(source_ref) + target_commit = target_repo.get_commit(target_ref) + + current_user = request.user.get_instance() review_data = get_default_reviewers_data( - self.db_repo.user, None, None, target_repo, None) + current_user, source_repo, source_commit, target_repo, target_commit) + return review_data diff --git a/rhodecode/lib/dbmigrate/versions/107_version_4_19_0.py b/rhodecode/lib/dbmigrate/versions/107_version_4_19_0.py new file mode 100644 --- /dev/null +++ b/rhodecode/lib/dbmigrate/versions/107_version_4_19_0.py @@ -0,0 +1,47 @@ +# -*- coding: utf-8 -*- + +import logging +from sqlalchemy import * + +from alembic.migration import MigrationContext +from alembic.operations import Operations +from sqlalchemy import BigInteger + +from rhodecode.lib.dbmigrate.versions import _reset_base +from rhodecode.model import init_model_encryption + + +log = logging.getLogger(__name__) + + +def upgrade(migrate_engine): + """ + Upgrade operations go here. + Don't create your own engine; bind migrate_engine to your metadata + """ + _reset_base(migrate_engine) + from rhodecode.lib.dbmigrate.schema import db_4_19_0_0 as db + + init_model_encryption(db) + + context = MigrationContext.configure(migrate_engine.connect()) + op = Operations(context) + + pull_requests = db.PullRequest.__table__ + with op.batch_alter_table(pull_requests.name) as batch_op: + new_column = Column('common_ancestor_id', Unicode(255), nullable=True) + batch_op.add_column(new_column) + + pull_request_version = db.PullRequestVersion.__table__ + with op.batch_alter_table(pull_request_version.name) as batch_op: + new_column = Column('common_ancestor_id', Unicode(255), nullable=True) + batch_op.add_column(new_column) + + +def downgrade(migrate_engine): + meta = MetaData() + meta.bind = migrate_engine + + +def fixups(models, _SESSION): + pass diff --git a/rhodecode/lib/vcs/backends/git/repository.py b/rhodecode/lib/vcs/backends/git/repository.py --- a/rhodecode/lib/vcs/backends/git/repository.py +++ b/rhodecode/lib/vcs/backends/git/repository.py @@ -580,6 +580,9 @@ class GitRepository(BaseRepository): return len(self.commit_ids) def get_common_ancestor(self, commit_id1, commit_id2, repo2): + log.debug('Calculating common ancestor between %sc1:%s and %sc2:%s', + self, commit_id1, repo2, commit_id2) + if commit_id1 == commit_id2: return commit_id1 @@ -600,6 +603,8 @@ class GitRepository(BaseRepository): ['merge-base', commit_id1, commit_id2]) ancestor_id = re.findall(r'[0-9a-fA-F]{40}', output)[0] + log.debug('Found common ancestor with sha: %s', ancestor_id) + return ancestor_id def compare(self, commit_id1, commit_id2, repo2, merge, pre_load=None): diff --git a/rhodecode/lib/vcs/backends/hg/repository.py b/rhodecode/lib/vcs/backends/hg/repository.py --- a/rhodecode/lib/vcs/backends/hg/repository.py +++ b/rhodecode/lib/vcs/backends/hg/repository.py @@ -297,13 +297,20 @@ class MercurialRepository(BaseRepository return update_cache def get_common_ancestor(self, commit_id1, commit_id2, repo2): + log.debug('Calculating common ancestor between %sc1:%s and %sc2:%s', + self, commit_id1, repo2, commit_id2) + if commit_id1 == commit_id2: return commit_id1 ancestors = self._remote.revs_from_revspec( "ancestor(id(%s), id(%s))", commit_id1, commit_id2, other_path=repo2.path) - return repo2[ancestors[0]].raw_id if ancestors else None + + ancestor_id = repo2[ancestors[0]].raw_id if ancestors else None + + log.debug('Found common ancestor with sha: %s', ancestor_id) + return ancestor_id def compare(self, commit_id1, commit_id2, repo2, merge, pre_load=None): if commit_id1 == commit_id2: diff --git a/rhodecode/model/db.py b/rhodecode/model/db.py --- a/rhodecode/model/db.py +++ b/rhodecode/model/db.py @@ -3989,6 +3989,8 @@ class _PullRequestBase(BaseModel): _revisions = Column( 'revisions', UnicodeText().with_variant(UnicodeText(20500), 'mysql')) + common_ancestor_id = Column('common_ancestor_id', Unicode(255), nullable=True) + @declared_attr def source_repo_id(cls): # TODO: dan: rename column to source_repo_id @@ -4302,7 +4304,7 @@ class PullRequest(Base, _PullRequestBase 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.common_ancestor_id = pull_request_obj.common_ancestor_id attrs.shadow_merge_ref = org_pull_request_obj.shadow_merge_ref attrs.reviewer_data = org_pull_request_obj.reviewer_data attrs.reviewer_data_json = org_pull_request_obj.reviewer_data_json 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 @@ -35,6 +35,7 @@ import collections from pyramid import compat from pyramid.threadlocal import get_current_request +from rhodecode.lib.vcs.nodes import FileNode from rhodecode.translation import lazy_ugettext from rhodecode.lib import helpers as h, hooks_utils, diffs from rhodecode.lib import audit_logger @@ -82,6 +83,126 @@ class UpdateResponse(object): self.target_changed = target_changed +def get_diff_info( + source_repo, source_ref, target_repo, target_ref, get_authors=False, + get_commit_authors=True): + """ + Calculates detailed diff information for usage in preview of creation of a pull-request. + This is also used for default reviewers logic + """ + + source_scm = source_repo.scm_instance() + target_scm = target_repo.scm_instance() + + ancestor_id = target_scm.get_common_ancestor(target_ref, source_ref, source_scm) + if not ancestor_id: + raise ValueError( + 'cannot calculate diff info without a common ancestor. ' + 'Make sure both repositories are related, and have a common forking commit.') + + # case here is that want a simple diff without incoming commits, + # previewing what will be merged based only on commits in the source. + log.debug('Using ancestor %s as source_ref instead of %s', + ancestor_id, source_ref) + + # source of changes now is the common ancestor + source_commit = source_scm.get_commit(commit_id=ancestor_id) + # target commit becomes the source ref as it is the last commit + # for diff generation this logic gives proper diff + target_commit = source_scm.get_commit(commit_id=source_ref) + + vcs_diff = \ + source_scm.get_diff(commit1=source_commit, commit2=target_commit, + ignore_whitespace=False, context=3) + + diff_processor = diffs.DiffProcessor( + vcs_diff, format='newdiff', diff_limit=None, + file_limit=None, show_full_diff=True) + + _parsed = diff_processor.prepare() + + all_files = [] + all_files_changes = [] + changed_lines = {} + stats = [0, 0] + for f in _parsed: + all_files.append(f['filename']) + all_files_changes.append({ + 'filename': f['filename'], + 'stats': f['stats'] + }) + stats[0] += f['stats']['added'] + stats[1] += f['stats']['deleted'] + + changed_lines[f['filename']] = [] + if len(f['chunks']) < 2: + continue + # first line is "context" information + for chunks in f['chunks'][1:]: + for chunk in chunks['lines']: + if chunk['action'] not in ('del', 'mod'): + continue + changed_lines[f['filename']].append(chunk['old_lineno']) + + commit_authors = [] + user_counts = {} + email_counts = {} + author_counts = {} + _commit_cache = {} + + commits = [] + if get_commit_authors: + commits = target_scm.compare( + target_ref, source_ref, source_scm, merge=True, + pre_load=["author"]) + + for commit in commits: + user = User.get_from_cs_author(commit.author) + if user and user not in commit_authors: + commit_authors.append(user) + + # lines + if get_authors: + target_commit = source_repo.get_commit(ancestor_id) + + for fname, lines in changed_lines.items(): + try: + node = target_commit.get_node(fname) + except Exception: + continue + + if not isinstance(node, FileNode): + continue + + for annotation in node.annotate: + line_no, commit_id, get_commit_func, line_text = annotation + if line_no in lines: + if commit_id not in _commit_cache: + _commit_cache[commit_id] = get_commit_func() + commit = _commit_cache[commit_id] + author = commit.author + email = commit.author_email + user = User.get_from_cs_author(author) + if user: + user_counts[user] = user_counts.get(user, 0) + 1 + author_counts[author] = author_counts.get(author, 0) + 1 + email_counts[email] = email_counts.get(email, 0) + 1 + + return { + 'commits': commits, + 'files': all_files_changes, + 'stats': stats, + 'ancestor': ancestor_id, + # original authors of modified files + 'original_authors': { + 'users': user_counts, + 'authors': author_counts, + 'emails': email_counts, + }, + 'commit_authors': commit_authors + } + + class PullRequestModel(BaseModel): cls = PullRequest @@ -453,6 +574,7 @@ class PullRequestModel(BaseModel): def create(self, created_by, source_repo, source_ref, target_repo, target_ref, revisions, reviewers, title, description=None, + common_ancestor_id=None, description_renderer=None, reviewer_data=None, translator=None, auth_user=None): translator = translator or get_current_request().translate @@ -474,6 +596,8 @@ class PullRequestModel(BaseModel): pull_request.author = created_by_user pull_request.reviewer_data = reviewer_data pull_request.pull_request_state = pull_request.STATE_CREATING + pull_request.common_ancestor_id = common_ancestor_id + Session().add(pull_request) Session().flush() @@ -805,8 +929,17 @@ class PullRequestModel(BaseModel): target_commit.raw_id, source_commit.raw_id, source_repo, merge=True, pre_load=pre_load) - ancestor_commit_id = source_repo.get_common_ancestor( - source_commit.raw_id, target_commit.raw_id, target_repo) + target_ref = target_commit.raw_id + source_ref = source_commit.raw_id + ancestor_commit_id = target_repo.get_common_ancestor( + target_ref, source_ref, source_repo) + + if not ancestor_commit_id: + raise ValueError( + 'cannot calculate diff info without a common ancestor. ' + 'Make sure both repositories are related, and have a common forking commit.') + + pull_request.common_ancestor_id = ancestor_commit_id pull_request.source_ref = '%s:%s:%s' % ( source_ref_type, source_ref_name, source_commit.raw_id) @@ -913,6 +1046,7 @@ class PullRequestModel(BaseModel): version.reviewer_data = pull_request.reviewer_data version.revisions = pull_request.revisions + version.common_ancestor_id = pull_request.common_ancestor_id version.pull_request = pull_request Session().add(version) Session().flush() 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 @@ -467,9 +467,9 @@ ul.auth_plugins { #pr_open_message { border: @border-thickness solid #fff; border-radius: @border-radius; - padding: @padding-large-vertical @padding-large-vertical @padding-large-vertical 0; text-align: left; overflow: hidden; + white-space: pre-line; } .pr-details-title { @@ -1796,7 +1796,7 @@ table.integrations { } div.ancestor { - line-height: 33px; + } .cs_icon_td input[type="checkbox"] { diff --git a/rhodecode/public/css/select2.less b/rhodecode/public/css/select2.less --- a/rhodecode/public/css/select2.less +++ b/rhodecode/public/css/select2.less @@ -205,7 +205,9 @@ select.select2{height:28px;visibility:hi font-family: @text-regular; color: @grey2; cursor: pointer; + white-space: nowrap; } + &.select2-result-with-children { .select2-result-label { @@ -220,6 +222,7 @@ select.select2{height:28px;visibility:hi font-family: @text-regular; color: @grey2; cursor: pointer; + white-space: nowrap; } } } diff --git a/rhodecode/public/js/src/i18n_utils.js b/rhodecode/public/js/src/i18n_utils.js --- a/rhodecode/public/js/src/i18n_utils.js +++ b/rhodecode/public/js/src/i18n_utils.js @@ -22,7 +22,7 @@ var _gettext = function (s) { if (_TM.hasOwnProperty(s)) { return _TM[s]; } - i18nLog.error( + i18nLog.warning( 'String `' + s + '` was requested but cannot be ' + 'found in translation table'); return s @@ -34,3 +34,5 @@ var _ngettext = function (singular, plur } return _gettext(plural) }; + + diff --git a/rhodecode/public/js/src/rhodecode/pullrequests.js b/rhodecode/public/js/src/rhodecode/pullrequests.js --- a/rhodecode/public/js/src/rhodecode/pullrequests.js +++ b/rhodecode/public/js/src/rhodecode/pullrequests.js @@ -75,12 +75,12 @@ var getTitleAndDescription = function(so var desc = ''; $.each($(elements).get().reverse().slice(0, limit), function(idx, value) { - var rawMessage = $(value).find('td.td-description .message').data('messageRaw').toString(); + var rawMessage = value['message']; desc += '- ' + rawMessage.split('\n')[0].replace(/\n+$/, "") + '\n'; }); // only 1 commit, use commit message as title if (elements.length === 1) { - var rawMessage = $(elements[0]).find('td.td-description .message').data('messageRaw').toString(); + var rawMessage = elements[0]['message']; title = rawMessage.split('\n')[0]; } else { @@ -92,7 +92,6 @@ var getTitleAndDescription = function(so }; - ReviewersController = function () { var self = this; this.$reviewRulesContainer = $('#review_rules'); @@ -100,28 +99,35 @@ ReviewersController = function () { this.forbidReviewUsers = undefined; this.$reviewMembers = $('#review_members'); this.currentRequest = null; + this.diffData = null; + //dummy handler, we might register our own later + this.diffDataHandler = function(data){}; - this.defaultForbidReviewUsers = function() { + this.defaultForbidReviewUsers = function () { return [ - {'username': 'default', - 'user_id': templateContext.default_user.user_id} + { + 'username': 'default', + 'user_id': templateContext.default_user.user_id + } ]; }; - this.hideReviewRules = function() { + this.hideReviewRules = function () { self.$reviewRulesContainer.hide(); }; - this.showReviewRules = function() { + this.showReviewRules = function () { self.$reviewRulesContainer.show(); }; - this.addRule = function(ruleText) { + this.addRule = function (ruleText) { self.showReviewRules(); return '
- {0}
'.format(ruleText) }; - this.loadReviewRules = function(data) { + this.loadReviewRules = function (data) { + self.diffData = data; + // reset forbidden Users this.forbidReviewUsers = self.defaultForbidReviewUsers(); @@ -141,7 +147,7 @@ ReviewersController = function () { if (data.rules.voting < 0) { self.$rulesList.append( self.addRule( - _gettext('All individual reviewers must vote.')) + _gettext('All individual reviewers must vote.')) ) } else if (data.rules.voting === 1) { self.$rulesList.append( @@ -158,7 +164,7 @@ ReviewersController = function () { } if (data.rules.voting_groups !== undefined) { - $.each(data.rules.voting_groups, function(index, rule_data) { + $.each(data.rules.voting_groups, function (index, rule_data) { self.$rulesList.append( self.addRule(rule_data.text) ) @@ -188,7 +194,7 @@ ReviewersController = function () { if (data.rules.forbid_commit_author_to_review) { if (data.rules_data.forbidden_users) { - $.each(data.rules_data.forbidden_users, function(index, member_data) { + $.each(data.rules_data.forbidden_users, function (index, member_data) { self.forbidReviewUsers.push(member_data) }); @@ -203,11 +209,10 @@ ReviewersController = function () { return self.forbidReviewUsers }; - this.loadDefaultReviewers = function(sourceRepo, sourceRef, targetRepo, targetRef) { + this.loadDefaultReviewers = function (sourceRepo, sourceRef, targetRepo, targetRef) { if (self.currentRequest) { - // make sure we cleanup old running requests before triggering this - // again + // make sure we cleanup old running requests before triggering this again self.currentRequest.abort(); } @@ -218,6 +223,9 @@ ReviewersController = function () { prButtonLock(true, null, 'reviewers'); $('#user').hide(); // hide user autocomplete before load + // lock PR button, so we cannot send PR before it's calculated + prButtonLock(true, _gettext('Loading diff ...'), 'compare'); + if (sourceRef.length !== 3 || targetRef.length !== 3) { // don't load defaults in case we're missing some refs... $('.calculate-reviewers').hide(); @@ -225,58 +233,80 @@ ReviewersController = function () { } var url = pyroutes.url('repo_default_reviewers_data', - { - 'repo_name': templateContext.repo_name, - 'source_repo': sourceRepo, - 'source_ref': sourceRef[2], - 'target_repo': targetRepo, - 'target_ref': targetRef[2] - }); + { + 'repo_name': templateContext.repo_name, + 'source_repo': sourceRepo, + 'source_ref': sourceRef[2], + 'target_repo': targetRepo, + 'target_ref': targetRef[2] + }); - self.currentRequest = $.get(url) - .done(function(data) { + self.currentRequest = $.ajax({ + url: url, + headers: {'X-PARTIAL-XHR': true}, + type: 'GET', + success: function (data) { + self.currentRequest = null; // review rules self.loadReviewRules(data); + self.handleDiffData(data["diff_info"]); for (var i = 0; i < data.reviewers.length; i++) { - var reviewer = data.reviewers[i]; - self.addReviewMember( - reviewer, reviewer.reasons, reviewer.mandatory); + var reviewer = data.reviewers[i]; + self.addReviewMember(reviewer, reviewer.reasons, reviewer.mandatory); } $('.calculate-reviewers').hide(); prButtonLock(false, null, 'reviewers'); $('#user').show(); // show user autocomplete after load - }); + + var commitElements = data["diff_info"]['commits']; + if (commitElements.length === 0) { + prButtonLock(true, _gettext('no commits'), 'all'); + + } else { + // un-lock PR button, so we cannot send PR before it's calculated + prButtonLock(false, null, 'compare'); + } + + }, + error: function (jqXHR, textStatus, errorThrown) { + var prefix = "Loading diff and reviewers failed\n" + var message = formatErrorMessage(jqXHR, textStatus, errorThrown, prefix); + ajaxErrorSwal(message); + } + }); + }; // check those, refactor - this.removeReviewMember = function(reviewer_id, mark_delete) { + this.removeReviewMember = function (reviewer_id, mark_delete) { var reviewer = $('#reviewer_{0}'.format(reviewer_id)); - if(typeof(mark_delete) === undefined){ + if (typeof (mark_delete) === undefined) { mark_delete = false; } - if(mark_delete === true){ - if (reviewer){ + if (mark_delete === true) { + if (reviewer) { // now delete the input $('#reviewer_{0} input'.format(reviewer_id)).remove(); // mark as to-delete var obj = $('#reviewer_{0}_name'.format(reviewer_id)); obj.addClass('to-delete'); - obj.css({"text-decoration":"line-through", "opacity": 0.5}); + obj.css({"text-decoration": "line-through", "opacity": 0.5}); } - } - else{ + } else { $('#reviewer_{0}'.format(reviewer_id)).remove(); } }; - this.reviewMemberEntry = function() { + + this.reviewMemberEntry = function () { }; - this.addReviewMember = function(reviewer_obj, reasons, mandatory) { + + this.addReviewMember = function (reviewer_obj, reasons, mandatory) { var members = self.$reviewMembers.get(0); var id = reviewer_obj.user_id; var username = reviewer_obj.username; @@ -287,13 +317,13 @@ ReviewersController = function () { // register IDS to check if we don't have this ID already in var currentIds = []; var _els = self.$reviewMembers.find('li').toArray(); - for (el in _els){ + for (el in _els) { currentIds.push(_els[el].id) } - var userAllowedReview = function(userId) { + var userAllowedReview = function (userId) { var allowed = true; - $.each(self.forbidReviewUsers, function(index, member_data) { + $.each(self.forbidReviewUsers, function (index, member_data) { if (parseInt(userId) === member_data['user_id']) { allowed = false; return false // breaks the loop @@ -303,35 +333,38 @@ ReviewersController = function () { }; var userAllowed = userAllowedReview(id); - if (!userAllowed){ - alert(_gettext('User `{0}` not allowed to be a reviewer').format(username)); + if (!userAllowed) { + alert(_gettext('User `{0}` not allowed to be a reviewer').format(username)); } else { // only add if it's not there - var alreadyReviewer = currentIds.indexOf('reviewer_'+id) != -1; + var alreadyReviewer = currentIds.indexOf('reviewer_' + id) != -1; if (alreadyReviewer) { alert(_gettext('User `{0}` already in reviewers').format(username)); } else { members.innerHTML += renderTemplate('reviewMemberEntry', { - 'member': reviewer_obj, - 'mandatory': mandatory, - 'allowed_to_update': true, - 'review_status': 'not_reviewed', - 'review_status_label': _gettext('Not Reviewed'), - 'reasons': reasons, - 'create': true - }); + 'member': reviewer_obj, + 'mandatory': mandatory, + 'allowed_to_update': true, + 'review_status': 'not_reviewed', + 'review_status_label': _gettext('Not Reviewed'), + 'reasons': reasons, + 'create': true + }); tooltipActivate(); } } }; - this.updateReviewers = function(repo_name, pull_request_id){ + this.updateReviewers = function (repo_name, pull_request_id) { var postData = $('#reviewers input').serialize(); _updatePullRequest(repo_name, pull_request_id, postData); }; + this.handleDiffData = function (data) { + self.diffDataHandler(data) + } }; diff --git a/rhodecode/public/js/src/select2/select2.css b/rhodecode/public/js/src/select2/select2.css --- a/rhodecode/public/js/src/select2/select2.css +++ b/rhodecode/public/js/src/select2/select2.css @@ -393,6 +393,7 @@ html[dir="rtl"] .select2-results { -moz-user-select: none; -ms-user-select: none; user-select: none; + white-space: nowrap; } .select2-results-dept-1 .select2-result-label { padding-left: 20px } diff --git a/rhodecode/templates/compare/compare_commits.mako b/rhodecode/templates/compare/compare_commits.mako --- a/rhodecode/templates/compare/compare_commits.mako +++ b/rhodecode/templates/compare/compare_commits.mako @@ -2,10 +2,8 @@ <%namespace name="base" file="/base/base.mako"/> %if c.ancestor: -
${_('Common Ancestor Commit')}: - - ${h.short_id(c.ancestor)} - . ${_('Compare was calculated based on this shared commit.')} +
${_('Compare was calculated based on this common ancestor commit')}: + ${h.short_id(c.ancestor)}
%endif diff --git a/rhodecode/templates/pullrequests/pullrequest.mako b/rhodecode/templates/pullrequests/pullrequest.mako --- a/rhodecode/templates/pullrequests/pullrequest.mako +++ b/rhodecode/templates/pullrequests/pullrequest.mako @@ -241,7 +241,93 @@ // custom code mirror var codeMirrorInstance = $('#pullrequest_desc').get(0).MarkupForm.cm; + var diffDataHandler = function(data) { + + $('#pull_request_overview').html(data); + + var commitElements = data['commits']; + var files = data['files']; + var added = data['stats'][0] + var deleted = data['stats'][1] + var commonAncestorId = data['ancestor']; + + var prTitleAndDesc = getTitleAndDescription( + sourceRef()[1], commitElements, 5); + + var title = prTitleAndDesc[0]; + var proposedDescription = prTitleAndDesc[1]; + + var useGeneratedTitle = ( + $('#pullrequest_title').hasClass('autogenerated-title') || + $('#pullrequest_title').val() === ""); + + if (title && useGeneratedTitle) { + // use generated title if we haven't specified our own + $('#pullrequest_title').val(title); + $('#pullrequest_title').addClass('autogenerated-title'); + + } + + var useGeneratedDescription = ( + !codeMirrorInstance._userDefinedValue || + codeMirrorInstance.getValue() === ""); + + if (proposedDescription && useGeneratedDescription) { + // set proposed content, if we haven't defined our own, + // or we don't have description written + codeMirrorInstance._userDefinedValue = false; // reset state + codeMirrorInstance.setValue(proposedDescription); + } + + // refresh our codeMirror so events kicks in and it's change aware + codeMirrorInstance.refresh(); + + var url_data = { + 'repo_name': targetRepo(), + 'target_repo': sourceRepo(), + 'source_ref': targetRef()[2], + 'source_ref_type': 'rev', + 'target_ref': sourceRef()[2], + 'target_ref_type': 'rev', + 'merge': true, + '_': Date.now() // bypass browser caching + }; // gather the source/target ref and repo here + var url = pyroutes.url('repo_compare', url_data); + + var msg = ''.format(commonAncestorId); + msg += '' + + $.each(commitElements, function(idx, value) { + msg += ''.format(value["raw_id"]); + }); + + msg += '' + msg += _ngettext( + 'This pull requests will consist of {0} commit.', + 'This pull requests will consist of {0} commits.', + commitElements.length).format(commitElements.length) + + msg += '\n'; + msg += _ngettext( + '{0} file changed, ', + '{0} files changed, ', + files.length).format(files.length) + msg += '{0} lines inserted, {1} lines deleted.'.format(added, deleted) + + msg += '\n\n ${_('Show detailed compare.')}'.format(url); + + if (commitElements.length) { + var commitsLink = '{0}'.format(commitElements.length); + prButtonLock(false, msg.replace('__COMMITS__', commitsLink), 'compare'); + } + else { + prButtonLock(true, "${_('There are no commits to merge.')}", 'compare'); + } + + }; + reviewersController = new ReviewersController(); + reviewersController.diffDataHandler = diffDataHandler; var queryTargetRepo = function(self, query) { // cache ALL results if query is empty @@ -286,99 +372,6 @@ query.callback({results: data.results}); }; - var loadRepoRefDiffPreview = function() { - - var url_data = { - 'repo_name': targetRepo(), - 'target_repo': sourceRepo(), - 'source_ref': targetRef()[2], - 'source_ref_type': 'rev', - 'target_ref': sourceRef()[2], - 'target_ref_type': 'rev', - 'merge': true, - '_': Date.now() // bypass browser caching - }; // gather the source/target ref and repo here - - if (sourceRef().length !== 3 || targetRef().length !== 3) { - prButtonLock(true, "${_('Please select source and target')}"); - return; - } - var url = pyroutes.url('repo_compare', url_data); - - // lock PR button, so we cannot send PR before it's calculated - prButtonLock(true, "${_('Loading compare ...')}", 'compare'); - - if (loadRepoRefDiffPreview._currentRequest) { - loadRepoRefDiffPreview._currentRequest.abort(); - } - - loadRepoRefDiffPreview._currentRequest = $.get(url) - .error(function(jqXHR, textStatus, errorThrown) { - if (textStatus !== 'abort') { - var prefix = "Error while processing request.\n" - var message = formatErrorMessage(jqXHR, textStatus, errorThrown, prefix); - ajaxErrorSwal(message); - } - - }) - .done(function(data) { - loadRepoRefDiffPreview._currentRequest = null; - $('#pull_request_overview').html(data); - - var commitElements = $(data).find('tr[commit_id]'); - - var prTitleAndDesc = getTitleAndDescription( - sourceRef()[1], commitElements, 5); - - var title = prTitleAndDesc[0]; - var proposedDescription = prTitleAndDesc[1]; - - var useGeneratedTitle = ( - $('#pullrequest_title').hasClass('autogenerated-title') || - $('#pullrequest_title').val() === ""); - - if (title && useGeneratedTitle) { - // use generated title if we haven't specified our own - $('#pullrequest_title').val(title); - $('#pullrequest_title').addClass('autogenerated-title'); - - } - - var useGeneratedDescription = ( - !codeMirrorInstance._userDefinedValue || - codeMirrorInstance.getValue() === ""); - - if (proposedDescription && useGeneratedDescription) { - // set proposed content, if we haven't defined our own, - // or we don't have description written - codeMirrorInstance._userDefinedValue = false; // reset state - codeMirrorInstance.setValue(proposedDescription); - } - - // refresh our codeMirror so events kicks in and it's change aware - codeMirrorInstance.refresh(); - - var msg = ''; - if (commitElements.length === 1) { - msg = "${_ungettext('This pull request will consist of __COMMITS__ commit.', 'This pull request will consist of __COMMITS__ commits.', 1)}"; - } else { - msg = "${_ungettext('This pull request will consist of __COMMITS__ commit.', 'This pull request will consist of __COMMITS__ commits.', 2)}"; - } - - msg += ' ${_('Show detailed compare.')}'.format(url); - - if (commitElements.length) { - var commitsLink = '{0}'.format(commitElements.length); - prButtonLock(false, msg.replace('__COMMITS__', commitsLink), 'compare'); - } - else { - prButtonLock(true, "${_('There are no commits to merge.')}", 'compare'); - } - - - }); - }; - var Select2Box = function(element, overrides) { var globalDefaults = { dropdownAutoWidth: true, @@ -476,13 +469,11 @@ targetRepoSelect2.initRepo(defaultTargetRepo, false); $sourceRef.on('change', function(e){ - loadRepoRefDiffPreview(); reviewersController.loadDefaultReviewers( sourceRepo(), sourceRef(), targetRepo(), targetRef()); }); $targetRef.on('change', function(e){ - loadRepoRefDiffPreview(); reviewersController.loadDefaultReviewers( sourceRepo(), sourceRef(), targetRepo(), targetRef()); }); @@ -502,7 +493,6 @@ success: function(data) { $('#target_ref_loading').hide(); targetRepoChanged(data); - loadRepoRefDiffPreview(); }, error: function(jqXHR, textStatus, errorThrown) { var prefix = "Error while fetching entries.\n" @@ -533,8 +523,8 @@ % if c.default_source_ref: // in case we have a pre-selected value, use it now $sourceRef.select2('val', '${c.default_source_ref}'); - // diff preview load - loadRepoRefDiffPreview(); + + // default reviewers reviewersController.loadDefaultReviewers( sourceRepo(), sourceRef(), targetRepo(), targetRef());