engineering/unrhodecode
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

pull-requests: fix way how pull-request calculates common ancestors.

Browse source 
- fixes #5561
- added new preview for size (commits/files) of PRs before opening, this is now based on the special logic that calculates common ancestor and has access to preview diff
- store common ancestor in DB so updates of pull-requests are consistent
This commit is contained in:
Marcin Kuzminski 2020-05-08 08:43:51 +02:00
parent 4b277c70fe
commit 293a3535ae
18 changed files with 456 additions and 205 deletions
Download patch file Download diff file Expand all files Collapse all files

2
rhodecode/__init__.py
View file

@ -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'

48
rhodecode/api/views/pull_request_api.py
View file

@ -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
)

3
rhodecode/apps/repository/tests/test_repo_compare.py
View file

@ -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:

12
rhodecode/apps/repository/utils.py
View file

@ -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, mandatory=False, rules=None, user_group
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': {},

34
rhodecode/apps/repository/views/repo_pull_requests.py
View file

@ -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, DataGridAppView):
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, DataGridAppView):
_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, DataGridAppView):
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, DataGridAppView):
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, DataGridAppView):
# 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, DataGridAppView):
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, DataGridAppView):
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, DataGridAppView):
target_repo=target_repo,
target_ref=target_ref,
revisions=commit_ids,
common_ancestor_id=common_ancestor_id,
reviewers=reviewers,
title=pullrequest_title,
description=description,

16
rhodecode/apps/repository/views/repo_review_rules.py
View file

@ -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

47
rhodecode/lib/dbmigrate/versions/107_version_4_19_0.py Normal file
View file

@ -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

5
rhodecode/lib/vcs/backends/git/repository.py
View file

@ -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):

9
rhodecode/lib/vcs/backends/hg/repository.py
View file

@ -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:

4
rhodecode/model/db.py
View file

@ -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

138
rhodecode/model/pull_request.py
View file

@ -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()

4
rhodecode/public/css/main.less
View file

@ -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"] {

3
rhodecode/public/css/select2.less
View file

@ -205,7 +205,9 @@ select.select2{height:28px;visibility:hidden}
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:hidden}
font-family: @text-regular;
color: @grey2;
cursor: pointer;
white-space: nowrap;
}
}
}

4
rhodecode/public/js/src/i18n_utils.js
View file

@ -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, plural, n) {
}
return _gettext(plural)
};

139
rhodecode/public/js/src/rhodecode/pullrequests.js
View file

@ -75,12 +75,12 @@ var getTitleAndDescription = function(sourceRef, elements, limit) {
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(sourceRef, elements, limit) {
};
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 '<div>- {0}</div>'.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 = $.ajax({
url: url,
headers: {'X-PARTIAL-XHR': true},
type: 'GET',
success: function (data) {
self.currentRequest = $.get(url)
.done(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)
}
};

1
rhodecode/public/js/src/select2/select2.css vendored
View file

@ -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 }

6
rhodecode/templates/compare/compare_commits.mako
View file

@ -2,10 +2,8 @@
<%namespace name="base" file="/base/base.mako"/>
%if c.ancestor:
<div class="ancestor">${_('Common Ancestor Commit')}:
<a href="${h.route_path('repo_commit', repo_name=c.repo_name, commit_id=c.ancestor)}">
${h.short_id(c.ancestor)}
</a>. ${_('Compare was calculated based on this shared commit.')}
<div class="ancestor">${_('Compare was calculated based on this common ancestor commit')}:
<a href="${h.route_path('repo_commit', repo_name=c.repo_name, commit_id=c.ancestor)}">${h.short_id(c.ancestor)}</a>
<input id="common_ancestor" type="hidden" name="common_ancestor" value="${c.ancestor}">
</div>
%endif

186
rhodecode/templates/pullrequests/pullrequest.mako
View file

@ -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 = '<input id="common_ancestor" type="hidden" name="common_ancestor" value="{0}">'.format(commonAncestorId);
msg += '<input type="hidden" name="__start__" value="revisions:sequence">'
$.each(commitElements, function(idx, value) {
msg += '<input type="hidden" name="revisions" value="{0}">'.format(value["raw_id"]);
});
msg += '<input type="hidden" name="__end__" value="revisions:sequence">'
msg += _ngettext(
'This pull requests will consist of <strong>{0} commit</strong>.',
'This pull requests will consist of <strong>{0} commits</strong>.',
commitElements.length).format(commitElements.length)
msg += '\n';
msg += _ngettext(
'<strong>{0} file</strong> changed, ',
'<strong>{0} files</strong> changed, ',
files.length).format(files.length)
msg += '<span class="op-added">{0} lines inserted</span>, <span class="op-deleted">{1} lines deleted</span>.'.format(added, deleted)
msg += '\n\n <a class="" id="pull_request_overview_url" href="{0}" target="_blank">${_('Show detailed compare.')}</a>'.format(url);
if (commitElements.length) {
var commitsLink = '<a href="#pull_request_overview"><strong>{0}</strong></a>'.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 += ' <a id="pull_request_overview_url" href="{0}" target="_blank">${_('Show detailed compare.')}</a>'.format(url);
if (commitElements.length) {
var commitsLink = '<a href="#pull_request_overview"><strong>{0}</strong></a>'.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());