reviewers: added observers as another way to define reviewers.

- fixes  #4183

rhodecode/lib/dbmigrate/versions/110_version_4_22_0.py

@@ -0,0 +1,68 b''
+# -*- 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.RepoReviewRuleUser.__table__
+    with op.batch_alter_table(table.name) as batch_op:
+        new_column = Column('role', Unicode(255), nullable=True)
+        batch_op.add_column(new_column)
+
+    _fill_rule_user_role(op, meta.Session)
+
+    table = db.RepoReviewRuleUserGroup.__table__
+    with op.batch_alter_table(table.name) as batch_op:
+        new_column = Column('role', Unicode(255), nullable=True)
+        batch_op.add_column(new_column)
+
+    _fill_rule_user_group_role(op, meta.Session)
+
+
+def downgrade(migrate_engine):
+    meta = MetaData()
+    meta.bind = migrate_engine
+
+
+def fixups(models, _SESSION):
+    pass
+
+
+def _fill_rule_user_role(op, session):
+    params = {'role': 'reviewer'}
+    query = text(
+        'UPDATE repo_review_rules_users SET role = :role'
+    ).bindparams(**params)
+    op.execute(query)
+    session().commit()
+
+
+def _fill_rule_user_group_role(op, session):
+    params = {'role': 'reviewer'}
+    query = text(
+        'UPDATE repo_review_rules_users_groups SET role = :role'
+    ).bindparams(**params)
+    op.execute(query)
+    session().commit()

rhodecode/lib/string_renderer.py

@@ -0,0 +1,35 b''
+# -*- coding: utf-8 -*-
+
+# Copyright (C) 2010-2020 RhodeCode GmbH
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License, version 3
+# (only), as published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+# This program is dual-licensed. If you wish to learn more about the
+# RhodeCode Enterprise Edition, including its added features, Support services,
+# and proprietary license terms, please see https://rhodecode.com/licenses/
+
+
+def html(info):
+    """
+    Custom string as html content_type renderer for pyramid
+    """
+    def _render(value, system):
+        request = system.get('request')
+        if request is not None:
+            response = request.response
+            ct = response.content_type
+            if ct == response.default_content_type:
+                response.content_type = 'text/html'
+        return value
+
+    return _render

rhodecode/__init__.py

@@ -48,7 +48,7 b' PYRAMID_SETTINGS = {}'
 EXTENSIONS = {}
 
 __version__ = ('.'.join((str(each) for each in VERSION[:3])))
-__dbversion__ = 109  # defines current db version for migrations
+__dbversion__ = 110  # defines current db version for migrations
 __platform__ = platform.system()
 __license__ = 'AGPLv3, and Commercial License'
 __author__ = 'RhodeCode GmbH'

rhodecode/api/views/pull_request_api.py

@@ -704,7 +704,7 b' def create_pull_request('
             user = get_user_or_error(reviewer_object['username'])
             reviewer_object['user_id'] = user.user_id
 
-    get_default_reviewers_data, validate_default_reviewers = \
+    get_default_reviewers_data, validate_default_reviewers, validate_observers = \
         PullRequestModel().get_reviewer_functions()
 
     # recalculate reviewers logic, to make sure we can validate this
@@ -865,14 +865,13 b' def update_pull_request('
             user = get_user_or_error(reviewer_object['username'])
             reviewer_object['user_id'] = user.user_id
 
-        get_default_reviewers_data, get_validated_reviewers = \
+        get_default_reviewers_data, get_validated_reviewers, validate_observers = \
             PullRequestModel().get_reviewer_functions()
 
         # re-use stored rules
         reviewer_rules = pull_request.reviewer_data
         try:
-            reviewers = get_validated_reviewers(
-                reviewer_objects, reviewer_rules)
+            reviewers = get_validated_reviewers(reviewer_objects, reviewer_rules)
         except ValueError as e:
             raise JSONRPCError('Reviewers Validation: {}'.format(e))
     else:

rhodecode/apps/debug_style/views.py

@@ -34,6 +34,7 b' log = logging.getLogger(__name__)'
 
 
 class DebugStyleView(BaseAppView):
+
     def load_default_context(self):
         c = self._get_local_tmpl_context()
 
@@ -75,6 +76,7 b' Check if we should use full-topic or min'
             source_ref_parts=AttributeDict(type='branch', name='fix-ticket-2000'),
             target_ref_parts=AttributeDict(type='branch', name='master'),
         )
+
         target_repo = AttributeDict(repo_name='repo_group/target_repo')
         source_repo = AttributeDict(repo_name='repo_group/source_repo')
         user = User.get_by_username(self.request.GET.get('user')) or self._rhodecode_db_user
@@ -83,6 +85,7 b' Check if we should use full-topic or min'
             'added': ['aaaaaaabbbbb', 'cccccccddddddd'],
             'removed': ['eeeeeeeeeee'],
         })
+
         file_changes = AttributeDict({
             'added': ['a/file1.md', 'file2.py'],
             'modified': ['b/modified_file.rst'],
@@ -97,15 +100,19 b' Check if we should use full-topic or min'
             'exc_message': 'Traceback (most recent call last):\n  File "/nix/store/s43k2r9rysfbzmsjdqnxgzvvb7zjhkxb-python2.7-pyramid-1.10.4/lib/python2.7/site-packages/pyramid/tweens.py", line 41, in excview_tween\n    response = handler(request)\n  File "/nix/store/s43k2r9rysfbzmsjdqnxgzvvb7zjhkxb-python2.7-pyramid-1.10.4/lib/python2.7/site-packages/pyramid/router.py", line 148, in handle_request\n    registry, request, context, context_iface, view_name\n  File "/nix/store/s43k2r9rysfbzmsjdqnxgzvvb7zjhkxb-python2.7-pyramid-1.10.4/lib/python2.7/site-packages/pyramid/view.py", line 667, in _call_view\n    response = view_callable(context, request)\n  File "/nix/store/s43k2r9rysfbzmsjdqnxgzvvb7zjhkxb-python2.7-pyramid-1.10.4/lib/python2.7/site-packages/pyramid/config/views.py", line 188, in attr_view\n    return view(context, request)\n  File "/nix/store/s43k2r9rysfbzmsjdqnxgzvvb7zjhkxb-python2.7-pyramid-1.10.4/lib/python2.7/site-packages/pyramid/config/views.py", line 214, in predicate_wrapper\n    return view(context, request)\n  File "/nix/store/s43k2r9rysfbzmsjdqnxgzvvb7zjhkxb-python2.7-pyramid-1.10.4/lib/python2.7/site-packages/pyramid/viewderivers.py", line 401, in viewresult_to_response\n    result = view(context, request)\n  File "/nix/store/s43k2r9rysfbzmsjdqnxgzvvb7zjhkxb-python2.7-pyramid-1.10.4/lib/python2.7/site-packages/pyramid/viewderivers.py", line 132, in _class_view\n    response = getattr(inst, attr)()\n  File "/mnt/hgfs/marcink/workspace/rhodecode-enterprise-ce/rhodecode/apps/debug_style/views.py", line 355, in render_email\n    template_type, **email_kwargs.get(email_id, {}))\n  File "/mnt/hgfs/marcink/workspace/rhodecode-enterprise-ce/rhodecode/model/notification.py", line 402, in render_email\n    body = email_template.render(None, **_kwargs)\n  File "/mnt/hgfs/marcink/workspace/rhodecode-enterprise-ce/rhodecode/lib/partial_renderer.py", line 95, in render\n    return self._render_with_exc(tmpl, args, kwargs)\n  File "/mnt/hgfs/marcink/workspace/rhodecode-enterprise-ce/rhodecode/lib/partial_renderer.py", line 79, in _render_with_exc\n    return render_func.render(*args, **kwargs)\n  File "/nix/store/dakh34sxz4yfr435c0cwjz0sd6hnd5g3-python2.7-mako-1.1.0/lib/python2.7/site-packages/mako/template.py", line 476, in render\n    return runtime._render(self, self.callable_, args, data)\n  File "/nix/store/dakh34sxz4yfr435c0cwjz0sd6hnd5g3-python2.7-mako-1.1.0/lib/python2.7/site-packages/mako/runtime.py", line 883, in _render\n    **_kwargs_for_callable(callable_, data)\n  File "/nix/store/dakh34sxz4yfr435c0cwjz0sd6hnd5g3-python2.7-mako-1.1.0/lib/python2.7/site-packages/mako/runtime.py", line 920, in _render_context\n    _exec_template(inherit, lclcontext, args=args, kwargs=kwargs)\n  File "/nix/store/dakh34sxz4yfr435c0cwjz0sd6hnd5g3-python2.7-mako-1.1.0/lib/python2.7/site-packages/mako/runtime.py", line 947, in _exec_template\n    callable_(context, *args, **kwargs)\n  File "rhodecode_templates_email_templates_base_mako", line 63, in render_body\n  File "rhodecode_templates_email_templates_exception_tracker_mako", line 43, in render_body\nAttributeError: \'str\' object has no attribute \'get\'\n',
             'exc_type': 'AttributeError'
         }
+
         email_kwargs = {
             'test': {},
+
             'message': {
                 'body': 'message body !'
             },
+
             'email_test': {
                 'user': user,
                 'date': datetime.datetime.now(),
             },
+
             'exception': {
                 'email_prefix': '[RHODECODE ERROR]',
                 'exc_id': exc_traceback['exc_id'],
@@ -113,6 +120,7 b' Check if we should use full-topic or min'
                 'exc_type_name': 'NameError',
                 'exc_traceback': exc_traceback,
             },
+
             'password_reset': {
                 'password_reset_url': 'http://example.com/reset-rhodecode-password/token',
 
@@ -121,6 +129,7 b' Check if we should use full-topic or min'
                 'email': '[e-mail]',
                 'first_admin_email': User.get_first_super_admin().email
             },
+
             'password_reset_confirmation': {
                 'new_password': 'new-password-example',
                 'user': user,
@@ -128,6 +137,7 b' Check if we should use full-topic or min'
                 'email': '[e-mail]',
                 'first_admin_email': User.get_first_super_admin().email
             },
+
             'registration': {
                 'user': user,
                 'date': datetime.datetime.now(),
@@ -161,6 +171,7 b' Check if we should use full-topic or min'
                 'mention': True,
 
             },
+
             'pull_request_comment+status': {
                 'user': user,
 
@@ -201,6 +212,7 b' def db():'
                 'mention': True,
 
             },
+
             'pull_request_comment+file': {
                 'user': user,
 
@@ -303,6 +315,7 b' This should work better !'
                 'renderer_type': 'markdown',
                 'mention': True,
             },
+
             'cs_comment+status': {
                 'user': user,
                 'commit': AttributeDict(idx=123, raw_id='a' * 40, message='Commit message'),
@@ -328,6 +341,7 b' This is a multiline comment :)'
                 'renderer_type': 'markdown',
                 'mention': True,
             },
+
             'cs_comment+file': {
                 'user': user,
                 'commit': AttributeDict(idx=123, raw_id='a' * 40, message='Commit message'),
@@ -371,8 +385,58 b' users: description edit fixes'
                 'pull_request_source_repo_url': 'http://source-repo/url',
 
                 'pull_request_url': 'http://code.rhodecode.com/_pull-request/123',
+                'user_role': 'reviewer',
+            },
+
+            'pull_request+reviewer_role': {
+                'user': user,
+                'pull_request': pr,
+                'pull_request_commits': [
+                    ('472d1df03bf7206e278fcedc6ac92b46b01c4e21', '''\
+my-account: moved email closer to profile as it's similar data just moved outside.                    
+                    '''),
+                    ('cbfa3061b6de2696c7161ed15ba5c6a0045f90a7', '''\
+users: description edit fixes
+
+- tests
+- added metatags info                    
+                    '''),
+                ],
+
+                'pull_request_target_repo': target_repo,
+                'pull_request_target_repo_url': 'http://target-repo/url',
+
+                'pull_request_source_repo': source_repo,
+                'pull_request_source_repo_url': 'http://source-repo/url',
+
+                'pull_request_url': 'http://code.rhodecode.com/_pull-request/123',
+                'user_role': 'reviewer',
+            },
+
+            'pull_request+observer_role': {
+                'user': user,
+                'pull_request': pr,
+                'pull_request_commits': [
+                    ('472d1df03bf7206e278fcedc6ac92b46b01c4e21', '''\
+        my-account: moved email closer to profile as it's similar data just moved outside.                    
+                            '''),
+                    ('cbfa3061b6de2696c7161ed15ba5c6a0045f90a7', '''\
+        users: description edit fixes
+
+        - tests
+        - added metatags info                    
+                            '''),
+                ],
+
+                'pull_request_target_repo': target_repo,
+                'pull_request_target_repo_url': 'http://target-repo/url',
+
+                'pull_request_source_repo': source_repo,
+                'pull_request_source_repo_url': 'http://source-repo/url',
+
+                'pull_request_url': 'http://code.rhodecode.com/_pull-request/123',
+                'user_role': 'observer'
             }
-
         }
 
         template_type = email_id.split('+')[0]
@@ -401,6 +465,7 b' users: description edit fixes'
         c = self.load_default_context()
         c.active = os.path.splitext(t_path)[0]
         c.came_from = ''
+        # NOTE(marcink): extend the email types with variations based on data sets
         c.email_types = {
             'cs_comment+file': {},
             'cs_comment+status': {},
@@ -409,6 +474,9 b' users: description edit fixes'
             'pull_request_comment+status': {},
 
             'pull_request_update': {},
+
+            'pull_request+reviewer_role': {},
+            'pull_request+observer_role': {},
         }
         c.email_types.update(EmailNotificationModel.email_types)
 

rhodecode/apps/repository/utils.py

@@ -21,11 +21,13 b''
 from rhodecode.lib import helpers as h
 from rhodecode.lib.utils2 import safe_int
 from rhodecode.model.pull_request import get_diff_info
-
-REVIEWER_API_VERSION = 'V3'
+from rhodecode.model.db import PullRequestReviewers
+# V3 - Reviewers, with default rules data
+# v4 - Added observers metadata
+REVIEWER_API_VERSION = 'V4'
 
 
-def reviewer_as_json(user, reasons=None, mandatory=False, rules=None, user_group=None):
+def reviewer_as_json(user, reasons=None, role=None, mandatory=False, rules=None, user_group=None):
     """
     Returns json struct of a reviewer for frontend
 
@@ -33,11 +35,15 b' def reviewer_as_json(user, reasons=None,'
     :param reasons: list of strings of why they are reviewers
     :param mandatory: bool, to set user as mandatory
     """
+    role = role or PullRequestReviewers.ROLE_REVIEWER
+    if role not in PullRequestReviewers.ROLES:
+        raise ValueError('role is not one of %s', PullRequestReviewers.ROLES)
 
     return {
         'user_id': user.user_id,
         'reasons': reasons or [],
         'rules': rules or [],
+        'role': role,
         'mandatory': mandatory,
         'user_group': user_group,
         'username': user.username,
@@ -48,8 +54,7 b' def reviewer_as_json(user, reasons=None,'
     }
 
 
-def get_default_reviewers_data(
-        current_user, source_repo, source_commit, target_repo, target_commit):
+def get_default_reviewers_data(current_user, source_repo, source_commit, target_repo, target_commit):
     """
     Return json for default reviewers of a repository
     """
@@ -59,7 +64,7 b' def get_default_reviewers_data('
 
     reasons = ['Default reviewer', 'Repository owner']
     json_reviewers = [reviewer_as_json(
-        user=target_repo.user, reasons=reasons, mandatory=False, rules=None)]
+        user=target_repo.user, reasons=reasons, mandatory=False, rules=None, role=None)]
 
     return {
         'api_ver': REVIEWER_API_VERSION,  # define version for later possible schema upgrade
@@ -73,15 +78,18 b' def get_default_reviewers_data('
 def validate_default_reviewers(review_members, reviewer_rules):
     """
     Function to validate submitted reviewers against the saved rules
-
     """
     reviewers = []
     reviewer_by_id = {}
     for r in review_members:
         reviewer_user_id = safe_int(r['user_id'])
-        entry = (reviewer_user_id, r['reasons'], r['mandatory'], r['rules'])
+        entry = (reviewer_user_id, r['reasons'], r['mandatory'], r['role'], r['rules'])
 
         reviewer_by_id[reviewer_user_id] = entry
         reviewers.append(entry)
 
     return reviewers
+
+
+def validate_observers(observer_members):
+    return {}

rhodecode/apps/repository/views/repo_commits.py

@@ -193,7 +193,7 b' class RepoCommitsView(RepoAppView):'
 
             for review_obj, member, reasons, mandatory, status in review_statuses:
                 member_reviewer = h.reviewer_as_json(
-                    member, reasons=reasons, mandatory=mandatory,
+                    member, reasons=reasons, mandatory=mandatory, role=None,
                     user_group=None
                 )
 

rhodecode/apps/repository/views/repo_pull_requests.py

@@ -39,14 +39,15 b' from rhodecode.lib.ext_json import json'
 from rhodecode.lib.auth import (
     LoginRequired, HasRepoPermissionAny, HasRepoPermissionAnyDecorator,
     NotAnonymous, CSRFRequired)
-from rhodecode.lib.utils2 import str2bool, safe_str, safe_unicode, safe_int
+from rhodecode.lib.utils2 import str2bool, safe_str, safe_unicode, safe_int, aslist
 from rhodecode.lib.vcs.backends.base import EmptyCommit, UpdateFailureReason
 from rhodecode.lib.vcs.exceptions import (
     CommitDoesNotExistError, RepositoryRequirementError, EmptyRepositoryError)
 from rhodecode.model.changeset_status import ChangesetStatusModel
 from rhodecode.model.comment import CommentsModel
 from rhodecode.model.db import (
-    func, or_, PullRequest, ChangesetComment, ChangesetStatus, Repository)
+    func, or_, PullRequest, ChangesetComment, ChangesetStatus, Repository,
+    PullRequestReviewers)
 from rhodecode.model.forms import PullRequestForm
 from rhodecode.model.meta import Session
 from rhodecode.model.pull_request import PullRequestModel, MergeCheck
