pull-requests: loosen strict view of pull-requests that state is changing

- we now replace it with limited view which still allows deletion/editing of reviewers or title and description - added labels for state changing instead of grey out (it was used for closed PRs) - small UI fixes
2019-11-27 12:43:52 +01:00 · 2019-11-27 12:43:52 +01:00 · 9143b86d83
commit 9143b86d83
parent ad10456d3a
9 changed files with 98 additions and 54 deletions
--- a/rhodecode/apps/my_account/views/my_account.py
+++ b/rhodecode/apps/my_account/views/my_account.py
@ -682,8 +682,8 @@ class MyAccountView(BaseAppView, DataGridAppView):
                'target_repo': _render('pullrequest_target_repo',
                                       pr.target_repo.repo_name),
                'name': _render('pullrequest_name',
-                                pr.pull_request_id, pr.work_in_progress,
-                                pr.target_repo.repo_name,
+                                pr.pull_request_id, pr.pull_request_state,
+                                pr.work_in_progress, pr.target_repo.repo_name,
                                short=True),
                'name_raw': pr.pull_request_id,
                'status': _render('pullrequest_status',
--- a/rhodecode/apps/repository/views/repo_pull_requests.py
+++ b/rhodecode/apps/repository/views/repo_pull_requests.py
@ -108,8 +108,8 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):

            data.append({
                'name': _render('pullrequest_name',
-                                pr.pull_request_id, pr.work_in_progress,
-                                pr.target_repo.repo_name),
+                                pr.pull_request_id, pr.pull_request_state,
+                                pr.work_in_progress, pr.target_repo.repo_name),
                'name_raw': pr.pull_request_id,
                'status': _render('pullrequest_status',
                                  pr.calculated_review_status()),
@ -273,15 +273,7 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):
            self.request.matchdict['pull_request_id'])
        pull_request_id = pull_request.pull_request_id

-        if pull_request.pull_request_state != PullRequest.STATE_CREATED:
-            log.debug('show: forbidden because pull request is in state %s',
-                      pull_request.pull_request_state)
-            msg = _(u'Cannot show pull requests in state other than `{}`. '
-                    u'Current state is: `{}`').format(PullRequest.STATE_CREATED,
-                                                      pull_request.pull_request_state)
-            h.flash(msg, category='error')
-            raise HTTPFound(h.route_path('pullrequest_show_all',
-                                         repo_name=self.db_repo_name))
+        c.state_progressing = pull_request.is_state_changing()

        version = self.request.GET.get('version')
        from_version = self.request.GET.get('from_version') or version
@ -1061,15 +1053,7 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):
            return {'response': True,
                    'redirect_url': redirect_url}

-        if pull_request.pull_request_state != PullRequest.STATE_CREATED:
-            log.debug('update: forbidden because pull request is in state %s',
-                      pull_request.pull_request_state)
-            msg = _(u'Cannot update pull requests in state other than `{}`. '
-                    u'Current state is: `{}`').format(PullRequest.STATE_CREATED,
-                                                      pull_request.pull_request_state)
-            h.flash(msg, category='error')
-            return {'response': True,
-                    'redirect_url': redirect_url}
+        is_state_changing = pull_request.is_state_changing()

        # only owner or admin can update it
        allowed_to_update = PullRequestModel().check_user_update(
@ -1083,6 +1067,16 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):
                    pull_request, controls['review_members'],
                    pull_request.reviewer_data)
            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',
+                              pull_request.pull_request_state)
+                    msg = _(u'Cannot update pull requests commits in state other than `{}`. '
+                            u'Current state is: `{}`').format(
+                        PullRequest.STATE_CREATED, pull_request.pull_request_state)
+                    h.flash(msg, category='error')
+                    return {'response': True,
+                            'redirect_url': redirect_url}
+
                self._update_commits(pull_request)
                if force_refresh:
                    redirect_url = h.route_path(
@ -1182,7 +1176,7 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):
            self.request.matchdict['pull_request_id'])
        _ = self.request.translate

