diff --git a/.bumpversion.cfg b/.bumpversion.cfg --- a/.bumpversion.cfg +++ b/.bumpversion.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 4.22.0 +current_version = 4.23.0 message = release: Bump version {current_version} to {new_version} [bumpversion:file:rhodecode/VERSION] diff --git a/.release.cfg b/.release.cfg --- a/.release.cfg +++ b/.release.cfg @@ -5,25 +5,20 @@ done = false done = true [task:rc_tools_pinned] -done = true [task:fixes_on_stable] -done = true [task:pip2nix_generated] -done = true [task:changelog_updated] -done = true [task:generate_api_docs] -done = true + +[task:updated_translation] [release] -state = prepared -version = 4.22.0 - -[task:updated_translation] +state = in_progress +version = 4.23.0 [task:generate_js_routes] diff --git a/docs/release-notes/release-notes-4.23.0.rst b/docs/release-notes/release-notes-4.23.0.rst new file mode 100644 --- /dev/null +++ b/docs/release-notes/release-notes-4.23.0.rst @@ -0,0 +1,89 @@ +|RCE| 4.23.0 |RNS| +------------------ + +Release Date +^^^^^^^^^^^^ + +- 2020-11-20 + + +New Features +^^^^^^^^^^^^ + +- Comments: introduced new draft comments. + + * drafts are private to author + * not triggering any notifications + * sidebar doesn't display draft comments + * They are just placeholders for longer review. + +- Comments: when channelstream is enabled, comments are pushed live, so there's no + need to refresh page to see other participant comments. + New comments are marker in the sidebar. + +- Comments: multiple changes on comments navigation/display logic. + + * toggle icon is smarter, open/hide windows according to actions. E.g commenting opens threads + * toggle are mor explicit + * possible to hide/show only single threads using the toggle icon. + * new UI for showing thread comments + +- Reviewers: new logic for author/commit-author rules. + It's not possible to define if author or commit author should be excluded, or always included in a review. +- Reviewers: no reviewers would now allow a PR to be merged, unless review rules require some. + Use case is that pr can be created without review needed, maybe just for sharing, or CI checks +- Pull requests: save permanently the state if sorting columns for pull-request grids. +- Commit ranges: enable combined diff compare directly from range selector. + + +General +^^^^^^^ + +- Authentication: enable custom names for auth plugins. It's possible to name the authentication + buttons now for SAML plugins. +- Login: optimized UI for login/register/password reset windows. +- Repo mapper: make it more resilient to errors, it's better it executes and skip certain + repositories, rather then crash whole mapper. +- Markdown: improved styling, and fixed nl2br extensions to only do br on new elements not inline. +- Pull requests: show pr version in the my-account and repo pr listing grids. +- Archives: allowing to obtain archives without the commit short id in the name for + better automation of obtained artifacts. + New url flag called `?=with_hash=1` controls this +- Error document: update info about stored exception retrieval. +- Range diff: enable hovercards for commits in range-diff. + + +Security +^^^^^^^^ + + + +Performance +^^^^^^^^^^^ + +- Improved logic of repo archive, now it's much faster to run archiver as VCSServer + communication was removed, and job is delegated to VCSServer itself. +- Improved VCSServer startup times. +- Notifications: skip double rendering just to generate email title/desc. + We'll re-use those now for better performance of creating notifications. +- App: improve logging, and remove DB calls on app startup. + + +Fixes +^^^^^ + +- Login/register: fixed header width problem on mobile devices +- Exception tracker: don't fail on empty request in context of celery app for example. +- Exceptions: improved reporting of unhandled vcsserver exceptions. +- Sidebar: fixed refresh of TODOs url. +- Remap-rescan: fixes #5636 initial rescan problem. +- API: fixed SVN raw diff export. The API method was inconsistent, and used different logic. + Now it shares the same code as raw-diff from web-ui. + + +Upgrade notes +^^^^^^^^^^^^^ + +- Scheduled feature release. + Please note that now the reviewers logic changed a bit, it's possible to create a pull request + Without any reviewers initially, and such pull request doesn't need to have an approval for merging. diff --git a/docs/release-notes/release-notes.rst b/docs/release-notes/release-notes.rst --- a/docs/release-notes/release-notes.rst +++ b/docs/release-notes/release-notes.rst @@ -9,6 +9,7 @@ Release Notes .. toctree:: :maxdepth: 1 + release-notes-4.23.0.rst release-notes-4.22.0.rst release-notes-4.21.0.rst release-notes-4.20.1.rst diff --git a/pkgs/python-packages.nix b/pkgs/python-packages.nix --- a/pkgs/python-packages.nix +++ b/pkgs/python-packages.nix @@ -1883,7 +1883,7 @@ self: super: { }; }; "rhodecode-enterprise-ce" = super.buildPythonPackage { - name = "rhodecode-enterprise-ce-4.22.0"; + name = "rhodecode-enterprise-ce-4.23.0"; buildInputs = [ self."pytest" self."py" diff --git a/rhodecode/VERSION b/rhodecode/VERSION --- a/rhodecode/VERSION +++ b/rhodecode/VERSION @@ -1,1 +1,1 @@ -4.22.0 \ No newline at end of file +4.23.0 \ No newline at end of file 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__ = 110 # defines current db version for migrations +__dbversion__ = 112 # defines current db version for migrations __platform__ = platform.system() __license__ = 'AGPLv3, and Commercial License' __author__ = 'RhodeCode GmbH' diff --git a/rhodecode/api/utils.py b/rhodecode/api/utils.py --- a/rhodecode/api/utils.py +++ b/rhodecode/api/utils.py @@ -351,7 +351,10 @@ def get_pull_request_or_error(pullreques return pull_request -def build_commit_data(commit, detail_level): +def build_commit_data(rhodecode_vcs_repo, commit, detail_level): + commit2 = commit + commit1 = commit.first_parent + parsed_diff = [] if detail_level == 'extended': for f_path in commit.added_paths: @@ -362,8 +365,11 @@ def build_commit_data(commit, detail_lev parsed_diff.append(_get_commit_dict(filename=f_path, op='D')) elif detail_level == 'full': - from rhodecode.lib.diffs import DiffProcessor - diff_processor = DiffProcessor(commit.diff()) + from rhodecode.lib import diffs + + _diff = rhodecode_vcs_repo.get_diff(commit1, commit2,) + diff_processor = diffs.DiffProcessor(_diff, format='newdiff', show_full_diff=True) + for dp in diff_processor.prepare(): del dp['stats']['ops'] _stats = dp['stats'] diff --git a/rhodecode/api/views/repo_api.py b/rhodecode/api/views/repo_api.py --- a/rhodecode/api/views/repo_api.py +++ b/rhodecode/api/views/repo_api.py @@ -317,17 +317,18 @@ def get_repo_changeset(request, apiuser, 'ret_type must be one of %s' % ( ','.join(_changes_details_types))) + vcs_repo = repo.scm_instance() pre_load = ['author', 'branch', 'date', 'message', 'parents', 'status', '_commit', '_file_paths'] try: - cs = repo.get_commit(commit_id=revision, pre_load=pre_load) + commit = repo.get_commit(commit_id=revision, pre_load=pre_load) except TypeError as e: raise JSONRPCError(safe_str(e)) - _cs_json = cs.__json__() - _cs_json['diff'] = build_commit_data(cs, changes_details) + _cs_json = commit.__json__() + _cs_json['diff'] = build_commit_data(vcs_repo, commit, changes_details) if changes_details == 'full': - _cs_json['refs'] = cs._get_refs() + _cs_json['refs'] = commit._get_refs() return _cs_json @@ -398,7 +399,7 @@ def get_repo_changesets(request, apiuser if cnt >= limit != -1: break _cs_json = commit.__json__() - _cs_json['diff'] = build_commit_data(commit, changes_details) + _cs_json['diff'] = build_commit_data(vcs_repo, commit, changes_details) if changes_details == 'full': _cs_json['refs'] = { 'branches': [commit.branch], diff --git a/rhodecode/apps/admin/views/users.py b/rhodecode/apps/admin/views/users.py --- a/rhodecode/apps/admin/views/users.py +++ b/rhodecode/apps/admin/views/users.py @@ -36,7 +36,7 @@ from rhodecode.authentication.plugins im from rhodecode.events import trigger from rhodecode.model.db import true, UserNotice -from rhodecode.lib import audit_logger, rc_cache +from rhodecode.lib import audit_logger, rc_cache, auth from rhodecode.lib.exceptions import ( UserCreationError, UserOwnsReposException, UserOwnsRepoGroupsException, UserOwnsUserGroupsException, UserOwnsPullRequestsException, @@ -295,6 +295,10 @@ class UsersView(UserAppView): c.allowed_extern_types = [ (x.uid, x.get_display_name()) for x in self.get_auth_plugins() ] + perms = req.registry.settings.get('available_permissions') + if not perms: + # inject info about available permissions + auth.set_available_permissions(req.registry.settings) c.available_permissions = req.registry.settings['available_permissions'] PermissionModel().set_global_permission_choices( diff --git a/rhodecode/apps/debug_style/views.py b/rhodecode/apps/debug_style/views.py --- a/rhodecode/apps/debug_style/views.py +++ b/rhodecode/apps/debug_style/views.py @@ -252,7 +252,7 @@ But please check this code var comment = $('#comment-'+commentId); var commentData = comment.data(); if (commentData.commentInline) { - this.createComment(comment, commentId) + this.createComment(comment, f_path, line_no, commentId) } else { Rhodecode.comments.createGeneralComment('general', "$placeholder", commentId) } diff --git a/rhodecode/apps/my_account/views/my_account.py b/rhodecode/apps/my_account/views/my_account.py --- a/rhodecode/apps/my_account/views/my_account.py +++ b/rhodecode/apps/my_account/views/my_account.py @@ -702,7 +702,9 @@ class MyAccountView(BaseAppView, DataGri **valid_data) if old_email != valid_data['email']: old = UserEmailMap.query() \ - .filter(UserEmailMap.user == c.user).filter(UserEmailMap.email == valid_data['email']).first() + .filter(UserEmailMap.user == c.user)\ + .filter(UserEmailMap.email == valid_data['email'])\ + .first() old.email = old_email h.flash(_('Your account was updated successfully'), category='success') Session().commit() @@ -718,6 +720,7 @@ class MyAccountView(BaseAppView, DataGri def _get_pull_requests_list(self, statuses): draw, start, limit = self._extract_chunk(self.request) search_q, order_by, order_dir = self._extract_ordering(self.request) + _render = self.request.get_partial_renderer( 'rhodecode:templates/data_table/_dt_elements.mako') @@ -735,7 +738,7 @@ class MyAccountView(BaseAppView, DataGri for pr in pull_requests: repo_id = pr.target_repo_id comments_count = comments_model.get_all_comments( - repo_id, pull_request=pr, count_only=True) + repo_id, pull_request=pr, include_drafts=False, count_only=True) owned = pr.user_id == self._rhodecode_user.user_id data.append({ @@ -751,7 +754,8 @@ class MyAccountView(BaseAppView, DataGri 'title': _render('pullrequest_title', pr.title, pr.description), 'description': h.escape(pr.description), 'updated_on': _render('pullrequest_updated_on', - h.datetime_to_time(pr.updated_on)), + h.datetime_to_time(pr.updated_on), + pr.versions_count), 'updated_on_raw': h.datetime_to_time(pr.updated_on), 'created_on': _render('pullrequest_updated_on', h.datetime_to_time(pr.created_on)), diff --git a/rhodecode/apps/repository/__init__.py b/rhodecode/apps/repository/__init__.py --- a/rhodecode/apps/repository/__init__.py +++ b/rhodecode/apps/repository/__init__.py @@ -355,6 +355,11 @@ def includeme(config): pattern='/{repo_name:.*?[^/]}/pull-request/{pull_request_id:\d+}/todos', repo_route=True) + config.add_route( + name='pullrequest_drafts', + pattern='/{repo_name:.*?[^/]}/pull-request/{pull_request_id:\d+}/drafts', + repo_route=True) + # Artifacts, (EE feature) config.add_route( name='repo_artifacts_list', diff --git a/rhodecode/apps/repository/tests/test_repo_pullrequests.py b/rhodecode/apps/repository/tests/test_repo_pullrequests.py --- a/rhodecode/apps/repository/tests/test_repo_pullrequests.py +++ b/rhodecode/apps/repository/tests/test_repo_pullrequests.py @@ -608,23 +608,23 @@ class TestPullrequestsView(object): pull_request.source_repo, pull_request=pull_request) assert status == ChangesetStatus.STATUS_REJECTED - comment_id = response.json.get('comment_id', None) - test_text = 'test' - response = self.app.post( - route_path( - 'pullrequest_comment_edit', - repo_name=target_scm_name, - pull_request_id=pull_request_id, - comment_id=comment_id, - ), - extra_environ=xhr_header, - params={ - 'csrf_token': csrf_token, - 'text': test_text, - }, - status=403, - ) - assert response.status_int == 403 + for comment_id in response.json.keys(): + test_text = 'test' + response = self.app.post( + route_path( + 'pullrequest_comment_edit', + repo_name=target_scm_name, + pull_request_id=pull_request_id, + comment_id=comment_id, + ), + extra_environ=xhr_header, + params={ + 'csrf_token': csrf_token, + 'text': test_text, + }, + status=403, + ) + assert response.status_int == 403 def test_comment_and_comment_edit(self, pr_util, csrf_token, xhr_header): pull_request = pr_util.create_pull_request() @@ -644,27 +644,27 @@ class TestPullrequestsView(object): ) assert response.json - comment_id = response.json.get('comment_id', None) - assert comment_id - test_text = 'test' - self.app.post( - route_path( - 'pullrequest_comment_edit', - repo_name=target_scm_name, - pull_request_id=pull_request.pull_request_id, - comment_id=comment_id, - ), - extra_environ=xhr_header, - params={ - 'csrf_token': csrf_token, - 'text': test_text, - 'version': '0', - }, + for comment_id in response.json.keys(): + assert comment_id + test_text = 'test' + self.app.post( + route_path( + 'pullrequest_comment_edit', + repo_name=target_scm_name, + pull_request_id=pull_request.pull_request_id, + comment_id=comment_id, + ), + extra_environ=xhr_header, + params={ + 'csrf_token': csrf_token, + 'text': test_text, + 'version': '0', + }, - ) - text_form_db = ChangesetComment.query().filter( - ChangesetComment.comment_id == comment_id).first().text - assert test_text == text_form_db + ) + text_form_db = ChangesetComment.query().filter( + ChangesetComment.comment_id == comment_id).first().text + assert test_text == text_form_db def test_comment_and_comment_edit(self, pr_util, csrf_token, xhr_header): pull_request = pr_util.create_pull_request() @@ -684,26 +684,25 @@ class TestPullrequestsView(object): ) assert response.json - comment_id = response.json.get('comment_id', None) - assert comment_id - test_text = 'init' - response = self.app.post( - route_path( - 'pullrequest_comment_edit', - repo_name=target_scm_name, - pull_request_id=pull_request.pull_request_id, - comment_id=comment_id, - ), - extra_environ=xhr_header, - params={ - 'csrf_token': csrf_token, - 'text': test_text, - 'version': '0', - }, - status=404, + for comment_id in response.json.keys(): + test_text = 'init' + response = self.app.post( + route_path( + 'pullrequest_comment_edit', + repo_name=target_scm_name, + pull_request_id=pull_request.pull_request_id, + comment_id=comment_id, + ), + extra_environ=xhr_header, + params={ + 'csrf_token': csrf_token, + 'text': test_text, + 'version': '0', + }, + status=404, - ) - assert response.status_int == 404 + ) + assert response.status_int == 404 def test_comment_and_try_edit_already_edited(self, pr_util, csrf_token, xhr_header): pull_request = pr_util.create_pull_request() @@ -722,48 +721,46 @@ class TestPullrequestsView(object): extra_environ=xhr_header, ) assert response.json - comment_id = response.json.get('comment_id', None) - assert comment_id - - test_text = 'test' - self.app.post( - route_path( - 'pullrequest_comment_edit', - repo_name=target_scm_name, - pull_request_id=pull_request.pull_request_id, - comment_id=comment_id, - ), - extra_environ=xhr_header, - params={ - 'csrf_token': csrf_token, - 'text': test_text, - 'version': '0', - }, + for comment_id in response.json.keys(): + test_text = 'test' + self.app.post( + route_path( + 'pullrequest_comment_edit', + repo_name=target_scm_name, + pull_request_id=pull_request.pull_request_id, + comment_id=comment_id, + ), + extra_environ=xhr_header, + params={ + 'csrf_token': csrf_token, + 'text': test_text, + 'version': '0', + }, - ) - test_text_v2 = 'test_v2' - response = self.app.post( - route_path( - 'pullrequest_comment_edit', - repo_name=target_scm_name, - pull_request_id=pull_request.pull_request_id, - comment_id=comment_id, - ), - extra_environ=xhr_header, - params={ - 'csrf_token': csrf_token, - 'text': test_text_v2, - 'version': '0', - }, - status=409, - ) - assert response.status_int == 409 + ) + test_text_v2 = 'test_v2' + response = self.app.post( + route_path( + 'pullrequest_comment_edit', + repo_name=target_scm_name, + pull_request_id=pull_request.pull_request_id, + comment_id=comment_id, + ), + extra_environ=xhr_header, + params={ + 'csrf_token': csrf_token, + 'text': test_text_v2, + 'version': '0', + }, + status=409, + ) + assert response.status_int == 409 - text_form_db = ChangesetComment.query().filter( - ChangesetComment.comment_id == comment_id).first().text + text_form_db = ChangesetComment.query().filter( + ChangesetComment.comment_id == comment_id).first().text - assert test_text == text_form_db - assert test_text_v2 != text_form_db + assert test_text == text_form_db + assert test_text_v2 != text_form_db def test_comment_and_comment_edit_permissions_forbidden( self, autologin_regular_user, user_regular, user_admin, pr_util, 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 @@ -24,7 +24,8 @@ from rhodecode.model.pull_request import from rhodecode.model.db import PullRequestReviewers # V3 - Reviewers, with default rules data # v4 - Added observers metadata -REVIEWER_API_VERSION = 'V4' +# v5 - pr_author/commit_author include/exclude logic +REVIEWER_API_VERSION = 'V5' def reviewer_as_json(user, reasons=None, role=None, mandatory=False, rules=None, user_group=None): @@ -88,6 +89,7 @@ def get_default_reviewers_data(current_u 'reviewers': json_reviewers, 'rules': {}, 'rules_data': {}, + 'rules_humanized': [], } diff --git a/rhodecode/apps/repository/views/repo_commits.py b/rhodecode/apps/repository/views/repo_commits.py --- a/rhodecode/apps/repository/views/repo_commits.py +++ b/rhodecode/apps/repository/views/repo_commits.py @@ -77,6 +77,7 @@ class RepoCommitsView(RepoAppView): _ = self.request.translate c = self.load_default_context() c.fulldiff = self.request.GET.get('fulldiff') + redirect_to_combined = str2bool(self.request.GET.get('redirect_combined')) # fetch global flags of ignore ws or context lines diff_context = get_diff_context(self.request) @@ -117,6 +118,19 @@ class RepoCommitsView(RepoAppView): raise HTTPNotFound() single_commit = len(c.commit_ranges) == 1 + if redirect_to_combined and not single_commit: + source_ref = getattr(c.commit_ranges[0].parents[0] + if c.commit_ranges[0].parents else h.EmptyCommit(), 'raw_id') + target_ref = c.commit_ranges[-1].raw_id + next_url = h.route_path( + 'repo_compare', + repo_name=c.repo_name, + source_ref_type='rev', + source_ref=source_ref, + target_ref_type='rev', + target_ref=target_ref) + raise HTTPFound(next_url) + c.changes = OrderedDict() c.lines_added = 0 c.lines_deleted = 0 @@ -366,6 +380,121 @@ class RepoCommitsView(RepoAppView): commit_id = self.request.matchdict['commit_id'] return self._commit(commit_id, method='download') + def _commit_comments_create(self, commit_id, comments): + _ = self.request.translate + data = {} + if not comments: + return + + commit = self.db_repo.get_commit(commit_id) + + all_drafts = len([x for x in comments if str2bool(x['is_draft'])]) == len(comments) + for entry in comments: + c = self.load_default_context() + comment_type = entry['comment_type'] + text = entry['text'] + status = entry['status'] + is_draft = str2bool(entry['is_draft']) + resolves_comment_id = entry['resolves_comment_id'] + f_path = entry['f_path'] + line_no = entry['line'] + target_elem_id = 'file-{}'.format(h.safeid(h.safe_unicode(f_path))) + + if status: + text = text or (_('Status change %(transition_icon)s %(status)s') + % {'transition_icon': '>', + 'status': ChangesetStatus.get_status_lbl(status)}) + + comment = CommentsModel().create( + text=text, + repo=self.db_repo.repo_id, + user=self._rhodecode_db_user.user_id, + commit_id=commit_id, + f_path=f_path, + line_no=line_no, + status_change=(ChangesetStatus.get_status_lbl(status) + if status else None), + status_change_type=status, + comment_type=comment_type, + is_draft=is_draft, + resolves_comment_id=resolves_comment_id, + auth_user=self._rhodecode_user, + send_email=not is_draft, # skip notification for draft comments + ) + is_inline = comment.is_inline + + # get status if set ! + if status: + # `dont_allow_on_closed_pull_request = True` means + # if latest status was from pull request and it's closed + # disallow changing status ! + + try: + ChangesetStatusModel().set_status( + self.db_repo.repo_id, + status, + self._rhodecode_db_user.user_id, + comment, + revision=commit_id, + dont_allow_on_closed_pull_request=True + ) + except StatusChangeOnClosedPullRequestError: + msg = _('Changing the status of a commit associated with ' + 'a closed pull request is not allowed') + log.exception(msg) + h.flash(msg, category='warning') + raise HTTPFound(h.route_path( + 'repo_commit', repo_name=self.db_repo_name, + commit_id=commit_id)) + + Session().flush() + # this is somehow required to get access to some relationship + # loaded on comment + Session().refresh(comment) + + # skip notifications for drafts + if not is_draft: + CommentsModel().trigger_commit_comment_hook( + self.db_repo, self._rhodecode_user, 'create', + data={'comment': comment, 'commit': commit}) + + comment_id = comment.comment_id + data[comment_id] = { + 'target_id': target_elem_id + } + Session().flush() + + c.co = comment + c.at_version_num = 0 + c.is_new = True + rendered_comment = render( + 'rhodecode:templates/changeset/changeset_comment_block.mako', + self._get_template_context(c), self.request) + + data[comment_id].update(comment.get_dict()) + data[comment_id].update({'rendered_text': rendered_comment}) + + # finalize, commit and redirect + Session().commit() + + # skip channelstream for draft comments + if not all_drafts: + comment_broadcast_channel = channelstream.comment_channel( + self.db_repo_name, commit_obj=commit) + + comment_data = data + posted_comment_type = 'inline' if is_inline else 'general' + if len(data) == 1: + msg = _('posted {} new {} comment').format(len(data), posted_comment_type) + else: + msg = _('posted {} new {} comments').format(len(data), posted_comment_type) + + channelstream.comment_channelstream_push( + self.request, comment_broadcast_channel, self._rhodecode_user, msg, + comment_data=comment_data) + + return data + @LoginRequired() @NotAnonymous() @HasRepoPermissionAnyDecorator( @@ -378,17 +507,6 @@ class RepoCommitsView(RepoAppView): _ = self.request.translate commit_id = self.request.matchdict['commit_id'] - c = self.load_default_context() - status = self.request.POST.get('changeset_status', None) - text = self.request.POST.get('text') - comment_type = self.request.POST.get('comment_type') - resolves_comment_id = self.request.POST.get('resolves_comment_id', None) - - if status: - text = text or (_('Status change %(transition_icon)s %(status)s') - % {'transition_icon': '>', - 'status': ChangesetStatus.get_status_lbl(status)}) - multi_commit_ids = [] for _commit_id in self.request.POST.get('commit_ids', '').split(','): if _commit_id not in ['', None, EmptyCommit.raw_id]: @@ -397,81 +515,23 @@ class RepoCommitsView(RepoAppView): commit_ids = multi_commit_ids or [commit_id] - comment = None + data = [] + # Multiple comments for each passed commit id for current_id in filter(None, commit_ids): - comment = CommentsModel().create( - text=text, - repo=self.db_repo.repo_id, - user=self._rhodecode_db_user.user_id, - commit_id=current_id, - f_path=self.request.POST.get('f_path'), - line_no=self.request.POST.get('line'), - status_change=(ChangesetStatus.get_status_lbl(status) - if status else None), - status_change_type=status, - comment_type=comment_type, - resolves_comment_id=resolves_comment_id, - auth_user=self._rhodecode_user - ) - is_inline = comment.is_inline - - # get status if set ! - if status: - # if latest status was from pull request and it's closed - # disallow changing status ! - # dont_allow_on_closed_pull_request = True ! + comment_data = { + 'comment_type': self.request.POST.get('comment_type'), + 'text': self.request.POST.get('text'), + 'status': self.request.POST.get('changeset_status', None), + 'is_draft': self.request.POST.get('draft'), + 'resolves_comment_id': self.request.POST.get('resolves_comment_id', None), + 'close_pull_request': self.request.POST.get('close_pull_request'), + 'f_path': self.request.POST.get('f_path'), + 'line': self.request.POST.get('line'), + } + comment = self._commit_comments_create(commit_id=current_id, comments=[comment_data]) + data.append(comment) - try: - ChangesetStatusModel().set_status( - self.db_repo.repo_id, - status, - self._rhodecode_db_user.user_id, - comment, - revision=current_id, - dont_allow_on_closed_pull_request=True - ) - except StatusChangeOnClosedPullRequestError: - msg = _('Changing the status of a commit associated with ' - 'a closed pull request is not allowed') - log.exception(msg) - h.flash(msg, category='warning') - raise HTTPFound(h.route_path( - 'repo_commit', repo_name=self.db_repo_name, - commit_id=current_id)) - - commit = self.db_repo.get_commit(current_id) - CommentsModel().trigger_commit_comment_hook( - self.db_repo, self._rhodecode_user, 'create', - data={'comment': comment, 'commit': commit}) - - # finalize, commit and redirect - Session().commit() - - data = { - 'target_id': h.safeid(h.safe_unicode( - self.request.POST.get('f_path'))), - } - if comment: - c.co = comment - c.at_version_num = 0 - rendered_comment = render( - 'rhodecode:templates/changeset/changeset_comment_block.mako', - self._get_template_context(c), self.request) - - data.update(comment.get_dict()) - data.update({'rendered_text': rendered_comment}) - - comment_broadcast_channel = channelstream.comment_channel( - self.db_repo_name, commit_obj=commit) - - comment_data = data - comment_type = 'inline' if is_inline else 'general' - channelstream.comment_channelstream_push( - self.request, comment_broadcast_channel, self._rhodecode_user, - _('posted a new {} comment').format(comment_type), - comment_data=comment_data) - - return data + return data if len(data) > 1 else data[0] @LoginRequired() @NotAnonymous() @@ -665,6 +725,7 @@ class RepoCommitsView(RepoAppView): def repo_commit_comment_edit(self): self.load_default_context() + commit_id = self.request.matchdict['commit_id'] comment_id = self.request.matchdict['comment_id'] comment = ChangesetComment.get_or_404(comment_id) @@ -717,11 +778,11 @@ class RepoCommitsView(RepoAppView): if not comment_history: raise HTTPNotFound() - commit_id = self.request.matchdict['commit_id'] - commit = self.db_repo.get_commit(commit_id) - CommentsModel().trigger_commit_comment_hook( - self.db_repo, self._rhodecode_user, 'edit', - data={'comment': comment, 'commit': commit}) + if not comment.draft: + commit = self.db_repo.get_commit(commit_id) + CommentsModel().trigger_commit_comment_hook( + self.db_repo, self._rhodecode_user, 'edit', + data={'comment': comment, 'commit': commit}) Session().commit() return { diff --git a/rhodecode/apps/repository/views/repo_files.py b/rhodecode/apps/repository/views/repo_files.py --- a/rhodecode/apps/repository/views/repo_files.py +++ b/rhodecode/apps/repository/views/repo_files.py @@ -325,6 +325,21 @@ class RepoFilesView(RepoAppView): return lf_enabled + def _get_archive_name(self, db_repo_name, commit_sha, ext, subrepos=False, path_sha=''): + # original backward compat name of archive + clean_name = safe_str(db_repo_name.replace('/', '_')) + + # e.g vcsserver.zip + # e.g vcsserver-abcdefgh.zip + # e.g vcsserver-abcdefgh-defghijk.zip + archive_name = '{}{}{}{}{}'.format( + clean_name, + '-sub' if subrepos else '', + commit_sha, + '-{}'.format(path_sha) if path_sha else '', + ext) + return archive_name + @LoginRequired() @HasRepoPermissionAnyDecorator( 'repository.read', 'repository.write', 'repository.admin') @@ -339,6 +354,7 @@ class RepoFilesView(RepoAppView): default_at_path = '/' fname = self.request.matchdict['fname'] subrepos = self.request.GET.get('subrepos') == 'true' + with_hash = str2bool(self.request.GET.get('with_hash', '1')) at_path = self.request.GET.get('at_path') or default_at_path if not self.db_repo.enable_downloads: @@ -364,30 +380,30 @@ class RepoFilesView(RepoAppView): except Exception: return Response(_('No node at path {} for this repository').format(at_path)) - path_sha = sha1(at_path)[:8] - - # original backward compat name of archive - clean_name = safe_str(self.db_repo_name.replace('/', '_')) - short_sha = safe_str(commit.short_id) + # path sha is part of subdir + path_sha = '' + if at_path != default_at_path: + path_sha = sha1(at_path)[:8] + short_sha = '-{}'.format(safe_str(commit.short_id)) + # used for cache etc + archive_name = self._get_archive_name( + self.db_repo_name, commit_sha=short_sha, ext=ext, subrepos=subrepos, + path_sha=path_sha) - if at_path == default_at_path: - archive_name = '{}-{}{}{}'.format( - clean_name, - '-sub' if subrepos else '', - short_sha, - ext) - # custom path and new name - else: - archive_name = '{}-{}{}-{}{}'.format( - clean_name, - '-sub' if subrepos else '', - short_sha, - path_sha, - ext) + if not with_hash: + short_sha = '' + path_sha = '' + + # what end client gets served + response_archive_name = self._get_archive_name( + self.db_repo_name, commit_sha=short_sha, ext=ext, subrepos=subrepos, + path_sha=path_sha) + # remove extension from our archive directory name + archive_dir_name = response_archive_name[:-len(ext)] use_cached_archive = False - archive_cache_enabled = CONFIG.get( - 'archive_cache_dir') and not self.request.GET.get('no_cache') + archive_cache_dir = CONFIG.get('archive_cache_dir') + archive_cache_enabled = archive_cache_dir and not self.request.GET.get('no_cache') cached_archive_path = None if archive_cache_enabled: @@ -403,12 +419,14 @@ class RepoFilesView(RepoAppView): else: log.debug('Archive %s is not yet cached', archive_name) + # generate new archive, as previous was not found in the cache if not use_cached_archive: - # generate new archive - fd, archive = tempfile.mkstemp() + _dir = os.path.abspath(archive_cache_dir) if archive_cache_dir else None + fd, archive = tempfile.mkstemp(dir=_dir) log.debug('Creating new temp archive in %s', archive) try: - commit.archive_repo(archive, kind=fileformat, subrepos=subrepos, + commit.archive_repo(archive, archive_dir_name=archive_dir_name, + kind=fileformat, subrepos=subrepos, archive_at_path=at_path) except ImproperArchiveTypeError: return _('Unknown archive type') @@ -445,8 +463,7 @@ class RepoFilesView(RepoAppView): yield data response = Response(app_iter=get_chunked_archive(archive)) - response.content_disposition = str( - 'attachment; filename=%s' % archive_name) + response.content_disposition = str('attachment; filename=%s' % response_archive_name) response.content_type = str(content_type) return response 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 @@ -47,7 +47,7 @@ from rhodecode.lib.vcs.exceptions import from rhodecode.model.changeset_status import ChangesetStatusModel from rhodecode.model.comment import CommentsModel from rhodecode.model.db import ( - func, or_, PullRequest, ChangesetComment, ChangesetStatus, Repository, + func, false, or_, PullRequest, ChangesetComment, ChangesetStatus, Repository, PullRequestReviewers) from rhodecode.model.forms import PullRequestForm from rhodecode.model.meta import Session @@ -107,7 +107,8 @@ class RepoPullRequestsView(RepoAppView, comments_model = CommentsModel() for pr in pull_requests: comments_count = comments_model.get_all_comments( - self.db_repo.repo_id, pull_request=pr, count_only=True) + self.db_repo.repo_id, pull_request=pr, + include_drafts=False, count_only=True) data.append({ 'name': _render('pullrequest_name', @@ -120,7 +121,8 @@ class RepoPullRequestsView(RepoAppView, 'title': _render('pullrequest_title', pr.title, pr.description), 'description': h.escape(pr.description), 'updated_on': _render('pullrequest_updated_on', - h.datetime_to_time(pr.updated_on)), + h.datetime_to_time(pr.updated_on), + pr.versions_count), 'updated_on_raw': h.datetime_to_time(pr.updated_on), 'created_on': _render('pullrequest_updated_on', h.datetime_to_time(pr.created_on)), @@ -268,12 +270,14 @@ class RepoPullRequestsView(RepoAppView, return diffset - def register_comments_vars(self, c, pull_request, versions): + def register_comments_vars(self, c, pull_request, versions, include_drafts=True): comments_model = CommentsModel() # GENERAL COMMENTS with versions # q = comments_model._all_general_comments_of_pull_request(pull_request) q = q.order_by(ChangesetComment.comment_id.asc()) + if not include_drafts: + q = q.filter(ChangesetComment.draft == false()) general_comments = q # pick comments we want to render at current version @@ -283,6 +287,8 @@ class RepoPullRequestsView(RepoAppView, # INLINE COMMENTS with versions # q = comments_model._all_inline_comments_of_pull_request(pull_request) q = q.order_by(ChangesetComment.comment_id.asc()) + if not include_drafts: + q = q.filter(ChangesetComment.draft == false()) inline_comments = q c.inline_versions = comments_model.aggregate_comments( @@ -422,16 +428,12 @@ class RepoPullRequestsView(RepoAppView, c.allowed_to_close = c.allowed_to_merge and not pr_closed c.forbid_adding_reviewers = False - c.forbid_author_to_review = False - c.forbid_commit_author_to_review = False if pull_request_latest.reviewer_data and \ 'rules' in pull_request_latest.reviewer_data: rules = pull_request_latest.reviewer_data['rules'] or {} try: c.forbid_adding_reviewers = rules.get('forbid_adding_reviewers') - c.forbid_author_to_review = rules.get('forbid_author_to_review') - c.forbid_commit_author_to_review = rules.get('forbid_commit_author_to_review') except Exception: pass @@ -499,6 +501,11 @@ class RepoPullRequestsView(RepoAppView, c.resolved_comments = CommentsModel() \ .get_pull_request_resolved_todos(pull_request_latest) + # Drafts + c.draft_comments = CommentsModel().get_pull_request_drafts( + self._rhodecode_db_user.user_id, + pull_request_latest) + # if we use version, then do not show later comments # than current version display_inline_comments = collections.defaultdict( @@ -979,8 +986,9 @@ class RepoPullRequestsView(RepoAppView, } return data - def _get_existing_ids(self, post_data): - return filter(lambda e: e, map(safe_int, aslist(post_data.get('comments'), ','))) + @classmethod + def get_comment_ids(cls, post_data): + return filter(lambda e: e > 0, map(safe_int, aslist(post_data.get('comments'), ','))) @LoginRequired() @NotAnonymous() @@ -1015,10 +1023,10 @@ class RepoPullRequestsView(RepoAppView, if at_version and at_version != PullRequest.LATEST_VER else None) - self.register_comments_vars(c, pull_request_latest, versions) + self.register_comments_vars(c, pull_request_latest, versions, include_drafts=False) all_comments = c.inline_comments_flat + c.comments - existing_ids = self._get_existing_ids(self.request.POST) + existing_ids = self.get_comment_ids(self.request.POST) return _render('comments_table', all_comments, len(all_comments), existing_ids=existing_ids) @@ -1055,12 +1063,12 @@ class RepoPullRequestsView(RepoAppView, else None) c.unresolved_comments = CommentsModel() \ - .get_pull_request_unresolved_todos(pull_request) + .get_pull_request_unresolved_todos(pull_request, include_drafts=False) c.resolved_comments = CommentsModel() \ - .get_pull_request_resolved_todos(pull_request) + .get_pull_request_resolved_todos(pull_request, include_drafts=False) all_comments = c.unresolved_comments + c.resolved_comments - existing_ids = self._get_existing_ids(self.request.POST) + existing_ids = self.get_comment_ids(self.request.POST) return _render('comments_table', all_comments, len(c.unresolved_comments), todo_comments=True, existing_ids=existing_ids) @@ -1068,6 +1076,48 @@ class RepoPullRequestsView(RepoAppView, @NotAnonymous() @HasRepoPermissionAnyDecorator( 'repository.read', 'repository.write', 'repository.admin') + @view_config( + route_name='pullrequest_drafts', request_method='POST', + renderer='string_html', xhr=True) + def pullrequest_drafts(self): + self.load_default_context() + + pull_request = PullRequest.get_or_404( + self.request.matchdict['pull_request_id']) + pull_request_id = pull_request.pull_request_id + version = self.request.GET.get('version') + + _render = self.request.get_partial_renderer( + 'rhodecode:templates/base/sidebar.mako') + c = _render.get_call_context() + + (pull_request_latest, + pull_request_at_ver, + pull_request_display_obj, + at_version) = PullRequestModel().get_pr_version( + pull_request_id, version=version) + versions = pull_request_display_obj.versions() + latest_ver = PullRequest.get_pr_display_object(pull_request_latest, pull_request_latest) + c.versions = versions + [latest_ver] + + c.at_version = at_version + c.at_version_num = (at_version + if at_version and at_version != PullRequest.LATEST_VER + else None) + + c.draft_comments = CommentsModel() \ + .get_pull_request_drafts(self._rhodecode_db_user.user_id, pull_request) + + all_comments = c.draft_comments + + existing_ids = self.get_comment_ids(self.request.POST) + return _render('comments_table', all_comments, len(all_comments), + existing_ids=existing_ids, draft_comments=True) + + @LoginRequired() + @NotAnonymous() + @HasRepoPermissionAnyDecorator( + 'repository.read', 'repository.write', 'repository.admin') @CSRFRequired() @view_config( route_name='pullrequest_create', request_method='POST', @@ -1514,6 +1564,152 @@ class RepoPullRequestsView(RepoAppView, self._rhodecode_user) raise HTTPNotFound() + def _pull_request_comments_create(self, pull_request, comments): + _ = self.request.translate + data = {} + if not comments: + return + pull_request_id = pull_request.pull_request_id + + all_drafts = len([x for x in comments if str2bool(x['is_draft'])]) == len(comments) + + for entry in comments: + c = self.load_default_context() + comment_type = entry['comment_type'] + text = entry['text'] + status = entry['status'] + is_draft = str2bool(entry['is_draft']) + resolves_comment_id = entry['resolves_comment_id'] + close_pull_request = entry['close_pull_request'] + f_path = entry['f_path'] + line_no = entry['line'] + target_elem_id = 'file-{}'.format(h.safeid(h.safe_unicode(f_path))) + + # the logic here should work like following, if we submit close + # pr comment, use `close_pull_request_with_comment` function + # else handle regular comment logic + + if close_pull_request: + # only owner or admin or person with write permissions + allowed_to_close = PullRequestModel().check_user_update( + pull_request, self._rhodecode_user) + if not allowed_to_close: + log.debug('comment: forbidden because not allowed to close ' + 'pull request %s', pull_request_id) + raise HTTPForbidden() + + # This also triggers `review_status_change` + comment, status = PullRequestModel().close_pull_request_with_comment( + pull_request, self._rhodecode_user, self.db_repo, message=text, + auth_user=self._rhodecode_user) + Session().flush() + is_inline = comment.is_inline + + PullRequestModel().trigger_pull_request_hook( + pull_request, self._rhodecode_user, 'comment', + data={'comment': comment}) + + else: + # regular comment case, could be inline, or one with status. + # for that one we check also permissions + # Additionally ENSURE if somehow draft is sent we're then unable to change status + allowed_to_change_status = PullRequestModel().check_user_change_status( + pull_request, self._rhodecode_user) and not is_draft + + if status and allowed_to_change_status: + message = (_('Status change %(transition_icon)s %(status)s') + % {'transition_icon': '>', + 'status': ChangesetStatus.get_status_lbl(status)}) + text = text or message + + comment = CommentsModel().create( + text=text, + repo=self.db_repo.repo_id, + user=self._rhodecode_user.user_id, + pull_request=pull_request, + f_path=f_path, + line_no=line_no, + status_change=(ChangesetStatus.get_status_lbl(status) + if status and allowed_to_change_status else None), + status_change_type=(status + if status and allowed_to_change_status else None), + comment_type=comment_type, + is_draft=is_draft, + resolves_comment_id=resolves_comment_id, + auth_user=self._rhodecode_user, + send_email=not is_draft, # skip notification for draft comments + ) + is_inline = comment.is_inline + + if allowed_to_change_status: + # calculate old status before we change it + old_calculated_status = pull_request.calculated_review_status() + + # get status if set ! + if status: + ChangesetStatusModel().set_status( + self.db_repo.repo_id, + status, + self._rhodecode_user.user_id, + comment, + pull_request=pull_request + ) + + Session().flush() + # this is somehow required to get access to some relationship + # loaded on comment + Session().refresh(comment) + + # skip notifications for drafts + if not is_draft: + PullRequestModel().trigger_pull_request_hook( + pull_request, self._rhodecode_user, 'comment', + data={'comment': comment}) + + # we now calculate the status of pull request, and based on that + # calculation we set the commits status + calculated_status = pull_request.calculated_review_status() + if old_calculated_status != calculated_status: + PullRequestModel().trigger_pull_request_hook( + pull_request, self._rhodecode_user, 'review_status_change', + data={'status': calculated_status}) + + comment_id = comment.comment_id + data[comment_id] = { + 'target_id': target_elem_id + } + Session().flush() + + c.co = comment + c.at_version_num = None + c.is_new = True + rendered_comment = render( + 'rhodecode:templates/changeset/changeset_comment_block.mako', + self._get_template_context(c), self.request) + + data[comment_id].update(comment.get_dict()) + data[comment_id].update({'rendered_text': rendered_comment}) + + Session().commit() + + # skip channelstream for draft comments + if not all_drafts: + comment_broadcast_channel = channelstream.comment_channel( + self.db_repo_name, pull_request_obj=pull_request) + + comment_data = data + posted_comment_type = 'inline' if is_inline else 'general' + if len(data) == 1: + msg = _('posted {} new {} comment').format(len(data), posted_comment_type) + else: + msg = _('posted {} new {} comments').format(len(data), posted_comment_type) + + channelstream.comment_channelstream_push( + self.request, comment_broadcast_channel, self._rhodecode_user, msg, + comment_data=comment_data) + + return data + @LoginRequired() @NotAnonymous() @HasRepoPermissionAnyDecorator( @@ -1525,9 +1721,7 @@ class RepoPullRequestsView(RepoAppView, def pull_request_comment_create(self): _ = self.request.translate - pull_request = PullRequest.get_or_404( - self.request.matchdict['pull_request_id']) - pull_request_id = pull_request.pull_request_id + pull_request = PullRequest.get_or_404(self.request.matchdict['pull_request_id']) if pull_request.is_closed(): log.debug('comment: forbidden because pull request is closed') @@ -1539,124 +1733,17 @@ class RepoPullRequestsView(RepoAppView, log.debug('comment: forbidden because pull request is from forbidden repo') raise HTTPForbidden() - c = self.load_default_context() - - status = self.request.POST.get('changeset_status', None) - text = self.request.POST.get('text') - comment_type = self.request.POST.get('comment_type') - resolves_comment_id = self.request.POST.get('resolves_comment_id', None) - close_pull_request = self.request.POST.get('close_pull_request') - - # the logic here should work like following, if we submit close - # pr comment, use `close_pull_request_with_comment` function - # else handle regular comment logic - - if close_pull_request: - # only owner or admin or person with write permissions - allowed_to_close = PullRequestModel().check_user_update( - pull_request, self._rhodecode_user) - if not allowed_to_close: - log.debug('comment: forbidden because not allowed to close ' - 'pull request %s', pull_request_id) - raise HTTPForbidden() - - # This also triggers `review_status_change` - comment, status = PullRequestModel().close_pull_request_with_comment( - pull_request, self._rhodecode_user, self.db_repo, message=text, - auth_user=self._rhodecode_user) - Session().flush() - is_inline = comment.is_inline - - PullRequestModel().trigger_pull_request_hook( - pull_request, self._rhodecode_user, 'comment', - data={'comment': comment}) - - else: - # regular comment case, could be inline, or one with status. - # for that one we check also permissions - - allowed_to_change_status = PullRequestModel().check_user_change_status( - pull_request, self._rhodecode_user) - - if status and allowed_to_change_status: - message = (_('Status change %(transition_icon)s %(status)s') - % {'transition_icon': '>', - 'status': ChangesetStatus.get_status_lbl(status)}) - text = text or message - - comment = CommentsModel().create( - text=text, - repo=self.db_repo.repo_id, - user=self._rhodecode_user.user_id, - pull_request=pull_request, - f_path=self.request.POST.get('f_path'), - line_no=self.request.POST.get('line'), - status_change=(ChangesetStatus.get_status_lbl(status) - if status and allowed_to_change_status else None), - status_change_type=(status - if status and allowed_to_change_status else None), - comment_type=comment_type, - resolves_comment_id=resolves_comment_id, - auth_user=self._rhodecode_user - ) - is_inline = comment.is_inline - - if allowed_to_change_status: - # calculate old status before we change it - old_calculated_status = pull_request.calculated_review_status() - - # get status if set ! - if status: - ChangesetStatusModel().set_status( - self.db_repo.repo_id, - status, - self._rhodecode_user.user_id, - comment, - pull_request=pull_request - ) - - Session().flush() - # this is somehow required to get access to some relationship - # loaded on comment - Session().refresh(comment) - - PullRequestModel().trigger_pull_request_hook( - pull_request, self._rhodecode_user, 'comment', - data={'comment': comment}) - - # we now calculate the status of pull request, and based on that - # calculation we set the commits status - calculated_status = pull_request.calculated_review_status() - if old_calculated_status != calculated_status: - PullRequestModel().trigger_pull_request_hook( - pull_request, self._rhodecode_user, 'review_status_change', - data={'status': calculated_status}) - - Session().commit() - - data = { - 'target_id': h.safeid(h.safe_unicode( - self.request.POST.get('f_path'))), + comment_data = { + 'comment_type': self.request.POST.get('comment_type'), + 'text': self.request.POST.get('text'), + 'status': self.request.POST.get('changeset_status', None), + 'is_draft': self.request.POST.get('draft'), + 'resolves_comment_id': self.request.POST.get('resolves_comment_id', None), + 'close_pull_request': self.request.POST.get('close_pull_request'), + 'f_path': self.request.POST.get('f_path'), + 'line': self.request.POST.get('line'), } - if comment: - c.co = comment - c.at_version_num = None - rendered_comment = render( - 'rhodecode:templates/changeset/changeset_comment_block.mako', - self._get_template_context(c), self.request) - - data.update(comment.get_dict()) - data.update({'rendered_text': rendered_comment}) - - comment_broadcast_channel = channelstream.comment_channel( - self.db_repo_name, pull_request_obj=pull_request) - - comment_data = data - comment_type = 'inline' if is_inline else 'general' - channelstream.comment_channelstream_push( - self.request, comment_broadcast_channel, self._rhodecode_user, - _('posted a new {} comment').format(comment_type), - comment_data=comment_data) + data = self._pull_request_comments_create(pull_request, [comment_data]) return data @@ -1741,11 +1828,6 @@ class RepoPullRequestsView(RepoAppView, log.debug('comment: forbidden because pull request is closed') raise HTTPForbidden() - if not comment: - log.debug('Comment with id:%s not found, skipping', comment_id) - # comment already deleted in another call probably - return True - if comment.pull_request.is_closed(): # don't allow deleting comments on closed pull request raise HTTPForbidden() @@ -1796,10 +1878,10 @@ class RepoPullRequestsView(RepoAppView, raise HTTPNotFound() Session().commit() - - PullRequestModel().trigger_pull_request_hook( - pull_request, self._rhodecode_user, 'comment_edit', - data={'comment': comment}) + if not comment.draft: + PullRequestModel().trigger_pull_request_hook( + pull_request, self._rhodecode_user, 'comment_edit', + data={'comment': comment}) return { 'comment_history_id': comment_history.comment_history_id, diff --git a/rhodecode/authentication/base.py b/rhodecode/authentication/base.py --- a/rhodecode/authentication/base.py +++ b/rhodecode/authentication/base.py @@ -215,9 +215,10 @@ class RhodeCodeAuthPluginBase(object): """ return self._plugin_id - def get_display_name(self): + def get_display_name(self, load_from_settings=False): """ Returns a translation string for displaying purposes. + if load_from_settings is set, plugin settings can override the display name """ raise NotImplementedError('Not implemented in base class') diff --git a/rhodecode/authentication/plugins/auth_crowd.py b/rhodecode/authentication/plugins/auth_crowd.py --- a/rhodecode/authentication/plugins/auth_crowd.py +++ b/rhodecode/authentication/plugins/auth_crowd.py @@ -213,7 +213,7 @@ class RhodeCodeAuthPlugin(RhodeCodeExter def get_settings_schema(self): return CrowdSettingsSchema() - def get_display_name(self): + def get_display_name(self, load_from_settings=False): return _('CROWD') @classmethod diff --git a/rhodecode/authentication/plugins/auth_headers.py b/rhodecode/authentication/plugins/auth_headers.py --- a/rhodecode/authentication/plugins/auth_headers.py +++ b/rhodecode/authentication/plugins/auth_headers.py @@ -95,7 +95,7 @@ class RhodeCodeAuthPlugin(RhodeCodeExter route_name='auth_home', context=HeadersAuthnResource) - def get_display_name(self): + def get_display_name(self, load_from_settings=False): return _('Headers') def get_settings_schema(self): diff --git a/rhodecode/authentication/plugins/auth_jasig_cas.py b/rhodecode/authentication/plugins/auth_jasig_cas.py --- a/rhodecode/authentication/plugins/auth_jasig_cas.py +++ b/rhodecode/authentication/plugins/auth_jasig_cas.py @@ -89,7 +89,7 @@ class RhodeCodeAuthPlugin(RhodeCodeExter def get_settings_schema(self): return JasigCasSettingsSchema() - def get_display_name(self): + def get_display_name(self, load_from_settings=False): return _('Jasig-CAS') @hybrid_property diff --git a/rhodecode/authentication/plugins/auth_ldap.py b/rhodecode/authentication/plugins/auth_ldap.py --- a/rhodecode/authentication/plugins/auth_ldap.py +++ b/rhodecode/authentication/plugins/auth_ldap.py @@ -421,7 +421,7 @@ class RhodeCodeAuthPlugin(RhodeCodeExter def get_settings_schema(self): return LdapSettingsSchema() - def get_display_name(self): + def get_display_name(self, load_from_settings=False): return _('LDAP') @classmethod diff --git a/rhodecode/authentication/plugins/auth_pam.py b/rhodecode/authentication/plugins/auth_pam.py --- a/rhodecode/authentication/plugins/auth_pam.py +++ b/rhodecode/authentication/plugins/auth_pam.py @@ -95,7 +95,7 @@ class RhodeCodeAuthPlugin(RhodeCodeExter route_name='auth_home', context=PamAuthnResource) - def get_display_name(self): + def get_display_name(self, load_from_settings=False): return _('PAM') @classmethod diff --git a/rhodecode/authentication/plugins/auth_rhodecode.py b/rhodecode/authentication/plugins/auth_rhodecode.py --- a/rhodecode/authentication/plugins/auth_rhodecode.py +++ b/rhodecode/authentication/plugins/auth_rhodecode.py @@ -75,7 +75,7 @@ class RhodeCodeAuthPlugin(RhodeCodeAuthP def get_settings_schema(self): return RhodeCodeSettingsSchema() - def get_display_name(self): + def get_display_name(self, load_from_settings=False): return _('RhodeCode Internal') @classmethod diff --git a/rhodecode/authentication/plugins/auth_token.py b/rhodecode/authentication/plugins/auth_token.py --- a/rhodecode/authentication/plugins/auth_token.py +++ b/rhodecode/authentication/plugins/auth_token.py @@ -73,7 +73,7 @@ class RhodeCodeAuthPlugin(RhodeCodeAuthP def get_settings_schema(self): return RhodeCodeSettingsSchema() - def get_display_name(self): + def get_display_name(self, load_from_settings=False): return _('Rhodecode Token') @classmethod diff --git a/rhodecode/config/middleware.py b/rhodecode/config/middleware.py --- a/rhodecode/config/middleware.py +++ b/rhodecode/config/middleware.py @@ -53,7 +53,7 @@ from rhodecode.lib.utils2 import aslist from rhodecode.lib.exc_tracking import store_exception from rhodecode.subscribers import ( scan_repositories_if_enabled, write_js_routes_if_enabled, - write_metadata_if_needed, write_usage_data, inject_app_settings) + write_metadata_if_needed, write_usage_data) log = logging.getLogger(__name__) @@ -310,8 +310,6 @@ def includeme(config): # Add subscribers. if load_all: - config.add_subscriber(inject_app_settings, - pyramid.events.ApplicationCreated) config.add_subscriber(scan_repositories_if_enabled, pyramid.events.ApplicationCreated) config.add_subscriber(write_metadata_if_needed, diff --git a/rhodecode/lib/bleach_whitelist.py b/rhodecode/lib/bleach_whitelist.py --- a/rhodecode/lib/bleach_whitelist.py +++ b/rhodecode/lib/bleach_whitelist.py @@ -67,7 +67,7 @@ markdown_tags = [ markdown_attrs = { "*": ["class", "style", "align"], - "img": ["src", "alt", "title"], + "img": ["src", "alt", "title", "width", "height", "hspace", "align"], "a": ["href", "alt", "title", "name", "data-hovercard-alt", "data-hovercard-url"], "abbr": ["title"], "acronym": ["title"], diff --git a/rhodecode/lib/channelstream.py b/rhodecode/lib/channelstream.py --- a/rhodecode/lib/channelstream.py +++ b/rhodecode/lib/channelstream.py @@ -339,13 +339,12 @@ def comment_channelstream_push(request, comment_data = kwargs.pop('comment_data', {}) user_data = kwargs.pop('user_data', {}) - comment_id = comment_data.get('comment_id') + comment_id = comment_data.keys()[0] if comment_data else '' - message = '{} {} #{}, {}'.format( + message = '{} {} #{}'.format( user.username, msg, comment_id, - _reload_link(_('Reload page to see new comments')), ) message_obj = { diff --git a/rhodecode/lib/dbmigrate/versions/111_version_4_23_0.py b/rhodecode/lib/dbmigrate/versions/111_version_4_23_0.py new file mode 100644 --- /dev/null +++ b/rhodecode/lib/dbmigrate/versions/111_version_4_23_0.py @@ -0,0 +1,53 @@ +# -*- coding: utf-8 -*- + +import logging +from sqlalchemy import * + +from alembic.migration import MigrationContext +from alembic.operations import Operations + +from rhodecode.lib.dbmigrate.versions import _reset_base +from rhodecode.model import meta, 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_20_0_0 as db + + init_model_encryption(db) + + context = MigrationContext.configure(migrate_engine.connect()) + op = Operations(context) + + table = db.ChangesetComment.__table__ + with op.batch_alter_table(table.name) as batch_op: + new_column = Column('draft', Boolean(), nullable=True) + batch_op.add_column(new_column) + + _set_default_as_non_draft(op, meta.Session) + + +def downgrade(migrate_engine): + meta = MetaData() + meta.bind = migrate_engine + + +def fixups(models, _SESSION): + pass + + +def _set_default_as_non_draft(op, session): + params = {'draft': False} + query = text( + 'UPDATE changeset_comments SET draft = :draft' + ).bindparams(**params) + op.execute(query) + session().commit() + diff --git a/rhodecode/lib/dbmigrate/versions/112_version_4_23_0.py b/rhodecode/lib/dbmigrate/versions/112_version_4_23_0.py new file mode 100644 --- /dev/null +++ b/rhodecode/lib/dbmigrate/versions/112_version_4_23_0.py @@ -0,0 +1,78 @@ +# -*- coding: utf-8 -*- + +import logging +from sqlalchemy import * + +from alembic.migration import MigrationContext +from alembic.operations import Operations + +from rhodecode.lib.dbmigrate.versions import _reset_base +from rhodecode.model import meta, 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_20_0_0 as db + + init_model_encryption(db) + + context = MigrationContext.configure(migrate_engine.connect()) + op = Operations(context) + + table = db.RepoReviewRule.__table__ + with op.batch_alter_table(table.name) as batch_op: + + new_column = Column('pr_author', UnicodeText().with_variant(UnicodeText(255), 'mysql'), nullable=True) + batch_op.add_column(new_column) + + new_column = Column('commit_author', UnicodeText().with_variant(UnicodeText(255), 'mysql'), nullable=True) + batch_op.add_column(new_column) + + _migrate_review_flags_to_new_cols(op, meta.Session) + + +def downgrade(migrate_engine): + meta = MetaData() + meta.bind = migrate_engine + + +def fixups(models, _SESSION): + pass + + +def _migrate_review_flags_to_new_cols(op, session): + + # set defaults for pr_author + query = text( + 'UPDATE repo_review_rules SET pr_author = :val' + ).bindparams(val='no_rule') + op.execute(query) + + # set defaults for commit_author + query = text( + 'UPDATE repo_review_rules SET commit_author = :val' + ).bindparams(val='no_rule') + op.execute(query) + + session().commit() + + # now change the flags to forbid based on + # forbid_author_to_review, forbid_commit_author_to_review + query = text( + 'UPDATE repo_review_rules SET pr_author = :val WHERE forbid_author_to_review = TRUE' + ).bindparams(val='forbid_pr_author') + op.execute(query) + + query = text( + 'UPDATE repo_review_rules SET commit_author = :val WHERE forbid_commit_author_to_review = TRUE' + ).bindparams(val='forbid_commit_author') + op.execute(query) + + session().commit() diff --git a/rhodecode/lib/diffs.py b/rhodecode/lib/diffs.py --- a/rhodecode/lib/diffs.py +++ b/rhodecode/lib/diffs.py @@ -1148,7 +1148,7 @@ class DiffLimitExceeded(Exception): # NOTE(marcink): if diffs.mako change, probably this # needs a bump to next version -CURRENT_DIFF_VERSION = 'v4' +CURRENT_DIFF_VERSION = 'v5' def _cleanup_cache_file(cached_diff_file): diff --git a/rhodecode/lib/exc_tracking.py b/rhodecode/lib/exc_tracking.py --- a/rhodecode/lib/exc_tracking.py +++ b/rhodecode/lib/exc_tracking.py @@ -110,7 +110,7 @@ def _store_exception(exc_id, exc_type_na mail_server = app.CONFIG.get('smtp_server') or None send_email = send_email and mail_server - if send_email: + if send_email and request: try: send_exc_email(request, exc_id, exc_type_name) except Exception: diff --git a/rhodecode/lib/markdown_ext.py b/rhodecode/lib/markdown_ext.py --- a/rhodecode/lib/markdown_ext.py +++ b/rhodecode/lib/markdown_ext.py @@ -18,17 +18,85 @@ # RhodeCode Enterprise Edition, including its added features, Support services, # and proprietary license terms, please see https://rhodecode.com/licenses/ +import re import markdown +import xml.etree.ElementTree as etree from markdown.extensions import Extension from markdown.extensions.fenced_code import FencedCodeExtension from markdown.extensions.smart_strong import SmartEmphasisExtension from markdown.extensions.tables import TableExtension -from markdown.extensions.nl2br import Nl2BrExtension +from markdown.inlinepatterns import Pattern import gfm +class InlineProcessor(Pattern): + """ + Base class that inline patterns subclass. + This is the newer style inline processor that uses a more + efficient and flexible search approach. + """ + + def __init__(self, pattern, md=None): + """ + Create an instant of an inline pattern. + Keyword arguments: + * pattern: A regular expression that matches a pattern + """ + self.pattern = pattern + self.compiled_re = re.compile(pattern, re.DOTALL | re.UNICODE) + + # Api for Markdown to pass safe_mode into instance + self.safe_mode = False + self.md = md + + def handleMatch(self, m, data): + """Return a ElementTree element from the given match and the + start and end index of the matched text. + If `start` and/or `end` are returned as `None`, it will be + assumed that the processor did not find a valid region of text. + Subclasses should override this method. + Keyword arguments: + * m: A re match object containing a match of the pattern. + * data: The buffer current under analysis + Returns: + * el: The ElementTree element, text or None. + * start: The start of the region that has been matched or None. + * end: The end of the region that has been matched or None. + """ + pass # pragma: no cover + + +class SimpleTagInlineProcessor(InlineProcessor): + """ + Return element of type `tag` with a text attribute of group(2) + of a Pattern. + """ + def __init__(self, pattern, tag): + InlineProcessor.__init__(self, pattern) + self.tag = tag + + def handleMatch(self, m, data): # pragma: no cover + el = etree.Element(self.tag) + el.text = m.group(2) + return el, m.start(0), m.end(0) + + +class SubstituteTagInlineProcessor(SimpleTagInlineProcessor): + """ Return an element of type `tag` with no children. """ + def handleMatch(self, m, data): + return etree.Element(self.tag), m.start(0), m.end(0) + + +class Nl2BrExtension(Extension): + BR_RE = r'\n' + + def extendMarkdown(self, md, md_globals): + br_tag = SubstituteTagInlineProcessor(self.BR_RE, 'br') + md.inlinePatterns.add('nl', br_tag, '_end') + + class GithubFlavoredMarkdownExtension(Extension): """ An extension that is as compatible as possible with GitHub-flavored @@ -51,6 +119,7 @@ class GithubFlavoredMarkdownExtension(Ex def extendMarkdown(self, md, md_globals): # Built-in extensions + Nl2BrExtension().extendMarkdown(md, md_globals) FencedCodeExtension().extendMarkdown(md, md_globals) SmartEmphasisExtension().extendMarkdown(md, md_globals) TableExtension().extendMarkdown(md, md_globals) @@ -68,7 +137,6 @@ class GithubFlavoredMarkdownExtension(Ex gfm.TaskListExtension([ ('list_attrs', {'class': 'checkbox'}) ]).extendMarkdown(md, md_globals) - Nl2BrExtension().extendMarkdown(md, md_globals) # Global Vars diff --git a/rhodecode/lib/rc_cache/__init__.py b/rhodecode/lib/rc_cache/__init__.py --- a/rhodecode/lib/rc_cache/__init__.py +++ b/rhodecode/lib/rc_cache/__init__.py @@ -74,7 +74,11 @@ def configure_dogpile_cache(settings): new_region.configure_from_config(settings, 'rc_cache.{}.'.format(region_name)) new_region.function_key_generator = backend_key_generator(new_region.actual_backend) - log.debug('dogpile: registering a new region %s[%s]', region_name, new_region.__dict__) + if log.isEnabledFor(logging.DEBUG): + region_args = dict(backend=new_region.actual_backend.__class__, + region_invalidator=new_region.region_invalidator.__class__) + log.debug('dogpile: registering a new region `%s` %s', region_name, region_args) + region_meta.dogpile_cache_regions[region_name] = new_region 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 @@ -915,8 +915,9 @@ class BaseCommit(object): list of parent commits """ + repository = None + branch = None - branch = None """ Depending on the backend this should be set to the branch name of the commit. Backends not supporting branches on commits should leave this @@ -1192,13 +1193,14 @@ class BaseCommit(object): return None def archive_repo(self, archive_dest_path, kind='tgz', subrepos=None, - prefix=None, write_metadata=False, mtime=None, archive_at_path='/'): + archive_dir_name=None, write_metadata=False, mtime=None, + archive_at_path='/'): """ Creates an archive containing the contents of the repository. :param archive_dest_path: path to the file which to create the archive. :param kind: one of following: ``"tbz2"``, ``"tgz"``, ``"zip"``. - :param prefix: name of root directory in archive. + :param archive_dir_name: name of root directory in archive. Default is repository name and commit's short_id joined with dash: ``"{repo_name}-{short_id}"``. :param write_metadata: write a metadata file into archive. @@ -1214,43 +1216,26 @@ class BaseCommit(object): 'Archive kind (%s) not supported use one of %s' % (kind, allowed_kinds)) - prefix = self._validate_archive_prefix(prefix) - + archive_dir_name = self._validate_archive_prefix(archive_dir_name) mtime = mtime is not None or time.mktime(self.date.timetuple()) - - file_info = [] - cur_rev = self.repository.get_commit(commit_id=self.raw_id) - for _r, _d, files in cur_rev.walk(archive_at_path): - for f in files: - f_path = os.path.join(prefix, f.path) - file_info.append( - (f_path, f.mode, f.is_link(), f.raw_bytes)) + commit_id = self.raw_id - if write_metadata: - metadata = [ - ('repo_name', self.repository.name), - ('commit_id', self.raw_id), - ('mtime', mtime), - ('branch', self.branch), - ('tags', ','.join(self.tags)), - ] - meta = ["%s:%s" % (f_name, value) for f_name, value in metadata] - file_info.append(('.archival.txt', 0o644, False, '\n'.join(meta))) + return self.repository._remote.archive_repo( + archive_dest_path, kind, mtime, archive_at_path, + archive_dir_name, commit_id) - connection.Hg.archive_repo(archive_dest_path, mtime, file_info, kind) - - def _validate_archive_prefix(self, prefix): - if prefix is None: - prefix = self._ARCHIVE_PREFIX_TEMPLATE.format( + def _validate_archive_prefix(self, archive_dir_name): + if archive_dir_name is None: + archive_dir_name = self._ARCHIVE_PREFIX_TEMPLATE.format( repo_name=safe_str(self.repository.name), short_id=self.short_id) - elif not isinstance(prefix, str): - raise ValueError("prefix not a bytes object: %s" % repr(prefix)) - elif prefix.startswith('/'): + elif not isinstance(archive_dir_name, str): + raise ValueError("prefix not a bytes object: %s" % repr(archive_dir_name)) + elif archive_dir_name.startswith('/'): raise VCSError("Prefix cannot start with leading slash") - elif prefix.strip() == '': + elif archive_dir_name.strip() == '': raise VCSError("Prefix cannot be empty") - return prefix + return archive_dir_name @LazyProperty def root(self): diff --git a/rhodecode/lib/vcs/exceptions.py b/rhodecode/lib/vcs/exceptions.py --- a/rhodecode/lib/vcs/exceptions.py +++ b/rhodecode/lib/vcs/exceptions.py @@ -214,16 +214,19 @@ def map_vcs_exceptions(func): # to translate them to the proper exception class in the vcs # client layer. kind = getattr(e, '_vcs_kind', None) + exc_name = getattr(e, '_vcs_server_org_exc_name', None) if kind: if any(e.args): - args = e.args + args = [a for a in e.args] + args[0] = '{}:'.format(exc_name) # prefix first arg with org exc name else: - args = [__traceback_info__ or 'unhandledException'] + args = [__traceback_info__ or '{}: UnhandledException'.format(exc_name)] if debug or __traceback_info__ and kind not in ['unhandled', 'lookup']: # for other than unhandled errors also log the traceback # can be useful for debugging log.error(__traceback_info__) + raise _EXCEPTION_MAP[kind](*args) else: raise diff --git a/rhodecode/model/comment.py b/rhodecode/model/comment.py --- a/rhodecode/model/comment.py +++ b/rhodecode/model/comment.py @@ -37,6 +37,7 @@ from rhodecode.lib.exceptions import Com from rhodecode.lib.utils2 import extract_mentioned_users, safe_str, safe_int from rhodecode.model import BaseModel from rhodecode.model.db import ( + false, true, ChangesetComment, User, Notification, @@ -160,7 +161,7 @@ class CommentsModel(BaseModel): return todos - def get_pull_request_unresolved_todos(self, pull_request, show_outdated=True): + def get_pull_request_unresolved_todos(self, pull_request, show_outdated=True, include_drafts=True): todos = Session().query(ChangesetComment) \ .filter(ChangesetComment.pull_request == pull_request) \ @@ -168,6 +169,9 @@ class CommentsModel(BaseModel): .filter(ChangesetComment.comment_type == ChangesetComment.COMMENT_TYPE_TODO) + if not include_drafts: + todos = todos.filter(ChangesetComment.draft == false()) + if not show_outdated: todos = todos.filter( coalesce(ChangesetComment.display_state, '') != @@ -177,7 +181,7 @@ class CommentsModel(BaseModel): return todos - def get_pull_request_resolved_todos(self, pull_request, show_outdated=True): + def get_pull_request_resolved_todos(self, pull_request, show_outdated=True, include_drafts=True): todos = Session().query(ChangesetComment) \ .filter(ChangesetComment.pull_request == pull_request) \ @@ -185,6 +189,9 @@ class CommentsModel(BaseModel): .filter(ChangesetComment.comment_type == ChangesetComment.COMMENT_TYPE_TODO) + if not include_drafts: + todos = todos.filter(ChangesetComment.draft == false()) + if not show_outdated: todos = todos.filter( coalesce(ChangesetComment.display_state, '') != @@ -194,7 +201,14 @@ class CommentsModel(BaseModel): return todos - def get_commit_unresolved_todos(self, commit_id, show_outdated=True): + def get_pull_request_drafts(self, user_id, pull_request): + drafts = Session().query(ChangesetComment) \ + .filter(ChangesetComment.pull_request == pull_request) \ + .filter(ChangesetComment.user_id == user_id) \ + .filter(ChangesetComment.draft == true()) + return drafts.all() + + def get_commit_unresolved_todos(self, commit_id, show_outdated=True, include_drafts=True): todos = Session().query(ChangesetComment) \ .filter(ChangesetComment.revision == commit_id) \ @@ -202,6 +216,9 @@ class CommentsModel(BaseModel): .filter(ChangesetComment.comment_type == ChangesetComment.COMMENT_TYPE_TODO) + if not include_drafts: + todos = todos.filter(ChangesetComment.draft == false()) + if not show_outdated: todos = todos.filter( coalesce(ChangesetComment.display_state, '') != @@ -211,7 +228,7 @@ class CommentsModel(BaseModel): return todos - def get_commit_resolved_todos(self, commit_id, show_outdated=True): + def get_commit_resolved_todos(self, commit_id, show_outdated=True, include_drafts=True): todos = Session().query(ChangesetComment) \ .filter(ChangesetComment.revision == commit_id) \ @@ -219,6 +236,9 @@ class CommentsModel(BaseModel): .filter(ChangesetComment.comment_type == ChangesetComment.COMMENT_TYPE_TODO) + if not include_drafts: + todos = todos.filter(ChangesetComment.draft == false()) + if not show_outdated: todos = todos.filter( coalesce(ChangesetComment.display_state, '') != @@ -228,11 +248,15 @@ class CommentsModel(BaseModel): return todos - def get_commit_inline_comments(self, commit_id): + def get_commit_inline_comments(self, commit_id, include_drafts=True): inline_comments = Session().query(ChangesetComment) \ .filter(ChangesetComment.line_no != None) \ .filter(ChangesetComment.f_path != None) \ .filter(ChangesetComment.revision == commit_id) + + if not include_drafts: + inline_comments = inline_comments.filter(ChangesetComment.draft == false()) + inline_comments = inline_comments.all() return inline_comments @@ -245,7 +269,7 @@ class CommentsModel(BaseModel): def create(self, text, repo, user, commit_id=None, pull_request=None, f_path=None, line_no=None, status_change=None, - status_change_type=None, comment_type=None, + status_change_type=None, comment_type=None, is_draft=False, resolves_comment_id=None, closing_pr=False, send_email=True, renderer=None, auth_user=None, extra_recipients=None): """ @@ -262,6 +286,7 @@ class CommentsModel(BaseModel): :param line_no: :param status_change: Label for status change :param comment_type: Type of comment + :param is_draft: is comment a draft only :param resolves_comment_id: id of comment which this one will resolve :param status_change_type: type of status change :param closing_pr: @@ -288,6 +313,7 @@ class CommentsModel(BaseModel): validated_kwargs = schema.deserialize(dict( comment_body=text, comment_type=comment_type, + is_draft=is_draft, comment_file=f_path, comment_line=line_no, renderer_type=renderer, @@ -296,6 +322,7 @@ class CommentsModel(BaseModel): repo=repo.repo_id, user=user.user_id, )) + is_draft = validated_kwargs['is_draft'] comment = ChangesetComment() comment.renderer = validated_kwargs['renderer_type'] @@ -303,6 +330,7 @@ class CommentsModel(BaseModel): comment.f_path = validated_kwargs['comment_file'] comment.line_no = validated_kwargs['comment_line'] comment.comment_type = validated_kwargs['comment_type'] + comment.draft = is_draft comment.repo = repo comment.author = user @@ -438,9 +466,6 @@ class CommentsModel(BaseModel): if send_email: recipients += [self._get_user(u) for u in (extra_recipients or [])] - # pre-generate the subject for notification itself - (subject, _e, body_plaintext) = EmailNotificationModel().render_email( - notification_type, **kwargs) mention_recipients = set( self._extract_mentions(text)).difference(recipients) @@ -448,8 +473,8 @@ class CommentsModel(BaseModel): # create notification objects, and emails NotificationModel().create( created_by=user, - notification_subject=subject, - notification_body=body_plaintext, + notification_subject='', # Filled in based on the notification_type + notification_body='', # Filled in based on the notification_type notification_type=notification_type, recipients=recipients, mention_recipients=mention_recipients, @@ -462,10 +487,11 @@ class CommentsModel(BaseModel): else: action = 'repo.commit.comment.create' - comment_data = comment.get_api_data() + if not is_draft: + comment_data = comment.get_api_data() - self._log_audit_action( - action, {'data': comment_data}, auth_user, comment) + self._log_audit_action( + action, {'data': comment_data}, auth_user, comment) return comment @@ -541,7 +567,8 @@ class CommentsModel(BaseModel): return comment - def get_all_comments(self, repo_id, revision=None, pull_request=None, count_only=False): + def get_all_comments(self, repo_id, revision=None, pull_request=None, + include_drafts=True, count_only=False): q = ChangesetComment.query()\ .filter(ChangesetComment.repo_id == repo_id) if revision: @@ -551,6 +578,8 @@ class CommentsModel(BaseModel): q = q.filter(ChangesetComment.pull_request_id == pull_request.pull_request_id) else: raise Exception('Please specify commit or pull_request') + if not include_drafts: + q = q.filter(ChangesetComment.draft == false()) q = q.order_by(ChangesetComment.created_on) if count_only: return q.count() @@ -697,7 +726,8 @@ class CommentsModel(BaseModel): path=comment.f_path, diff_line=diff_line) except (diffs.LineNotInDiffException, diffs.FileNotInDiffException): - comment.display_state = ChangesetComment.COMMENT_OUTDATED + if not comment.draft: + comment.display_state = ChangesetComment.COMMENT_OUTDATED return if old_context == new_context: @@ -707,14 +737,15 @@ class CommentsModel(BaseModel): new_diff_lines = new_diff_proc.find_context( path=comment.f_path, context=old_context, offset=self.DIFF_CONTEXT_BEFORE) - if not new_diff_lines: + if not new_diff_lines and not comment.draft: comment.display_state = ChangesetComment.COMMENT_OUTDATED else: new_diff_line = self._choose_closest_diff_line( diff_line, new_diff_lines) comment.line_no = _diff_to_comment_line_number(new_diff_line) else: - comment.display_state = ChangesetComment.COMMENT_OUTDATED + if not comment.draft: + comment.display_state = ChangesetComment.COMMENT_OUTDATED def _should_relocate_diff_line(self, diff_line): """ diff --git a/rhodecode/model/db.py b/rhodecode/model/db.py --- a/rhodecode/model/db.py +++ b/rhodecode/model/db.py @@ -3767,6 +3767,7 @@ class ChangesetComment(Base, BaseModel): renderer = Column('renderer', Unicode(64), nullable=True) display_state = Column('display_state', Unicode(128), nullable=True) immutable_state = Column('immutable_state', Unicode(128), nullable=True, default=OP_CHANGEABLE) + draft = Column('draft', Boolean(), nullable=True, default=False) comment_type = Column('comment_type', Unicode(128), nullable=True, default=COMMENT_TYPE_NOTE) resolved_comment_id = Column('resolved_comment_id', Integer(), ForeignKey('changeset_comments.comment_id'), nullable=True) @@ -5057,8 +5058,14 @@ class RepoReviewRule(Base, BaseModel): _file_pattern = Column("file_pattern", UnicodeText().with_variant(UnicodeText(255), 'mysql'), default=u'*') # glob use_authors_for_review = Column("use_authors_for_review", Boolean(), nullable=False, default=False) - forbid_author_to_review = Column("forbid_author_to_review", Boolean(), nullable=False, default=False) - forbid_commit_author_to_review = Column("forbid_commit_author_to_review", Boolean(), nullable=False, default=False) + + # Legacy fields, just for backward compat + _forbid_author_to_review = Column("forbid_author_to_review", Boolean(), nullable=False, default=False) + _forbid_commit_author_to_review = Column("forbid_commit_author_to_review", Boolean(), nullable=False, default=False) + + pr_author = Column("pr_author", UnicodeText().with_variant(UnicodeText(255), 'mysql'), nullable=True) + commit_author = Column("commit_author", UnicodeText().with_variant(UnicodeText(255), 'mysql'), nullable=True) + forbid_adding_reviewers = Column("forbid_adding_reviewers", Boolean(), nullable=False, default=False) rule_users = relationship('RepoReviewRuleUser') @@ -5094,6 +5101,22 @@ class RepoReviewRule(Base, BaseModel): self._validate_pattern(value) self._file_pattern = value or '*' + @hybrid_property + def forbid_pr_author_to_review(self): + return self.pr_author == 'forbid_pr_author' + + @hybrid_property + def include_pr_author_to_review(self): + return self.pr_author == 'include_pr_author' + + @hybrid_property + def forbid_commit_author_to_review(self): + return self.commit_author == 'forbid_commit_author' + + @hybrid_property + def include_commit_author_to_review(self): + return self.commit_author == 'include_commit_author' + def matches(self, source_branch, target_branch, files_changed): """ Check if this review rule matches a branch/files in a pull request diff --git a/rhodecode/model/notification.py b/rhodecode/model/notification.py --- a/rhodecode/model/notification.py +++ b/rhodecode/model/notification.py @@ -55,7 +55,7 @@ class NotificationModel(BaseModel): ' of Notification got %s' % type(notification)) def create( - self, created_by, notification_subject, notification_body, + self, created_by, notification_subject='', notification_body='', notification_type=Notification.TYPE_MESSAGE, recipients=None, mention_recipients=None, with_email=True, email_kwargs=None): """ @@ -64,11 +64,12 @@ class NotificationModel(BaseModel): :param created_by: int, str or User instance. User who created this notification - :param notification_subject: subject of notification itself + :param notification_subject: subject of notification itself, + it will be generated automatically from notification_type if not specified :param notification_body: body of notification text + it will be generated automatically from notification_type if not specified :param notification_type: type of notification, based on that we pick templates - :param recipients: list of int, str or User objects, when None is given send to all admins :param mention_recipients: list of int, str or User objects, @@ -82,14 +83,19 @@ class NotificationModel(BaseModel): if recipients and not getattr(recipients, '__iter__', False): raise Exception('recipients must be an iterable object') + if not (notification_subject and notification_body) and not notification_type: + raise ValueError('notification_subject, and notification_body ' + 'cannot be empty when notification_type is not specified') + created_by_obj = self._get_user(created_by) + + if not created_by_obj: + raise Exception('unknown user %s' % created_by) + # default MAIN body if not given email_kwargs = email_kwargs or {'body': notification_body} mention_recipients = mention_recipients or set() - if not created_by_obj: - raise Exception('unknown user %s' % created_by) - if recipients is None: # recipients is None means to all admins recipients_objs = User.query().filter(User.admin == true()).all() @@ -113,6 +119,15 @@ class NotificationModel(BaseModel): # add mentioned users into recipients final_recipients = set(recipients_objs).union(mention_recipients) + (subject, email_body, email_body_plaintext) = \ + EmailNotificationModel().render_email(notification_type, **email_kwargs) + + if not notification_subject: + notification_subject = subject + + if not notification_body: + notification_body = email_body_plaintext + notification = Notification.create( created_by=created_by_obj, subject=notification_subject, body=notification_body, recipients=final_recipients, diff --git a/rhodecode/model/permission.py b/rhodecode/model/permission.py --- a/rhodecode/model/permission.py +++ b/rhodecode/model/permission.py @@ -578,7 +578,7 @@ class PermissionModel(BaseModel): return user_group_write_permissions def trigger_permission_flush(self, affected_user_ids=None): - affected_user_ids or User.get_all_user_ids() + affected_user_ids = affected_user_ids or User.get_all_user_ids() events.trigger(events.UserPermissionsChange(affected_user_ids)) def flush_user_permission_caches(self, changes, affected_user_ids=None): 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 @@ -1502,15 +1502,11 @@ class PullRequestModel(BaseModel): 'user_role': role } - # pre-generate the subject for notification itself - (subject, _e, body_plaintext) = EmailNotificationModel().render_email( - notification_type, **kwargs) - # create notification objects, and emails NotificationModel().create( created_by=current_rhodecode_user, - notification_subject=subject, - notification_body=body_plaintext, + notification_subject='', # Filled in based on the notification_type + notification_body='', # Filled in based on the notification_type notification_type=notification_type, recipients=recipients, email_kwargs=kwargs, @@ -1579,14 +1575,11 @@ class PullRequestModel(BaseModel): 'thread_ids': [pr_url], } - (subject, _e, body_plaintext) = EmailNotificationModel().render_email( - EmailNotificationModel.TYPE_PULL_REQUEST_UPDATE, **email_kwargs) - # create notification objects, and emails NotificationModel().create( created_by=updating_user, - notification_subject=subject, - notification_body=body_plaintext, + notification_subject='', # Filled in based on the notification_type + notification_body='', # Filled in based on the notification_type notification_type=EmailNotificationModel.TYPE_PULL_REQUEST_UPDATE, recipients=recipients, email_kwargs=email_kwargs, @@ -2067,6 +2060,8 @@ class MergeCheck(object): self.error_details = OrderedDict() self.source_commit = AttributeDict() self.target_commit = AttributeDict() + self.reviewers_count = 0 + self.observers_count = 0 def __repr__(self): return ''.format( @@ -2128,11 +2123,12 @@ class MergeCheck(object): # review status, must be always present review_status = pull_request.calculated_review_status() merge_check.review_status = review_status + merge_check.reviewers_count = pull_request.reviewers_count + merge_check.observers_count = pull_request.observers_count status_approved = review_status == ChangesetStatus.STATUS_APPROVED - if not status_approved: + if not status_approved and merge_check.reviewers_count: log.debug("MergeCheck: cannot merge, approval is pending.") - msg = _('Pull request reviewer approval is pending.') merge_check.push_error('warning', msg, cls.REVIEW_CHECK, review_status) diff --git a/rhodecode/model/scm.py b/rhodecode/model/scm.py --- a/rhodecode/model/scm.py +++ b/rhodecode/model/scm.py @@ -231,6 +231,10 @@ class ScmModel(BaseModel): with_wire={"cache": False}) except OSError: continue + except RepositoryError: + log.exception('Failed to create a repo') + continue + log.debug('found %s paths with repositories', len(repos)) return repos diff --git a/rhodecode/model/user.py b/rhodecode/model/user.py --- a/rhodecode/model/user.py +++ b/rhodecode/model/user.py @@ -425,15 +425,12 @@ class UserModel(BaseModel): 'date': datetime.datetime.now() } notification_type = EmailNotificationModel.TYPE_REGISTRATION - # pre-generate the subject for notification itself - (subject, _e, body_plaintext) = EmailNotificationModel().render_email( - notification_type, **kwargs) # create notification objects, and emails NotificationModel().create( created_by=new_user, - notification_subject=subject, - notification_body=body_plaintext, + notification_subject='', # Filled in based on the notification_type + notification_body='', # Filled in based on the notification_type notification_type=notification_type, recipients=None, # all admins email_kwargs=kwargs, diff --git a/rhodecode/model/validation_schema/schemas/comment_schema.py b/rhodecode/model/validation_schema/schemas/comment_schema.py --- a/rhodecode/model/validation_schema/schemas/comment_schema.py +++ b/rhodecode/model/validation_schema/schemas/comment_schema.py @@ -60,7 +60,7 @@ class CommentSchema(colander.MappingSche colander.String(), validator=colander.OneOf(ChangesetComment.COMMENT_TYPES), missing=ChangesetComment.COMMENT_TYPE_NOTE) - + is_draft = colander.SchemaNode(colander.Boolean(),missing=False) comment_file = colander.SchemaNode(colander.String(), missing=None) comment_line = colander.SchemaNode(colander.String(), missing=None) status_change = colander.SchemaNode( diff --git a/rhodecode/public/css/buttons.less b/rhodecode/public/css/buttons.less --- a/rhodecode/public/css/buttons.less +++ b/rhodecode/public/css/buttons.less @@ -162,7 +162,6 @@ input[type="button"] { } } -.btn-warning, .btn-danger, .revoke_perm, .btn-x, @@ -196,6 +195,36 @@ input[type="button"] { } } +.btn-warning { + .border ( @border-thickness, @alert3 ); + background-color: white; + color: @alert3; + + a { + color: @alert3; + } + + &:hover, + &.active { + .border ( @border-thickness, @alert3 ); + color: white; + background-color: @alert3; + + a { + color: white; + } + } + + i { + display:none; + } + + &:disabled { + background-color: white; + color: @alert3; + } +} + .btn-approved-status { .border ( @border-thickness, @alert1 ); background-color: white; @@ -264,7 +293,6 @@ input[type="button"] { margin-left: -1px; padding-left: 2px; padding-right: 2px; - border-left: 1px solid @grey3; } } @@ -342,7 +370,7 @@ input[type="button"] { color: @alert2; &:hover { - color: darken(@alert2,30%); + color: darken(@alert2, 30%); } &:disabled { @@ -402,6 +430,37 @@ input[type="button"] { } +input[type="submit"].btn-warning { + &:extend(.btn-warning); + + &:focus { + outline: 0; + } + + &:hover { + &:extend(.btn-warning:hover); + } + + &.btn-link { + &:extend(.btn-link); + color: @alert3; + + &:disabled { + color: @alert3; + background-color: transparent; + } + } + + &:disabled { + .border ( @border-thickness-buttons, @alert3 ); + background-color: white; + color: @alert3; + opacity: 0.5; + } +} + + + // TODO: johbo: Form button tweaks, check if we can use the classes instead input[type="submit"] { &:extend(.btn-primary); diff --git a/rhodecode/public/css/code-block.less b/rhodecode/public/css/code-block.less --- a/rhodecode/public/css/code-block.less +++ b/rhodecode/public/css/code-block.less @@ -1002,7 +1002,7 @@ input.filediff-collapse-state { .nav-chunk { position: absolute; right: 20px; - margin-top: -17px; + margin-top: -15px; } .nav-chunk.selected { diff --git a/rhodecode/public/css/comments.less b/rhodecode/public/css/comments.less --- a/rhodecode/public/css/comments.less +++ b/rhodecode/public/css/comments.less @@ -4,7 +4,7 @@ // Comments -@comment-outdated-opacity: 0.6; +@comment-outdated-opacity: 1.0; .comments { width: 100%; @@ -61,28 +61,37 @@ tr.inline-comments div { visibility: hidden; } +.comment-draft { + float: left; + margin-right: 10px; + font-weight: 400; + color: @color-draft; +} + +.comment-new { + float: left; + margin-right: 10px; + font-weight: 400; + color: @color-new; +} + .comment-label { float: left; - padding: 0.4em 0.4em; - margin: 2px 4px 0px 0px; - display: inline-block; + padding: 0 8px 0 0; min-height: 0; text-align: center; font-size: 10px; - line-height: .8em; font-family: @text-italic; font-style: italic; background: #fff none; color: @grey3; - border: 1px solid @grey4; white-space: nowrap; text-transform: uppercase; min-width: 50px; - border-radius: 4px; &.todo { color: @color5; @@ -270,64 +279,165 @@ tr.inline-comments div { .comment-outdated { opacity: @comment-outdated-opacity; } + + .comment-outdated-label { + color: @grey3; + padding-right: 4px; + } } .inline-comments { - border-radius: @border-radius; + .comment { margin: 0; - border-radius: @border-radius; } + .comment-outdated { opacity: @comment-outdated-opacity; } + .comment-outdated-label { + color: @grey3; + padding-right: 4px; + } + .comment-inline { + + &:first-child { + margin: 4px 4px 0 4px; + border-top: 1px solid @grey5; + border-bottom: 0 solid @grey5; + border-left: 1px solid @grey5; + border-right: 1px solid @grey5; + .border-radius-top(4px); + } + + &:only-child { + margin: 4px 4px 0 4px; + border-top: 1px solid @grey5; + border-bottom: 0 solid @grey5; + border-left: 1px solid @grey5; + border-right: 1px solid @grey5; + .border-radius-top(4px); + } + background: white; padding: @comment-padding @comment-padding; - border: @comment-padding solid @grey6; + margin: 0 4px 0 4px; + border-top: 0 solid @grey5; + border-bottom: 0 solid @grey5; + border-left: 1px solid @grey5; + border-right: 1px solid @grey5; .text { border: none; } + .meta { border-bottom: 1px solid @grey6; margin: -5px 0px; line-height: 24px; } + } .comment-selected { border-left: 6px solid @comment-highlight-color; } + + .comment-inline-form-open { + display: block !important; + } + .comment-inline-form { - padding: @comment-padding; display: none; } - .cb-comment-add-button { - margin: @comment-padding; + + .comment-inline-form-edit { + padding: 0; + margin: 0px 4px 2px 4px; + } + + .reply-thread-container { + display: table; + width: 100%; + padding: 0px 4px 4px 4px; + } + + .reply-thread-container-wrapper { + margin: 0 4px 4px 4px; + border-top: 0 solid @grey5; + border-bottom: 1px solid @grey5; + border-left: 1px solid @grey5; + border-right: 1px solid @grey5; + .border-radius-bottom(4px); + } + + .reply-thread-gravatar { + display: table-cell; + width: 24px; + height: 24px; + padding-top: 10px; + padding-left: 10px; + background-color: #eeeeee; + vertical-align: top; } - /* hide add comment button when form is open */ + + .reply-thread-reply-button { + display: table-cell; + width: 100%; + height: 33px; + padding: 3px 8px; + margin-left: 8px; + background-color: #eeeeee; + } + + .reply-thread-reply-button .cb-comment-add-button { + border-radius: 4px; + width: 100%; + padding: 6px 2px; + text-align: left; + cursor: text; + color: @grey3; + } + .reply-thread-reply-button .cb-comment-add-button:hover { + background-color: white; + color: @grey2; + } + + .reply-thread-last { + display: table-cell; + width: 10px; + } + + /* Hide reply box when it's a first element, + can happen when drafts are saved but not shown to specific user, + or there are outdated comments hidden + */ + .reply-thread-container-wrapper:first-child:not(.comment-form-active) { + display: none; + } + + .reply-thread-container-wrapper.comment-outdated { + display: none + } + + /* hide add comment button when form is open */ .comment-inline-form-open ~ .cb-comment-add-button { display: none; } - .comment-inline-form-open { - display: block; - } - /* hide add comment button when form but no comments */ - .comment-inline-form:first-child + .cb-comment-add-button { - display: none; - } - /* hide add comment button when no comments or form */ - .cb-comment-add-button:first-child { - display: none; - } + /* hide add comment button when only comment is being deleted */ .comment-deleting:first-child + .cb-comment-add-button { display: none; } + + /* hide add comment button when form but no comments */ + .comment-inline-form:first-child + .cb-comment-add-button { + display: none; + } + } - .show-outdated-comments { display: inline; color: @rcblue; @@ -380,23 +490,36 @@ form.comment-form { } .comment-footer { - position: relative; + display: table; width: 100%; - min-height: 42px; + height: 42px; - .status_box, + .comment-status-box, .cancel-button { - float: left; display: inline-block; } - .status_box { + .comment-status-box { margin-left: 10px; } .action-buttons { - float: left; - display: inline-block; + display: table-cell; + padding: 5px 0 5px 2px; + } + + .toolbar-text { + height: 28px; + display: table-cell; + vertical-align: baseline; + font-size: 11px; + color: @grey4; + text-align: right; + + a { + color: @grey4; + } + } .action-buttons-extra { @@ -427,10 +550,10 @@ form.comment-form { margin-right: 0; } - .comment-footer { - margin-bottom: 50px; - margin-top: 10px; + #save_general { + margin-left: -6px; } + } @@ -482,8 +605,8 @@ form.comment-form { .injected_diff .comment-inline-form, .comment-inline-form { background-color: white; - margin-top: 10px; - margin-bottom: 20px; + margin-top: 4px; + margin-bottom: 10px; } .inline-form { @@ -519,9 +642,6 @@ form.comment-form { margin: 0px; } -.comment-inline-form .comment-footer { - margin: 10px 0px 0px 0px; -} .hide-inline-form-button { margin-left: 5px; @@ -547,6 +667,7 @@ comment-area-text { .comment-area-header { height: 35px; + border-bottom: 1px solid @grey5; } .comment-area-header .nav-links { @@ -554,6 +675,7 @@ comment-area-text { flex-flow: row wrap; -webkit-flex-flow: row wrap; width: 100%; + border: none; } .comment-area-footer { @@ -622,14 +744,3 @@ comment-area-text { border-bottom: 2px solid transparent; } -.toolbar-text { - float: right; - font-size: 11px; - color: @grey4; - text-align: right; - - a { - color: @grey4; - } -} - diff --git a/rhodecode/public/css/legacy_code_styles.less b/rhodecode/public/css/legacy_code_styles.less --- a/rhodecode/public/css/legacy_code_styles.less +++ b/rhodecode/public/css/legacy_code_styles.less @@ -213,7 +213,6 @@ div.markdown-block pre { div.markdown-block img { border-style: none; background-color: #fff; - padding-right: 20px; max-width: 100%; } @@ -274,6 +273,13 @@ div.markdown-block #ws { background-color: @grey6; } +div.markdown-block p { + margin-top: 0; + margin-bottom: 16px; + padding: 0; + line-height: unset; +} + div.markdown-block code, div.markdown-block pre, div.markdown-block #ws, diff --git a/rhodecode/public/css/login.less b/rhodecode/public/css/login.less --- a/rhodecode/public/css/login.less +++ b/rhodecode/public/css/login.less @@ -2,7 +2,7 @@ .loginbox { - max-width: 65%; + max-width: 960px; margin: @pagepadding auto; font-family: @text-light; border: @border-thickness solid @grey5; @@ -22,13 +22,27 @@ float: none; } - .header { + .header-account { + min-height: 49px; width: 100%; - padding: 0 35px; + padding: 0 @header-padding; box-sizing: border-box; + position: relative; + vertical-align: bottom; + + background-color: @grey1; + color: @grey5; .title { padding: 0; + overflow: visible; + } + + &:before, + &:after { + content: ""; + clear: both; + width: 100%; } } @@ -69,7 +83,7 @@ .sign-in-image { display: block; width: 65%; - margin: 5% auto; + margin: 1% auto; } .sign-in-title { 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 @@ -263,9 +263,6 @@ input.inline[type="file"] { // HEADER .header { - // TODO: johbo: Fix login pages, so that they work without a min-height - // for the header and then remove the min-height. I chose a smaller value - // intentionally here to avoid rendering issues in the main navigation. min-height: 49px; min-width: 1024px; @@ -1143,9 +1140,8 @@ label { margin-left: -15px; } -#rev_range_container, #rev_range_clear, #rev_range_more { - margin-top: -5px; - margin-bottom: -5px; +#rev_range_action { + margin-bottom: -8px; } #filter_changelog { @@ -1591,9 +1587,9 @@ table.integrations { } .pr-details-title { height: 20px; - line-height: 20px; - - padding-bottom: 8px; + line-height: 16px; + + padding-bottom: 4px; border-bottom: @border-thickness solid @grey5; .action_button.disabled { @@ -3212,7 +3208,12 @@ details:not([open]) > :not(summary) { .sidebar-element { margin-top: 20px; -} + + .icon-draft { + color: @color-draft + } +} + .right-sidebar-collapsed-state { display: flex; @@ -3235,5 +3236,4 @@ details:not([open]) > :not(summary) { .old-comments-marker td { padding-top: 15px; - border-bottom: 1px solid @grey5; -} +} diff --git a/rhodecode/public/css/readme-box.less b/rhodecode/public/css/readme-box.less --- a/rhodecode/public/css/readme-box.less +++ b/rhodecode/public/css/readme-box.less @@ -115,11 +115,9 @@ div.readme_box pre { div.readme_box img { border-style: none; background-color: #fff; - padding-right: 20px; max-width: 100%; } - div.readme_box strong { font-weight: 600; margin: 0; @@ -152,6 +150,13 @@ div.readme_box a:visited { } */ +div.readme_box p { + margin-top: 0; + margin-bottom: 16px; + padding: 0; + line-height: unset; +} + div.readme_box button { font-size: @basefontsize; diff --git a/rhodecode/public/css/variables.less b/rhodecode/public/css/variables.less --- a/rhodecode/public/css/variables.less +++ b/rhodecode/public/css/variables.less @@ -47,6 +47,8 @@ // Highlight color for lines and colors @comment-highlight-color: #ffd887; +@color-draft: darken(@alert3, 30%); +@color-new: darken(@alert1, 5%); // FONTS @basefontsize: 13px; diff --git a/rhodecode/public/js/rhodecode/routes.js b/rhodecode/public/js/rhodecode/routes.js --- a/rhodecode/public/js/rhodecode/routes.js +++ b/rhodecode/public/js/rhodecode/routes.js @@ -248,6 +248,7 @@ function registerRCRoutes() { pyroutes.register('pullrequest_comment_delete', '/%(repo_name)s/pull-request/%(pull_request_id)s/comment/%(comment_id)s/delete', ['repo_name', 'pull_request_id', 'comment_id']); pyroutes.register('pullrequest_comments', '/%(repo_name)s/pull-request/%(pull_request_id)s/comments', ['repo_name', 'pull_request_id']); pyroutes.register('pullrequest_todos', '/%(repo_name)s/pull-request/%(pull_request_id)s/todos', ['repo_name', 'pull_request_id']); + pyroutes.register('pullrequest_drafts', '/%(repo_name)s/pull-request/%(pull_request_id)s/drafts', ['repo_name', 'pull_request_id']); pyroutes.register('edit_repo', '/%(repo_name)s/settings', ['repo_name']); pyroutes.register('edit_repo_advanced', '/%(repo_name)s/settings/advanced', ['repo_name']); pyroutes.register('edit_repo_advanced_archive', '/%(repo_name)s/settings/advanced/archive', ['repo_name']); @@ -386,6 +387,8 @@ function registerRCRoutes() { pyroutes.register('my_account_auth_tokens_add', '/_admin/my_account/auth_tokens/new', []); pyroutes.register('my_account_external_identity', '/_admin/my_account/external-identity', []); pyroutes.register('my_account_external_identity_delete', '/_admin/my_account/external-identity/delete', []); + pyroutes.register('pullrequest_draft_comments_submit', '/%(repo_name)s/pull-request/%(pull_request_id)s/draft_comments_submit', ['repo_name', 'pull_request_id']); + pyroutes.register('commit_draft_comments_submit', '/%(repo_name)s/changeset/%(commit_id)s/draft_comments_submit', ['repo_name', 'commit_id']); pyroutes.register('repo_artifacts_list', '/%(repo_name)s/artifacts', ['repo_name']); pyroutes.register('repo_artifacts_data', '/%(repo_name)s/artifacts_data', ['repo_name']); pyroutes.register('repo_artifacts_new', '/%(repo_name)s/artifacts/new', ['repo_name']); diff --git a/rhodecode/public/js/src/components/rhodecode-app/rhodecode-app.js b/rhodecode/public/js/src/components/rhodecode-app/rhodecode-app.js --- a/rhodecode/public/js/src/components/rhodecode-app/rhodecode-app.js +++ b/rhodecode/public/js/src/components/rhodecode-app/rhodecode-app.js @@ -71,14 +71,20 @@ export class RhodecodeApp extends Polyme if (elem) { elem.handleNotification(data); } - } handleComment(data) { - if (data.message.comment_id) { + + if (data.message.comment_data.length !== 0) { if (window.refreshAllComments !== undefined) { refreshAllComments() } + var json_data = data.message.comment_data; + + if (window.commentsController !== undefined) { + + window.commentsController.attachComment(json_data) + } } } diff --git a/rhodecode/public/js/src/rhodecode.js b/rhodecode/public/js/src/rhodecode.js --- a/rhodecode/public/js/src/rhodecode.js +++ b/rhodecode/public/js/src/rhodecode.js @@ -704,3 +704,13 @@ var storeUserSessionAttr = function (key ajaxPOST(pyroutes.url('store_user_session_value'), postData, success); return false; }; + + +var getUserSessionAttr = function(key) { + var storeKey = templateContext.session_attrs; + var val = storeKey[key] + if (val !== undefined) { + return JSON.parse(val) + } + return null +} diff --git a/rhodecode/public/js/src/rhodecode/codemirror.js b/rhodecode/public/js/src/rhodecode/codemirror.js --- a/rhodecode/public/js/src/rhodecode/codemirror.js +++ b/rhodecode/public/js/src/rhodecode/codemirror.js @@ -349,7 +349,12 @@ var initCommentBoxCodeMirror = function( }; var submitForm = function(cm, pred) { - $(cm.display.input.textarea.form).submit(); + $(cm.display.input.textarea.form).find('.submit-comment-action').click(); + return CodeMirror.Pass; + }; + + var submitFormAsDraft = function(cm, pred) { + $(cm.display.input.textarea.form).find('.submit-draft-action').click(); return CodeMirror.Pass; }; @@ -475,9 +480,11 @@ var initCommentBoxCodeMirror = function( // submit form on Meta-Enter if (OSType === "mac") { extraKeys["Cmd-Enter"] = submitForm; + extraKeys["Shift-Cmd-Enter"] = submitFormAsDraft; } else { extraKeys["Ctrl-Enter"] = submitForm; + extraKeys["Shift-Ctrl-Enter"] = submitFormAsDraft; } if (triggerActions) { diff --git a/rhodecode/public/js/src/rhodecode/comments.js b/rhodecode/public/js/src/rhodecode/comments.js --- a/rhodecode/public/js/src/rhodecode/comments.js +++ b/rhodecode/public/js/src/rhodecode/comments.js @@ -124,16 +124,20 @@ var _submitAjaxPOST = function(url, post this.statusChange = this.withLineNo('#change_status'); this.submitForm = formElement; - this.submitButton = $(this.submitForm).find('input[type="submit"]'); + + this.submitButton = $(this.submitForm).find('.submit-comment-action'); this.submitButtonText = this.submitButton.val(); + this.submitDraftButton = $(this.submitForm).find('.submit-draft-action'); + this.submitDraftButtonText = this.submitDraftButton.val(); this.previewUrl = pyroutes.url('repo_commit_comment_preview', {'repo_name': templateContext.repo_name, 'commit_id': templateContext.commit_data.commit_id}); if (edit){ - this.submitButtonText = _gettext('Updated Comment'); + this.submitDraftButton.hide(); + this.submitButtonText = _gettext('Update Comment'); $(this.commentType).prop('disabled', true); $(this.commentType).addClass('disabled'); var editInfo = @@ -215,10 +219,17 @@ var _submitAjaxPOST = function(url, post this.getCommentStatus = function() { return $(this.submitForm).find(this.statusChange).val(); }; + this.getCommentType = function() { return $(this.submitForm).find(this.commentType).val(); }; + this.getDraftState = function () { + var submitterElem = $(this.submitForm).find('input[type="submit"].submitter'); + var data = $(submitterElem).data('isDraft'); + return data + } + this.getResolvesId = function() { return $(this.submitForm).find(this.resolvesId).val() || null; }; @@ -233,7 +244,9 @@ var _submitAjaxPOST = function(url, post }; this.isAllowedToSubmit = function() { - return !$(this.submitButton).prop('disabled'); + var commentDisabled = $(this.submitButton).prop('disabled'); + var draftDisabled = $(this.submitDraftButton).prop('disabled'); + return !commentDisabled && !draftDisabled; }; this.initStatusChangeSelector = function(){ @@ -259,11 +272,13 @@ var _submitAjaxPOST = function(url, post dropdownAutoWidth: true, minimumResultsForSearch: -1 }); + $(this.submitForm).find(this.statusChange).on('change', function() { var status = self.getCommentStatus(); if (status && !self.isInline()) { $(self.submitButton).prop('disabled', false); + $(self.submitDraftButton).prop('disabled', false); } var placeholderText = _gettext('Comment text will be set automatically based on currently selected status ({0}) ...').format(status); @@ -295,10 +310,10 @@ var _submitAjaxPOST = function(url, post $(this.statusChange).select2('readonly', false); }; - this.globalSubmitSuccessCallback = function(){ + this.globalSubmitSuccessCallback = function(comment){ // default behaviour is to call GLOBAL hook, if it's registered. if (window.commentFormGlobalSubmitSuccessCallback !== undefined){ - commentFormGlobalSubmitSuccessCallback(); + commentFormGlobalSubmitSuccessCallback(comment); } }; @@ -321,6 +336,7 @@ var _submitAjaxPOST = function(url, post var text = self.cm.getValue(); var status = self.getCommentStatus(); var commentType = self.getCommentType(); + var isDraft = self.getDraftState(); var resolvesCommentId = self.getResolvesId(); var closePullRequest = self.getClosePr(); @@ -348,12 +364,15 @@ var _submitAjaxPOST = function(url, post postData['close_pull_request'] = true; } - var submitSuccessCallback = function(o) { + // submitSuccess for general comments + var submitSuccessCallback = function(json_data) { // reload page if we change status for single commit. if (status && self.commitId) { location.reload(true); } else { - $('#injected_page_comments').append(o.rendered_text); + // inject newly created comments, json_data is {: {}} + self.attachGeneralComment(json_data) + self.resetCommentFormState(); timeagoActivate(); tooltipActivate(); @@ -365,7 +384,7 @@ var _submitAjaxPOST = function(url, post } // run global callback on submit - self.globalSubmitSuccessCallback(); + self.globalSubmitSuccessCallback({draft: isDraft, comment_id: comment_id}); }; var submitFailCallback = function(jqXHR, textStatus, errorThrown) { @@ -409,10 +428,20 @@ var _submitAjaxPOST = function(url, post } $(this.submitButton).prop('disabled', submitState); + $(this.submitDraftButton).prop('disabled', submitState); + if (submitEvent) { - $(this.submitButton).val(_gettext('Submitting...')); + var isDraft = self.getDraftState(); + + if (isDraft) { + $(this.submitDraftButton).val(_gettext('Saving Draft...')); + } else { + $(this.submitButton).val(_gettext('Submitting...')); + } + } else { $(this.submitButton).val(this.submitButtonText); + $(this.submitDraftButton).val(this.submitDraftButtonText); } }; @@ -488,6 +517,7 @@ var _submitAjaxPOST = function(url, post if (!allowedToSubmit){ return false; } + self.handleFormSubmit(); }); @@ -538,26 +568,6 @@ var CommentsController = function() { var mainComment = '#text'; var self = this; - this.cancelComment = function (node) { - var $node = $(node); - var edit = $(this).attr('edit'); - if (edit) { - var $general_comments = null; - var $inline_comments = $node.closest('div.inline-comments'); - if (!$inline_comments.length) { - $general_comments = $('#comments'); - var $comment = $general_comments.parent().find('div.comment:hidden'); - // show hidden general comment form - $('#cb-comment-general-form-placeholder').show(); - } else { - var $comment = $inline_comments.find('div.comment:hidden'); - } - $comment.show(); - } - $node.closest('.comment-inline-form').remove(); - return false; - }; - this.showVersion = function (comment_id, comment_history_id) { var historyViewUrl = pyroutes.url( @@ -655,12 +665,51 @@ var CommentsController = function() { return self.scrollToComment(node, -1, true); }; + this.cancelComment = function (node) { + var $node = $(node); + var edit = $(this).attr('edit'); + var $inlineComments = $node.closest('div.inline-comments'); + + if (edit) { + var $general_comments = null; + if (!$inlineComments.length) { + $general_comments = $('#comments'); + var $comment = $general_comments.parent().find('div.comment:hidden'); + // show hidden general comment form + $('#cb-comment-general-form-placeholder').show(); + } else { + var $comment = $inlineComments.find('div.comment:hidden'); + } + $comment.show(); + } + var $replyWrapper = $node.closest('.comment-inline-form').closest('.reply-thread-container-wrapper') + $replyWrapper.removeClass('comment-form-active'); + + var lastComment = $inlineComments.find('.comment-inline').last(); + if ($(lastComment).hasClass('comment-outdated')) { + $replyWrapper.hide(); + } + + $node.closest('.comment-inline-form').remove(); + return false; + }; + this._deleteComment = function(node) { var $node = $(node); var $td = $node.closest('td'); var $comment = $node.closest('.comment'); - var comment_id = $comment.attr('data-comment-id'); - var url = AJAX_COMMENT_DELETE_URL.replace('__COMMENT_ID__', comment_id); + var comment_id = $($comment).data('commentId'); + var isDraft = $($comment).data('commentDraft'); + + var pullRequestId = templateContext.pull_request_data.pull_request_id; + var commitId = templateContext.commit_data.commit_id; + + if (pullRequestId) { + var url = pyroutes.url('pullrequest_comment_delete', {"comment_id": comment_id, "repo_name": templateContext.repo_name, "pull_request_id": pullRequestId}) + } else if (commitId) { + var url = pyroutes.url('repo_commit_comment_delete', {"comment_id": comment_id, "repo_name": templateContext.repo_name, "commit_id": commitId}) + } + var postData = { 'csrf_token': CSRF_TOKEN }; @@ -677,10 +726,14 @@ var CommentsController = function() { updateSticky() } - if (window.refreshAllComments !== undefined) { + if (window.refreshAllComments !== undefined && !isDraft) { // if we have this handler, run it, and refresh all comments boxes refreshAllComments() } + else if (window.refreshDraftComments !== undefined && isDraft) { + // if we have this handler, run it, and refresh all comments boxes + refreshDraftComments(); + } return false; }; @@ -695,8 +748,6 @@ var CommentsController = function() { }; ajaxPOST(url, postData, success, failure); - - } this.deleteComment = function(node) { @@ -716,7 +767,59 @@ var CommentsController = function() { }) }; + this._finalizeDrafts = function(commentIds) { + + var pullRequestId = templateContext.pull_request_data.pull_request_id; + var commitId = templateContext.commit_data.commit_id; + + if (pullRequestId) { + var url = pyroutes.url('pullrequest_draft_comments_submit', {"repo_name": templateContext.repo_name, "pull_request_id": pullRequestId}) + } else if (commitId) { + var url = pyroutes.url('commit_draft_comments_submit', {"repo_name": templateContext.repo_name, "commit_id": commitId}) + } + + // remove the drafts so we can lock them before submit. + $.each(commentIds, function(idx, val){ + $('#comment-{0}'.format(val)).remove(); + }) + + var postData = {'comments': commentIds, 'csrf_token': CSRF_TOKEN}; + + var submitSuccessCallback = function(json_data) { + self.attachInlineComment(json_data); + + if (window.refreshDraftComments !== undefined) { + // if we have this handler, run it, and refresh all comments boxes + refreshDraftComments() + } + + return false; + }; + + ajaxPOST(url, postData, submitSuccessCallback) + + } + + this.finalizeDrafts = function(commentIds, callback) { + + SwalNoAnimation.fire({ + title: _ngettext('Submit {0} draft comment.', 'Submit {0} draft comments.', commentIds.length).format(commentIds.length), + icon: 'warning', + showCancelButton: true, + confirmButtonText: _gettext('Yes'), + + }).then(function(result) { + if (result.value) { + if (callback !== undefined) { + callback(result) + } + self._finalizeDrafts(commentIds); + } + }) + }; + this.toggleWideMode = function (node) { + if ($('#content').hasClass('wrapper')) { $('#content').removeClass("wrapper"); $('#content').addClass("wide-mode-wrapper"); @@ -731,16 +834,49 @@ var CommentsController = function() { }; - this.toggleComments = function(node, show) { + /** + * Turn off/on all comments in file diff + */ + this.toggleDiffComments = function(node) { + // Find closes filediff container var $filediff = $(node).closest('.filediff'); + if ($(node).hasClass('toggle-on')) { + var show = false; + } else if ($(node).hasClass('toggle-off')) { + var show = true; + } + + // Toggle each individual comment block, so we can un-toggle single ones + $.each($filediff.find('.toggle-comment-action'), function(idx, val) { + self.toggleLineComments($(val), show) + }) + + // since we change the height of the diff container that has anchor points for upper + // sticky header, we need to tell it to re-calculate those + if (window.updateSticky !== undefined) { + // potentially our comments change the active window size, so we + // notify sticky elements + updateSticky() + } + + return false; + } + + this.toggleLineComments = function(node, show) { + + var trElem = $(node).closest('tr') + if (show === true) { - $filediff.removeClass('hide-comments'); + // mark outdated comments as visible before the toggle; + $(trElem).find('.comment-outdated').show(); + $(trElem).removeClass('hide-line-comments'); } else if (show === false) { - $filediff.find('.hide-line-comments').removeClass('hide-line-comments'); - $filediff.addClass('hide-comments'); + $(trElem).find('.comment-outdated').hide(); + $(trElem).addClass('hide-line-comments'); } else { - $filediff.find('.hide-line-comments').removeClass('hide-line-comments'); - $filediff.toggleClass('hide-comments'); + // mark outdated comments as visible before the toggle; + $(trElem).find('.comment-outdated').show(); + $(trElem).toggleClass('hide-line-comments'); } // since we change the height of the diff container that has anchor points for upper @@ -751,15 +887,6 @@ var CommentsController = function() { updateSticky() } - return false; - }; - - this.toggleLineComments = function(node) { - self.toggleComments(node, true); - var $node = $(node); - // mark outdated comments as visible before the toggle; - $(node.closest('tr')).find('.comment-outdated').show(); - $node.closest('tr').toggleClass('hide-line-comments'); }; this.createCommentForm = function(formElement, lineno, placeholderText, initAutocompleteActions, resolvesCommentId, edit, comment_id){ @@ -913,63 +1040,58 @@ var CommentsController = function() { return commentForm; }; - this.editComment = function(node) { + this.editComment = function(node, line_no, f_path) { + self.edit = true; var $node = $(node); + var $td = $node.closest('td'); + var $comment = $(node).closest('.comment'); - var comment_id = $comment.attr('data-comment-id'); - var $form = null + var comment_id = $($comment).data('commentId'); + var isDraft = $($comment).data('commentDraft'); + var $editForm = null var $comments = $node.closest('div.inline-comments'); var $general_comments = null; - var lineno = null; if($comments.length){ // inline comments setup - $form = $comments.find('.comment-inline-form'); - lineno = self.getLineNumber(node) + $editForm = $comments.find('.comment-inline-form'); + line_no = self.getLineNumber(node) } else{ // general comments setup $comments = $('#comments'); - $form = $comments.find('.comment-inline-form'); - lineno = $comment[0].id + $editForm = $comments.find('.comment-inline-form'); + line_no = $comment[0].id $('#cb-comment-general-form-placeholder').hide(); } - this.edit = true; + if ($editForm.length === 0) { - if (!$form.length) { - + // unhide all comments if they are hidden for a proper REPLY mode var $filediff = $node.closest('.filediff'); $filediff.removeClass('hide-comments'); - var f_path = $filediff.attr('data-f-path'); - - // create a new HTML from template - var tmpl = $('#cb-comment-inline-form-template').html(); - tmpl = tmpl.format(escapeHtml(f_path), lineno); - $form = $(tmpl); - $comment.after($form) + $editForm = self.createNewFormWrapper(f_path, line_no); + if(f_path && line_no) { + $editForm.addClass('comment-inline-form-edit') + } - var _form = $($form[0]).find('form'); + $comment.after($editForm) + + var _form = $($editForm[0]).find('form'); var autocompleteActions = ['as_note',]; var commentForm = this.createCommentForm( - _form, lineno, '', autocompleteActions, resolvesCommentId, + _form, line_no, '', autocompleteActions, resolvesCommentId, this.edit, comment_id); var old_comment_text_binary = $comment.attr('data-comment-text'); var old_comment_text = b64DecodeUnicode(old_comment_text_binary); commentForm.cm.setValue(old_comment_text); $comment.hide(); + tooltipActivate(); - $.Topic('/ui/plugins/code/comment_form_built').prepareOrPublish({ - form: _form, - parent: $comments, - lineno: lineno, - f_path: f_path} - ); - - // set a CUSTOM submit handler for inline comments. - commentForm.setHandleFormSubmit(function(o) { + // set a CUSTOM submit handler for inline comment edit action. + commentForm.setHandleFormSubmit(function(o) { var text = commentForm.cm.getValue(); var commentType = commentForm.getCommentType(); @@ -1000,14 +1122,15 @@ var CommentsController = function() { var postData = { 'text': text, 'f_path': f_path, - 'line': lineno, + 'line': line_no, 'comment_type': commentType, + 'draft': isDraft, 'version': version, 'csrf_token': CSRF_TOKEN }; var submitSuccessCallback = function(json_data) { - $form.remove(); + $editForm.remove(); $comment.show(); var postData = { 'text': text, @@ -1072,8 +1195,7 @@ var CommentsController = function() { 'commit_id': templateContext.commit_data.commit_id}); _submitAjaxPOST( - previewUrl, postData, successRenderCommit, - failRenderCommit + previewUrl, postData, successRenderCommit, failRenderCommit ); try { @@ -1084,7 +1206,7 @@ var CommentsController = function() { $comments.find('.cb-comment-add-button').before(html); // run global callback on submit - commentForm.globalSubmitSuccessCallback(); + commentForm.globalSubmitSuccessCallback({draft: isDraft, comment_id: comment_id}); } catch (e) { console.error(e); @@ -1101,10 +1223,14 @@ var CommentsController = function() { updateSticky() } - if (window.refreshAllComments !== undefined) { + if (window.refreshAllComments !== undefined && !isDraft) { // if we have this handler, run it, and refresh all comments boxes refreshAllComments() } + else if (window.refreshDraftComments !== undefined && isDraft) { + // if we have this handler, run it, and refresh all comments boxes + refreshDraftComments(); + } commentForm.setActionButtonsDisabled(false); @@ -1129,66 +1255,122 @@ var CommentsController = function() { }); } - $form.addClass('comment-inline-form-open'); + $editForm.addClass('comment-inline-form-open'); }; - this.createComment = function(node, resolutionComment) { - var resolvesCommentId = resolutionComment || null; + this.attachComment = function(json_data) { + var self = this; + $.each(json_data, function(idx, val) { + var json_data_elem = [val] + var isInline = val.comment_f_path && val.comment_lineno + + if (isInline) { + self.attachInlineComment(json_data_elem) + } else { + self.attachGeneralComment(json_data_elem) + } + }) + + } + + this.attachGeneralComment = function(json_data) { + $.each(json_data, function(idx, val) { + $('#injected_page_comments').append(val.rendered_text); + }) + } + + this.attachInlineComment = function(json_data) { + + $.each(json_data, function (idx, val) { + var line_qry = '*[data-line-no="{0}"]'.format(val.line_no); + var html = val.rendered_text; + var $inlineComments = $('#' + val.target_id) + .find(line_qry) + .find('.inline-comments'); + + var lastComment = $inlineComments.find('.comment-inline').last(); + + if (lastComment.length === 0) { + // first comment, we append simply + $inlineComments.find('.reply-thread-container-wrapper').before(html); + } else { + $(lastComment).after(html) + } + + }) + + }; + + this.createNewFormWrapper = function(f_path, line_no) { + // create a new reply HTML form from template + var tmpl = $('#cb-comment-inline-form-template').html(); + tmpl = tmpl.format(escapeHtml(f_path), line_no); + return $(tmpl); + } + + this.createComment = function(node, f_path, line_no, resolutionComment) { + self.edit = false; var $node = $(node); var $td = $node.closest('td'); - var $form = $td.find('.comment-inline-form'); - this.edit = false; + var resolvesCommentId = resolutionComment || null; - if (!$form.length) { + var $replyForm = $td.find('.comment-inline-form'); - var $filediff = $node.closest('.filediff'); - $filediff.removeClass('hide-comments'); - var f_path = $filediff.attr('data-f-path'); - var lineno = self.getLineNumber(node); - // create a new HTML from template - var tmpl = $('#cb-comment-inline-form-template').html(); - tmpl = tmpl.format(escapeHtml(f_path), lineno); - $form = $(tmpl); + // if form isn't existing, we're generating a new one and injecting it. + if ($replyForm.length === 0) { + + // unhide/expand all comments if they are hidden for a proper REPLY mode + self.toggleLineComments($node, true); + + $replyForm = self.createNewFormWrapper(f_path, line_no); var $comments = $td.find('.inline-comments'); - if (!$comments.length) { - $comments = $( - $('#cb-comments-inline-container-template').html()); - $td.append($comments); + + // There aren't any comments, we init the `.inline-comments` with `reply-thread-container` first + if ($comments.length===0) { + var replBtn = ''.format(f_path, line_no) + var $reply_container = $('#cb-comments-inline-container-template') + $reply_container.find('button.cb-comment-add-button').replaceWith(replBtn); + $td.append($($reply_container).html()); } - $td.find('.cb-comment-add-button').before($form); + // default comment button exists, so we prepend the form for leaving initial comment + $td.find('.cb-comment-add-button').before($replyForm); + // set marker, that we have a open form + var $replyWrapper = $td.find('.reply-thread-container-wrapper') + $replyWrapper.addClass('comment-form-active'); - var placeholderText = _gettext('Leave a comment on line {0}.').format(lineno); - var _form = $($form[0]).find('form'); + var lastComment = $comments.find('.comment-inline').last(); + if ($(lastComment).hasClass('comment-outdated')) { + $replyWrapper.show(); + } + + var _form = $($replyForm[0]).find('form'); var autocompleteActions = ['as_note', 'as_todo']; var comment_id=null; - var commentForm = this.createCommentForm( - _form, lineno, placeholderText, autocompleteActions, resolvesCommentId, this.edit, comment_id); - - $.Topic('/ui/plugins/code/comment_form_built').prepareOrPublish({ - form: _form, - parent: $td[0], - lineno: lineno, - f_path: f_path} - ); + var placeholderText = _gettext('Leave a comment on file {0} line {1}.').format(f_path, line_no); + var commentForm = self.createCommentForm( + _form, line_no, placeholderText, autocompleteActions, resolvesCommentId, + self.edit, comment_id); // set a CUSTOM submit handler for inline comments. commentForm.setHandleFormSubmit(function(o) { var text = commentForm.cm.getValue(); var commentType = commentForm.getCommentType(); var resolvesCommentId = commentForm.getResolvesId(); + var isDraft = commentForm.getDraftState(); if (text === "") { return; } - if (lineno === undefined) { - alert('missing line !'); + if (line_no === undefined) { + alert('Error: unable to fetch line number for this inline comment !'); return; } + if (f_path === undefined) { - alert('missing file path !'); + alert('Error: unable to fetch file path for this inline comment !'); return; } @@ -1199,66 +1381,79 @@ var CommentsController = function() { var postData = { 'text': text, 'f_path': f_path, - 'line': lineno, + 'line': line_no, 'comment_type': commentType, + 'draft': isDraft, 'csrf_token': CSRF_TOKEN }; if (resolvesCommentId){ postData['resolves_comment_id'] = resolvesCommentId; } + // submitSuccess for inline commits var submitSuccessCallback = function(json_data) { - $form.remove(); - try { - var html = json_data.rendered_text; - var lineno = json_data.line_no; - var target_id = json_data.target_id; + + $replyForm.remove(); + $td.find('.reply-thread-container-wrapper').removeClass('comment-form-active'); + + try { + + // inject newly created comments, json_data is {: {}} + self.attachInlineComment(json_data) - $comments.find('.cb-comment-add-button').before(html); + //mark visually which comment was resolved + if (resolvesCommentId) { + commentForm.markCommentResolved(resolvesCommentId); + } - //mark visually which comment was resolved - if (resolvesCommentId) { - commentForm.markCommentResolved(resolvesCommentId); + // run global callback on submit + commentForm.globalSubmitSuccessCallback({ + draft: isDraft, + comment_id: comment_id + }); + + } catch (e) { + console.error(e); } - // run global callback on submit - commentForm.globalSubmitSuccessCallback(); - - } catch (e) { - console.error(e); - } - - // re trigger the linkification of next/prev navigation - linkifyComments($('.inline-comment-injected')); - timeagoActivate(); - tooltipActivate(); - if (window.updateSticky !== undefined) { // potentially our comments change the active window size, so we // notify sticky elements updateSticky() } - if (window.refreshAllComments !== undefined) { + if (window.refreshAllComments !== undefined && !isDraft) { // if we have this handler, run it, and refresh all comments boxes refreshAllComments() } + else if (window.refreshDraftComments !== undefined && isDraft) { + // if we have this handler, run it, and refresh all comments boxes + refreshDraftComments(); + } commentForm.setActionButtonsDisabled(false); + // re trigger the linkification of next/prev navigation + linkifyComments($('.inline-comment-injected')); + timeagoActivate(); + tooltipActivate(); }; + var submitFailCallback = function(jqXHR, textStatus, errorThrown) { var prefix = "Error while submitting comment.\n" var message = formatErrorMessage(jqXHR, textStatus, errorThrown, prefix); ajaxErrorSwal(message); commentForm.resetCommentFormState(text) }; + commentForm.submitAjaxPOST( commentForm.submitUrl, postData, submitSuccessCallback, submitFailCallback); }); } - $form.addClass('comment-inline-form-open'); + // Finally "open" our reply form, since we know there are comments and we have the "attached" old form + $replyForm.addClass('comment-inline-form-open'); + tooltipActivate(); }; this.createResolutionComment = function(commentId){ @@ -1268,9 +1463,12 @@ var CommentsController = function() { var comment = $('#comment-'+commentId); var commentData = comment.data(); if (commentData.commentInline) { - this.createComment(comment, commentId) + var f_path = commentData.fPath; + var line_no = commentData.lineNo; + //TODO check this if we need to give f_path/line_no + this.createComment(comment, f_path, line_no, commentId) } else { - Rhodecode.comments.createGeneralComment('general', "$placeholder", commentId) + this.createGeneralComment('general', "$placeholder", commentId) } return false; @@ -1296,3 +1494,8 @@ var CommentsController = function() { }; }; + +window.commentHelp = function(renderer) { + var funcData = {'renderer': renderer} + return renderTemplate('commentHelpHovercard', funcData) +} \ No newline at end of file diff --git a/rhodecode/public/js/src/rhodecode/menus.js b/rhodecode/public/js/src/rhodecode/menus.js --- a/rhodecode/public/js/src/rhodecode/menus.js +++ b/rhodecode/public/js/src/rhodecode/menus.js @@ -42,12 +42,22 @@ window.toggleElement = function (elem, t var $elem = $(elem); var $target = $(target); - if ($target.is(':visible') || $target.length === 0) { + if (target !== undefined) { + var show = $target.is(':visible') || $target.length === 0; + } else { + var show = $elem.hasClass('toggle-off') + } + + if (show) { $target.hide(); $elem.html($elem.data('toggleOn')) + $elem.addClass('toggle-on') + $elem.removeClass('toggle-off') } else { $target.show(); $elem.html($elem.data('toggleOff')) + $elem.addClass('toggle-off') + $elem.removeClass('toggle-on') } return false 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 @@ -182,83 +182,34 @@ window.ReviewersController = function () if (!data || data.rules === undefined || $.isEmptyObject(data.rules)) { // default rule, case for older repo that don't have any rules stored self.$rulesList.append( - self.addRule( - _gettext('All reviewers must vote.')) + self.addRule(_gettext('All reviewers must vote.')) ); return self.forbidUsers } - if (data.rules.voting !== undefined) { - if (data.rules.voting < 0) { - self.$rulesList.append( - self.addRule( - _gettext('All individual reviewers must vote.')) - ) - } else if (data.rules.voting === 1) { - self.$rulesList.append( - self.addRule( - _gettext('At least {0} reviewer must vote.').format(data.rules.voting)) - ) - - } else { - self.$rulesList.append( - self.addRule( - _gettext('At least {0} reviewers must vote.').format(data.rules.voting)) - ) - } + if (data.rules.forbid_adding_reviewers) { + $('#add_reviewer_input').remove(); } - if (data.rules.voting_groups !== undefined) { - $.each(data.rules.voting_groups, function (index, rule_data) { - self.$rulesList.append( - self.addRule(rule_data.text) - ) - }); - } - - if (data.rules.use_code_authors_for_review) { - self.$rulesList.append( - self.addRule( - _gettext('Reviewers picked from source code changes.')) - ) + if (data.rules_data !== undefined && data.rules_data.forbidden_users !== undefined) { + $.each(data.rules_data.forbidden_users, function(idx, val){ + self.forbidUsers.push(val) + }) } - if (data.rules.forbid_adding_reviewers) { - $('#add_reviewer_input').remove(); + if (data.rules_humanized !== undefined && data.rules_humanized.length > 0) { + $.each(data.rules_humanized, function(idx, val) { + self.$rulesList.append( + self.addRule(val) + ) + }) + } else { + // we don't have any rules set, so we inform users about it self.$rulesList.append( - self.addRule( - _gettext('Adding new reviewers is forbidden.')) - ) - } - - if (data.rules.forbid_author_to_review) { - self.forbidUsers.push(data.rules_data.pr_author); - self.$rulesList.append( - self.addRule( - _gettext('Author is not allowed to be a reviewer.')) + self.addRule(_gettext('No additional review rules set.')) ) } - if (data.rules.forbid_commit_author_to_review) { - - if (data.rules_data.forbidden_users) { - $.each(data.rules_data.forbidden_users, function (index, member_data) { - self.forbidUsers.push(member_data) - }); - } - - self.$rulesList.append( - self.addRule( - _gettext('Commit Authors are not allowed to be a reviewer.')) - ) - } - - // we don't have any rules set, so we inform users about it - if (self.enabledRules.length === 0) { - self.addRule( - _gettext('No review rules set.')) - } - return self.forbidUsers }; @@ -1066,14 +1017,14 @@ window.ReviewerPresenceController = func this.handlePresence = function (data) { if (data.type == 'presence' && data.channel === self.channel) { this.storeUsers(data.users); - this.render() + this.render(); } }; this.handleChannelUpdate = function (data) { if (data.channel === this.channel) { this.storeUsers(data.state.users); - this.render() + this.render(); } }; @@ -1085,6 +1036,30 @@ window.ReviewerPresenceController = func }; +window.refreshCommentsSuccess = function(targetNode, counterNode, extraCallback) { + var $targetElem = targetNode; + var $counterElem = counterNode; + + return function (data) { + var newCount = $(data).data('counter'); + if (newCount !== undefined) { + var callback = function () { + $counterElem.animate({'opacity': 1.00}, 200) + $counterElem.html(newCount); + }; + $counterElem.animate({'opacity': 0.15}, 200, callback); + } + + $targetElem.css('opacity', 1); + $targetElem.html(data); + tooltipActivate(); + + if (extraCallback !== undefined) { + extraCallback(data) + } + } +} + window.refreshComments = function (version) { version = version || templateContext.pull_request_data.pull_request_version || ''; @@ -1109,23 +1084,8 @@ window.refreshComments = function (versi var $targetElem = $('.comments-content-table'); $targetElem.css('opacity', 0.3); - - var success = function (data) { - var $counterElem = $('#comments-count'); - var newCount = $(data).data('counter'); - if (newCount !== undefined) { - var callback = function () { - $counterElem.animate({'opacity': 1.00}, 200) - $counterElem.html(newCount); - }; - $counterElem.animate({'opacity': 0.15}, 200, callback); - } - - $targetElem.css('opacity', 1); - $targetElem.html(data); - tooltipActivate(); - } - + var $counterElem = $('#comments-count'); + var success = refreshCommentsSuccess($targetElem, $counterElem); ajaxPOST(loadUrl, data, success, null, {}) } @@ -1139,7 +1099,7 @@ window.refreshTODOs = function (version) 'repo_name': templateContext.repo_name, 'version': version, }; - var loadUrl = pyroutes.url('pullrequest_comments', params); + var loadUrl = pyroutes.url('pullrequest_todos', params); } // commit case else { return @@ -1153,27 +1113,46 @@ window.refreshTODOs = function (version) var data = {"comments": currentIDs}; var $targetElem = $('.todos-content-table'); $targetElem.css('opacity', 0.3); - - var success = function (data) { - var $counterElem = $('#todos-count') - var newCount = $(data).data('counter'); - if (newCount !== undefined) { - var callback = function () { - $counterElem.animate({'opacity': 1.00}, 200) - $counterElem.html(newCount); - }; - $counterElem.animate({'opacity': 0.15}, 200, callback); - } - - $targetElem.css('opacity', 1); - $targetElem.html(data); - tooltipActivate(); - } + var $counterElem = $('#todos-count'); + var success = refreshCommentsSuccess($targetElem, $counterElem); ajaxPOST(loadUrl, data, success, null, {}) } +window.refreshDraftComments = function () { + + // Pull request case + if (templateContext.pull_request_data.pull_request_id !== null) { + var params = { + 'pull_request_id': templateContext.pull_request_data.pull_request_id, + 'repo_name': templateContext.repo_name, + }; + var loadUrl = pyroutes.url('pullrequest_drafts', params); + } // commit case + else { + return + } + + var data = {}; + + var $targetElem = $('.drafts-content-table'); + $targetElem.css('opacity', 0.3); + var $counterElem = $('#drafts-count'); + var extraCallback = function(data) { + if ($(data).data('counter') == 0){ + $('#draftsTable').hide(); + } else { + $('#draftsTable').show(); + } + // uncheck on load the select all checkbox + $('[name=select_all_drafts]').prop('checked', 0); + } + var success = refreshCommentsSuccess($targetElem, $counterElem, extraCallback); + + ajaxPOST(loadUrl, data, success, null, {}) +}; + window.refreshAllComments = function (version) { version = version || templateContext.pull_request_data.pull_request_version || ''; diff --git a/rhodecode/public/js/topics_list.txt b/rhodecode/public/js/topics_list.txt --- a/rhodecode/public/js/topics_list.txt +++ b/rhodecode/public/js/topics_list.txt @@ -1,7 +1,5 @@ /__MAIN_APP__ - launched when rhodecode-app element is attached to DOM /plugins/__REGISTER__ - launched after the onDomReady() code from rhodecode.js is executed -/ui/plugins/code/anchor_focus - launched when rc starts to scroll on load to anchor on PR/Codeview -/ui/plugins/code/comment_form_built - launched when injectInlineForm() is executed and the form object is created /notifications - shows new event notifications /connection_controller/subscribe - subscribes user to new channels /connection_controller/presence - receives presence change messages diff --git a/rhodecode/subscribers.py b/rhodecode/subscribers.py --- a/rhodecode/subscribers.py +++ b/rhodecode/subscribers.py @@ -104,12 +104,6 @@ def add_request_user_context(event): request.environ['rc_req_id'] = req_id -def inject_app_settings(event): - settings = event.app.registry.settings - # inject info about available permissions - auth.set_available_permissions(settings) - - def scan_repositories_if_enabled(event): """ This is subscribed to the `pyramid.events.ApplicationCreated` event. It diff --git a/rhodecode/templates/admin/my_account/my_account_pullrequests.mako b/rhodecode/templates/admin/my_account/my_account_pullrequests.mako --- a/rhodecode/templates/admin/my_account/my_account_pullrequests.mako +++ b/rhodecode/templates/admin/my_account/my_account_pullrequests.mako @@ -46,6 +46,8 @@ $pullRequestListTable.DataTable({ processing: true, serverSide: true, + stateSave: true, + stateDuration: -1, ajax: { "url": "${h.route_path('my_account_pullrequests_data')}", "data": function (d) { @@ -119,6 +121,10 @@ if (data['owned']) { $(row).addClass('owned'); } + }, + "stateSaveParams": function (settings, data) { + data.search.search = ""; // Don't save search + data.start = 0; // don't save pagination } }); $pullRequestListTable.on('xhr.dt', function (e, settings, json, xhr) { @@ -129,6 +135,7 @@ $pullRequestListTable.css('opacity', 0.3); }); + // filter $('#q_filter').on('keyup', $.debounce(250, function () { diff --git a/rhodecode/templates/base/base.mako b/rhodecode/templates/base/base.mako --- a/rhodecode/templates/base/base.mako +++ b/rhodecode/templates/base/base.mako @@ -1225,6 +1225,14 @@ (function () { "use sctrict"; + // details block auto-hide menu + $(document).mouseup(function(e) { + var container = $('.details-inline-block'); + if (!container.is(e.target) && container.has(e.target).length === 0) { + $('.details-inline-block[open]').removeAttr('open') + } + }); + var $sideBar = $('.right-sidebar'); var expanded = $sideBar.hasClass('right-sidebar-expanded'); var sidebarState = templateContext.session_attrs.sidebarState; diff --git a/rhodecode/templates/base/sidebar.mako b/rhodecode/templates/base/sidebar.mako --- a/rhodecode/templates/base/sidebar.mako +++ b/rhodecode/templates/base/sidebar.mako @@ -4,7 +4,7 @@ ## ${sidebar.comments_table()} <%namespace name="base" file="/base/base.mako"/> -<%def name="comments_table(comments, counter_num, todo_comments=False, existing_ids=None, is_pr=True)"> +<%def name="comments_table(comments, counter_num, todo_comments=False, draft_comments=False, existing_ids=None, is_pr=True)"> <% if todo_comments: cls_ = 'todos-content-table' @@ -15,10 +15,13 @@ # own comments first user_id = 0 return '{}'.format(str(entry.comment_id).zfill(10000)) + elif draft_comments: + cls_ = 'drafts-content-table' + def sorter(entry): + return '{}'.format(str(entry.comment_id).zfill(10000)) else: cls_ = 'comments-content-table' def sorter(entry): - user_id = entry.author.user_id return '{}'.format(str(entry.comment_id).zfill(10000)) existing_ids = existing_ids or [] @@ -31,8 +34,12 @@ <% display = '' _cls = '' + ## Extra precaution to not show drafts in the sidebar for todo/comments + if comment_obj.draft and not draft_comments: + continue %> + <% comment_ver_index = comment_obj.get_index_version(getattr(c, 'versions', [])) prev_comment_ver_index = 0 @@ -83,6 +90,11 @@ % endif + % if draft_comments: + + ${h.checkbox('submit_draft', id=None, value=comment_obj.comment_id)} + + % endif <% version_info = '' diff --git a/rhodecode/templates/changeset/changeset.mako b/rhodecode/templates/changeset/changeset.mako --- a/rhodecode/templates/changeset/changeset.mako +++ b/rhodecode/templates/changeset/changeset.mako @@ -24,8 +24,6 @@ <%def name="main()"> diff --git a/rhodecode/templates/changeset/changeset_comment_block.mako b/rhodecode/templates/changeset/changeset_comment_block.mako --- a/rhodecode/templates/changeset/changeset_comment_block.mako +++ b/rhodecode/templates/changeset/changeset_comment_block.mako @@ -1,4 +1,4 @@ ## this is a dummy html file for partial rendering on server and sending ## generated output via ajax after comment submit <%namespace name="comment" file="/changeset/changeset_file_comment.mako"/> -${comment.comment_block(c.co, inline=c.co.is_inline)} +${comment.comment_block(c.co, inline=c.co.is_inline, is_new=c.is_new)} diff --git a/rhodecode/templates/changeset/changeset_file_comment.mako b/rhodecode/templates/changeset/changeset_file_comment.mako --- a/rhodecode/templates/changeset/changeset_file_comment.mako +++ b/rhodecode/templates/changeset/changeset_file_comment.mako @@ -3,20 +3,25 @@ ## <%namespace name="comment" file="/changeset/changeset_file_comment.mako"/> ## ${comment.comment_block(comment)} ## +<%namespace name="base" file="/base/base.mako"/> <%! from rhodecode.lib import html_filters %> -<%namespace name="base" file="/base/base.mako"/> -<%def name="comment_block(comment, inline=False, active_pattern_entries=None)"> + +<%def name="comment_block(comment, inline=False, active_pattern_entries=None, is_new=False)"> <% - from rhodecode.model.comment import CommentsModel - comment_model = CommentsModel() + from rhodecode.model.comment import CommentsModel + comment_model = CommentsModel() + + comment_ver = comment.get_index_version(getattr(c, 'versions', [])) + latest_ver = len(getattr(c, 'versions', [])) + visible_for_user = True + if comment.draft: + visible_for_user = comment.user_id == c.rhodecode_user.user_id %> - <% comment_ver = comment.get_index_version(getattr(c, 'versions', [])) %> - <% latest_ver = len(getattr(c, 'versions', [])) %> % if inline: <% outdated_at_ver = comment.outdated_at_version(c.at_version_num) %> @@ -24,6 +29,7 @@ <% outdated_at_ver = comment.older_than_version(c.at_version_num) %> % endif + % if visible_for_user:
+ % if comment.draft: +
+ DRAFT +
+ % elif is_new: +
+ NEW +
+ % endif +
## TODO COMMENT @@ -90,7 +108,7 @@
- + ## NOTE 0 and .. => because we disable it for now until UI ready % if 0 and comment.status_change:
@@ -100,10 +118,12 @@
% endif - + ## Since only author can see drafts, we don't show it + % if not comment.draft:
${base.gravatar_with_user(comment.author.email, 16, tooltip=True)}
+ % endif
${h.age_component(comment.modified_at, time_is_local=True)} @@ -164,7 +184,7 @@ % if inline: % if outdated_at_ver: - outdated ${'v{}'.format(comment_ver)} + outdated ${'v{}'.format(comment_ver)} | % elif comment_ver: ${'v{}'.format(comment_ver)} @@ -210,11 +230,17 @@ %if comment.immutable is False and (c.is_super_admin or h.HasRepoPermissionAny('repository.admin')(c.repo_name) or comment.author.user_id == c.rhodecode_user.user_id):
- ${_('Edit')} + ${_('Edit')}
${_('Delete')}
+ ## Only available in EE edition + % if comment.draft and c.rhodecode_edition_id == 'EE': +
+ ${_('Submit draft')} +
+ % endif %else:
@@ -252,6 +278,7 @@
+ % endif ## generate main comments @@ -298,10 +325,9 @@ ## inject form here
+ + + + ##// END OF EJS Templates
diff --git a/rhodecode/templates/email_templates/base.mako b/rhodecode/templates/email_templates/base.mako --- a/rhodecode/templates/email_templates/base.mako +++ b/rhodecode/templates/email_templates/base.mako @@ -337,7 +337,6 @@ text_monospace = "'Menlo', 'Liberation M div.markdown-block img { border-style: none; background-color: #fff; - padding-right: 20px; max-width: 100% } @@ -395,6 +394,13 @@ text_monospace = "'Menlo', 'Liberation M background-color: #eeeeee } + div.markdown-block p { + margin-top: 0; + margin-bottom: 16px; + padding: 0; + line-height: unset; + } + div.markdown-block code, div.markdown-block pre, div.markdown-block #ws, diff --git a/rhodecode/templates/errors/error_document.mako b/rhodecode/templates/errors/error_document.mako --- a/rhodecode/templates/errors/error_document.mako +++ b/rhodecode/templates/errors/error_document.mako @@ -82,8 +82,8 @@

Exception ID: ${c.exception_id}
- Super-admins can see detailed traceback information from this exception by checking the below Exception ID.
- Please include the above link for further details of this exception. + Super-admins can see details of the above error in the exception tracker found under + admin > settings > exception tracker.

% endif diff --git a/rhodecode/templates/files/files_browser.mako b/rhodecode/templates/files/files_browser.mako --- a/rhodecode/templates/files/files_browser.mako +++ b/rhodecode/templates/files/files_browser.mako @@ -29,7 +29,7 @@
-