@@ -455,14 +456,18 b' class RepoPullRequestsView(RepoAppView, '
             return self._get_template_context(c)
 
         c.allowed_reviewers = [obj.user_id for obj in pull_request.reviewers if obj.user]
+        c.reviewers_count = pull_request.reviewers_count
+        c.observers_count = pull_request.observers_count
 
         # reviewers and statuses
         c.pull_request_default_reviewers_data_json = json.dumps(pull_request.reviewer_data)
         c.pull_request_set_reviewers_data_json = collections.OrderedDict({'reviewers': []})
+        c.pull_request_set_observers_data_json = collections.OrderedDict({'observers': []})
 
         for review_obj, member, reasons, mandatory, status in pull_request_at_ver.reviewers_statuses():
             member_reviewer = h.reviewer_as_json(
                 member, reasons=reasons, mandatory=mandatory,
+                role=review_obj.role,
                 user_group=review_obj.rule_user_group_data()
             )
 
@@ -474,6 +479,17 b' class RepoPullRequestsView(RepoAppView, '
 
         c.pull_request_set_reviewers_data_json = json.dumps(c.pull_request_set_reviewers_data_json)
 
+        for observer_obj, member in pull_request_at_ver.observers():
+            member_observer = h.reviewer_as_json(
+                member, reasons=[], mandatory=False,
+                role=observer_obj.role,
+                user_group=observer_obj.rule_user_group_data()
+            )
+            member_observer['allowed_to_update'] = c.allowed_to_update
+            c.pull_request_set_observers_data_json['observers'].append(member_observer)
+
+        c.pull_request_set_observers_data_json = json.dumps(c.pull_request_set_observers_data_json)
+
         general_comments, inline_comments = \
             self.register_comments_vars(c, pull_request_latest, versions)
 
@@ -967,7 +983,7 b' class RepoPullRequestsView(RepoAppView, '
         'repository.read', 'repository.write', 'repository.admin')
     @view_config(
         route_name='pullrequest_comments', request_method='POST',
-        renderer='string', xhr=True)
+        renderer='string_html', xhr=True)
     def pullrequest_comments(self):
         self.load_default_context()
 
@@ -998,7 +1014,8 b' class RepoPullRequestsView(RepoAppView, '
         all_comments = c.inline_comments_flat + c.comments
 
         existing_ids = filter(
-            lambda e: e, map(safe_int, self.request.POST.getall('comments[]')))
+            lambda e: e, map(safe_int, aslist(self.request.POST.get('comments'))))
+
         return _render('comments_table', all_comments, len(all_comments),
                        existing_ids=existing_ids)
 
@@ -1008,7 +1025,7 b' class RepoPullRequestsView(RepoAppView, '
         'repository.read', 'repository.write', 'repository.admin')
     @view_config(
         route_name='pullrequest_todos', request_method='POST',
-        renderer='string', xhr=True)
+        renderer='string_html', xhr=True)
     def pullrequest_todos(self):
         self.load_default_context()
 
@@ -1138,7 +1155,7 b' class RepoPullRequestsView(RepoAppView, '
         target_ref_type, target_ref_name, __ = _form['target_ref'].split(':')
         target_ref = ':'.join((target_ref_type, target_ref_name, ancestor))
 
-        get_default_reviewers_data, validate_default_reviewers = \
+        get_default_reviewers_data, validate_default_reviewers, validate_observers = \
             PullRequestModel().get_reviewer_functions()
 
         # recalculate reviewers logic, to make sure we can validate this
@@ -1146,9 +1163,8 b' class RepoPullRequestsView(RepoAppView, '
             self._rhodecode_db_user, source_db_repo,
             source_commit, target_db_repo, target_commit)
 
-        given_reviewers = _form['review_members']
-        reviewers = validate_default_reviewers(
-            given_reviewers, reviewer_rules)
+        reviewers = validate_default_reviewers(_form['review_members'], reviewer_rules)
+        observers = validate_observers(_form['observer_members'], reviewer_rules)
 
         pullrequest_title = _form['pullrequest_title']
         title_source_ref = source_ref.split(':', 2)[1]
@@ -1172,6 +1188,7 b' class RepoPullRequestsView(RepoAppView, '
                 revisions=commit_ids,
                 common_ancestor_id=common_ancestor_id,
                 reviewers=reviewers,
+                observers=observers,
                 title=pullrequest_title,
                 description=description,
                 description_renderer=description_renderer,
@@ -1227,14 +1244,23 b' class RepoPullRequestsView(RepoAppView, '
         # only owner or admin can update it
         allowed_to_update = PullRequestModel().check_user_update(
             pull_request, self._rhodecode_user)
