Changeset - 82ec506f7378
[Not reviewed]
default
0 2 0
Mads Kiilerich - 10 years ago 2016-02-01 21:22:39
madski@unity3d.com
pullrequests: try to improve usability of PR update functionality
2 files changed with 3 insertions and 2 deletions:
0 comments (0 inline, 0 general)
kallithea/controllers/pullrequests.py
Show inline comments
 
@@ -518,193 +518,193 @@ class PullrequestsController(BaseRepoCon
 
    @NotAnonymous()
 
    @HasRepoPermissionAnyDecorator('repository.read', 'repository.write',
 
                                   'repository.admin')
 
    @jsonify
 
    def delete(self, repo_name, pull_request_id):
 
        pull_request = PullRequest.get_or_404(pull_request_id)
 
        #only owner can delete it !
 
        if pull_request.owner.user_id == c.authuser.user_id:
 
            PullRequestModel().delete(pull_request)
 
            Session().commit()
 
            h.flash(_('Successfully deleted pull request'),
 
                    category='success')
 
            raise HTTPFound(location=url('my_pullrequests'))
 
        raise HTTPForbidden()
 

	
 
    @LoginRequired()
 
    @HasRepoPermissionAnyDecorator('repository.read', 'repository.write',
 
                                   'repository.admin')
 
    def show(self, repo_name, pull_request_id, extra=None):
 
        repo_model = RepoModel()
 
        c.users_array = repo_model.get_users_js()
 
        c.user_groups_array = repo_model.get_user_groups_js()
 
        c.pull_request = PullRequest.get_or_404(pull_request_id)
 
        c.allowed_to_change_status = self._get_is_allowed_change_status(c.pull_request)
 
        cc_model = ChangesetCommentsModel()
 
        cs_model = ChangesetStatusModel()
 

	
 
        # pull_requests repo_name we opened it against
 
        # ie. other_repo must match
 
        if repo_name != c.pull_request.other_repo.repo_name:
 
            raise HTTPNotFound
 

	
 
        # load compare data into template context
 
        c.cs_repo = c.pull_request.org_repo
 
        (c.cs_ref_type,
 
         c.cs_ref_name,
 
         c.cs_rev) = c.pull_request.org_ref.split(':')
 

	
 
        c.a_repo = c.pull_request.other_repo
 
        (c.a_ref_type,
 
         c.a_ref_name,
 
         c.a_rev) = c.pull_request.other_ref.split(':') # other_rev is ancestor
 

	
 
        org_scm_instance = c.cs_repo.scm_instance # property with expensive cache invalidation check!!!
 
        c.cs_repo = c.cs_repo
 
        c.cs_ranges = [org_scm_instance.get_changeset(x) for x in c.pull_request.revisions]
 
        c.cs_ranges_org = None # not stored and not important and moving target - could be calculated ...
 
        revs = [ctx.revision for ctx in reversed(c.cs_ranges)]
 
        c.jsdata = json.dumps(graph_data(org_scm_instance, revs))
 

	
 
        c.is_range = False
 
        if c.a_ref_type == 'rev': # this looks like a free range where target is ancestor
 
            cs_a = org_scm_instance.get_changeset(c.a_rev)
 
            root_parents = c.cs_ranges[0].parents
 
            c.is_range = cs_a in root_parents
 
            #c.merge_root = len(root_parents) > 1 # a range starting with a merge might deserve a warning
 

	
 
        avail_revs = set()
 
        avail_show = []
 
        c.cs_branch_name = c.cs_ref_name
 
        other_scm_instance = c.a_repo.scm_instance
 
        c.update_msg = ""
 
        c.update_msg_other = ""
 
        if org_scm_instance.alias == 'hg' and c.a_ref_name != 'ancestor':
 
            if c.cs_ref_type != 'branch':
 
                c.cs_branch_name = org_scm_instance.get_changeset(c.cs_ref_name).branch # use ref_type ?
 
            c.a_branch_name = c.a_ref_name
 
            if c.a_ref_type != 'branch':
 
                try:
 
                    c.a_branch_name = other_scm_instance.get_changeset(c.a_ref_name).branch # use ref_type ?
 
                except EmptyRepositoryError:
 
                    c.a_branch_name = 'null' # not a branch name ... but close enough
 
            # candidates: descendants of old head that are on the right branch
 
            #             and not are the old head itself ...
 
            #             and nothing at all if old head is a descendant of target ref name
 
            if not c.is_range and other_scm_instance._repo.revs('present(%s)::&%s', c.cs_ranges[-1].raw_id, c.a_branch_name):
 
                c.update_msg = _('This pull request has already been merged to %s.') % c.a_branch_name
 
            elif c.pull_request.is_closed():
 
                c.update_msg = _('This pull request has been closed and can not be updated.')
 
            else: # look for descendants of PR head on source branch in org repo
 
                avail_revs = org_scm_instance._repo.revs('%s:: & branch(%s)',
 
                                                         revs[0], c.cs_branch_name)
 
                if len(avail_revs) > 1: # more than just revs[0]
 
                    # also show changesets that not are descendants but would be merged in
 
                    targethead = other_scm_instance.get_changeset(c.a_branch_name).raw_id
 
                    if org_scm_instance.path != other_scm_instance.path:
 
                        # Note: org_scm_instance.path must come first so all
 
                        # valid revision numbers are 100% org_scm compatible
 
                        # - both for avail_revs and for revset results
 
                        hgrepo = unionrepo.unionrepository(org_scm_instance.baseui,
 
                                                           org_scm_instance.path,
 
                                                           other_scm_instance.path)
 
                    else:
 
                        hgrepo = org_scm_instance._repo
 
                    show = set(hgrepo.revs('::%ld & !::parents(%s) & !::%s',
 
                                           avail_revs, revs[0], targethead))
 
                    c.update_msg = _('This pull request can be updated with changes on %s:') % c.cs_branch_name
 
                    c.update_msg = _('The following changes are available on %s:') % c.cs_branch_name
 
                else:
 
                    show = set()
 
                    avail_revs = set() # drop revs[0]
 
                    c.update_msg = _('No changesets found for updating this pull request.')
 

	
 
                # TODO: handle branch heads that not are tip-most
 
                brevs = org_scm_instance._repo.revs('%s - %ld - %s', c.cs_branch_name, avail_revs, revs[0])
 
                if brevs:
 
                    # also show changesets that are on branch but neither ancestors nor descendants
 
                    show.update(org_scm_instance._repo.revs('::%ld - ::%ld - ::%s', brevs, avail_revs, c.a_branch_name))
 
                    show.add(revs[0]) # make sure graph shows this so we can see how they relate
 
                    c.update_msg_other = _('Note: Branch %s has another head: %s.') % (c.cs_branch_name,
 
                        h.short_id(org_scm_instance.get_changeset((max(brevs))).raw_id))
 

	
 
                avail_show = sorted(show, reverse=True)
 

	
 
        elif org_scm_instance.alias == 'git':
 
            c.update_msg = _("Git pull requests don't support updates yet.")
 

	
 
        c.avail_revs = avail_revs
 
        c.avail_cs = [org_scm_instance.get_changeset(r) for r in avail_show]
 
        c.avail_jsdata = json.dumps(graph_data(org_scm_instance, avail_show))
 

	
 
        raw_ids = [x.raw_id for x in c.cs_ranges]
 
        c.cs_comments = c.cs_repo.get_comments(raw_ids)
 
        c.statuses = c.cs_repo.statuses(raw_ids)
 

	
 
        ignore_whitespace = request.GET.get('ignorews') == '1'
 
        line_context = request.GET.get('context', 3)
 
        c.ignorews_url = _ignorews_url
 
        c.context_url = _context_url
 
        c.fulldiff = request.GET.get('fulldiff')
 
        diff_limit = self.cut_off_limit if not c.fulldiff else None
 

	
 
        # we swap org/other ref since we run a simple diff on one repo
 
        log.debug('running diff between %s and %s in %s',
 
                  c.a_rev, c.cs_rev, org_scm_instance.path)
 
        txtdiff = org_scm_instance.get_diff(rev1=safe_str(c.a_rev), rev2=safe_str(c.cs_rev),
 
                                      ignore_whitespace=ignore_whitespace,
 
                                      context=line_context)
 

	
 
        diff_processor = diffs.DiffProcessor(txtdiff or '', format='gitdiff',
 
                                             diff_limit=diff_limit)
 
        _parsed = diff_processor.prepare()
 

	
 
        c.limited_diff = False
 
        if isinstance(_parsed, LimitedDiffContainer):
 
            c.limited_diff = True
 

	
 
        c.files = []
 
        c.changes = {}
 
        c.lines_added = 0
 
        c.lines_deleted = 0
 

	
 
        for f in _parsed:
 
            st = f['stats']
 
            c.lines_added += st['added']
 
            c.lines_deleted += st['deleted']
 
            fid = h.FID('', f['filename'])
 
            c.files.append([fid, f['operation'], f['filename'], f['stats']])
 
            htmldiff = diff_processor.as_html(enable_comments=True,
 
                                              parsed_lines=[f])
 
            c.changes[fid] = [f['operation'], f['filename'], htmldiff]
 

	
 
        # inline comments
 
        c.inline_cnt = 0
 
        c.inline_comments = cc_model.get_inline_comments(
 
                                c.db_repo.repo_id,
 
                                pull_request=pull_request_id)
 
        # count inline comments
 
        for __, lines in c.inline_comments:
 
            for comments in lines.values():
 
                c.inline_cnt += len(comments)
 
        # comments
 
        c.comments = cc_model.get_comments(c.db_repo.repo_id,
 
                                           pull_request=pull_request_id)
 

	
 
        # (badly named) pull-request status calculation based on reviewer votes
 
        (c.pull_request_reviewers,
 
         c.pull_request_pending_reviewers,
 
         c.current_voting_result,
 
         ) = cs_model.calculate_pull_request_result(c.pull_request)
 
        c.changeset_statuses = ChangesetStatus.STATUSES
 

	
 
        c.as_form = False
 
        c.ancestor = None # there is one - but right here we don't know which
 
        return render('/pullrequests/pullrequest_show.html')
 

	
 
    @LoginRequired()
 
    @NotAnonymous()
 
    @HasRepoPermissionAnyDecorator('repository.read', 'repository.write',
 
                                   'repository.admin')
 
    @jsonify
 
    def comment(self, repo_name, pull_request_id):
 
        pull_request = PullRequest.get_or_404(pull_request_id)
 

	
kallithea/templates/pullrequests/pullrequest_show.html
Show inline comments
 
@@ -109,273 +109,274 @@ ${self.repo_context_bar('showpullrequest
 
          <div class="input">
 
            <div>
 
              ${h.link_to_ref(c.pull_request.org_repo.repo_name, c.cs_ref_type, c.cs_ref_name, c.cs_rev)}
 
              %if c.cs_ref_type != 'branch':
 
                ${_('on')} ${h.link_to_ref(c.pull_request.org_repo.repo_name, 'branch', c.cs_branch_name)}
 
              %endif
 
            </div>
 
          </div>
 
        </div>
 
        <div class="field">
 
          <div class="label-summary">
 
              <label>${_('Target')}:</label>
 
          </div>
 
          <div class="input">
 
            %if c.is_range:
 
              ${_("This is just a range of changesets and doesn't have a target or a real merge ancestor.")}
 
            %else:
 
              ${h.link_to_ref(c.pull_request.other_repo.repo_name, c.a_ref_type, c.a_ref_name)}
 
              ## we don't know other rev - c.a_rev is ancestor and not necessarily on other_name_branch branch
 
            %endif
 
          </div>
 
        </div>
 
        <div class="field">
 
          <div class="label-summary">
 
              <label>${_('Pull changes')}:</label>
 
          </div>
 
          <div class="input">
 
              <div>
 
               ## TODO: use cs_ranges[-1] or org_ref_parts[1] in both cases?
 
               %if h.is_hg(c.pull_request.org_repo):
 
                 <span style="font-family: monospace">hg pull ${c.pull_request.org_repo.clone_url()} -r ${h.short_id(c.cs_ranges[-1].raw_id)}</span>
 
               %elif h.is_git(c.pull_request.org_repo):
 
                 <span style="font-family: monospace">git pull ${c.pull_request.org_repo.clone_url()} ${c.pull_request.org_ref_parts[1]}</span>
 
               %endif
 
              </div>
 
          </div>
 
        </div>
 
        <div class="field">
 
          <div class="label-summary">
 
              <label>${_('Created on')}:</label>
 
          </div>
 
          <div class="input">
 
              <div>${h.fmt_date(c.pull_request.created_on)}</div>
 
          </div>
 
        </div>
 
        <div class="field">
 
          <div class="label-summary">
 
              <label>${_('Owner')}:</label>
 
          </div>
 
          <div class="input pr-not-edit">
 
                  <div class="gravatar">
 
                    ${h.gravatar(c.pull_request.owner.email, size=20)}
 
                  </div>
 
                  <span>${c.pull_request.owner.full_name_and_username}</span><br/>
 
                  <span><a href="mailto:${c.pull_request.owner.email}">${c.pull_request.owner.email}</a></span><br/>
 
          </div>
 
          <div class="input pr-do-edit ac" style="display:none">
 
               ${h.text('owner', class_="large", value=c.pull_request.owner.username, placeholder=_('Username'))}
 
               <div id="owner_completion_container"></div>
 
          </div>
 
        </div>
 

	
 
        <div class="field">
 
            <div class="label-summary">
 
              <label>${_('Update')}:</label>
 
            </div>
 
            <div class="input">
 
              <div class="msg-div">${c.update_msg}</div>
 
              %if c.avail_revs:
 
              <div id="updaterevs" style="max-height:200px; overflow-y:auto; overflow-x:hidden; margin-bottom: 10px; padding: 1px 0">
 
                <div style="height:0">
 
                  <canvas id="avail_graph_canvas" style="width:0"></canvas>
 
                </div>
 
                <table id="updaterevs-table" class="noborder" style="padding-left:50px">
 
                  %for cnt, cs in enumerate(c.avail_cs):
 
                    <tr id="chg_available_${cnt+1}">
 
                      %if cs.revision == c.cs_ranges[-1].revision:
 
                        <td>
 
                          %if editable:
 
                            ${h.radio(name='updaterev', value='', checked=True)}
 
                          %endif
 
                        </td>
 
                        <td colspan="2">${_("Current revision - no change")}</td>
 
                      %else:
 
                        <td>
 
                          %if editable and cs.revision in c.avail_revs:
 
                            ${h.radio(name='updaterev', value=cs.raw_id)}
 
                          %endif
 
                        </td>
 
                        <td>${h.link_to(h.show_id(cs),h.url('changeset_home',repo_name=c.cs_repo.repo_name,revision=cs.raw_id))}</td>
 
                        <td><div class="message" style="white-space:normal; height:1.1em; max-width: 500px; padding:0">${h.urlify_commit(cs.message, c.repo_name)}</div></td>
 
                      %endif
 
                    </tr>
 
                  %endfor
 
                </table>
 
              </div>
 
              <div class="msg-div">${_("Pull requests do not change once created. Select a revision and save to replace this pull request with a new one.")}</div>
 
              %endif
 
              <div class="msg-div">${c.update_msg_other}</div>
 
            </div>
 
        </div>
 
      </div>
 
    </div>
 
    ## REVIEWERS
 
    <div style="float:left; border-left:1px dashed #eee">
 
        <div class="pr-details-title">${_('Pull Request Reviewers')}</div>
 
        <div id="reviewers" style="padding:0px 0px 5px 10px">
 
          ## members goes here !
 
          <div>
 
            <ul id="review_members" class="group_members">
 
            %for member,status in c.pull_request_reviewers:
 
              ## WARNING: the HTML below is duplicate with
 
              ## kallithea/public/js/base.js
 
              ## If you change something here it should be reflected in the template too.
 
              <li id="reviewer_${member.user_id}">
 
                <div class="reviewers_member">
 
                    <div class="reviewer_status tooltip" title="${h.changeset_status_lbl(status.status if status else 'not_reviewed')}">
 
                      <i class="icon-circle changeset-status-${status.status if status else 'not_reviewed'}"></i>
 
                    </div>
 
                  <div class="reviewer_gravatar gravatar">
 
                    ${h.gravatar(member.email, size=14)}
 
                  </div>
 
                  <div style="float:left;">
 
                    ${member.full_name_and_username}
 
                    %if c.pull_request.user_id == member.user_id:
 
                      (${_('Owner')})
 
                    %endif
 
                  </div>
 
                  <input type="hidden" value="${member.user_id}" name="review_members" />
 
                  %if editable:
 
                  <div class="reviewer_member_remove action_button" onclick="removeReviewMember(${member.user_id})" title="${_('Remove reviewer')}">
 
                      <i class="icon-minus-circled"></i>
 
                  </div>
 
                  %endif
 
                </div>
 
              </li>
 
            %endfor
 
            </ul>
 
          </div>
 
          %if editable:
 
          <div class='ac'>
 
            <div class="reviewer_ac">
 
               ${h.text('user', class_='yui-ac-input',placeholder=_('Type name of reviewer to add'))}
 
               <div id="reviewers_container"></div>
 
            </div>
 
          </div>
 
          %endif
 
        </div>
 

	
 
        %if not c.pull_request_reviewers:
 
        <div class="pr-details-title">${_('Potential Reviewers')}</div>
 
        <div style="margin: 10px 0 10px 10px; max-width: 250px">
 
          <div>
 
            ${_('Click to add the repository owner as reviewer:')}
 
          </div>
 
          <ul style="margin-top: 10px">
 
            %for u in [c.pull_request.other_repo.user]:
 
              <li>
 
                <a class="missing_reviewer missing_reviewer_${u.user_id}"
 
                  user_id="${u.user_id}"
 
                  fname="${u.name}"
 
                  lname="${u.lastname}"
 
                  nname="${u.username}"
 
                  gravatar_lnk="${h.gravatar_url(u.email, size=28)}"
 
                  gravatar_size="14"
 
                  title="Click to add reviewer to the list, then Save Changes.">${u.full_name}</a>
 
              </li>
 
            %endfor
 
          </ul>
 
        </div>
 
        %endif
 
    </div>
 
    <div class="form" style="clear:both">
 
      <div class="fields">
 
        %if editable:
 
          <div class="buttons">
 
            ${h.submit('pr-form-save',_('Save Changes'),class_="btn btn-small")}
 
            ${h.submit('pr-form-clone',_('Save as New Pull Request'),class_="btn btn-small",disabled='disabled')}
 
            ${h.submit('pr-form-clone',_('Save Updates as New Pull Request'),class_="btn btn-small",disabled='disabled')}
 
            ${h.reset('pr-form-reset',_('Cancel Changes'),class_="btn btn-small")}
 
          </div>
 
        %endif
 
      </div>
 
    </div>
 
  ${h.end_form()}
 
</div>
 

	
 
<div class="box">
 
    <div class="title">
 
      <div class="breadcrumbs">${_('Pull Request Content')}</div>
 
    </div>
 
    <div class="table">
 
          <div id="changeset_compare_view_content">
 
              <div style="font-size:1.1em;font-weight: bold;clear:both;padding-top:10px">
 
                  ${comment.comment_count(c.inline_cnt, len(c.comments))}
 
              </div>
 
              ##CS
 
              <div style="font-size:1.1em;font-weight: bold;clear:both;padding-top:10px">
 
                ${ungettext('Showing %s commit','Showing %s commits', len(c.cs_ranges)) % len(c.cs_ranges)}
 
              </div>
 
              <%include file="/compare/compare_cs.html" />
 

	
 
              <div style="font-size:1.1em;font-weight: bold;clear:both;padding-top:10px">
 
              ${_('Common ancestor')}:
 
              ${h.link_to(h.short_id(c.a_rev),h.url('changeset_home',repo_name=c.a_repo.repo_name,revision=c.a_rev))}
 
              </div>
 

	
 
              ## FILES
 
              <div style="font-size:1.1em;font-weight: bold;clear:both;padding-top:10px">
 

	
 
              % if c.limited_diff:
 
                  ${ungettext('%s file changed', '%s files changed', len(c.files)) % len(c.files)}
 
              % else:
 
                  ${ungettext('%s file changed with %s insertions and %s deletions','%s files changed with %s insertions and %s deletions', len(c.files)) % (len(c.files),c.lines_added,c.lines_deleted)}:
 
              %endif
 

	
 
              </div>
 
              <div class="cs_files">
 
                %if not c.files:
 
                   <span class="empty_data">${_('No files')}</span>
 
                %endif
 
                %for fid, change, f, stat in c.files:
 
                    <div class="cs_${change}">
 
                      <div class="node">
 
                          <i class="icon-diff-${change}"></i>
 
                          ${h.link_to(h.safe_unicode(f),'#' + fid)}
 
                      </div>
 
                      <div class="changes">${h.fancy_file_stats(stat)}</div>
 
                    </div>
 
                %endfor
 
              </div>
 
              % if c.limited_diff:
 
                <h5>${_('Changeset was too big and was cut off...')} <a href="${h.url.current(fulldiff=1, **request.GET.mixed())}">${_('Show full diff anyway')}</a></h5>
 
              % endif
 
          </div>
 
    </div>
 
    <script>
 
    var _USERS_AC_DATA = ${c.users_array|n};
 
    var _GROUPS_AC_DATA = ${c.user_groups_array|n};
 
    // TODO: switch this to pyroutes
 
    AJAX_COMMENT_URL = "${url('pullrequest_comment',repo_name=c.repo_name,pull_request_id=c.pull_request.pull_request_id)}";
 
    AJAX_COMMENT_DELETE_URL = "${url('pullrequest_comment_delete',repo_name=c.repo_name,comment_id='__COMMENT_ID__')}";
 

	
 
    pyroutes.register('pullrequest_comment', "${url('pullrequest_comment',repo_name='%(repo_name)s',pull_request_id='%(pull_request_id)s')}", ['repo_name', 'pull_request_id']);
 
    pyroutes.register('pullrequest_comment_delete', "${url('pullrequest_comment_delete',repo_name='%(repo_name)s',comment_id='%(comment_id)s')}", ['repo_name', 'comment_id']);
 

	
 
    </script>
 

	
 
    ## diff block
 
    <div class="commentable-diff">
 
    <%namespace name="diff_block" file="/changeset/diff_block.html"/>
 
    ${diff_block.diff_block_js()}
 
    ${diff_block.diff_block_simple(c.files, c.changes)}
 
    % if c.limited_diff:
 
      <h4>${_('Changeset was too big and was cut off...')} <a href="${h.url.current(fulldiff=1, **request.GET.mixed())}">${_('Show full diff anyway')}</a></h4>
 
    % endif
 
    </div>
 

	
 
    ## template for inline comment form
 
    ${comment.comment_inline_form()}
 

	
 
    ## render comments and inlines
 
    ${comment.generate_comments()}
 

	
 
    ## main comment form and it status
 
    ${comment.comments(change_status=c.allowed_to_change_status)}
 

	
 
    <script type="text/javascript">
 
      $(document).ready(function(){
 
          PullRequestAutoComplete($('#user'), $('#reviewers_container'), _USERS_AC_DATA);
 
          SimpleUserAutoComplete($('#owner'), $('#owner_completion_container'), _USERS_AC_DATA);
 

	
 
          $('.code-difftable').on('click', '.add-bubble', function(e){
 
              show_comment_form($(this));
 
          });
0 comments (0 inline, 0 general)