-        if pull_request.pull_request_state != PullRequest.STATE_CREATED:
+        if pull_request.is_state_changing():
            log.debug('show: forbidden because pull request is in state %s',
                      pull_request.pull_request_state)
            msg = _(u'Cannot merge pull requests in state other than `{}`. '
--- a/rhodecode/model/db.py
+++ b/rhodecode/model/db.py
@ -4225,6 +4225,9 @@ class PullRequest(Base, _PullRequestBase):
            def is_closed(self):
                return pull_request_obj.is_closed()

+            def is_state_changing(self):
+                return pull_request_obj.is_state_changing()
+
            @property
            def pull_request_version_id(self):
                return getattr(pull_request_obj, 'pull_request_version_id', None)
@ -4256,6 +4259,9 @@ class PullRequest(Base, _PullRequestBase):
    def is_closed(self):
        return self.status == self.STATUS_CLOSED

+    def is_state_changing(self):
+        return self.pull_request_state != PullRequest.STATE_CREATED
+
    def __json__(self):
        return {
            'revisions': self.revisions,
@ -4313,6 +4319,9 @@ class PullRequestVersion(Base, _PullRequestBase):
        # calculate from original
        return self.pull_request.status == self.STATUS_CLOSED

+    def is_state_changing(self):
+        return self.pull_request.pull_request_state != PullRequest.STATE_CREATED
+
    def calculated_review_status(self):
        return self.pull_request.calculated_review_status()

--- a/rhodecode/public/css/main.less
+++ b/rhodecode/public/css/main.less
@ -414,16 +414,28 @@ ul.auth_plugins {
 }


-#pr-title-input {
-  width: 72%;
+.pr-title-input {
+  width: 80%;
  font-size: 1em;
-  margin: 0;
-  padding: 0 0 0 @padding/4;
+  margin:  0 0 4px 0;
+  padding: 0;
  line-height: 1.7em;
  color: @text-color;
  letter-spacing: .02em;
  font-weight: @text-bold-weight;
  font-family: @text-bold;
+
+  &:hover {
+    box-shadow: none;
+  }
+}
+
+#pr-title {
+  input {
+    border: 1px transparent;
+    color: black;
+    opacity: 1
+  }
 }

 #pullrequest_title {
--- a/rhodecode/public/css/tags.less
+++ b/rhodecode/public/css/tags.less
@ -62,6 +62,23 @@
 }