+
         if allowed_to_update:
             controls = peppercorn.parse(self.request.POST.items())
             force_refresh = str2bool(self.request.POST.get('force_refresh'))
 
             if 'review_members' in controls:
                 self._update_reviewers(
+                    c,
                     pull_request, controls['review_members'],
-                    pull_request.reviewer_data)
+                    pull_request.reviewer_data,
+                    PullRequestReviewers.ROLE_REVIEWER)
+            elif 'observer_members' in controls:
+                self._update_reviewers(
+                    c,
+                    pull_request, controls['observer_members'],
+                    pull_request.reviewer_data,
+                    PullRequestReviewers.ROLE_OBSERVER)
             elif str2bool(self.request.POST.get('update_commits', 'false')):
                 if is_state_changing:
                     log.debug('commits update: forbidden because pull request is in state %s',
@@ -1255,6 +1281,7 b' class RepoPullRequestsView(RepoAppView, '
             elif str2bool(self.request.POST.get('edit_pull_request', 'false')):
                 self._edit_pull_request(pull_request)
             else:
+                log.error('Unhandled update data.')
                 raise HTTPBadRequest()
 
             return {'response': True,
@@ -1262,6 +1289,9 b' class RepoPullRequestsView(RepoAppView, '
         raise HTTPForbidden()
 
     def _edit_pull_request(self, pull_request):
+        """
+        Edit title and description
+        """
         _ = self.request.translate
 
         try:
@@ -1302,27 +1332,14 b' class RepoPullRequestsView(RepoAppView, '
 
             msg = _(u'Pull request updated to "{source_commit_id}" with '
                     u'{count_added} added, {count_removed} removed commits. '
-                    u'Source of changes: {change_source}')
+                    u'Source of changes: {change_source}.')
             msg = msg.format(
                 source_commit_id=pull_request.source_ref_parts.commit_id,
                 count_added=len(resp.changes.added),
                 count_removed=len(resp.changes.removed),
                 change_source=changed)
             h.flash(msg, category='success')
-
-            message = msg + (
-                ' - <a onclick="window.location.reload()">'
-                '<strong>{}</strong></a>'.format(_('Reload page')))
-
-            message_obj = {
-                'message': message,
-                'level': 'success',
-                'topic': '/notifications'
-            }
-
-            channelstream.post_message(
-                c.pr_broadcast_channel, message_obj, self._rhodecode_user.username,
-                registry=self.request.registry)
+            self._pr_update_channelstream_push(c.pr_broadcast_channel, msg)
         else:
             msg = PullRequestModel.UPDATE_STATUS_MESSAGES[resp.reason]
             warning_reasons = [
@@ -1332,6 +1349,53 b' class RepoPullRequestsView(RepoAppView, '
             category = 'warning' if resp.reason in warning_reasons else 'error'
             h.flash(msg, category=category)
 
+    def _update_reviewers(self, c, pull_request, review_members, reviewer_rules, role):
+        _ = self.request.translate
+
+        get_default_reviewers_data, validate_default_reviewers, validate_observers = \
+            PullRequestModel().get_reviewer_functions()
+
+        if role == PullRequestReviewers.ROLE_REVIEWER:
+            try:
+                reviewers = validate_default_reviewers(review_members, reviewer_rules)
+            except ValueError as e:
+                log.error('Reviewers Validation: {}'.format(e))
+                h.flash(e, category='error')
+                return
+
+            old_calculated_status = pull_request.calculated_review_status()
+            PullRequestModel().update_reviewers(
+                pull_request, reviewers, self._rhodecode_user)
+
+            Session().commit()
+
+            msg = _('Pull request reviewers updated.')
+            h.flash(msg, category='success')
+            self._pr_update_channelstream_push(c.pr_broadcast_channel, msg)
+
+            # trigger status changed if change in reviewers changes the 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})
+
+        elif role == PullRequestReviewers.ROLE_OBSERVER:
+            try:
+                observers = validate_observers(review_members, reviewer_rules)
+            except ValueError as e:
+                log.error('Observers Validation: {}'.format(e))
+                h.flash(e, category='error')
+                return
+
+            PullRequestModel().update_observers(
+                pull_request, observers, self._rhodecode_user)
+
+            Session().commit()
+            msg = _('Pull request observers updated.')
+            h.flash(msg, category='success')
+            self._pr_update_channelstream_push(c.pr_broadcast_channel, msg)
+
     @LoginRequired()
     @NotAnonymous()
     @HasRepoPermissionAnyDecorator(
@@ -1408,32 +1472,6 b' class RepoPullRequestsView(RepoAppView, '
             msg = merge_resp.merge_status_message
             h.flash(msg, category='error')
 
-    def _update_reviewers(self, pull_request, review_members, reviewer_rules):
-        _ = self.request.translate
-
-        get_default_reviewers_data, validate_default_reviewers = \
-            PullRequestModel().get_reviewer_functions()
-
-        try:
-            reviewers = validate_default_reviewers(review_members, reviewer_rules)
-        except ValueError as e:
-            log.error('Reviewers Validation: {}'.format(e))
-            h.flash(e, category='error')
-            return
-
-        old_calculated_status = pull_request.calculated_review_status()
-        PullRequestModel().update_reviewers(
-            pull_request, reviewers, self._rhodecode_user)
-        h.flash(_('Pull request reviewers updated.'), category='success')
-        Session().commit()
-
-        # trigger status changed if change in reviewers changes the 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})
-
     @LoginRequired()
     @NotAnonymous()
     @HasRepoPermissionAnyDecorator(
@@ -1488,8 +1526,7 b' class RepoPullRequestsView(RepoAppView, '
         allowed_to_comment = PullRequestModel().check_user_comment(
             pull_request, self._rhodecode_user)
         if not allowed_to_comment:
-            log.debug(
-                'comment: forbidden because pull request is from forbidden repo')
+            log.debug('comment: forbidden because pull request is from forbidden repo')
             raise HTTPForbidden()
 
         c = self.load_default_context()

rhodecode/config/middleware.py

@@ -341,6 +341,10 b' def includeme(config):'
         name='json_ext',
         factory='rhodecode.lib.ext_json_renderer.pyramid_ext_json')
 
+    config.add_renderer(
+        name='string_html',
+        factory='rhodecode.lib.string_renderer.html')
+
     # include RhodeCode plugins
     includes = aslist(settings.get('rhodecode.includes', []))
     for inc in includes:

rhodecode/lib/audit_logger.py

@@ -88,6 +88,9 b' ACTIONS_V1 = {'
     'repo.pull_request.reviewer.add': '',
     'repo.pull_request.reviewer.delete': '',
 
+    'repo.pull_request.observer.add': '',
+    'repo.pull_request.observer.delete': '',
+
     'repo.commit.strip': {'commit_id': ''},
     'repo.commit.comment.create': {'data': {}},
     'repo.commit.comment.delete': {'data': {}},

rhodecode/lib/helpers.py

@@ -1104,6 +1104,10 b' def bool2icon(value, show_at_false=True)'
             return HTML.tag('i', class_="icon-false", title='False')
         return HTML.tag('i')
 
+
+def b64(inp):
+    return base64.b64encode(inp)
+
 #==============================================================================
 # PERMS
 #==============================================================================

rhodecode/model/changeset_status.py

@@ -25,7 +25,7 b' import collections'
 
 from rhodecode.model import BaseModel
 from rhodecode.model.db import (
-    ChangesetStatus, ChangesetComment, PullRequest, Session)
+    ChangesetStatus, ChangesetComment, PullRequest, PullRequestReviewers, Session)
 from rhodecode.lib.exceptions import StatusChangeOnClosedPullRequestError
 from rhodecode.lib.markup_renderer import (
     DEFAULT_COMMENTS_RENDERER, RstTemplateRenderer)
@@ -383,15 +383,14 b' class ChangesetStatusModel(BaseModel):'
             pull_request.source_repo,
             pull_request=pull_request,
             with_revisions=True)
+        reviewers = pull_request.get_pull_request_reviewers(
+            role=PullRequestReviewers.ROLE_REVIEWER)
+        return self.aggregate_votes_by_user(_commit_statuses, reviewers)
 
-        return self.aggregate_votes_by_user(_commit_statuses, pull_request.reviewers)
-
-    def calculated_review_status(self, pull_request, reviewers_statuses=None):
+    def calculated_review_status(self, pull_request):
         """
         calculate pull request status based on reviewers, it should be a list
         of two element lists.
-
-        :param reviewers_statuses:
         """
-        reviewers = reviewers_statuses or self.reviewers_statuses(pull_request)
+        reviewers = self.reviewers_statuses(pull_request)
         return self.calculate_status(reviewers)

rhodecode/model/comment.py

@@ -436,9 +436,8 b' class CommentsModel(BaseModel):'
                 'thread_ids': [pr_url, pr_comment_url],
             })
 
+        if send_email:
         recipients += [self._get_user(u) for u in (extra_recipients or [])]
-
-        if send_email:
             # pre-generate the subject for notification itself
             (subject, _e, body_plaintext) = EmailNotificationModel().render_email(
                 notification_type, **kwargs)

rhodecode/model/db.py

@@ -4465,6 +4465,37 b' class PullRequest(Base, _PullRequestBase'
         from rhodecode.model.changeset_status import ChangesetStatusModel
         return ChangesetStatusModel().reviewers_statuses(self)
 
+    def get_pull_request_reviewers(self, role=None):
+        qry = PullRequestReviewers.query()\
+            .filter(PullRequestReviewers.pull_request_id == self.pull_request_id)
+        if role:
+            qry = qry.filter(PullRequestReviewers.role == role)
+
+        return qry.all()
+
+    @property
+    def reviewers_count(self):
+        qry = PullRequestReviewers.query()\
+            .filter(PullRequestReviewers.pull_request_id == self.pull_request_id)\
+            .filter(PullRequestReviewers.role == PullRequestReviewers.ROLE_REVIEWER)
+        return qry.count()
+
+    @property
+    def observers_count(self):
+        qry = PullRequestReviewers.query()\
+            .filter(PullRequestReviewers.pull_request_id == self.pull_request_id)\
+            .filter(PullRequestReviewers.role == PullRequestReviewers.ROLE_OBSERVER)
+        return qry.count()
+
+    def observers(self):
+        qry = PullRequestReviewers.query()\
+            .filter(PullRequestReviewers.pull_request_id == self.pull_request_id)\
+            .filter(PullRequestReviewers.role == PullRequestReviewers.ROLE_OBSERVER)\
+            .all()
+
+        for entry in qry:
+            yield entry, entry.user
+
     @property
     def workspace_id(self):
         from rhodecode.model.pull_request import PullRequestModel
@@ -4530,6 +4561,9 b' class PullRequestVersion(Base, _PullRequ'
     def reviewers_statuses(self):
         return self.pull_request.reviewers_statuses()
 
+    def observer(self):
+        return self.pull_request.observers()
+
 
 class PullRequestReviewers(Base, BaseModel):
     __tablename__ = 'pull_request_reviewers'
@@ -4538,6 +4572,7 b' class PullRequestReviewers(Base, BaseMod'
     )
     ROLE_REVIEWER = u'reviewer'
     ROLE_OBSERVER = u'observer'
+    ROLES = [ROLE_REVIEWER, ROLE_OBSERVER]
 
     @hybrid_property
     def reasons(self):
@@ -4589,6 +4624,15 b' class PullRequestReviewers(Base, BaseMod'
 
             return user_group_data
 
+    @classmethod
+    def get_pull_request_reviewers(cls, pull_request_id, role=None):
+        qry = PullRequestReviewers.query()\
+            .filter(PullRequestReviewers.pull_request_id == pull_request_id)
+        if role:
+            qry = qry.filter(PullRequestReviewers.role == role)
+
+        return qry.all()
+
     def __unicode__(self):
         return u"<%s('id:%s')>" % (self.__class__.__name__,
                                    self.pull_requests_reviewers_id)
@@ -4954,16 +4998,21 b' class RepoReviewRuleUser(Base, BaseModel'
     __table_args__ = (
         base_table_args
     )
+    ROLE_REVIEWER = u'reviewer'
+    ROLE_OBSERVER = u'observer'
+    ROLES = [ROLE_REVIEWER, ROLE_OBSERVER]
 
     repo_review_rule_user_id = Column('repo_review_rule_user_id', Integer(), primary_key=True)
     repo_review_rule_id = Column("repo_review_rule_id", Integer(), ForeignKey('repo_review_rules.repo_review_rule_id'))
     user_id = Column("user_id", Integer(), ForeignKey('users.user_id'), nullable=False)
     mandatory = Column("mandatory", Boolean(), nullable=False, default=False)
+    role = Column('role', Unicode(255), nullable=True, default=ROLE_REVIEWER)
     user = relationship('User')
 
     def rule_data(self):
         return {
-            'mandatory': self.mandatory
+            'mandatory': self.mandatory,
+            'role': self.role,
         }
 
 
@@ -4974,17 +5023,22 b' class RepoReviewRuleUserGroup(Base, Base'
     )
 
     VOTE_RULE_ALL = -1
+    ROLE_REVIEWER = u'reviewer'
+    ROLE_OBSERVER = u'observer'
+    ROLES = [ROLE_REVIEWER, ROLE_OBSERVER]
 
     repo_review_rule_users_group_id = Column('repo_review_rule_users_group_id', Integer(), primary_key=True)
     repo_review_rule_id = Column("repo_review_rule_id", Integer(), ForeignKey('repo_review_rules.repo_review_rule_id'))
     users_group_id = Column("users_group_id", Integer(),ForeignKey('users_groups.users_group_id'), nullable=False)
     mandatory = Column("mandatory", Boolean(), nullable=False, default=False)
+    role = Column('role', Unicode(255), nullable=True, default=ROLE_REVIEWER)
     vote_rule = Column("vote_rule", Integer(), nullable=True, default=VOTE_RULE_ALL)
     users_group = relationship('UserGroup')
 
     def rule_data(self):
         return {
             'mandatory': self.mandatory,
+            'role': self.role,
             'vote_rule': self.vote_rule
         }
 

rhodecode/model/forms.py

@@ -601,6 +601,14 b' def PullRequestForm(localizer, repo_id):'
         reasons = All()
         rules = All(v.UniqueList(localizer, convert=int)())
         mandatory = v.StringBoolean()
