pagure

Clone

#5027 Change the PR flag API endpoints to use commit flags
Merged 3 years ago by pingou. Opened 3 years ago by pingou.
commit_flags  into  master

file modified
+51 -4 
@@ -928,6 +928,13 @@ 
 

      -------------------
 

      Add or edit flags on a pull-request.
 

  

+     This is an utility method which add a flag to the latest commit of the
 

+     specified pull-request.
 

+ 

+     Note that using it has a risk of race-condition if the pull-request changes
 

+     at the time the flag is being added. Using the commit flag endpoint prevents
 

+     this risk of race-condition.
 

+ 

      ::
 

  

          POST /api/0/<repo>/pull-request/<request id>/flag
 
@@ -1064,9 +1071,27 @@ 
 

                      else pagure_config["FLAG_FAILURE"]
 

                  )
 

          try:
 

+             if not request.commit_stop:
 

+                 repopath = None
 

+                 parentpath = pagure.utils.get_repo_path(request.project)
 

+                 if request.remote:
 

+                     repopath = pagure.utils.get_remote_repo_path(
 

+                         request.remote_git, request.branch_from
 

+                     )
 

+                 elif request.project_from:
 

+                     repopath = pagure.utils.get_repo_path(request.project_from)
 

+ 

+                 repo_obj = None
 

+                 if repopath:
 

+                     repo_obj = pygit2.Repository(repopath)
 

+                 orig_repo = pygit2.Repository(parentpath)
 

+                 pagure.lib.git.diff_pull_request(
 

+                     flask.g.session, request, repo_obj, orig_repo
 

+                 )
 

+ 

              # New Flag
 

              message, uid = pagure.lib.query.add_pull_request_flag(
 

-                 flask.g.session,
 

+                 session=flask.g.session,
 

                  request=request,
 

                  username=username,
 

                  status=status,
 
@@ -1078,8 +1103,8 @@ 
 

                  token=flask.g.token.id,
 

              )
 

              flask.g.session.commit()
 

-             pr_flag = pagure.lib.query.get_pull_request_flag_by_uid(
 

-                 flask.g.session, request, uid
 

+             pr_flag = pagure.lib.query.get_commit_flag_by_uid(
 

+                 flask.g.session, request.commit_stop, uid
 

              )
 

              output["message"] = message
 

              output["uid"] = uid
 
@@ -1182,9 +1207,31 @@ 
 

      _check_pull_request(repo)
 

      request = _get_request(repo, requestid)
 

  

+     if not request.commit_stop:
 

+         repopath = None
 

+         parentpath = pagure.utils.get_repo_path(request.project)
 

+         if request.remote:
 

+             repopath = pagure.utils.get_remote_repo_path(
 

+                 request.remote_git, request.branch_from
 

+             )
 

+         elif request.project_from:
 

+             repopath = pagure.utils.get_repo_path(request.project_from)
 

+ 

+         repo_obj = None
 

+         if repopath:
 

+             repo_obj = pygit2.Repository(repopath)
 

+         orig_repo = pygit2.Repository(parentpath)
 

+         pagure.lib.git.diff_pull_request(
 

+             flask.g.session, request, repo_obj, orig_repo
 

+         )
 

+ 

      output = {"flags": []}
 

  

-     for flag in request.flags:
 

+     flags = pagure.lib.query.get_commit_flag(
 

+         flask.g.session, request.project, request.commit_stop
 

+     )
 

+ 

+     for flag in flags:
 

          output["flags"].append(flag.to_json(public=True))
 

  

      jsonout = flask.jsonify(output)

file modified
+10 -12 
@@ -1016,7 +1016,7 @@ 
 

      )
 

  

  

- def notify_pull_request_flag(flag, user):
 

+ def notify_pull_request_flag(flag, request, user):
 

      """ Notify the people following a pull-request that a new flag was
 

      added to it.
 

      """
 