+.tag-merge-state-created {
+    color: @color1;
+}
+
+.tag-merge-state-creating {
+    color: @color1;
+}
+
+.tag-merge-state-merging {
+    color: @color3;
+}
+
+.tag-merge-state-updating {
+    color: @color3;
+}
+
+
 .metatag-list {
    margin: 0;
    padding: 0;
--- a/rhodecode/templates/admin/my_account/my_account_pullrequests.mako
+++ b/rhodecode/templates/admin/my_account/my_account_pullrequests.mako
@ -47,10 +47,10 @@ $(document).ready(function() {
          pageLength: ${c.visual.dashboard_items},
          order: [[ 2, "desc" ]],
          columns: [
-             { data: {"_": "status",
-                      "sort": "status"}, title: "", className: "td-status", orderable: false},
             { data: {"_": "target_repo",
                      "sort": "target_repo"}, title: "${_('Target Repo')}", className: "td-targetrepo", orderable: false},
+             { data: {"_": "status",
+                      "sort": "status"}, title: "", className: "td-status", orderable: false},
             { data: {"_": "name",
                      "sort": "name_raw"}, title: "${_('Id')}", className: "td-componentname", "type": "num" },
             { data: {"_": "title",
@ -78,9 +78,6 @@ $(document).ready(function() {
              if (data['owned']) {
                $(row).addClass('owned');
              }
-              if (data['state'] !== 'created') {
-                  $(row).addClass('state-' + data['state']);
-              }
          }
        });
        $pullRequestListTable.on('xhr.dt', function(e, settings, json, xhr){
--- a/rhodecode/templates/data_table/_dt_elements.mako
+++ b/rhodecode/templates/data_table/_dt_elements.mako
@ -370,17 +370,22 @@ ${h.style_metatag(tag_type, tag)|n,trim}
    <i class="icon-comment"></i> ${comments_nr}
 </%def>

-<%def name="pullrequest_name(pull_request_id, is_wip, target_repo_name, short=False)">
+<%def name="pullrequest_name(pull_request_id, state, is_wip, target_repo_name, short=False)">
    <a href="${h.route_path('pullrequest_show',repo_name=target_repo_name,pull_request_id=pull_request_id)}">
-      % if is_wip:
-          <span class="tag tooltip" title="${_('Work in progress')}">wip</span>
-      % endif

      % if short:
        !${pull_request_id}
      % else:
        ${_('Pull request !{}').format(pull_request_id)}
      % endif
+
+      % if state not in ['created']:
+          <span class="tag tag-merge-state-${state} tooltip" title="Pull request state is changing">${state}</span>
+      % endif
+
+      % if is_wip:
+          <span class="tag tooltip" title="${_('Work in progress')}">wip</span>
+      % endif
    </a>
 </%def>

--- a/rhodecode/templates/pullrequests/pullrequest_show.mako
+++ b/rhodecode/templates/pullrequests/pullrequest_show.mako
@ -10,14 +10,17 @@
 </%def>

 <%def name="breadcrumbs_links()">
-    <span id="pr-title">
-    ${c.pull_request.title}
-    %if c.pull_request.is_closed():
-        (${_('Closed')})
-    %endif
-    </span>
+    <%
+    pr_title = c.pull_request.title
+    if c.pull_request.is_closed():
+        pr_title = '[{}] {}'.format(_('Closed'), pr_title)
+    %>
+
+    <div id="pr-title">
+        <input class="pr-title-input large disabled" disabled="disabled" name="pullrequest_title" type="text" value="${pr_title}">
+    </div>
    <div id="pr-title-edit" class="input" style="display: none;">
-        ${h.text('pullrequest_title', id_="pr-title-input", class_="large", value=c.pull_request.title)}
+        <input class="pr-title-input large" id="pr-title-input" name="pullrequest_title" type="text" value="${c.pull_request.title}">
    </div>
 </%def>

@ -151,7 +154,7 @@
            <div class="input">
              %if c.pull_request_review_status:
                <i class="icon-circle review-status-${c.pull_request_review_status}"></i>
-                <span class="changeset-status-lbl tooltip">
+                <span class="changeset-status-lbl">
                  %if c.pull_request.is_closed():
                      ${_('Closed')},
                  %endif
@ -208,13 +211,13 @@
                                    </code>
                               </td>
                               <td>
-                                   <input ${'checked="checked"' if c.from_version_num == ver_pr else ''} class="compare-radio-button" type="radio" name="ver_source" value="${ver_pr or 'latest'}" data-ver-pos="${ver_pos}"/>
-                                   <input ${'checked="checked"' if c.at_version_num == ver_pr else ''} class="compare-radio-button" type="radio" name="ver_target" value="${ver_pr or 'latest'}" data-ver-pos="${ver_pos}"/>
+                                   <input ${('checked="checked"' if c.from_version_num == ver_pr else '')} class="compare-radio-button" type="radio" name="ver_source" value="${ver_pr or 'latest'}" data-ver-pos="${ver_pos}"/>
+                                   <input ${('checked="checked"' if c.at_version_num == ver_pr else '')} class="compare-radio-button" type="radio" name="ver_target" value="${ver_pr or 'latest'}" data-ver-pos="${ver_pos}"/>
                               </td>
                               <td>
                                <% review_status = c.review_versions[ver_pr].status if ver_pr in c.review_versions else 'not_reviewed' %>
                                <i class="tooltip icon-circle review-status-${review_status}" title="${_('Your review status at this version')}"></i>
-                                </div>
+
                               </td>
                               <td>
                                   % if c.at_version_num != ver_pr:
@ -350,8 +353,17 @@
        </div>
      </div>
  </div>
+
  <div class="box">
-      ##DIFF
+
+  % if c.state_progressing:
+    <h2 style="text-align: center">
+        ${_('Cannot show diff when pull request state is changing. Current progress state')}: <span class="tag tag-merge-state-${c.pull_request.state}">${c.pull_request.state}</span>
+    </h2>
+
+  % else:
+
+      ## Diffs rendered here
      <div class="table" >
          <div id="changeset_compare_view_content">
              ##CS
@ -599,7 +611,10 @@

      %endif

-      <script type="text/javascript">
+   % endif
+  </div>
+
+<script type="text/javascript">

        versionController = new VersionController();
        versionController.init();
@ -802,8 +817,6 @@
        })

      </script>
-
-  </div>
 </div>

 </%def>
--- a/rhodecode/templates/pullrequests/pullrequests.mako
+++ b/rhodecode/templates/pullrequests/pullrequests.mako
@ -114,9 +114,6 @@ $(document).ready(function() {
          if (data['closed']) {
              $(row).addClass('closed');
          }
-          if (data['state'] !== 'created') {
-              $(row).addClass('state-' + data['state']);
-          }
      }
    });