+        role = v.String(if_missing='reviewer')
+
+    class ObserverForm(formencode.Schema):
+        user_id = v.Int(not_empty=True)
+        reasons = All()
+        rules = All(v.UniqueList(localizer, convert=int)())
+        mandatory = v.StringBoolean()
+        role = v.String(if_missing='observer')
 
     class _PullRequestForm(formencode.Schema):
         allow_extra_fields = True
@@ -614,6 +622,7 b' def PullRequestForm(localizer, repo_id):'
         revisions = All(#v.NotReviewedRevisions(localizer, repo_id)(),
                         v.UniqueList(localizer)(not_empty=True))
         review_members = formencode.ForEach(ReviewerForm())
+        observer_members = formencode.ForEach(ObserverForm())
         pullrequest_title = v.UnicodeString(strip=True, required=True, min=1, max=255)
         pullrequest_desc = v.UnicodeString(strip=True, required=False)
         description_renderer = v.UnicodeString(strip=True, required=False)

rhodecode/model/pull_request.py

@@ -575,7 +575,7 b' class PullRequestModel(BaseModel):'
                pull_request_display_obj, at_version
 
     def create(self, created_by, source_repo, source_ref, target_repo,
-               target_ref, revisions, reviewers, title, description=None,
+               target_ref, revisions, reviewers, observers, title, description=None,
                common_ancestor_id=None,
                description_renderer=None,
                reviewer_data=None, translator=None, auth_user=None):
@@ -606,7 +606,7 b' class PullRequestModel(BaseModel):'
         reviewer_ids = set()
         # members / reviewers
         for reviewer_object in reviewers:
-            user_id, reasons, mandatory, rules = reviewer_object
+            user_id, reasons, mandatory, role, rules = reviewer_object
             user = self._get_user(user_id)
 
             # skip duplicates
@@ -620,6 +620,7 b' class PullRequestModel(BaseModel):'
             reviewer.pull_request = pull_request
             reviewer.reasons = reasons
             reviewer.mandatory = mandatory
+            reviewer.role = role
 
             # NOTE(marcink): pick only first rule for now
             rule_id = list(rules)[0] if rules else None
@@ -653,6 +654,33 b' class PullRequestModel(BaseModel):'
             Session().add(reviewer)
             Session().flush()
 
