source: peerreviewplugin/trunk/codereview/changeset.py

Last change on this file was 18287, checked in by Cinc-th, 2 years ago

PeerReviewPlugin: improvements to changeset reviews:

  • Allow to create another changeset review when old review was closed.
  • Show pin icon in lines with comments
  • Hide/show comments

Some minor improvements to presentation like titles for revision links.

Refs #14007

File size: 17.2 KB
RevLine 
[17458]1# -*- coding: utf-8 -*-
[18053]2#
3# Copyright (C) 2019-2021 Cinc
4#
5# All rights reserved.
6#
7# This software is licensed as described in the file COPYING.txt, which
8# you should have received as part of this distribution.
[17458]9import itertools
10
11from codereview.model import get_users, PeerReviewModel, PeerReviewerModel, \
12    ReviewDataModel, ReviewFileModel
13from codereview.peerReviewCommentCallback import writeJSONResponse, writeResponse
[18284]14from codereview.repo import file_lines_from_node, hash_from_file_node
[17458]15from codereview.repobrowser import get_node_from_repo
[18287]16from codereview.util import get_files_for_review_id, to_trac_path
[18266]17from functools import partial
[17458]18from trac.core import Component, implements
19from trac.resource import get_resource_url, Resource
[18048]20from trac.util.translation import _
[18286]21from trac.versioncontrol.api import Changeset, Node, RepositoryManager
[18284]22from trac.versioncontrol.diff import diff_blocks
23from trac.versioncontrol.web_ui.util import get_existing_node
[17458]24from trac.web.chrome import Chrome
25from trac.web.api import IRequestFilter, IRequestHandler
[18260]26from trac.web.chrome import add_script, add_script_data, add_stylesheet, web_context
27from trac.wiki.formatter import format_to_oneliner
[17458]28
29
30class PeerChangeset(Component):
[18260]31    """Create review for a changeset from the changeset page.
32
33    The created review holds the files from the changeset.
34
35    '''Note''': This plugin may be disabled without side effects.
36    """
[17458]37    implements(IRequestFilter, IRequestHandler)
38
39    # IRequestFilter methods
40
41    def pre_process_request(self, req, handler):
[18243]42        """Always returns the request handler, even if unchanged."""
[17458]43        return handler
44
45    def post_process_request(self, req, template, data, content_type):
[18243]46        """Do any post-processing the request might need;
[17458]47        `data` may be updated in place.
48
49        Always returns a tuple of (template, data, content_type), even if
50        unchanged.
51
52        Note that `template`, `data`, `content_type` will be `None` if:
53         - called when processing an error page
54         - the default request handler did not return any result
55        """
56        # Note that data is already filled with information about the source file, repo and what not
57        # We only handle the changeset page
58        if req.path_info.startswith('/changeset/'):
59            if data and 'changes' in data and data['changes']:
[18263]60                cset = data.get('new_rev', '')
[18271]61                reponame = data.get('reponame', '')
[18263]62                f_data = {}
[18271]63                review = get_review_for_changeset(self.env, cset, reponame)
[18265]64                if 'CODE_REVIEW_VIEW' in req.perm and cset:
[18271]65                    f_data = self.file_dict_from_changeset(req, cset, reponame, review)
[18263]66
[17458]67                add_stylesheet(req, 'hw/css/peerreview.css')
[18271]68                jdata = {'peer_repo': reponame,
[18264]69                         'peer_rev': cset,
70                         'peer_changeset_url': req.href.peerreviewchangeset(),
71                         'peer_comment_url': req.href.peercomment(),
72                         'tacUrl': req.href.chrome('/hw/images/thumbtac11x11.gif'),
[18265]73                         'peer_perm_dev': 0,
[18287]74                         'peer_new_file': 0,  # for deciding if we need to reload the page.
75                         'peer_pin_icon': req.href.chrome('/hw/images/thumbtac11x11.gif'),
[18264]76                         }
77                if f_data:
78                    jdata['peer_file_comments'] = f_data
[18265]79                if 'CODE_REVIEW_DEV' in req.perm:
80                    jdata['peer_perm_dev'] = 1
[18264]81
[18287]82                # These are files which were copied, moved or added. Trac normally doesn't
83                # have a diff view for them so add the data here.
84                for change in data['changes']:
85                    if change['change'] in ('add', 'copy', 'move') and not change['diffs']:
86                        jdata['peer_new_file'] = 1  # We reload the page after review creation
87                        if review:
[18284]88                            node = get_existing_node(self.env, data['repos'],
89                                                     change['new']['path'], change['new']['rev'])
90                            lines = file_lines_from_node(node)
91                            if lines:
92                                change['diffs'] = diff_blocks([], lines)
[18287]93                                # Don't add 'href' to this dict otherwise the header with revision
94                                # info will become a link. A value of None or '' won't prevent it.
[18284]95                                change['old'] = {'rev': ' ---',
[18287]96                                                 'shortrev': ' ---',
97                                                 }
[18286]98
[18287]99                add_script_data(req, jdata)
100                add_script(req, "hw/js/peer_trac_changeset.js")
101                add_script(req, "hw/js/peer_user_list.js")
102                Chrome(self.env).add_jquery_ui(req)
103                add_script(req, 'common/js/folding.js')
104
[17458]105        return template, data, content_type
106
[18271]107    def file_dict_from_changeset(self, req, cset, reponame, review=None):
108        """Get a dict with files belonging to this changeset when a review exists.
[18264]109
[18271]110        :param req: Request object. Needed internally to getz the authname
111        :param cset: this changeset revision. Not a changeset object of any kind.
112        :param reponame: name of the repository this changest belongs to.
113        :param review: Review object associated with this changeset. If this is None it will
114                       be queried here.
[18264]115        :return: a dict with key: file path, val: list [fileid, [line #, line #, ...]]
116                 note: the list contains line numbers.
[18271]117                 If there is no review already created for this changeset the dict is
118                 empty.
[18264]119        """
120        f_data = {}
[18271]121        review = review or get_review_for_changeset(self.env, cset, reponame)
[18264]122        if review:
[18287]123            # We ask for include comment inforamtion here
[18264]124            rfiles = get_files_for_review_id(self.env, req, review['review_id'], True)
125            for rfile in rfiles:
[18287]126                path = to_trac_path(rfile['path'])  # Trac path doesn't start with '/'. Db path does.
[18264]127                lines = set([comment['line_num'] for comment in rfile.comment_data])
128                if lines:
[18273]129                    f_data[path] = [rfile['file_id'], list(lines)]
[18264]130                else:
[18273]131                    f_data[path] = [rfile['file_id'], []]
[18264]132        return f_data
133
[17458]134    # IRequestHandler methods
135
136    def create_review_form(self, req):
137        _form = """
138<form id="create-peerreview-form" action="">
139  <input type="hidden" name="peer_repo" value="{reponame}" />
140  <input type="hidden" name="peer_rev" value="{rev}" />
141  <input type="hidden" name="Name" value="Changeset {rev} in repository '{reponame}'" />
142  <input type="hidden" name="__FORM_TOKEN" value="{form-token}" />
143  <fieldset>
144    <legend>Select reviewers for your review.</legend>
[18287]145    {userselect}
[17458]146    <div class="buttons">
147      <input id="create-review-submit" type="submit" name="create" value="Create Code Review"/>
148    </div>
[18287]149    <div><p class="help">{reload}</p></div>
[17458]150  </fieldset>
151</form>
152<div id="user-rem-confirm" title="Remove user?">
153    <p>
154    <span class="ui-icon ui-icon-alert" style="float:left; margin:0 7px 20px 0;"></span>
155    Really remove the user <strong id="user-rem-name"></strong> from the list?</p>
156</div>
157"""
[18263]158        tmpl_permission = """
159        <div class="collapsed">
160          <h3 class="foldable">{title}</h3>
161          <div id="peer-codereview">
162             %s
163          </div>
164        </div>
165        """.format(title=_('Codereview'))
[17459]166
[18264]167        if 'CODE_REVIEW_DEV' not in req.perm:
[17459]168            res = '<div id="peer-msg" class="system-message warning">%s</div>' % \
169                  _("You don't have permission to create a code review.")
[18263]170            return tmpl_permission % res
[17459]171
[17458]172        users = get_users(self.env)
[18266]173        chrome = Chrome(self.env)
[17458]174        data = {
175            'form-token': req.form_token,
176            'reponame': req.args.get('peer_repo', ''),
177            'rev': req.args.get('peer_rev', ''),
178            'new': 'yes',
179            'users': users,
[18266]180            'cycle': itertools.cycle,
[18287]181            'authorinfo': partial(chrome.authorinfo, req),
182            'reload': _("The page will be reloaded.") if req.args.getint('peer_new_file') == 1 else ""
[17458]183        }
[18257]184        if hasattr(Chrome, 'jenv'):
185            template = chrome.load_template('peerreviewuser_jinja.html')
[18287]186            data['userselect'] = chrome.render_template_string(template, data)
[18257]187        else:
188            template = chrome.load_template('user_list.html', None)
[18287]189            data['userselect'] = template.generate(**data).render()
[17458]190
191        peerreview_div = '<div class="collapsed"><h3 class="foldable">%s</h3>' \
[18263]192                         '<div id="peer-codereview">%s</div>' \
193                         '</div><div><p class="help">%s</p></div>' % \
[18287]194                         (_('Codereview'), _form.format(**data),
[18263]195                          _("You need to create a review if you want to comment on files."))
[17458]196        return peerreview_div
197
[18263]198    def create_review_info(self, req, review, create=False):
199        """Create html with inforamtion about the current changeset review.
200        :param req: Request object
201        :param review: Review object associated with this review
202        :param create: True if the review was just created and the page is updated,
203                       False if the page is normally loaded. There may be some
204                       presentation differences.
205        """
[18243]206        _rev_info = u"""
[18258]207            <dt class="property review">{review_id_label}</dt>
[18260]208            <dd class="review">{review_wiki}
[18243]209              <small><em>{reviewer_id_help}</em></small>
[17458]210            </dd>
[18243]211            <dt class="property">{status_label}</dt>
212            <dd>{status}</dd>
213            <dt class="property">{reviewers_label}</dt>
214            <dd>{user_list}</dd>
[17458]215        """
[18263]216        review_tmpl = """
217        <div>
218            <dl id="peer-review-info">
219              %s
220            </dl>
221        </div>
222        """.format(title=_('Codereview'))
[18287]223
224        def create_user_list(reviewer):
[18258]225            chrome = Chrome(self.env)
[18243]226            if reviewer:
[18262]227                usr = [u'<tr><td class="user-icon"><span class="ui-icon ui-icon-person"></span></td><td>%s</td></tr>' % chrome.authorinfo(req,
228                                                                                                      item['reviewer'])
229                       for item in reviewer]
[18243]230                li = u"".join(usr)
231            else:
[18262]232                li = '<tr><td>{msg}</td></tr>'.format(msg=_("There are no users included in this code review."))
233            return u'<table id="userlist">{li}</table>'.format(li=li)
[17458]234
[17459]235        if 'CODE_REVIEW_VIEW' not in req.perm:
[18264]236            no_perm_tmpl = """<dt class="property" style="margin-top: 1em">Review:</dt>
237            <dd style="margin-top: 1em"><span>{msg}</span></dd>"""
[18265]238            # TODO: this kind of information disclosure (you learn that a review exists) should be
239            #       removed. Same for the permission msg when creating reviews
[18243]240            return no_perm_tmpl.format(msg=_("You don't have permission to view code review information."))
[17459]241
[17458]242        res = Resource('peerreview', review['review_id'])
243        data = {
[18243]244            'status': review['status'],
[18287]245            'user_list': create_user_list(list(PeerReviewerModel.select_by_review_id(self.env, review['review_id']))),
[18243]246            'review_url': get_resource_url(self.env, res, req.href),
247            'review_id': review['review_id'],
248            'status_label': _("Status:"),
[18260]249            'review_id_label': _("Review:"),
[18243]250            'reviewers_label': _("Reviewers:"),
[18263]251            'reviewer_id_help': _("(click to open review page)"),
[18260]252            'review_wiki': format_to_oneliner(self.env, web_context(req),
253                                              "[review:{r_id} Review {r_id}]".format(r_id=review['review_id']))
[17458]254        }
255
[18263]256        if create:
257            return review_tmpl % _rev_info.format(**data)
[17458]258        else:
[18243]259            return _rev_info.format(**data)
[17458]260
261    def match_request(self, req):
262        return req.path_info == '/peerreviewchangeset'
263
264    def process_request(self, req):
265
266        if req.method == 'POST':
[17459]267            req.perm.require('CODE_REVIEW_DEV')
268
[17458]269            review = create_changeset_review(self, req)
270            if not review:
[18263]271                data = {'html': '<div id="peer-msg" class="system-message warning">%s</div>' %
[18264]272                                _('Error while creating Review.'),
273                        'success': 0}
[18263]274                writeJSONResponse(req, data)
[17458]275            else:
[18264]276                f_data = self.file_dict_from_changeset(req, req.args.get('peer_rev'), req.args.get('peer_repo'))
277
278                data = {'html': self.create_review_info(req, review, True),
279                        'filedata': f_data,
280                        'success': 1}
[18263]281                writeJSONResponse(req, data)
[17458]282            return
283
284        dm = ReviewDataModel(self.env)
285        dm['type'] = 'changeset'
286        dm['data'] = "%s:%s" % (req.args.get('peer_repo', ''), req.args.get('peer_rev', ''))
287        rev_data = list(dm.list_matching_objects())
288
[17459]289        # Permission handling is done in the called methods so the proper message is created there.
[17458]290        if not rev_data:
291            data = {'action': 'create',
292                    'html': self.create_review_form(req)}
293            writeJSONResponse(req, data)
294        else:
295            review = PeerReviewModel(self.env, rev_data[-1]['review_id'])
296            data = {'action': 'info',
[18263]297                    'html': self.create_review_info(req, review, False)}
[17458]298            writeJSONResponse(req, data)
299
300
301def create_changeset_review(self, req):
302    """Create a new code review from the data in the request object req.
303
304    Takes the information given when the page is posted and creates a
305    new code review in the database and populates it with the
306    information. Also creates new reviewer and file data for
307    the review.
308    """
309    rev = req.args.get('peer_rev')
310    reponame = req.args.get('peer_repo')
311    repo = RepositoryManager(self.env).get_repository(reponame)
312    if not repo:
313        return None
314
315    changeset = repo.get_changeset(rev)
316    if not changeset:
317        return None
318
319    review = PeerReviewModel(self.env)
320    review['owner'] = req.authname
321    review['name'] = req.args.get('Name')
322    review['notes'] = req.args.get('Notes')
323    if req.args.get('project'):
324        review['project'] = req.args.get('project')
325    review.insert()
326    id_ = review['review_id']
327
328    # Create reviewer entries
[18253]329    user = req.args.getlist('user')
[17458]330    if not type(user) is list:
331        user = [user]
332    for name in user:
333        if name != "":
334            reviewer = PeerReviewerModel(self.env)
335            reviewer['review_id'] = id_
336            reviewer['reviewer'] = name
337            reviewer['vote'] = -1
338            reviewer.insert()
339
340    # Create file entries
341    path, kind, change, base_path, base_rev = range(0, 5)
342    for item in changeset.get_changes():
[18286]343        if item[change] != Changeset.DELETE:
344            rfile = ReviewFileModel(self.env)
345            rfile['review_id'] = id_
346            # Path must have leading '/' in the database for historical reasons.
347            rfile['path'] = u'/' + item[path].lstrip('/')
348            rfile['revision'] = rev
349            rfile['line_start'] = 0
350            rfile['line_end'] = 0
351            rfile['repo'] = reponame
352            node, display_rev, context = get_node_from_repo(req, repo, rfile['path'], rfile['revision'])
353            if node and node.kind == Node.FILE:
354                rfile['changerevision'] = rev
355                rfile['hash'] = hash_from_file_node(node)
356                rfile.insert()
[17458]357
[18260]358    # Mark that this is a changeset review
[17458]359    dm = ReviewDataModel(self.env)
360    dm['review_id'] = id_
361    dm['type'] = 'changeset'
362    dm['data'] = "%s:%s" % (reponame, rev)
363    dm.insert()
364    return review
[17460]365
366
[18263]367def get_review_for_changeset(env, cs_num, repo_name):
368    """Get a PeerReview from the given repo:changset combination.
369     Return None if not found."""
370    dm = ReviewDataModel(env)
371    dm['type'] = 'changeset'
[18287]372    dm['data'] = "%s:%s" % (repo_name, cs_num)  # this will automatically skip closed reviews
[18263]373    rev_data = list(dm.list_matching_objects())
374
375    # Permission handling is done in the called methods so the proper message is created there.
376    if rev_data:
377        return PeerReviewModel(env, rev_data[-1]['review_id'])
378    else:
379        return None
380
381
[17460]382def get_changeset_data(env, review_id):
383    """Return changeset information for the given review id if any.
384
385    Checks the table 'peerreviewdata' for a changeset entry for this
386    review id.
387
388    @param review_id: numeric id of a review
389    @return: list [reponame, changeset] or ['', ''] if no changeset review
390    """
391    dm = ReviewDataModel(env)
392    dm['type'] = 'changeset'
393    dm['review_id'] = review_id
394    rev_data = list(dm.list_matching_objects())
395    if not rev_data:
396        return ['', '']
[18287]397    return rev_data[-1]['data'].split(':')[:2]
Note: See TracBrowser for help on using the repository browser.