@@ -1026,31 +1026,29 @@ 
 

  %s
 

  """ % (
 

          flag.username,
 

-         flag.pull_request.title,
 

+         request.title,
 

          flag.status,
 

          flag.comment,
 

          _build_url(
 

              pagure_config["APP_URL"],
 

-             _fullname_to_url(flag.pull_request.project.fullname),
 

+             _fullname_to_url(request.project.fullname),
 

              "pull-request",
 

-             flag.pull_request.id,
 

+             request.id,
 

          ),
 

      )
 

-     mail_to = _get_emails_for_obj(flag.pull_request)
 

+     mail_to = _get_emails_for_obj(request)
 

  

-     assignee = (
 

-         flag.pull_request.assignee.user if flag.pull_request.assignee else None
 

-     )
 

+     assignee = request.assignee.user if request.assignee else None
 

  

      send_email(
 

          text,
 

-         "PR #%s - %s: %s" % (flag.pull_request.id, flag.username, flag.status),
 

+         "PR #%s - %s: %s" % (request.id, flag.username, flag.status),
 

          ",".join(mail_to),
 

          mail_id=flag.mail_id,
 

-         in_reply_to=flag.pull_request.mail_id,
 

-         project_name=flag.pull_request.project.fullname,
 

+         in_reply_to=request.mail_id,
 

+         project_name=request.project.fullname,
 

          user_from=flag.username,
 

-         reporter=flag.pull_request.user.user,
 

+         reporter=request.user.user,
 

          assignee=assignee,
 

      )

file modified
+21 -28 
@@ -1502,35 +1502,26 @@ 
 

      """ Add a flag to a pull-request. """
 

      user_obj = get_user(session, user)
 

  

-     action = "added"
 

-     pr_flag = None
 

-     if uid:
 

-         pr_flag = get_pull_request_flag_by_uid(session, request, uid)
 

-     if pr_flag:
 

-         action = "updated"
 

-         pr_flag.comment = comment
 

-         pr_flag.status = status
 

-         pr_flag.percent = percent
 

-         pr_flag.url = url
 

-     else:
 

-         pr_flag = model.PullRequestFlag(
 

-             pull_request_uid=request.uid,
 

-             uid=uid or uuid.uuid4().hex,
 

-             username=username,
 

-             percent=percent,
 

-             comment=comment,
 

-             status=status,
 

-             url=url,
 

-             user_id=user_obj.id,
 

-             token_id=token,
 

-         )
 

-     request.updated_on = datetime.datetime.utcnow()
 

-     session.add(pr_flag)
 

-     # Make sure we won't have SQLAlchemy error before we continue
 

-     session.flush()
 

+     action, flag_uid = add_commit_flag(
 

+         session=session,
 

+         repo=request.project,
 

+         commit_hash=request.commit_stop,
 

+         username=username,
 

+         status=status,
 

+         percent=percent,
 

+         comment=comment,
 

+         url=url,
 

+         uid=uid,
 

+         user=user,
 

+         token=token,
 

+     )
 

+ 

+     pr_flag = pagure.lib.query.get_commit_flag_by_uid(
 

+         session, request.commit_stop, flag_uid
 

+     )
 

  

      if request.project.settings.get("notify_on_pull-request_flag"):
 

-         pagure.lib.notify.notify_pull_request_flag(pr_flag, username)
 

+         pagure.lib.notify.notify_pull_request_flag(pr_flag, request, username)
 

  

      pagure.lib.git.update_git(request, repo=request.project)
 

  
@@ -1544,6 +1535,8 @@ 
 

          ),
 

      )
 

  

+     action = action.replace("Flag ", "")
 

+ 

      return ("Flag %s" % action, pr_flag.uid)
 

  

  
@@ -1619,7 +1612,7 @@ 
 

          session.query(model.CommitFlag)
 

          .filter(model.CommitFlag.project_id == project.id)
 

          .filter(model.CommitFlag.commit_hash == commit_hash)
 

-     )
 

+     ).order_by(model.CommitFlag.date_updated)
 

  

      return query.all()

file modified
+105 -6 
@@ -97,7 +97,7 @@ 
 

                {% if pull_request.project_from.is_fork -%}
 

                  {{ pull_request.project_from.user.user }}/
 

                {%- endif -%}
 

-               {{ pull_request.project_from.name }}  

+               {{ pull_request.project_from.name }}
 

              </span>
 

              {% elif pull_request.project_from is none and pull_request.remote_git is none %}
 

              <span class="badge badge-light badge-pill border border-seconday font-1em">
 
@@ -336,7 +336,8 @@ 
 

                <div class="col-xs-auto pr-3 text-right">
 

                    <div class="btn-group">
 

                      <a href="{{ commit_link }}"
 

-                       class="btn btn-outline-primary font-weight-bold {{'disabled' if not commit_link}}">
 

+                       data-commithash="{{ commit.hex }}"
 

+                       class="btn btn-outline-primary font-weight-bold {{'disabled' if not commit_link}} commithash">
 

                        <code>{{ commit.hex|short }}</code>
 

                      </a>
 

                      <a class="btn btn-outline-primary font-weight-bold {{'disabled' if not commit_link}}" href="{{tree_link}}"><span class="fa fa-file-code-o fa-fw"></span></a>
 
@@ -409,6 +410,12 @@ 
 

        </form>
 

      </section>
 

  

+     <div class="mb-4">
 

+         <div class="list-group list-group-flush" id="flags">
 

+         </div>
 

+     </div>
 

+ 

+ 

      {% if g.authenticated and mergeform %}
 

      <div class="card mt-5">
 

      {% if g.authenticated %}
 
@@ -458,7 +465,7 @@ 
 

          </form>
 

  

            {% endif %}
 

-   </div>
 

+     </div>
 

        <div class="small">
 

          <p>
 

            Pull this pull-request locally
 
@@ -470,9 +477,8 @@ 
 

          <pre id="local_pull_info" class="hidden">git fetch {{ config.get('GIT_URL_GIT') }}{{ repo.fullname }}.git refs/pull/{{ pull_request.id }}/head:pr{{ pull_request.id }}</pre>
 

        </div>
 

      {% endif %}
 

- 

- 

    </div>
 

+ 

    <div class="col-md-4">
 

      <div>
 

      <div class="mb-4">
 
@@ -580,7 +586,6 @@ 
 

          {% endif %}
 

      </div>
 

  

- 

      {% if pull_request.flags %}
 

      <div class="mb-4">
 

          <h5 class="d-flex align-items-center font-weight-bold border-bottom">
 
@@ -882,7 +887,101 @@ 
 

  }
 

  {% endif %}
 

  

+ var statusesLabels = {{ flag_statuses_labels|safe }}
 

+ 

  $(document).ready(function() {
 

+ 

+   {# Show latest tag #}
 

+   {% if pull_request.commit_stop %};
 

+     var url = '{{ url_for("api_ns.api_commit_flags",
 

+                   repo=repo.name,
 

+                   username=repo.user.user if repo.is_fork else none,
 

+                   namespace=repo.namespace,
 

+                   commit_hash=pull_request.commit_stop) }}'
 

+     var commitUrl = '{{ url_for("ui_ns.view_commit",
 

+                         repo=repo.name,
 

+                         username=username,
 

+                         namespace=repo.namespace,
 

+                         commitid=pull_request.commit_stop) }}';
 

+     $.ajax({
 

+       url: url,
 

+       type: 'GET',
 

+       dataType: 'json',
 

+       success: function(res) {
 

+         var statusesLabels = {{ flag_statuses_labels|safe }};
 

+         var flags_html = '';
 

+ 

+         for (var i in res['flags']) {
 

+           var f = res['flags'][i];
 

+           var t = f.date_created == f.date_updated ? "Created at": "Updated at";
 

+           var d = new Date(f.date_updated * 1000);
 

+           var html = '<a href="' + f.url + '" class="list-group-item list-group-item-action border-0 pl-2 pr-2">'
 

+             + ' <div>'
 

+             + '    <span class="font-weight-bold">' + f.username
 

+             + '    </span>'
 

+             + '    <div class="float-right">'
 

+             + '      <span class="badge ' + statusesLabels[f.status] + ' font-size-09">' + f.status
 

+             + '      </span>'
 

+             + '    </div>'
 

+             + '  </div>'
 

+             + '  <small><div class="clearfix">'
 

+             + '      <span>' + f.comment + '</span>'
 

+             + '      <div title="' + t + ' ' + d.toUTCString() + '" class="float-right">'
 

+             + '      ' + d.toUTCString() + '</div>'
 

+             + '  </div>'
 

+             + '  </small>'
 

+             + '</a>';
 

+             flags_html += html;
 

+         }
 

+         $("#flags").html(flags_html);
 

+       }
 

+     });
 

+   {% endif %}
 

+ 

+   {# Show flags in commit list #}
 

+   $(".commithash").each(function(idx, item) {
 

+     var url = '{{ url_for("api_ns.api_commit_flags",
 

+                   repo=repo.name,
 

+                   username=repo.user.user if repo.is_fork else none,
 

+                   namespace=repo.namespace,
 

+                   commit_hash='COMMIT_HASH') }}'
 

+     var commitUrl = '{{ url_for("ui_ns.view_commit",
 

+                         repo=repo.name,
 

+                         username=username,
 

+                         namespace=repo.namespace,
 

+                         commitid="COMMIT_HASH") }}';
 

+     var commithash= $(item).attr('data-commithash');
 

+     url = url.replace("COMMIT_HASH", commithash);
 

+     commitUrl = commitUrl.replace("COMMIT_HASH", commithash);
 

+     $.ajax({
 

+       url: url,
 

+       type: 'GET',
 

+       dataType: 'json',
 

+       success: function(res) {
 

+         var statuses = {}
 

+         for (var i in res['flags']) {
 

+           var f = res['flags'][i]
 

+           if (!(f['status'] in statuses)) {
 

+             statuses[f['status']] = []
 

+           }
 

+           statuses[f['status']].push(f)
 

+         }
 

+         var html = ''
 

+         var sortedStatuses = Object.keys(statuses).sort()
 

+         for (var i in sortedStatuses) {
 

+           s = sortedStatuses[i]
 

+           numStatuses = statuses[s].length
 

+           html += '<a href="' + commitUrl + '" title="' + numStatuses
 

+           html += ' ' + s + ' flag' + (numStatuses > 1 ? 's' : '')
 

+           html += '" class="btn ' + statusesLabels[s].replace('badge', 'btn-outline') + '">'
 

+           html += statuses[s].length + '</a>\n'
 

+         }
 

+         $(html).insertBefore(item);
 

+       }
 

+     });
 

+   });
 

+ 

+ 

    $('#merge_btn').click(function() {
 

        return confirm('Confirm merging this pull-request');
 

    });

file modified
+2 
@@ -19,6 +19,7 @@ 
 

  from __future__ import unicode_literals, absolute_import
 

  

  import logging
 

+ import json
 

  import os
 

  from math import ceil
 

  
@@ -369,6 +370,7 @@ 
 

          can_delete_branch=can_delete_branch,
 

          trigger_ci=trigger_ci,
 

          trigger_ci_pr_form=trigger_ci_pr_form,
 

+         flag_statuses_labels=json.dumps(pagure_config["FLAG_STATUSES_LABELS"]),
 

      )

file modified
+112 -72 
@@ -65,6 +65,9 @@ 
 

          request = pagure.lib.query.search_pull_requests(
 

              self.session, project_id=1, requestid=1
 

          )
 

+         request.commit_stop = "hash_commit_stop"
 

+         self.session.add(request)
 

+         self.session.commit()
 

          self.assertEqual(len(request.flags), 0)
 

  

      def test_invalid_project(self):
 
@@ -244,8 +247,8 @@ 
 

          data = json.loads(output.get_data(as_text=True))
 

          data["flag"]["date_created"] = "1510742565"
 

          data["flag"]["date_updated"] = "1510742565"
 

-         pr_uid = data["flag"]["pull_request_uid"]
 

-         data["flag"]["pull_request_uid"] = "62b49f00d489452994de5010565fab81"
 

+         commit_hash = data["flag"]["commit_hash"]
 

+         data["flag"]["commit_hash"] = "62b49f00d489452994de5010565fab81"
 

          data["avatar_url"] = "https://seccdn.libravatar.org/avatar/..."
 

          self.assertDictEqual(
 

              data,
 
@@ -255,7 +258,7 @@ 
 

                      "date_created": "1510742565",
 

                      "date_updated": "1510742565",
 

                      "percent": None,
 

-                     "pull_request_uid": "62b49f00d489452994de5010565fab81",
 

+                     "commit_hash": "62b49f00d489452994de5010565fab81",
 

                      "status": "pending",
 

                      "url": "http://jenkins.cloud.fedoraproject.org/",
 

                      "user": {
 
@@ -279,9 +282,13 @@ 
 

          request = pagure.lib.query.search_pull_requests(
 

              self.session, project_id=1, requestid=1
 

          )
 

-         self.assertEqual(len(request.flags), 1)
 

-         self.assertEqual(request.flags[0].comment, "Tests running")
 

-         self.assertEqual(request.flags[0].percent, None)
 

+         self.assertEqual(len(request.flags), 0)
 

+ 

+         flags = pagure.lib.query.get_commit_flag(
 

+             self.session, request.project, commit_hash
 

+         )
 

+         self.assertEqual(flags[0].comment, "Tests running")
 

+         self.assertEqual(flags[0].percent, None)
 

  

          # Check the notification sent
 

          mock_email.assert_called_once_with(
 
@@ -291,8 +298,8 @@ 
 

              "PR #1 - Jenkins: pending",
 

              "bar@pingou.com",
 

              assignee=None,
 

-             in_reply_to="test-pull-request-" + pr_uid,
 

-             mail_id="test-pull-request-" + pr_uid + "-1",
 

+             in_reply_to="test-pull-request-%s" % request.uid,
 

+             mail_id="test-commit-1-1",
 

              project_name="test",
 

              reporter="pingou",
 

              user_from="Jenkins",
 
@@ -317,17 +324,16 @@ 
 

          data = json.loads(output.get_data(as_text=True))
 

          data["flag"]["date_created"] = "1510742565"
 

          data["flag"]["date_updated"] = "1510742565"
 

-         data["flag"]["pull_request_uid"] = "62b49f00d489452994de5010565fab81"
 

          data["avatar_url"] = "https://seccdn.libravatar.org/avatar/..."
 

          self.assertDictEqual(
 

              data,
 

              {
 

                  "flag": {
 

                      "comment": "Tests running",
 

+                     "commit_hash": "hash_commit_stop",
 

                      "date_created": "1510742565",
 

                      "date_updated": "1510742565",
 

                      "percent": None,
 

-                     "pull_request_uid": "62b49f00d489452994de5010565fab81",
 

                      "status": "pending",
 

                      "url": "http://jenkins.cloud.fedoraproject.org/",
 

                      "user": {
 
@@ -351,9 +357,13 @@ 
 

          request = pagure.lib.query.search_pull_requests(
 

              self.session, project_id=1, requestid=1
 

          )
 

-         self.assertEqual(len(request.flags), 1)
 

-         self.assertEqual(request.flags[0].comment, "Tests running")
 

-         self.assertEqual(request.flags[0].percent, None)
 

+         self.assertEqual(len(request.flags), 0)
 

+ 

+         flags = pagure.lib.query.get_commit_flag(
 

+             self.session, request.project, "hash_commit_stop"
 

+         )
 

+         self.assertEqual(flags[0].comment, "Tests running")
 

+         self.assertEqual(flags[0].percent, None)
 

  

          # Update flag  -  w/o providing the status
 

          data = {
 
@@ -371,17 +381,16 @@ 
 

          data = json.loads(output.get_data(as_text=True))
 

          data["flag"]["date_created"] = "1510742565"
 

          data["flag"]["date_updated"] = "1510742565"
 

-         data["flag"]["pull_request_uid"] = "62b49f00d489452994de5010565fab81"
 

          data["avatar_url"] = "https://seccdn.libravatar.org/avatar/..."
 

          self.assertDictEqual(
 

              data,
 

              {
 

                  "flag": {
 

                      "comment": "Tests passed",
 

+                     "commit_hash": "hash_commit_stop",
 

                      "date_created": "1510742565",
 

                      "date_updated": "1510742565",
 

                      "percent": 100,
 

-                     "pull_request_uid": "62b49f00d489452994de5010565fab81",
 

                      "status": "success",
 

                      "url": "http://jenkins.cloud.fedoraproject.org/",
 

                      "user": {
 
@@ -405,9 +414,13 @@ 
 

          request = pagure.lib.query.search_pull_requests(
 

              self.session, project_id=1, requestid=1
 

          )
 

-         self.assertEqual(len(request.flags), 1)
 

-         self.assertEqual(request.flags[0].comment, "Tests passed")
 

-         self.assertEqual(request.flags[0].percent, 100)
 

+         self.assertEqual(len(request.flags), 0)
 

+ 

+         flags = pagure.lib.query.get_commit_flag(
 

+             self.session, request.project, "hash_commit_stop"
 

+         )
 

+         self.assertEqual(flags[0].comment, "Tests passed")
 

+         self.assertEqual(flags[0].percent, 100)
 

  

      def test_adding_two_flags(self):
 

          """ Test the adding two flags to a PR. """
 
@@ -430,17 +443,16 @@ 
 

          data = json.loads(output.get_data(as_text=True))
 

          data["flag"]["date_created"] = "1510742565"
 

          data["flag"]["date_updated"] = "1510742565"
 

-         data["flag"]["pull_request_uid"] = "62b49f00d489452994de5010565fab81"
 

          data["avatar_url"] = "https://seccdn.libravatar.org/avatar/..."
 

          self.assertDictEqual(
 

              data,
 

              {
 

                  "flag": {
 

                      "comment": "Tests passed",
 

+                     "commit_hash": "hash_commit_stop",
 

                      "date_created": "1510742565",
 

                      "date_updated": "1510742565",
 

                      "percent": 100,
 

-                     "pull_request_uid": "62b49f00d489452994de5010565fab81",
 

                      "status": "success",
 

                      "url": "http://jenkins.cloud.fedoraproject.org/",
 

                      "user": {
 
@@ -459,14 +471,19 @@ 
 

              },
 

          )
 

  

-         # One flag added
 

+         # One flag added - but no longer on the request object
 

          self.session.commit()
 

          request = pagure.lib.query.search_pull_requests(
 

              self.session, project_id=1, requestid=1
 

          )
 

-         self.assertEqual(len(request.flags), 1)
 

-         self.assertEqual(request.flags[0].comment, "Tests passed")
 

-         self.assertEqual(request.flags[0].percent, 100)
 

+         self.assertEqual(len(request.flags), 0)
 

+ 

+         flags = pagure.lib.query.get_commit_flag(
 

+             self.session, request.project, "hash_commit_stop"
 

+         )
 

+         self.assertEqual(len(flags), 1)
 

+         self.assertEqual(flags[0].comment, "Tests passed")
 

+         self.assertEqual(flags[0].percent, 100)
 

  

          data = {
 

              "username": "Jenkins",
 
@@ -482,7 +499,6 @@ 
 

          data = json.loads(output.get_data(as_text=True))
 

          data["flag"]["date_created"] = "1510742565"
 

          data["flag"]["date_updated"] = "1510742565"
 

-         data["flag"]["pull_request_uid"] = "62b49f00d489452994de5010565fab81"
 

          self.assertNotEqual(data["uid"], "jenkins_build_pagure_100+seed")
 

          data["uid"] = "jenkins_build_pagure_100+seed"
 

          data["avatar_url"] = "https://seccdn.libravatar.org/avatar/..."
 
@@ -491,10 +507,10 @@ 
 

              {
 

                  "flag": {
 

                      "comment": "Tests running again",
 

+                     "commit_hash": "hash_commit_stop",
 

                      "date_created": "1510742565",
 

                      "date_updated": "1510742565",
 

                      "percent": None,
 

-                     "pull_request_uid": "62b49f00d489452994de5010565fab81",
 

                      "status": "pending",
 

                      "url": "http://jenkins.cloud.fedoraproject.org/",
 

                      "user": {
 
@@ -513,16 +529,21 @@ 
 

              },
 

          )
 

  

-         # Two flag added
 

+         # Two flags added - but no longer on the request object
 

          self.session.commit()
 

          request = pagure.lib.query.search_pull_requests(
 

              self.session, project_id=1, requestid=1
 

          )
 

-         self.assertEqual(len(request.flags), 2)
 

-         self.assertEqual(request.flags[0].comment, "Tests running again")
 

-         self.assertEqual(request.flags[0].percent, None)
 

-         self.assertEqual(request.flags[1].comment, "Tests passed")
 

-         self.assertEqual(request.flags[1].percent, 100)
 

+         self.assertEqual(len(request.flags), 0)
 

+ 

+         flags = pagure.lib.query.get_commit_flag(
 

+             self.session, request.project, "hash_commit_stop"
 

+         )
 

+         self.assertEqual(len(flags), 2)
 

+         self.assertEqual(flags[1].comment, "Tests running again")
 

+         self.assertEqual(flags[1].percent, None)
 

+         self.assertEqual(flags[0].comment, "Tests passed")
 

+         self.assertEqual(flags[0].percent, 100)
 

  

      @patch.dict(
 

          "pagure.config.config",
 
@@ -642,6 +663,9 @@ 
 

          request = pagure.lib.query.search_pull_requests(
 

              self.session, project_id=1, requestid=1
 

          )
 

+         request.commit_stop = "hash_commit_stop"
 

+         self.session.add(request)
 

+         self.session.commit()
 

          self.assertEqual(len(request.flags), 0)
 

  

      def test_no_pr(self):
 
@@ -787,17 +811,16 @@ 
 

          data = json.loads(output.get_data(as_text=True))
 

          data["flag"]["date_created"] = "1510742565"
 

          data["flag"]["date_updated"] = "1510742565"
 

-         data["flag"]["pull_request_uid"] = "62b49f00d489452994de5010565fab81"
 

          data["avatar_url"] = "https://seccdn.libravatar.org/avatar/..."
 

          self.assertDictEqual(
 

              data,
 

              {
 

                  "flag": {
 

                      "comment": "Tests failed",
 

+                     "commit_hash": "hash_commit_stop",
 

                      "date_created": "1510742565",
 

                      "date_updated": "1510742565",
 

                      "percent": 0,
 

-                     "pull_request_uid": "62b49f00d489452994de5010565fab81",
 

                      "status": "failure",
 

                      "url": "http://jenkins.cloud.fedoraproject.org/",
 

                      "user": {
 
@@ -821,9 +844,14 @@ 
 

          request = pagure.lib.query.search_pull_requests(
 

              self.session, project_id=1, requestid=1
 

          )
 

-         self.assertEqual(len(request.flags), 1)
 

-         self.assertEqual(request.flags[0].comment, "Tests failed")
 

-         self.assertEqual(request.flags[0].percent, 0)
 

+         self.assertEqual(len(request.flags), 0)
 

+ 

+         flags = pagure.lib.query.get_commit_flag(
 

+             self.session, request.project, "hash_commit_stop"
 

+         )
 

+         self.assertEqual(len(flags), 1)
 

+         self.assertEqual(flags[0].comment, "Tests failed")
 

+         self.assertEqual(flags[0].percent, 0)
 

  

          # no notifications sent
 

          mock_email.assert_not_called()
 
@@ -849,17 +877,16 @@ 
 

          data = json.loads(output.get_data(as_text=True))
 

          data["flag"]["date_created"] = "1510742565"
 

          data["flag"]["date_updated"] = "1510742565"
 

-         data["flag"]["pull_request_uid"] = "62b49f00d489452994de5010565fab81"
 

          data["avatar_url"] = "https://seccdn.libravatar.org/avatar/..."
 

          self.assertDictEqual(
 

              data,
 

              {
 

                  "flag": {
 

                      "comment": "Tests failed",
 

+                     "commit_hash": "hash_commit_stop",
 

                      "date_created": "1510742565",
 

                      "date_updated": "1510742565",
 

                      "percent": None,
 

-                     "pull_request_uid": "62b49f00d489452994de5010565fab81",
 

                      "status": "failure",
 

                      "url": "http://jenkins.cloud.fedoraproject.org/",
 

                      "user": {
 
@@ -883,9 +910,14 @@ 
 

          request = pagure.lib.query.search_pull_requests(
 

              self.session, project_id=1, requestid=1
 

          )
 

-         self.assertEqual(len(request.flags), 1)
 

-         self.assertEqual(request.flags[0].comment, "Tests failed")
 

-         self.assertEqual(request.flags[0].percent, None)
 

+         self.assertEqual(len(request.flags), 0)
 

+ 

+         flags = pagure.lib.query.get_commit_flag(
 

+             self.session, request.project, "hash_commit_stop"
 

+         )
 

+         self.assertEqual(len(flags), 1)
 

+         self.assertEqual(flags[0].comment, "Tests failed")
 

+         self.assertEqual(flags[0].percent, None)
 

  

          # Update flag
 

          data = {
 
@@ -904,17 +936,16 @@ 
 

          data = json.loads(output.get_data(as_text=True))
 

          data["flag"]["date_created"] = "1510742565"
 

          data["flag"]["date_updated"] = "1510742565"
 

-         data["flag"]["pull_request_uid"] = "62b49f00d489452994de5010565fab81"
 

          data["avatar_url"] = "https://seccdn.libravatar.org/avatar/..."
 

          self.assertDictEqual(
 

              data,
 

              {
 

                  "flag": {
 

                      "comment": "Tests passed",
 

+                     "commit_hash": "hash_commit_stop",
 

                      "date_created": "1510742565",
 

                      "date_updated": "1510742565",
 

                      "percent": 100,
 

-                     "pull_request_uid": "62b49f00d489452994de5010565fab81",
 

                      "status": "success",
 

                      "url": "http://jenkins.cloud.fedoraproject.org/",
 

                      "user": {
 
@@ -938,9 +969,14 @@ 
 

          request = pagure.lib.query.search_pull_requests(
 

              self.session, project_id=1, requestid=1
 

          )
 

-         self.assertEqual(len(request.flags), 1)
 

-         self.assertEqual(request.flags[0].comment, "Tests passed")
 

-         self.assertEqual(request.flags[0].percent, 100)
 

+         self.assertEqual(len(request.flags), 0)
 

+ 

+         flags = pagure.lib.query.get_commit_flag(
 

+             self.session, request.project, "hash_commit_stop"
 

+         )
 

+         self.assertEqual(len(flags), 1)
 

+         self.assertEqual(flags[0].comment, "Tests passed")
 

+         self.assertEqual(flags[0].percent, 100)
 

  

  

  class PagureFlaskApiGetPRFlagtests(tests.Modeltests):
 
@@ -983,6 +1019,9 @@ 
 

          request = pagure.lib.query.search_pull_requests(
 

              self.session, project_id=1, requestid=1
 

          )
 

+         request.commit_stop = "hash_commit_stop"
 

+         self.session.add(request)
 

+         self.session.commit()
 

          self.assertEqual(len(request.flags), 0)
 

  

      def test_invalid_project(self):
 
@@ -1060,8 +1099,13 @@ 
 

          self.assertEqual(msg, ("Flag added", "jenkins_build_pagure_34"))
 

          self.session.commit()
 

  

-         self.assertEqual(len(request.flags), 1)
 

-         self.assertEqual(request.flags[0].token_id, "aaabbbcccddd")
 

+         self.assertEqual(len(request.flags), 0)
 

+ 

+         flags = pagure.lib.query.get_commit_flag(
 

+             self.session, request.project, "hash_commit_stop"
 

+         )
 

+         self.assertEqual(len(flags), 1)
 

+         self.assertEqual(flags[0].token_id, "aaabbbcccddd")
 

  

          # 1 flag
 

          output = self.app.get("/api/0/test/pull-request/1/flag")
 
@@ -1069,19 +1113,16 @@ 
 

          data = json.loads(output.get_data(as_text=True))
 

          data["flags"][0]["date_created"] = "1541413645"
 

          data["flags"][0]["date_updated"] = "1541413645"
 

-         data["flags"][0][
 

-             "pull_request_uid"
 

-         ] = "72a61033c2fc464aa9ef514c057aa62c"
 

          self.assertDictEqual(
 

              data,
 

              {
 

                  "flags": [
 

                      {
 

                          "comment": "Build passes",
 

+                         "commit_hash": "hash_commit_stop",
 

                          "date_created": "1541413645",
 

                          "date_updated": "1541413645",
 

                          "percent": None,
 

-                         "pull_request_uid": "72a61033c2fc464aa9ef514c057aa62c",
 

                          "status": "success",
 

                          "url": "http://jenkins.cloud.fedoraproject.org",
 

                          "user": {
 
@@ -1132,9 +1173,14 @@ 
 

          self.assertEqual(msg, ("Flag added", "travis_build_pagure_34"))
 

          self.session.commit()
 

  

-         self.assertEqual(len(request.flags), 2)
 

-         self.assertEqual(request.flags[1].token_id, "aaabbbcccddd")
 

-         self.assertEqual(request.flags[0].token_id, "aaabbbcccddd")
 

+         self.assertEqual(len(request.flags), 0)
 

+ 

+         flags = pagure.lib.query.get_commit_flag(
 

+             self.session, request.project, "hash_commit_stop"
 

+         )
 

+         self.assertEqual(len(flags), 2)
 

+         self.assertEqual(flags[1].token_id, "aaabbbcccddd")
 

+         self.assertEqual(flags[0].token_id, "aaabbbcccddd")
 

  

          # 1 flag
 

          output = self.app.get("/api/0/test/pull-request/1/flag")
 
@@ -1142,47 +1188,41 @@ 
 

          data = json.loads(output.get_data(as_text=True))
 

          data["flags"][0]["date_created"] = "1541413645"
 

          data["flags"][0]["date_updated"] = "1541413645"
 

-         data["flags"][0][
 

-             "pull_request_uid"
 

-         ] = "72a61033c2fc464aa9ef514c057aa62c"
 

          data["flags"][1]["date_created"] = "1541413645"
 

          data["flags"][1]["date_updated"] = "1541413645"
 

-         data["flags"][1][
 

-             "pull_request_uid"
 

-         ] = "72a61033c2fc464aa9ef514c057aa62c"
 

          self.assertDictEqual(
 

              data,
 

              {
 

                  "flags": [
 

                      {
 

-                         "comment": "Build pending",
 

+                         "comment": "Build passes",
 

+                         "commit_hash": "hash_commit_stop",
 

                          "date_created": "1541413645",
 

                          "date_updated": "1541413645",
 

                          "percent": None,
 

-                         "pull_request_uid": "72a61033c2fc464aa9ef514c057aa62c",
 

-                         "status": "pending",
 

-                         "url": "http://travis.io",
 

+                         "status": "success",
 

+                         "url": "http://jenkins.cloud.fedoraproject.org",
 

                          "user": {
 

                              "fullname": "foo bar",
 

                              "name": "foo",
 

                              "url_path": "user/foo",
 

                          },
 

-                         "username": "travis",
 

+                         "username": "jenkins",
 

                      },
 

                      {
 

-                         "comment": "Build passes",
 

+                         "comment": "Build pending",
 

+                         "commit_hash": "hash_commit_stop",
 

                          "date_created": "1541413645",
 

                          "date_updated": "1541413645",
 

                          "percent": None,
 

-                         "pull_request_uid": "72a61033c2fc464aa9ef514c057aa62c",
 

-                         "status": "success",
 

-                         "url": "http://jenkins.cloud.fedoraproject.org",
 

+                         "status": "pending",
 

+                         "url": "http://travis.io",
 

                          "user": {
 

                              "fullname": "foo bar",
 

                              "name": "foo",
 

                              "url_path": "user/foo",
 

                          },
 

-                         "username": "jenkins",
 

+                         "username": "travis",
 

                      },
 

                  ]
 

              },

file modified
+20 -10 
@@ -1985,6 +1985,8 @@ 
 

              title="test pull-request",
 

              user="pingou",
 

          )
 

+         req.commit_stop = "hash_commit_stop"
 

+         self.session.add(req)
 

          self.session.commit()
 

          self.assertEqual(req.id, 1)
 

          self.assertEqual(req.title, "test pull-request")
 
@@ -2041,17 +2043,16 @@ 
 

          data = json.loads(output.get_data(as_text=True))
 

          data["flag"]["date_created"] = "1510742565"
 

          data["flag"]["date_updated"] = "1510742565"
 

-         data["flag"]["pull_request_uid"] = "62b49f00d489452994de5010565fab81"
 

          data["avatar_url"] = "https://seccdn.libravatar.org/avatar/..."
 

          self.assertDictEqual(
 

              data,
 

              {
 

                  "flag": {
 

                      "comment": "Tests failed",
 

+                     "commit_hash": "hash_commit_stop",
 

                      "date_created": "1510742565",
 

                      "date_updated": "1510742565",
 

                      "percent": 0,
 

-                     "pull_request_uid": "62b49f00d489452994de5010565fab81",
 

                      "status": "failure",
 

                      "url": "http://jenkins.cloud.fedoraproject.org/",
 

                      "user": {
 
@@ -2075,9 +2076,14 @@ 
 

          request = pagure.lib.query.search_pull_requests(
 

              self.session, project_id=1, requestid=1
 

          )
 

-         self.assertEqual(len(request.flags), 1)
 

-         self.assertEqual(request.flags[0].comment, "Tests failed")
 

-         self.assertEqual(request.flags[0].percent, 0)
 

+         self.assertEqual(len(request.flags), 0)
 

+ 

+         flags = pagure.lib.query.get_commit_flag(
 

+             self.session, request.project, "hash_commit_stop"
 

+         )
 

+         self.assertEqual(len(flags), 1)
 

+         self.assertEqual(flags[0].comment, "Tests failed")
 

+         self.assertEqual(flags[0].percent, 0)
 

  

          # Update flag
 

          data = {
 
@@ -2095,17 +2101,16 @@ 
 

          data = json.loads(output.get_data(as_text=True))
 

          data["flag"]["date_created"] = "1510742565"
 

          data["flag"]["date_updated"] = "1510742565"
 

-         data["flag"]["pull_request_uid"] = "62b49f00d489452994de5010565fab81"
 

          data["avatar_url"] = "https://seccdn.libravatar.org/avatar/..."
 

          self.assertDictEqual(
 

              data,
 

              {
 

                  "flag": {
 

                      "comment": "Tests passed",
 

+                     "commit_hash": "hash_commit_stop",
 

                      "date_created": "1510742565",
 

                      "date_updated": "1510742565",
 

                      "percent": 100,
 

-                     "pull_request_uid": "62b49f00d489452994de5010565fab81",
 

                      "status": "success",
 

                      "url": "http://jenkins.cloud.fedoraproject.org/",
 

                      "user": {
 
@@ -2129,9 +2134,14 @@ 
 

          request = pagure.lib.query.search_pull_requests(
 

              self.session, project_id=1, requestid=1
 

          )
 

-         self.assertEqual(len(request.flags), 1)
 

-         self.assertEqual(request.flags[0].comment, "Tests passed")
 

-         self.assertEqual(request.flags[0].percent, 100)
 

+         self.assertEqual(len(request.flags), 0)
 

+ 

+         flags = pagure.lib.query.get_commit_flag(
 

+             self.session, request.project, "hash_commit_stop"
 

+         )
 

+         self.assertEqual(len(flags), 1)
 

+         self.assertEqual(flags[0].comment, "Tests passed")
 

+         self.assertEqual(flags[0].percent, 100)
 

  

      @patch("pagure.lib.notify.send_email")
 

      def test_api_private_repo_pr_close(self, send_email):

file modified
+14 -2 
@@ -2655,6 +2655,12 @@ 
 

          request = pagure.lib.query.search_pull_requests(
 

              self.session, requestid=1
 

          )
 

+         request.commit_stop = "hash_commit_stop"
 

+         self.session.add(request)
 

+         self.session.commit()
 

+         request = pagure.lib.query.search_pull_requests(
 

+             self.session, requestid=1
 

+         )
 

          self.assertEqual(len(request.flags), 0)
 

  

          msg = pagure.lib.query.add_pull_request_flag(
 
@@ -2672,8 +2678,14 @@ 
 

          self.assertEqual(msg, ("Flag added", "jenkins_build_pagure_34"))
 

          self.session.commit()
 

  

-         self.assertEqual(len(request.flags), 1)
 

-         self.assertEqual(request.flags[0].token_id, "aaabbbcccddd")
 

+         # We're no longer adding a PR flag but instead a commit flag to the
 

+         # latest commit of the PR
 

+         self.assertEqual(len(request.flags), 0)
 

+         flags = pagure.lib.query.get_commit_flag(
 

+             self.session, request.project, "hash_commit_stop"
 

+         )
 

+         self.assertEqual(flags[0].comment, "Build passes")
 

+         self.assertEqual(flags[0].token_id, "aaabbbcccddd")
 

  

      @patch("pagure.lib.notify.send_email", MagicMock(return_value=True))
 

      def test_search_pull_requests(self):

no initial comment

pretty please pagure-ci rebuild
3 years ago

3 new commits added

  • Rework how PR flags are show in the UI
  • Change the PR flag API endpoints to use commit flags
  • When returning the commits flags in the API, returned them by update date
3 years ago

3 new commits added

  • Rework how PR flags are show in the UI
  • Change the PR flag API endpoints to use commit flags
  • When returning the commits flags in the API, returned them by update date
3 years ago

3 new commits added

  • Rework how PR flags are show in the UI
  • Change the PR flag API endpoints to use commit flags
  • When returning the commits flags in the API, returned them by update date
3 years ago

:thumbsup:

rebased onto 83845808fe15b193e7f34a1e92f1150642317119
3 years ago

:thumbsup:

@pingou Does it make sense to have the flags column on the right side if they're all in the PR body now?

@pingou Does it make sense to have the flags column on the right side if they're all in the PR body now?

For now yes, as otherwise existing flags will not show at all

Okay, then this is good to merge.

1 new commit added

  • Document the risk of race-condition in the PR flag API endpoint
3 years ago

rebased onto 56c3184
3 years ago

I'm sure there will be things to adjust (I know the PR list will need to be adjusted for example), but let's start with this and see how we do :)

Pull-Request has been merged by pingou
3 years ago
Changes Summary 8
+51 -4
file changed
pagure/api/fork.py
+10 -12
file changed
pagure/lib/notify.py
+21 -28
file changed
pagure/lib/query.py
+105 -6
file changed
pagure/templates/repo_pull_request.html
+2 -0
file changed
pagure/ui/fork.py
+112 -72
file changed
tests/test_pagure_flask_api_pr_flag.py
+20 -10
file changed
tests/test_pagure_flask_api_ui_private_repo.py
+14 -2
file changed
tests/test_pagure_lib.py
Powered by Pagure 5.14.1
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.