+        for observer_object in observers:
+            user_id, reasons, mandatory, role, rules = observer_object
+            user = self._get_user(user_id)
+
+            # skip duplicates from reviewers
+            if user.user_id in reviewer_ids:
+                continue
+
+            #reviewer_ids.add(user.user_id)
+
+            observer = PullRequestReviewers()
+            observer.user = user
+            observer.pull_request = pull_request
+            observer.reasons = reasons
+            observer.mandatory = mandatory
+            observer.role = role
+
+            # NOTE(marcink): pick only first rule for now
+            rule_id = list(rules)[0] if rules else None
+            rule = RepoReviewRule.get(rule_id) if rule_id else None
+            if rule:
+                # TODO(marcink): do we need this for observers ??
+                pass
+
+            Session().add(observer)
+            Session().flush()
+
         # Set approval status to "Under Review" for all commits which are
         # part of this pull request.
         ChangesetStatusModel().set_status(
@@ -1204,23 +1232,25 b' class PullRequestModel(BaseModel):'
 
         :param pull_request: the pr to update
         :param reviewer_data: list of tuples
-            [(user, ['reason1', 'reason2'], mandatory_flag, [rules])]
+            [(user, ['reason1', 'reason2'], mandatory_flag, role, [rules])]
+        :param user: current use who triggers this action
         """
+
         pull_request = self.__get_pull_request(pull_request)
         if pull_request.is_closed():
             raise ValueError('This pull request is closed')
 
         reviewers = {}
-        for user_id, reasons, mandatory, rules in reviewer_data:
+        for user_id, reasons, mandatory, role, rules in reviewer_data:
             if isinstance(user_id, (int, compat.string_types)):
                 user_id = self._get_user(user_id).user_id
             reviewers[user_id] = {
-                'reasons': reasons, 'mandatory': mandatory}
+                'reasons': reasons, 'mandatory': mandatory, 'role': role}
 
         reviewers_ids = set(reviewers.keys())
-        current_reviewers = PullRequestReviewers.query()\
-            .filter(PullRequestReviewers.pull_request ==
-                    pull_request).all()
+        current_reviewers = PullRequestReviewers.get_pull_request_reviewers(
+            pull_request.pull_request_id, role=PullRequestReviewers.ROLE_REVIEWER)
+
         current_reviewers_ids = set([x.user.user_id for x in current_reviewers])
 
         ids_to_add = reviewers_ids.difference(current_reviewers_ids)
@@ -1241,16 +1271,19 b' class PullRequestModel(BaseModel):'
             reviewer.reasons = reviewers[uid]['reasons']
             # NOTE(marcink): mandatory shouldn't be changed now
             # reviewer.mandatory = reviewers[uid]['reasons']
+            # NOTE(marcink): role should be hardcoded, so we won't edit it.
+            reviewer.role = PullRequestReviewers.ROLE_REVIEWER
             Session().add(reviewer)
             added_audit_reviewers.append(reviewer.get_dict())
 
         for uid in ids_to_remove:
             changed = True
-            # NOTE(marcink): we fetch "ALL" reviewers using .all(). This is an edge case
-            # that prevents and fixes cases that we added the same reviewer twice.
+            # NOTE(marcink): we fetch "ALL" reviewers objects using .all().
+            # This is an edge case that handles previous state of having the same reviewer twice.
             # this CAN happen due to the lack of DB checks
             reviewers = PullRequestReviewers.query()\
                 .filter(PullRequestReviewers.user_id == uid,
+                        PullRequestReviewers.role == PullRequestReviewers.ROLE_REVIEWER,
                         PullRequestReviewers.pull_request == pull_request)\
                 .all()
 
@@ -1273,7 +1306,90 b' class PullRequestModel(BaseModel):'
                 'repo.pull_request.reviewer.delete', {'old_data': user_data},
                 user, pull_request)
 
-        self.notify_reviewers(pull_request, ids_to_add)
+        self.notify_reviewers(pull_request, ids_to_add, user.get_instance())
+        return ids_to_add, ids_to_remove
+
+    def update_observers(self, pull_request, observer_data, user):
+        """
+        Update the observers in the pull request
+
+        :param pull_request: the pr to update
+        :param observer_data: list of tuples
+            [(user, ['reason1', 'reason2'], mandatory_flag, role, [rules])]
+        :param user: current use who triggers this action
+        """
+        pull_request = self.__get_pull_request(pull_request)
+        if pull_request.is_closed():
+            raise ValueError('This pull request is closed')
+
+        observers = {}
+        for user_id, reasons, mandatory, role, rules in observer_data:
+            if isinstance(user_id, (int, compat.string_types)):
+                user_id = self._get_user(user_id).user_id
+            observers[user_id] = {
+                'reasons': reasons, 'observers': mandatory, 'role': role}
+
+        observers_ids = set(observers.keys())
+        current_observers = PullRequestReviewers.get_pull_request_reviewers(
+            pull_request.pull_request_id, role=PullRequestReviewers.ROLE_OBSERVER)
+
+        current_observers_ids = set([x.user.user_id for x in current_observers])
+
+        ids_to_add = observers_ids.difference(current_observers_ids)
+        ids_to_remove = current_observers_ids.difference(observers_ids)
+
+        log.debug("Adding %s observer", ids_to_add)
+        log.debug("Removing %s observer", ids_to_remove)
+        changed = False
+        added_audit_observers = []
+        removed_audit_observers = []
+
+        for uid in ids_to_add:
+            changed = True
+            _usr = self._get_user(uid)
+            observer = PullRequestReviewers()
+            observer.user = _usr
+            observer.pull_request = pull_request
+            observer.reasons = observers[uid]['reasons']
+            # NOTE(marcink): mandatory shouldn't be changed now
+            # observer.mandatory = observer[uid]['reasons']
+
+            # NOTE(marcink): role should be hardcoded, so we won't edit it.
+            observer.role = PullRequestReviewers.ROLE_OBSERVER
+            Session().add(observer)
+            added_audit_observers.append(observer.get_dict())
+
+        for uid in ids_to_remove:
+            changed = True
+            # NOTE(marcink): we fetch "ALL" reviewers objects using .all().
+            # This is an edge case that handles previous state of having the same reviewer twice.
+            # this CAN happen due to the lack of DB checks
+            observers = PullRequestReviewers.query()\
+                .filter(PullRequestReviewers.user_id == uid,
+                        PullRequestReviewers.role == PullRequestReviewers.ROLE_OBSERVER,
+                        PullRequestReviewers.pull_request == pull_request)\
+                .all()
+
+            for obj in observers:
+                added_audit_observers.append(obj.get_dict())
+                Session().delete(obj)
+
+        if changed:
+            Session().expire_all()
+            pull_request.updated_on = datetime.datetime.now()
+            Session().add(pull_request)
+
+        # finally store audit logs
+        for user_data in added_audit_observers:
+            self._log_audit_action(
+                'repo.pull_request.observer.add', {'data': user_data},
+                user, pull_request)
+        for user_data in removed_audit_observers:
+            self._log_audit_action(
+                'repo.pull_request.observer.delete', {'old_data': user_data},
+                user, pull_request)
+
+        self.notify_observers(pull_request, ids_to_add, user.get_instance())
         return ids_to_add, ids_to_remove
 
     def get_url(self, pull_request, request=None, permalink=False):
@@ -1301,16 +1417,16 b' class PullRequestModel(BaseModel):'
             pr_url = urllib.unquote(self.get_url(pull_request, request=request))
             return safe_unicode('{pr_url}/repository'.format(pr_url=pr_url))
 
-    def notify_reviewers(self, pull_request, reviewers_ids):
-        # notification to reviewers
-        if not reviewers_ids:
+    def _notify_reviewers(self, pull_request, user_ids, role, user):
+        # notification to reviewers/observers
+        if not user_ids:
             return
 
-        log.debug('Notify following reviewers about pull-request %s', reviewers_ids)
+        log.debug('Notify following %s users about pull-request %s', role, user_ids)
 
         pull_request_obj = pull_request
         # get the current participants of this pull request
-        recipients = reviewers_ids
+        recipients = user_ids
         notification_type = EmailNotificationModel.TYPE_PULL_REQUEST
 
         pr_source_repo = pull_request_obj.source_repo
@@ -1332,8 +1448,10 b' class PullRequestModel(BaseModel):'
             (x.raw_id, x.message)
             for x in map(pr_source_repo.get_commit, pull_request.revisions)]
 
+        current_rhodecode_user = user
         kwargs = {
-            'user': pull_request.author,
+            'user': current_rhodecode_user,
+            'pull_request_author': pull_request.author,
             'pull_request': pull_request_obj,
             'pull_request_commits': pull_request_commits,
 
@@ -1345,6 +1463,7 b' class PullRequestModel(BaseModel):'
 
             'pull_request_url': pr_url,
             'thread_ids': [pr_url],
+            'user_role': role
         }
 
         # pre-generate the subject for notification itself
@@ -1353,7 +1472,7 b' class PullRequestModel(BaseModel):'
 
         # create notification objects, and emails
         NotificationModel().create(
-            created_by=pull_request.author,
+            created_by=current_rhodecode_user,
             notification_subject=subject,
             notification_body=body_plaintext,
             notification_type=notification_type,
@@ -1361,6 +1480,14 b' class PullRequestModel(BaseModel):'
             email_kwargs=kwargs,
         )
 
+    def notify_reviewers(self, pull_request, reviewers_ids, user):
+        return self._notify_reviewers(pull_request, reviewers_ids,
+                                      PullRequestReviewers.ROLE_REVIEWER, user)
+
+    def notify_observers(self, pull_request, observers_ids, user):
+        return self._notify_reviewers(pull_request, observers_ids,
+                                      PullRequestReviewers.ROLE_OBSERVER, user)
+
     def notify_users(self, pull_request, updating_user, ancestor_commit_id,
                      commit_changes, file_changes):
 
@@ -1874,11 +2001,13 b' class PullRequestModel(BaseModel):'
         try:
             from rc_reviewers.utils import get_default_reviewers_data
             from rc_reviewers.utils import validate_default_reviewers
+            from rc_reviewers.utils import validate_observers
         except ImportError:
             from rhodecode.apps.repository.utils import get_default_reviewers_data
             from rhodecode.apps.repository.utils import validate_default_reviewers
+            from rhodecode.apps.repository.utils import validate_observers
 
-        return get_default_reviewers_data, validate_default_reviewers
+        return get_default_reviewers_data, validate_default_reviewers, validate_observers
 
 
 class MergeCheck(object):

rhodecode/public/css/main.less

@@ -1700,8 +1700,33 b' table.group_members {'
 }
 
 .reviewer_ac .ac-input {
+    width: 98%;
+    margin-bottom: 1em;
+}
+
+.observer_ac .ac-input {
+    width: 98%;
+    margin-bottom: 1em;
+}
+
+.rule-table {
     width: 100%;
-    margin-bottom: 1em;
+}
+
+.rule-table td {
+
+}
+
+.rule-table .td-role {
+  width: 100px
+}
+
+.rule-table .td-mandatory {
+  width: 100px
+}
+
+.rule-table .td-group-votes {
+  width: 150px
 }
 
 .compare_view_commits tr{

rhodecode/public/js/src/rhodecode/pullrequests.js

@@ -94,21 +94,26 b' var getTitleAndDescription = function(so'
 };
 
 
-ReviewersController = function () {
+window.ReviewersController = function () {
     var self = this;
+    this.$loadingIndicator = $('.calculate-reviewers');
     this.$reviewRulesContainer = $('#review_rules');
     this.$rulesList = this.$reviewRulesContainer.find('.pr-reviewer-rules');
     this.$userRule = $('.pr-user-rule-container');
-    this.forbidReviewUsers = undefined;
     this.$reviewMembers = $('#review_members');
+    this.$observerMembers = $('#observer_members');
+
     this.currentRequest = null;
     this.diffData = null;
     this.enabledRules = [];
+    // sync with db.py entries
+    this.ROLE_REVIEWER = 'reviewer';
+    this.ROLE_OBSERVER = 'observer'
 
     //dummy handler, we might register our own later
     this.diffDataHandler = function(data){};
 
-    this.defaultForbidReviewUsers = function () {
+    this.defaultForbidUsers = function () {
         return [
             {
                 'username': 'default',
@@ -117,6 +122,9 b' ReviewersController = function () {'
         ];
     };
 
+    // init default forbidden users
+    this.forbidUsers = this.defaultForbidUsers();
+
     this.hideReviewRules = function () {
         self.$reviewRulesContainer.hide();
         $(self.$userRule.selector).hide();
@@ -133,11 +141,40 b' ReviewersController = function () {'
         return '<div>- {0}</div>'.format(ruleText)
     };
 
+    this.increaseCounter = function(role) {
+        if (role === self.ROLE_REVIEWER) {
+            var $elem = $('#reviewers-cnt')
+            var cnt = parseInt($elem.data('count') || 0)
+            cnt +=1
+            $elem.html(cnt);
+            $elem.data('count', cnt);
+        }
+        else if (role === self.ROLE_OBSERVER) {
+            var $elem = $('#observers-cnt');
+            var cnt = parseInt($elem.data('count') || 0)
+            cnt +=1
+            $elem.html(cnt);
+            $elem.data('count', cnt);
+        }
+    }
+
+    this.resetCounter = function () {
+        var $elem = $('#reviewers-cnt');
+
+        $elem.data('count', 0);
+        $elem.html(0);
+
+        var $elem = $('#observers-cnt');
+
+        $elem.data('count', 0);
+        $elem.html(0);
+    }
+
     this.loadReviewRules = function (data) {
         self.diffData = data;
 
         // reset forbidden Users
-        this.forbidReviewUsers = self.defaultForbidReviewUsers();
+        this.forbidUsers = self.defaultForbidUsers();
 
         // reset state of review rules
         self.$rulesList.html('');
@@ -148,7 +185,7 b' ReviewersController = function () {'
                 self.addRule(
                     _gettext('All reviewers must vote.'))
             );
-            return self.forbidReviewUsers
+            return self.forbidUsers
         }
 
         if (data.rules.voting !== undefined) {
@@ -195,7 +232,7 b' ReviewersController = function () {'
         }
 
         if (data.rules.forbid_author_to_review) {
-            self.forbidReviewUsers.push(data.rules_data.pr_author);
+            self.forbidUsers.push(data.rules_data.pr_author);
             self.$rulesList.append(
                 self.addRule(
                     _gettext('Author is not allowed to be a reviewer.'))
@@ -206,9 +243,8 b' ReviewersController = function () {'
 
             if (data.rules_data.forbidden_users) {
                 $.each(data.rules_data.forbidden_users, function (index, member_data) {
-                    self.forbidReviewUsers.push(member_data)
+                    self.forbidUsers.push(member_data)
                 });
-
             }
 
             self.$rulesList.append(
@@ -223,9 +259,31 b' ReviewersController = function () {'
                 _gettext('No review rules set.'))
         }
 
-        return self.forbidReviewUsers
+        return self.forbidUsers
     };
 
+    this.emptyTables = function () {
+        self.emptyReviewersTable();
+        self.emptyObserversTable();
+
+        // Also reset counters.
+        self.resetCounter();
+    }
+
+    this.emptyReviewersTable = function (withText) {
+        self.$reviewMembers.empty();
+        if (withText !== undefined) {
+            self.$reviewMembers.html(withText)
+        }
+    };
+
+    this.emptyObserversTable = function (withText) {
+        self.$observerMembers.empty();
+        if (withText !== undefined) {
+            self.$observerMembers.html(withText)
+        }
+    }
+
     this.loadDefaultReviewers = function (sourceRepo, sourceRef, targetRepo, targetRef) {
 
         if (self.currentRequest) {
@@ -233,19 +291,21 b' ReviewersController = function () {'
             self.currentRequest.abort();
         }
 
-        $('.calculate-reviewers').show();
-        // reset reviewer members
-        self.$reviewMembers.empty();
+        self.$loadingIndicator.show();
+
+        // reset reviewer/observe members
+        self.emptyTables();
 
         prButtonLock(true, null, 'reviewers');
         $('#user').hide(); // hide user autocomplete before load
+        $('#observer').hide(); //hide observer autocomplete before load
 
         // lock PR button, so we cannot send PR before it's calculated
         prButtonLock(true, _gettext('Loading diff ...'), 'compare');
 
         if (sourceRef.length !== 3 || targetRef.length !== 3) {
             // don't load defaults in case we're missing some refs...
-            $('.calculate-reviewers').hide();
+            self.$loadingIndicator.hide();
             return
         }
 
@@ -272,11 +332,16 b' ReviewersController = function () {'
 
                 for (var i = 0; i < data.reviewers.length; i++) {
                     var reviewer = data.reviewers[i];
-                    self.addReviewMember(reviewer, reviewer.reasons, reviewer.mandatory);
+                    // load reviewer rules from the repo data
+                    self.addMember(reviewer, reviewer.reasons, reviewer.mandatory, reviewer.role);
                 }
-                $('.calculate-reviewers').hide();
+
+
+                self.$loadingIndicator.hide();
                 prButtonLock(false, null, 'reviewers');
-                $('#user').show(); // show user autocomplete after load
+
+                $('#user').show(); // show user autocomplete before load
+                $('#observer').show(); // show observer autocomplete before load
 
                 var commitElements = data["diff_info"]['commits'];
 
@@ -292,7 +357,7 b' ReviewersController = function () {'
 
             },
             error: function (jqXHR, textStatus, errorThrown) {
-                var prefix = "Loading diff and reviewers failed\n"
+                var prefix = "Loading diff and reviewers/observers failed\n"
                 var message = formatErrorMessage(jqXHR, textStatus, errorThrown, prefix);
                 ajaxErrorSwal(message);
             }
@@ -301,7 +366,7 b' ReviewersController = function () {'
     };
 
     // check those, refactor
-    this.removeReviewMember = function (reviewer_id, mark_delete) {
+    this.removeMember = function (reviewer_id, mark_delete) {
         var reviewer = $('#reviewer_{0}'.format(reviewer_id));
 
         if (typeof (mark_delete) === undefined) {
@@ -312,6 +377,7 b' ReviewersController = function () {'
             if (reviewer) {
                 // now delete the input
                 $('#reviewer_{0} input'.format(reviewer_id)).remove();
+                $('#reviewer_{0}_rules input'.format(reviewer_id)).remove();
                 // mark as to-delete
                 var obj = $('#reviewer_{0}_name'.format(reviewer_id));
                 obj.addClass('to-delete');
@@ -322,27 +388,26 b' ReviewersController = function () {'
         }
     };
 
-    this.reviewMemberEntry = function () {
+    this.addMember = function (reviewer_obj, reasons, mandatory, role) {
 
-    };
-
-    this.addReviewMember = function (reviewer_obj, reasons, mandatory) {
         var id = reviewer_obj.user_id;
         var username = reviewer_obj.username;
 
-        var reasons = reasons || [];
-        var mandatory = mandatory || false;
+        reasons = reasons || [];
+        mandatory = mandatory || false;
+        role = role || self.ROLE_REVIEWER
 
-        // register IDS to check if we don't have this ID already in
+        // register current set IDS to check if we don't have this ID already in
+        // and prevent duplicates
         var currentIds = [];
 
-        $.each(self.$reviewMembers.find('.reviewer_entry'), function (index, value) {
+        $.each($('.reviewer_entry'), function (index, value) {
             currentIds.push($(value).data('reviewerUserId'))
         })
 
         var userAllowedReview = function (userId) {
             var allowed = true;
-            $.each(self.forbidReviewUsers, function (index, member_data) {
+            $.each(self.forbidUsers, function (index, member_data) {
                 if (parseInt(userId) === member_data['user_id']) {
                     allowed = false;
                     return false // breaks the loop
@@ -352,6 +417,7 b' ReviewersController = function () {'
         };
 
         var userAllowed = userAllowedReview(id);
+
         if (!userAllowed) {
             alert(_gettext('User `{0}` not allowed to be a reviewer').format(username));
         } else {
@@ -359,11 +425,13 b' ReviewersController = function () {'
             var alreadyReviewer = currentIds.indexOf(id) != -1;
 
             if (alreadyReviewer) {
-                alert(_gettext('User `{0}` already in reviewers').format(username));
+                alert(_gettext('User `{0}` already in reviewers/observers').format(username));
             } else {
+
                 var reviewerEntry = renderTemplate('reviewMemberEntry', {
                     'member': reviewer_obj,
                     'mandatory': mandatory,
+                    'role': role,
                     'reasons': reasons,
                     'allowed_to_update': true,
                     'review_status': 'not_reviewed',
@@ -372,16 +440,32 b' ReviewersController = function () {'
                     'create': true,
                     'rule_show': true,
                 })
+
+                if (role === self.ROLE_REVIEWER) {
                 $(self.$reviewMembers.selector).append(reviewerEntry);
+                    self.increaseCounter(self.ROLE_REVIEWER);
+                    $('#reviewer-empty-msg').remove()
+                }
+                else if (role === self.ROLE_OBSERVER) {
+                    $(self.$observerMembers.selector).append(reviewerEntry);
+                    self.increaseCounter(self.ROLE_OBSERVER);
+                    $('#observer-empty-msg').remove();
+                }
+
                 tooltipActivate();
             }
         }
 
     };
 
-    this.updateReviewers = function (repo_name, pull_request_id) {
+    this.updateReviewers = function (repo_name, pull_request_id, role) {
+        if (role === 'reviewer') {
         var postData = $('#reviewers input').serialize();
         _updatePullRequest(repo_name, pull_request_id, postData);
+        } else if (role === 'observer') {
+            var postData = $('#observers input').serialize();
+            _updatePullRequest(repo_name, pull_request_id, postData);
+        }
     };
 
     this.handleDiffData = function (data) {
@@ -449,23 +533,14 b' var editPullRequest = function(repo_name'
 
 
 /**
- * Reviewer autocomplete
+ * autocomplete handler for reviewers/observers
  */
-var ReviewerAutoComplete = function(inputId) {
-  $(inputId).autocomplete({
-    serviceUrl: pyroutes.url('user_autocomplete_data'),
-    minChars:2,
-    maxHeight:400,
-    deferRequestBy: 300, //miliseconds
-    showNoSuggestionNotice: true,
-    tabDisabled: true,
-    autoSelectFirst: true,
-    params: { user_id: templateContext.rhodecode_user.user_id, user_groups:true, user_groups_expand:true, skip_default_user:true },
-    formatResult: autocompleteFormatResult,
-    lookupFilter: autocompleteFilterResult,
-    onSelect: function(element, data) {
+var autoCompleteHandler = function (inputId, controller, role) {
+
+    return function (element, data) {
         var mandatory = false;
-        var reasons = [_gettext('added manually by "{0}"').format(templateContext.rhodecode_user.username)];
+        var reasons = [_gettext('added manually by "{0}"').format(
+            templateContext.rhodecode_user.username)];
 
         // add whole user groups
         if (data.value_type == 'user_group') {
@@ -477,7 +552,7 b' var ReviewerAutoComplete = function(inpu'
                 reviewer['gravatar_link'] = member_data['icon_link'];
                 reviewer['user_link'] = member_data['profile_link'];
                 reviewer['rules'] = [];
-                reviewersController.addReviewMember(reviewer, reasons, mandatory);
+                controller.addMember(reviewer, reasons, mandatory, role);
             })
         }
         // add single user
@@ -487,14 +562,71 b' var ReviewerAutoComplete = function(inpu'
             reviewer['gravatar_link'] = data['icon_link'];
             reviewer['user_link'] = data['profile_link'];
             reviewer['rules'] = [];
-            reviewersController.addReviewMember(reviewer, reasons, mandatory);
+            controller.addMember(reviewer, reasons, mandatory, role);
         }
 
       $(inputId).val('');
     }
+}
+
+/**
+ * Reviewer autocomplete
+ */
+var ReviewerAutoComplete = function (inputId, controller) {
+    var self = this;
+    self.controller = controller;
+    self.inputId = inputId;
+    var handler = autoCompleteHandler(inputId, controller, controller.ROLE_REVIEWER);
+
+    $(inputId).autocomplete({
+        serviceUrl: pyroutes.url('user_autocomplete_data'),
+        minChars: 2,
+        maxHeight: 400,
+        deferRequestBy: 300, //miliseconds
+        showNoSuggestionNotice: true,
+        tabDisabled: true,
+        autoSelectFirst: true,
+        params: {
+            user_id: templateContext.rhodecode_user.user_id,
+            user_groups: true,
+            user_groups_expand: true,
+            skip_default_user: true
+        },
+        formatResult: autocompleteFormatResult,
+        lookupFilter: autocompleteFilterResult,
+        onSelect: handler
   });
 };
 
+/**
+ * Observers autocomplete
+ */
+var ObserverAutoComplete = function(inputId, controller) {
+    var self = this;
+    self.controller = controller;
+    self.inputId = inputId;
+    var handler = autoCompleteHandler(inputId, controller, controller.ROLE_OBSERVER);
+
+    $(inputId).autocomplete({
+        serviceUrl: pyroutes.url('user_autocomplete_data'),
+        minChars: 2,
+        maxHeight: 400,
+        deferRequestBy: 300, //miliseconds
+        showNoSuggestionNotice: true,
+        tabDisabled: true,
+        autoSelectFirst: true,
+        params: {
+            user_id: templateContext.rhodecode_user.user_id,
+            user_groups: true,
+            user_groups_expand: true,
+            skip_default_user: true
+        },
+        formatResult: autocompleteFormatResult,
+        lookupFilter: autocompleteFilterResult,
+        onSelect: handler
+    });
+}
+
 
 window.VersionController = function () {
     var self = this;
@@ -504,7 +636,7 b' window.VersionController = function () {'
 
     this.adjustRadioSelectors = function (curNode) {
         var getVal = function (item) {
-            if (item == 'latest') {
+            if (item === 'latest') {
                 return Number.MAX_SAFE_INTEGER
             }
             else {
@@ -663,6 +795,7 b' window.UpdatePrController = function () '
     };
 };
 
+
 /**
  * Reviewer display panel
  */
@@ -702,14 +835,24 b' window.ReviewersPanel = {'
     },
 
     renderReviewers: function () {
+        if (this.setReviewers.reviewers === undefined) {
+            return
+        }
+        if (this.setReviewers.reviewers.length === 0) {
+            reviewersController.emptyReviewersTable('<tr id="reviewer-empty-msg"><td colspan="6">No reviewers</td></tr>');
+            return
+        }
 
-        $('#review_members').html('')
+        reviewersController.emptyReviewersTable();
+
         $.each(this.setReviewers.reviewers, function (key, val) {
+
             var member = val;
-
+            if (member.role === reviewersController.ROLE_REVIEWER) {
             var entry = renderTemplate('reviewMemberEntry', {
                 'member': member,
                 'mandatory': member.mandatory,
+                    'role': member.role,
                 'reasons': member.reasons,
                 'allowed_to_update': member.allowed_to_update,
                 'review_status': member.review_status,
@@ -718,10 +861,11 b' window.ReviewersPanel = {'
                 'create': false
             });
 
-            $('#review_members').append(entry)
+                $(reviewersController.$reviewMembers.selector).append(entry)
+            }
         });
+
         tooltipActivate();
-
     },
 
     edit: function (event) {
@@ -739,10 +883,142 b' window.ReviewersPanel = {'
         this.addButton.hide();
         $(this.removeButtons.selector).css('visibility', 'hidden');
         // hide review rules
-        reviewersController.hideReviewRules()
+        reviewersController.hideReviewRules();
     }
 };
 
+/**
+ * Reviewer display panel
+ */
+window.ObserversPanel = {
+    editButton: null,
+    closeButton: null,
+    addButton: null,
+    removeButtons: null,
+    reviewRules: null,
+    setReviewers: null,
+
+    setSelectors: function () {
+        var self = this;
+        self.editButton = $('#open_edit_observers');
+        self.closeButton =$('#close_edit_observers');
+        self.addButton = $('#add_observer');
+        self.removeButtons = $('.observer_member_remove,.observer_member_mandatory_remove');
+    },
+
+    init: function (reviewRules, setReviewers) {
+        var self = this;
+        self.setSelectors();
+
+        this.reviewRules = reviewRules;
+        this.setReviewers = setReviewers;
+
+        this.editButton.on('click', function (e) {
+            self.edit();
+        });
+        this.closeButton.on('click', function (e) {
+            self.close();
+            self.renderObservers();
+        });
+
+        self.renderObservers();
+
+    },
+
+    renderObservers: function () {
+        if (this.setReviewers.observers === undefined) {
+            return
+        }
+        if (this.setReviewers.observers.length === 0) {
+            reviewersController.emptyObserversTable('<tr id="observer-empty-msg"><td colspan="6">No observers</td></tr>');
+            return
+        }
+
+        reviewersController.emptyObserversTable();
+
+        $.each(this.setReviewers.observers, function (key, val) {
+            var member = val;
+            if (member.role === reviewersController.ROLE_OBSERVER) {
+                var entry = renderTemplate('reviewMemberEntry', {
+                    'member': member,
+                    'mandatory': member.mandatory,
+                    'role': member.role,
+                    'reasons': member.reasons,
+                    'allowed_to_update': member.allowed_to_update,
+                    'review_status': member.review_status,
+                    'review_status_label': member.review_status_label,
+                    'user_group': member.user_group,
+                    'create': false
+                });
+
+                $(reviewersController.$observerMembers.selector).append(entry)
+            }
+        });
+
+        tooltipActivate();
+    },
+
+    edit: function (event) {
+        this.editButton.hide();
+        this.closeButton.show();
+        this.addButton.show();
+        $(this.removeButtons.selector).css('visibility', 'visible');
+    },
+
+    close: function (event) {
+        this.editButton.show();
+        this.closeButton.hide();
+        this.addButton.hide();
+        $(this.removeButtons.selector).css('visibility', 'hidden');
+    }
+
+};
+
+window.PRDetails = {
+    editButton: null,
+    closeButton: null,
+    deleteButton: null,
+    viewFields: null,
+    editFields: null,
+
+    setSelectors: function () {
+        var self = this;
+        self.editButton = $('#open_edit_pullrequest')
+        self.closeButton = $('#close_edit_pullrequest')
+        self.deleteButton = $('#delete_pullrequest')
+        self.viewFields = $('#pr-desc, #pr-title')
+        self.editFields = $('#pr-desc-edit, #pr-title-edit, .pr-save')
+    },
+
+    init: function () {
+        var self = this;
+        self.setSelectors();
+        self.editButton.on('click', function (e) {
+            self.edit();
+        });
+        self.closeButton.on('click', function (e) {
+            self.view();
+        });
+    },
+
+    edit: function (event) {
+        var cmInstance = $('#pr-description-input').get(0).MarkupForm.cm;
+        this.viewFields.hide();
+        this.editButton.hide();
+        this.deleteButton.hide();
+        this.closeButton.show();
+        this.editFields.show();
+        cmInstance.refresh();
+    },
+
+    view: function (event) {
+        this.editButton.show();
+        this.deleteButton.show();
+        this.editFields.hide();
+        this.closeButton.hide();
+        this.viewFields.show();
+    }
+};
 
 /**
  * OnLine presence using channelstream
@@ -813,17 +1089,14 b' window.refreshComments = function (versi'
     $.each($('.comment'), function (idx, element) {
         currentIDs.push($(element).data('commentId'));
     });
-    var data = {"comments[]": currentIDs};
+    var data = {"comments": currentIDs};
 
     var $targetElem = $('.comments-content-table');
     $targetElem.css('opacity', 0.3);
-    $targetElem.load(
-        loadUrl, data, function (responseText, textStatus, jqXHR) {
-            if (jqXHR.status !== 200) {
-                return false;
-            }
+
+    var success = function (data) {
             var $counterElem = $('#comments-count');
-            var newCount = $(responseText).data('counter');
+        var newCount = $(data).data('counter');
             if (newCount !== undefined) {
                 var callback = function () {
                     $counterElem.animate({'opacity': 1.00}, 200)
@@ -833,9 +1106,12 b' window.refreshComments = function (versi'
             }
 
             $targetElem.css('opacity', 1);
+        $targetElem.html(data);
             tooltipActivate();
         }
-    );
+
+    ajaxPOST(loadUrl, data, success, null, {})
+
 }
 
 window.refreshTODOs = function (version) {
@@ -858,16 +1134,13 b' window.refreshTODOs = function (version)'
         currentIDs.push($(element).data('commentId'));
     });
 
-    var data = {"comments[]": currentIDs};
+    var data = {"comments": currentIDs};
     var $targetElem = $('.todos-content-table');
     $targetElem.css('opacity', 0.3);
-    $targetElem.load(
-        loadUrl, data, function (responseText, textStatus, jqXHR) {
-            if (jqXHR.status !== 200) {
-                return false;
-            }
+
+    var success = function (data) {
             var $counterElem = $('#todos-count')
-            var newCount = $(responseText).data('counter');
+        var newCount = $(data).data('counter');
             if (newCount !== undefined) {
                 var callback = function () {
                     $counterElem.animate({'opacity': 1.00}, 200)
@@ -877,9 +1150,12 b' window.refreshTODOs = function (version)'
             }
 
             $targetElem.css('opacity', 1);
+        $targetElem.html(data);
             tooltipActivate();
         }
-    );
+
+    ajaxPOST(loadUrl, data, success, null, {})
+
 }
 
 window.refreshAllComments = function (version) {
@@ -888,3 +1164,12 b' window.refreshAllComments = function (ve'
     refreshComments(version);
     refreshTODOs(version);
 };
+
+window.sidebarComment = function (commentId) {
+    var jsonData = $('#commentHovercard{0}'.format(commentId)).data('commentJsonB64');
+    if (!jsonData) {
+        return 'Failed to load comment {0}'.format(commentId)
+    }
+    var funcData = JSON.parse(atob(jsonData));
+    return renderTemplate('sideBarCommentHovercard', funcData)
+};

rhodecode/public/js/src/rhodecode/utils/ajax.js

@@ -57,15 +57,18 b' var ajaxGET = function (url, success, fa'
     return request;
 };
 
-var ajaxPOST = function (url, postData, success, failure) {
-    var sUrl = url;
-    var postData = toQueryString(postData);
-    var request = $.ajax({
+var ajaxPOST = function (url, postData, success, failure, options) {
+
+    var ajaxSettings = $.extend({
         type: 'POST',
-        url: sUrl,
-        data: postData,
+        url: url,
+        data: toQueryString(postData),
         headers: {'X-PARTIAL-XHR': true}
-        })
+    }, options);
+
+    var request = $.ajax(
+        ajaxSettings
+        )
         .done(function (data) {
             success(data);
         })
@@ -126,7 +129,8 b' function formatErrorMessage(jqXHR, textS'
     } else if (errorThrown === 'abort') {
         return (prefix + 'Ajax request aborted.');
     } else {
-        return (prefix + 'Uncaught Error.\n' + jqXHR.responseText);
+        var errInfo = 'Uncaught Error. code: {0}\n'.format(jqXHR.status)
+        return (prefix + errInfo + jqXHR.responseText);
     }
 }
 

rhodecode/templates/base/sidebar.mako

@@ -89,36 +89,41 b''
                     if is_pr:
                         version_info = (' made in older version (v{})'.format(comment_ver_index) if is_from_old_ver == 'true' else ' made in this version')
                   %>
-
-                  <script type="text/javascript">
-                      // closure function helper
-                      var sidebarComment${comment_obj.comment_id} = function() {
-                        return renderTemplate('sideBarCommentHovercard', {
-                            version_info: "${version_info}",
-                            file_name: "${comment_obj.f_path}",
-                            line_no: "${comment_obj.line_no}",
-                            outdated: ${h.json.dumps(comment_obj.outdated)},
-                            inline: ${h.json.dumps(comment_obj.is_inline)},
-                            is_todo: ${h.json.dumps(comment_obj.is_todo)},
-                            created_on: "${h.format_date(comment_obj.created_on)}",
-                            datetime: "${comment_obj.created_on}${h.get_timezone(comment_obj.created_on, time_is_local=True)}",
-                            review_status: "${(comment_obj.review_status or '')}"
-                        })
-                      }
-                  </script>
-
-                  % if comment_obj.outdated:
-                    <i class="icon-comment-toggle tooltip-hovercard" data-hovercard-url="javascript:sidebarComment${comment_obj.comment_id}()"></i>
-                  % elif comment_obj.is_inline:
-                    <i class="icon-code tooltip-hovercard" data-hovercard-url="javascript:sidebarComment${comment_obj.comment_id}()"></i>
-                  % else:
-                    <i class="icon-comment tooltip-hovercard" data-hovercard-url="javascript:sidebarComment${comment_obj.comment_id}()"></i>
+                  ## NEW, since refresh
+                  % if existing_ids and comment_obj.comment_id not in existing_ids:
+                      <div class="tooltip" style="position: absolute; left: 8px" title="New comment">
+                          !
+                      </div>
                   % endif
 
-                  ## NEW, since refresh
-                  % if existing_ids and comment_obj.comment_id not in existing_ids:
-                      <span class="tag">NEW</span>
-                  % endif
+                  <%
+                      data = h.json.dumps({
+                            'comment_id': comment_obj.comment_id,
+                            'version_info': version_info,
+                            'file_name': comment_obj.f_path,
+                            'line_no': comment_obj.line_no,
+                            'outdated': comment_obj.outdated,
+                            'inline': comment_obj.is_inline,
+                            'is_todo': comment_obj.is_todo,
+                            'created_on': h.format_date(comment_obj.created_on),
+                            'datetime': '{}{}'.format(comment_obj.created_on, h.get_timezone(comment_obj.created_on, time_is_local=True)),
+                            'review_status': (comment_obj.review_status or '')
+                      })
+
+                      if comment_obj.outdated:
+                          icon = 'icon-comment-toggle'
+                      elif comment_obj.is_inline:
+                          icon = 'icon-code'
+                      else:
+                          icon = 'icon-comment'
+                  %>
+
+                  <i id="commentHovercard${comment_obj.comment_id}"
+                     class="${icon} tooltip-hovercard"
+                     data-hovercard-url="javascript:sidebarComment(${comment_obj.comment_id})"
+                     data-comment-json-b64='${h.b64(data)}'>
+                  </i>
+
               </td>
 
               <td class="td-todo-gravatar">

rhodecode/templates/changeset/changeset.mako

@@ -187,12 +187,12 b''
             <div class="sidebar-element clear-both">
                 <% vote_title = _ungettext(
                         'Status calculated based on votes from {} reviewer',
-                        'Status calculated based on votes from {} reviewers', len(c.allowed_reviewers)).format(len(c.allowed_reviewers))
+                        'Status calculated based on votes from {} reviewers', c.reviewers_count).format(c.reviewers_count)
                 %>
 
                 <div class="tooltip right-sidebar-collapsed-state" style="display: none" onclick="toggleSidebar(); return false" title="${vote_title}">
                     <i class="icon-circle review-status-${c.commit_review_status}"></i>
-                    ${len(c.allowed_reviewers)}
+                    ${c.reviewers_count}
                 </div>
             </div>
 

rhodecode/templates/debug_style/collapsable-content.html

@@ -149,7 +149,7 b''
                   <span class="user"> <a href="/_profiles/jenkins-tests">jenkins-tests</a> (reviewer)</span>
                 </div>
                   <input id="reviewer_70_input" type="hidden" value="70" name="review_members">
-                  <div class="reviewer_member_remove action_button" onclick="removeReviewMember(70, true)" style="visibility: hidden;">
+                  <div class="reviewer_member_remove action_button" onclick="removeMember(70, true)" style="visibility: hidden;">
                     <i class="icon-remove"></i>
                   </div>
               </li>

rhodecode/templates/ejs_templates/templates.html

@@ -66,9 +66,18 b" var data_hovercard_url = pyroutes.url('h"
 <tr id="reviewer_<%= member.user_id %>" class="reviewer_entry" tooltip="Review Group" data-reviewer-user-id="<%= member.user_id %>">
 
     <td style="width: 20px">
+        <div class="tooltip presence-state" style="display: none; position: absolute; left: 2px" title="This users is currently at this page">
+          <i class="icon-eye" style="color: #0ac878"></i>
+        </div>
+        <% if (role === 'reviewer') { %>
         <div class="reviewer_status tooltip" title="<%= review_status_label %>">
           <i class="icon-circle review-status-<%= review_status %>"></i>
         </div>
+        <% }  else if (role === 'observer') { %>
+        <div class="tooltip" title="Observer without voting right.">
+        <i class="icon-circle-thin"></i>
+        </div>
+        <% } %>
     </td>
 
     <td>
@@ -84,9 +93,6 b" var data_hovercard_url = pyroutes.url('h"
             'gravatar_url':  member.gravatar_link
             })
         %>
-        <span class="tooltip presence-state" style="display: none" title="This users is currently at this page">
-          <i class="icon-eye" style="color: #0ac878"></i>
-        </span>
       </div>
     </td>
 
@@ -108,7 +114,7 b" var data_hovercard_url = pyroutes.url('h"
     <% } else { %>
     <td style="text-align: right;width: 10px;">
         <% if (allowed_to_update) { %>
-          <div class="reviewer_member_remove" onclick="reviewersController.removeReviewMember(<%= member.user_id %>, true)" style="visibility: <%= edit_visibility %>;">
+          <div class="<%=role %>_member_remove" onclick="reviewersController.removeMember(<%= member.user_id %>, true)" style="visibility: <%= edit_visibility %>;">
               <i class="icon-remove"></i>
           </div>
         <% } %>
@@ -117,7 +123,7 b" var data_hovercard_url = pyroutes.url('h"
 
 </tr>
 
-<tr>
+<tr id="reviewer_<%= member.user_id %>_rules">
     <td colspan="4" style="display: <%= rule_visibility %>" class="pr-user-rule-container">
       <input type="hidden" name="__start__" value="reviewer:mapping">
 
@@ -149,6 +155,7 b" var data_hovercard_url = pyroutes.url('h"
 
           <input id="reviewer_<%= member.user_id %>_input" type="hidden" value="<%= member.user_id %>" name="user_id" />
           <input type="hidden" name="mandatory" value="<%= mandatory %>"/>
+          <input type="hidden" name="role" value="<%= role %>"/>
 
       <input type="hidden" name="__end__" value="reviewer:mapping">
     </td>

rhodecode/templates/email_templates/pull_request_review.mako

@@ -11,6 +11,9 b' data = {'
     'pr_title': pull_request.title,
 }
 
+if user_role == 'observer':
+    subject_template = email_pr_review_subject_template or _('{user} added you as observer to pull request. !{pr_id}: "{pr_title}"')
+else:
 subject_template = email_pr_review_subject_template or _('{user} requested a pull request review. !{pr_id}: "{pr_title}"')
 %>
 
@@ -34,6 +37,7 b' data = {'
     'source_repo_url': pull_request_source_repo_url,
     'target_repo_url': pull_request_target_repo_url,
 }
+
 %>
 
 * ${_('Pull Request link')}: ${pull_request_url}
@@ -51,7 +55,7 b' data = {'
 
 % for commit_id, message in pull_request_commits:
     - ${h.short_id(commit_id)}
-      ${h.chop_at_smart(message, '\n', suffix_if_chopped='...')}
+    ${h.chop_at_smart(message.lstrip(), '\n', suffix_if_chopped='...')}
 
 % endfor
 
@@ -78,19 +82,23 b' data = {'
 <table style="text-align:left;vertical-align:middle;width: 100%">
     <tr>
     <td style="width:100%;border-bottom:1px solid #dbd9da;">
-
         <div style="margin: 0; font-weight: bold">
+            % if user_role == 'observer':
+            <div class="clear-both" class="clear-both" style="margin-bottom: 4px">
+                <span style="color:#7E7F7F">@${h.person(user.username)}</span>
+                ${_('added you as observer to')}
+                <a href="${pull_request_url}" style="${base.link_css()}">pull request</a>.
+            </div>
+            % else:
             <div class="clear-both" class="clear-both" style="margin-bottom: 4px">
                 <span style="color:#7E7F7F">@${h.person(user.username)}</span>
                 ${_('requested a')}
-                <a href="${pull_request_url}" style="${base.link_css()}">
-                ${_('pull request review.').format(**data) }
-                </a>
+                <a href="${pull_request_url}" style="${base.link_css()}">pull request</a> review.
             </div>
+            % endif
             <div style="margin-top: 10px"></div>
             ${_('Pull request')} <code>!${data['pr_id']}: ${data['pr_title']}</code>
         </div>
-
     </td>
     </tr>
 

rhodecode/templates/pullrequests/pullrequest.mako

@@ -112,7 +112,7 b''
                         ## REVIEWERS
                         <div class="field">
                             <div class="label label-textarea">
-                                <label for="pullrequest_reviewers">${_('Reviewers')}:</label>
+                                <label for="pullrequest_reviewers">${_('Reviewers / Observers')}:</label>
                             </div>
                             <div class="content">
                                 ## REVIEW RULES
@@ -125,18 +125,40 b''
                                     </div>
                                 </div>
 
-                                ## REVIEWERS
+                                ## REVIEWERS / OBSERVERS
                                 <div class="reviewers-title">
-                                    <div class="pr-details-title">
-                                        ${_('Pull request reviewers')}
-                                        <span class="calculate-reviewers"> - ${_('loading...')}</span>
-                                    </div>
-                                </div>
+
+                                    <ul class="nav-links clearfix">
+
+                                        ## TAB1 MANDATORY REVIEWERS
+                                        <li class="active">
+                                            <a id="reviewers-btn" href="#showReviewers" tabindex="-1">
+                                                Reviewers
+                                                <span id="reviewers-cnt" data-count="0" class="menulink-counter">0</span>
+                                            </a>
+                                        </li>
+
+                                        ## TAB2 OBSERVERS
+                                        <li class="">
+                                            <a id="observers-btn" href="#showObservers" tabindex="-1">
+                                                Observers
+                                                <span id="observers-cnt"  data-count="0" class="menulink-counter">0</span>
+                                            </a>
+                                        </li>
+
+                                    </ul>
+
+                                    ## TAB1 MANDATORY REVIEWERS
+                                    <div id="reviewers-container">
+                                        <span class="calculate-reviewers">
+                                            <h4>${_('loading...')}</h4>
+                                        </span>
+
                                 <div id="reviewers" class="pr-details-content reviewers">
                                     ## members goes here, filled via JS based on initial selection !
                                     <input type="hidden" name="__start__" value="review_members:sequence">
                                     <table id="review_members" class="group_members">
-                                    ## This content is loaded via JS and ReviewersPanel
+                                            ## This content is loaded via JS and ReviewersPanel, an sets reviewer_entry class on each element
                                     </table>
                                     <input type="hidden" name="__end__" value="review_members:sequence">
 
@@ -149,6 +171,34 b''
 
                                 </div>
                             </div>
+
+                                    ## TAB2 OBSERVERS
+                                    <div id="observers-container" style="display: none">
+                                        <span class="calculate-reviewers">
+                                            <h4>${_('loading...')}</h4>
+                                        </span>
+
+                                        <div id="observers" class="pr-details-content observers">
+                                            ## members goes here, filled via JS based on initial selection !
+                                            <input type="hidden" name="__start__" value="observer_members:sequence">
+                                            <table id="observer_members" class="group_members">
+                                            ## This content is loaded via JS and ReviewersPanel, an sets reviewer_entry class on each element
+                                            </table>
+                                            <input type="hidden" name="__end__" value="observer_members:sequence">
+
+                                            <div id="add_observer_input" class='ac'>
+                                                <div class="observer_ac">
+                                                    ${h.text('observer', class_='ac-input', placeholder=_('Add observer or observer group'))}
+                                                    <div id="observers_container"></div>
+                                                </div>
+                                            </div>
+                                        </div>
+
+                                    </div>
+
+                                </div>
+
+                            </div>
                         </div>
 
                         ## SUBMIT
@@ -339,7 +389,6 b''
 
        //make both panels equal
        $('.target-panel').height($('.source-panel').height())
-
    };
 
    reviewersController = new ReviewersController();
@@ -465,8 +514,7 b''
          queryTargetRefs(initialData, query)
        },
        initSelection: initRefSelection()
-     }
-   );
+   });
 
    var sourceRepoSelect2 = Select2Box($sourceRepo, {
      query: function(query) {}
@@ -543,12 +591,44 b''
    $sourceRef.select2('val', '${c.default_source_ref}');
 
 
-   // default reviewers
+   // default reviewers / observers
    reviewersController.loadDefaultReviewers(
        sourceRepo(), sourceRef(), targetRepo(), targetRef());
    % endif
 
-   ReviewerAutoComplete('#user');
+   ReviewerAutoComplete('#user', reviewersController);
+   ObserverAutoComplete('#observer', reviewersController);
+
+   // TODO, move this to another handler
+
+   var $reviewersBtn = $('#reviewers-btn');
+   var $reviewersContainer = $('#reviewers-container');
+
+   var $observersBtn = $('#observers-btn')
+   var $observersContainer = $('#observers-container');
+
+   $reviewersBtn.on('click', function (e) {
+
+       $observersContainer.hide();
+       $reviewersContainer.show();
+
+       $observersBtn.parent().removeClass('active');
+       $reviewersBtn.parent().addClass('active');
+       e.preventDefault();
+
+   })
+
+   $observersBtn.on('click', function (e) {
+
+       $reviewersContainer.hide();
+       $observersContainer.show();
+
+       $reviewersBtn.parent().removeClass('active');
+       $observersBtn.parent().addClass('active');
+       e.preventDefault();
+
+   })
+
  });
 </script>
 

rhodecode/templates/pullrequests/pullrequest_show.mako

@@ -556,12 +556,12 b''
             <div class="sidebar-element clear-both">
                 <% vote_title = _ungettext(
                         'Status calculated based on votes from {} reviewer',
-                        'Status calculated based on votes from {} reviewers', len(c.allowed_reviewers)).format(len(c.allowed_reviewers))
+                        'Status calculated based on votes from {} reviewers', c.reviewers_count).format(c.reviewers_count)
                 %>
 
                 <div class="tooltip right-sidebar-collapsed-state" style="display: none" onclick="toggleSidebar(); return false" title="${vote_title}">
                     <i class="icon-circle review-status-${c.pull_request_review_status}"></i>
-                    ${len(c.allowed_reviewers)}
+                    ${c.reviewers_count}
                 </div>
 
                 ## REVIEW RULES
@@ -609,13 +609,13 b''
                         <div id="add_reviewer" class="ac" style="display: none;">
                         %if c.allowed_to_update:
                             % if not c.forbid_adding_reviewers:
-                                <div id="add_reviewer_input" class="reviewer_ac">
-                                   ${h.text('user', class_='ac-input', placeholder=_('Add reviewer or reviewer group'))}
+                                <div id="add_reviewer_input" class="reviewer_ac" style="width: 240px">
+                                   <input class="ac-input" id="user" name="user" placeholder="${_('Add reviewer or reviewer group')}" type="text" autocomplete="off">
                                    <div id="reviewers_container"></div>
                                 </div>
                             % endif
-                            <div class="pull-right">
-                                <button id="update_pull_request" class="btn btn-small no-margin">${_('Save Changes')}</button>
+                            <div class="pull-right" style="margin-bottom: 15px">
+                                <button data-role="reviewer" id="update_reviewers" class="btn btn-small no-margin">${_('Save Changes')}</button>
                             </div>
                         %endif
                         </div>
@@ -623,23 +623,52 b''
                 </div>
             </div>
 
-##             ## OBSERVERS
-##             <div class="sidebar-element clear-both">
-##                 <div class="tooltip right-sidebar-collapsed-state" style="display: none" onclick="toggleSidebar(); return false" title="${_('Observers')}">
-##                     <i class="icon-eye"></i>
-##                     0
-##                 </div>
-##
-##                 <div class="right-sidebar-expanded-state pr-details-title">
-##                   <span class="sidebar-heading">
-##                     <i class="icon-eye"></i>
-##                     ${_('Observers')}
-##                   </span>
-##                 </div>
-##                 <div class="right-sidebar-expanded-state pr-details-content">
-##                     No observers
-##                 </div>
-##             </div>
+            ## OBSERVERS
+            <div class="sidebar-element clear-both">
+                <div class="tooltip right-sidebar-collapsed-state" style="display: none" onclick="toggleSidebar(); return false" title="${_('Observers')}">
+                    <i class="icon-circle-thin"></i>
+                    ${c.observers_count}
+                </div>
+
+                <div class="right-sidebar-expanded-state pr-details-title">
+                  <span class="sidebar-heading">
+                    <i class="icon-circle-thin"></i>
+                    ${_('Observers')}
+                  </span>
+                  %if c.allowed_to_update:
+                      <span id="open_edit_observers" class="block-right action_button last-item">${_('Edit')}</span>
+                      <span id="close_edit_observers" class="block-right action_button last-item" style="display: none;">${_('Close')}</span>
+                  %endif
+                </div>
+
+                <div id="observers" class="right-sidebar-expanded-state pr-details-content reviewers">
+                    ## members redering block
+                    <input type="hidden" name="__start__" value="observer_members:sequence">
+
+                    <table id="observer_members" class="group_members">
+                    ## This content is loaded via JS and ReviewersPanel
+                    </table>
+
+                    <input type="hidden" name="__end__" value="observer_members:sequence">
+                    ## end members redering block
+
+                    %if not c.pull_request.is_closed():
+                        <div id="add_observer" class="ac" style="display: none;">
+                        %if c.allowed_to_update:
+                            % if not c.forbid_adding_reviewers or 1:
+                                <div id="add_reviewer_input" class="reviewer_ac" style="width: 240px" >
+                                   <input class="ac-input" id="observer" name="observer" placeholder="${_('Add observer or observer group')}" type="text" autocomplete="off">
+                                   <div id="observers_container"></div>
+                                </div>
+                            % endif
+                            <div class="pull-right" style="margin-bottom: 15px">
+                                <button data-role="observer" id="update_observers" class="btn btn-small no-margin">${_('Save Changes')}</button>
+                            </div>
+                        %endif
+                        </div>
+                    %endif
+                </div>
+            </div>
 
             ## TODOs
             <div class="sidebar-element clear-both">
@@ -815,6 +844,7 b' updateController = new UpdatePrControlle'
 
 window.reviewerRulesData = ${c.pull_request_default_reviewers_data_json | n};
 window.setReviewersData = ${c.pull_request_set_reviewers_data_json | n};
+window.setObserversData = ${c.pull_request_set_observers_data_json | n};
 
 (function () {
     "use strict";
@@ -822,44 +852,9 b' window.setReviewersData = ${c.pull_reque'
     // custom code mirror
     var codeMirrorInstance = $('#pr-description-input').get(0).MarkupForm.cm;
 
-    var PRDetails = {
-        editButton: $('#open_edit_pullrequest'),
-        closeButton: $('#close_edit_pullrequest'),
-        deleteButton: $('#delete_pullrequest'),
-        viewFields: $('#pr-desc, #pr-title'),
-        editFields: $('#pr-desc-edit, #pr-title-edit, .pr-save'),
-
-        init: function () {
-            var that = this;
-            this.editButton.on('click', function (e) {
-                that.edit();
-            });
-            this.closeButton.on('click', function (e) {
-                that.view();
-            });
-        },
-
-        edit: function (event) {
-            var cmInstance = $('#pr-description-input').get(0).MarkupForm.cm;
-            this.viewFields.hide();
-            this.editButton.hide();
-            this.deleteButton.hide();
-            this.closeButton.show();
-            this.editFields.show();
-            cmInstance.refresh();
-        },
-
-        view: function (event) {
-            this.editButton.show();
-            this.deleteButton.show();
-            this.editFields.hide();
-            this.closeButton.hide();
-            this.viewFields.show();
-        }
-    };
-
     PRDetails.init();
     ReviewersPanel.init(reviewerRulesData, setReviewersData);
+    ObserversPanel.init(reviewerRulesData, setObserversData);
 
     window.showOutdated = function (self) {
         $('.comment-inline.comment-outdated').show();
@@ -929,12 +924,17 b' window.setReviewersData = ${c.pull_reque'
                 title, description, renderer);
     });
 
-    $('#update_pull_request').on('click', function (e) {
-        $(this).attr('disabled', 'disabled');
-        $(this).addClass('disabled');
-        $(this).html(_gettext('Saving...'));
+    var $updateButtons = $('#update_reviewers,#update_observers');
+    $updateButtons.on('click', function (e) {
+        var role = $(this).data('role');
+        $updateButtons.attr('disabled', 'disabled');
+        $updateButtons.addClass('disabled');
+        $updateButtons.html(_gettext('Saving...'));
         reviewersController.updateReviewers(
-                "${c.repo_name}", "${c.pull_request.pull_request_id}");
+                templateContext.repo_name,
+                templateContext.pull_request_data.pull_request_id,
+                role
+        );
     });
 
     // fixing issue with caches on firefox
@@ -978,7 +978,8 b' window.setReviewersData = ${c.pull_reque'
         refreshMergeChecks();
     };
 
-    ReviewerAutoComplete('#user');
+    ReviewerAutoComplete('#user', reviewersController);
+    ObserverAutoComplete('#observer', reviewersController);
 
 })();
 

rhodecode/tests/lib/test_mako_emails.py

@@ -23,7 +23,7 b' import collections'
 
 from rhodecode.lib.partial_renderer import PyramidPartialRenderer
 from rhodecode.lib.utils2 import AttributeDict
-from rhodecode.model.db import User
+from rhodecode.model.db import User, PullRequestReviewers
 from rhodecode.model.notification import EmailNotificationModel
 
 
@@ -52,7 +52,8 b' def test_render_email(app, http_host_onl'
     assert 'Email Body' in body
 
 
-def test_render_pr_email(app, user_admin):
[e-mail]('role', PullRequestReviewers.ROLES)
+def test_render_pr_email(app, user_admin, role):
     ref = collections.namedtuple(
         'Ref', 'name, type')('fxies123', 'book')
 
@@ -75,13 +76,17 b' def test_render_pr_email(app, user_admin'
         'pull_request_source_repo_url': 'x',
 
         'pull_request_url': 'http://localhost/pr1',
+        'user_role': role,
     }
 
     subject, body, body_plaintext = EmailNotificationModel().render_email(
         EmailNotificationModel.TYPE_PULL_REQUEST, **kwargs)
 
     # subject
+    if role == PullRequestReviewers.ROLE_REVIEWER:
     assert subject == '@test_admin (RhodeCode Admin) requested a pull request review. !200: "Example Pull Request"'
+    elif role == PullRequestReviewers.ROLE_OBSERVER:
+        assert subject == '@test_admin (RhodeCode Admin) added you as observer to pull request. !200: "Example Pull Request"'
 
 
 def test_render_pr_update_email(app, user_admin):