#4620 Tag filtering support on pull requests list view and api endpoint
Merged 4 years ago by pingou. Opened 4 years ago by jlanda.

file modified
+20 -2
@@ -87,6 +87,13 @@ 

      | ``author``    | string   | Optional     | | Filter the author of     |

      |               |          |              |   pull requests            |

      +---------------+----------+--------------+----------------------------+

+     | ``tags``      | string   | Optional     | | A list of tags you       |

+     |               |          |              |   wish to filter. If you   |

+     |               |          |              |   want to filter for pull  |

+     |               |          |              |   requests not having a    |

+     |               |          |              |   tag, add an exclamation  |

+     |               |          |              |   mark in front of it      |

+     +---------------+----------+--------------+----------------------------+

  

      Sample response

      ^^^^^^^^^^^^^^^
@@ -97,7 +104,8 @@ 

            "args": {

              "assignee": null,

              "author": null,

-             "status": true

+             "status": true,

+             "tags": null

            },

            "total_requests": 1,

            "requests": [
@@ -154,6 +162,8 @@ 

      status = flask.request.args.get("status", True)

      assignee = flask.request.args.get("assignee", None)

      author = flask.request.args.get("author", None)

+     tags = flask.request.args.getlist("tags")

+     tags = [tag.strip() for tag in tags if tag.strip()]

  

      status_text = ("%s" % status).lower()

      requests = []
@@ -164,6 +174,7 @@ 

              status=False,

              assignee=assignee,

              author=author,

+             tags=tags,

          )

  

      elif status_text == "all":
@@ -173,6 +184,7 @@ 

              status=None,

              assignee=assignee,

              author=author,

+             tags=tags,

          )

  

      else:
@@ -182,6 +194,7 @@ 

              assignee=assignee,

              author=author,

              status=status,

+             tags=tags,

          )

  

      page = get_page()
@@ -201,7 +214,12 @@ 

          "requests": [

              request.to_json(public=True, api=True) for request in requests_page

          ],

-         "args": {"status": status, "assignee": assignee, "author": author},

+         "args": {

+             "status": status,

+             "assignee": assignee,

+             "author": author,

+             "tags": tags,

+         },

      }

      if pagination_metadata:

          jsonout["args"]["page"] = page

file modified
+52
@@ -3210,6 +3210,7 @@ 

      status=None,

      author=None,

      assignee=None,

+     tags=None,

      count=False,

      offset=None,

      limit=None,
@@ -3285,6 +3286,57 @@ 

      if branch_from is not None:

          query = query.filter(model.PullRequest.branch_from == branch_from)

  

+     if tags is not None and tags != []:

+         if isinstance(tags, six.string_types):

+             tags = [tags]

+         notags = []

+         ytags = []

+         for tag in tags:

+             if tag.startswith("!"):

+                 notags.append(tag[1:])

+             else:

+                 ytags.append(tag)

+ 

+         if ytags:

+             sub_q2 = session.query(sqlalchemy.distinct(model.PullRequest.uid))

+             if project_id is not None:

+                 sub_q2 = sub_q2.filter(

+                     model.PullRequest.project_id == project_id

+                 )

+             sub_q2 = (

+                 sub_q2.filter(

+                     model.PullRequest.uid == model.TagPullRequest.request_uid

+                 )

+                 .filter(model.TagPullRequest.tag_id == model.TagColored.id)

+                 .filter(model.TagColored.tag.in_(ytags))

+             )

+         if notags:

+             sub_q3 = session.query(sqlalchemy.distinct(model.PullRequest.uid))

+             if project_id is not None:

+                 sub_q3 = sub_q3.filter(

+                     model.PullRequest.project_id == project_id

+                 )

+             sub_q3 = (

+                 sub_q3.filter(

+                     model.PullRequest.uid == model.TagPullRequest.request_uid

+                 )

+                 .filter(model.TagPullRequest.tag_id == model.TagColored.id)

+                 .filter(model.TagColored.tag.in_(notags))

+             )

+         # Adjust the main query based on the parameters specified

+         if ytags and not notags:

+             query = query.filter(model.PullRequest.uid.in_(sub_q2))

+         elif not ytags and notags:

+             query = query.filter(~model.PullRequest.uid.in_(sub_q3))

+         elif ytags and notags:

+             final_set = set([i[0] for i in sub_q2.all()]) - set(

+                 [i[0] for i in sub_q3.all()]

+             )

+             if final_set:

+                 query = query.filter(

+                     model.PullRequest.uid.in_(list(final_set))

+                 )

+ 

      if search_pattern is not None:

          if "*" in search_pattern:

              search_pattern = search_pattern.replace("*", "%")

@@ -11,6 +11,20 @@ 

  {% block header %}

  <link type="text/css" rel="stylesheet" nonce="{{ g.nonce }}" href="{{

    url_for('static', filename='vendor/selectize/selectize.bootstrap3.css') }}"/>

+ <style nonce="{{ g.nonce }}">

+   #tags-filter-group .selectize-input .item{

+     background-color: {{tag.tag_color}};

+     color:white;

+     font-weight:bold;

+     padding-left:6px;

+     width:100%;

+   }

+   {% for tag in tag_list %}

+   #tags-filter-group .selectize-input .item[data-value='{{tag.tag}}']{

+     background-color: {{tag.tag_color}};

+   }

+   {% endfor %}

+ </style>

  {% endblock %}

  

  {% block repo %}
@@ -133,6 +147,20 @@ 

                              repo=repo.name ) }}" method="GET">

                          <input type="hidden" name="status" value="{{ status }}" />

  

+                         <div class="form-group row mb-1" id="tags-filter-group">

+                           <label for="tags" class="col-auto align-self-center pl-1 pr-0"><i class="text-muted fa fa-fw fa-tag"></i></label>

+                           <div class="col pl-1">

+                             <select name="tags" multiple id="tags-selectize" placeholder="Tags">

+                               {% for tag in tag_list %}

+                               <option value="{{ tag.tag }}" {% if tag.tag in tags %}selected="selected"{% endif %}>{{tag.tag}}</option>

+                               {% endfor %}

+                             </select>

+                           </div>

+                           <div class="col-auto pl-0 pr-1 pt-1">

+                             <i class="fa fa-times fa-fw text-muted" id="tags-selectize-reset"></i>

+                           </div>

+                         </div>

+ 

                          <div class="form-group row mb-2">

                            <label for="search_pattern" class="col-auto align-self-center pl-1 pr-0"><i class="text-muted fa fa-fw fa-search"></i></label>

                            <div class="col pl-1">
@@ -309,6 +337,25 @@ 

        $('input[name="search_pattern"]').val('');

      });

  

+     var $tags_selectize = $('#tags-selectize').selectize({

+     plugins: ['remove_button'],

+     closeAfterSelect: true,

+     onInitialize: function(){

+       $("#tags-filter-group .selectize-control").on('click', function(event){

+         event.stopPropagation();

+       })

+       $("#filters-dropdown").on('click', function(event){

+         event.stopPropagation();

+       })

+     }

+     });

+ 

+   var tags_selectize_control = $tags_selectize[0].selectize;

+ 

+   $("#tags-selectize-reset").on('click', function(e){

+     tags_selectize_control.clear();

+   });

+ 

      var $assignee_selectize = $('#assignee-selectize').selectize({

        valueField: 'user',

        labelField: 'user',

file modified
+13
@@ -69,6 +69,8 @@ 

      """ List all Pull-requests associated to a repo

      """

      status = flask.request.args.get("status", "Open")

+     tags = flask.request.args.getlist("tags")

+     tags = [tag.strip() for tag in tags if tag.strip()]

      assignee = flask.request.args.get("assignee", None)

      search_pattern = flask.request.args.get("search_pattern", None)

      author = flask.request.args.get("author", None)
@@ -98,6 +100,7 @@ 

              order_key=order_key,

              assignee=assignee,

              author=author,

+             tags=tags,

              search_pattern=search_pattern,

              offset=flask.g.offset,

              limit=flask.g.limit,
@@ -112,6 +115,7 @@ 

              order_key=order_key,

              assignee=assignee,

              author=author,

+             tags=tags,

              search_pattern=search_pattern,

              offset=flask.g.offset,

              limit=flask.g.limit,
@@ -126,6 +130,7 @@ 

              order_key=order_key,

              assignee=assignee,

              author=author,

+             tags=tags,

              search_pattern=search_pattern,

              offset=flask.g.offset,

              limit=flask.g.limit,
@@ -140,6 +145,7 @@ 

              order_key=order_key,

              assignee=assignee,

              author=author,

+             tags=tags,

              search_pattern=search_pattern,

              offset=flask.g.offset,

              limit=flask.g.limit,
@@ -151,6 +157,7 @@ 

          status="Open",

          assignee=assignee,

          author=author,

+         tags=tags,

          search_pattern=search_pattern,

          count=True,

      )
@@ -161,6 +168,7 @@ 

          status="Merged",

          assignee=assignee,

          author=author,

+         tags=tags,

          search_pattern=search_pattern,

          count=True,

      )
@@ -171,6 +179,7 @@ 

          status="Closed",

          assignee=assignee,

          author=author,

+         tags=tags,

          search_pattern=search_pattern,

          count=True,

      )
@@ -193,11 +202,15 @@ 

              total_requests = closed_cnt + merged_cnt + open_cnt

          total_page = int(ceil(total_requests / float(flask.g.limit)))

  

+     tag_list = pagure.lib.query.get_tags_of_project(flask.g.session, repo)

+ 

      return flask.render_template(

          "requests.html",

          select="requests",

          repo=repo,

          username=username,

+         tag_list=tag_list,

+         tags=tags,

          requests=requests,

          open_cnt=open_cnt,

          merged_cnt=merged_cnt,

@@ -123,6 +123,7 @@ 

                  "args": {

                      "assignee": None,

                      "author": None,

+                     "tags": [],

                      "page": 1,

                      "per_page": 20,

                      "status": "closed",
@@ -158,6 +159,7 @@ 

              {

                  "assignee": None,

                  "author": None,

+                 "tags": [],

                  "page": 1,

                  "per_page": 20,

                  "status": "closed",
@@ -301,6 +303,7 @@ 

              {

                  "assignee": None,

                  "author": None,

+                 "tags": [],

                  "page": 1,

                  "per_page": 20,

                  "status": "all",
@@ -325,6 +328,7 @@ 

              {

                  "assignee": None,

                  "author": None,

+                 "tags": [],

                  "page": 1,

                  "per_page": 20,

                  "status": "all",
@@ -386,6 +390,7 @@ 

              "args": {

                  "assignee": None,

                  "author": None,

+                 "tags": [],

                  "page": 1,

                  "per_page": 20,

                  "status": True,
@@ -514,6 +519,126 @@ 

          self.assertDictEqual(data, data2)

  

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

+     def test_api_pull_request_view_tag_filtered(self, send_email):

+         """ Test the api_pull_request_view method of the flask api to list

+             tag filtered open PRs. """

+         send_email.return_value = True

+         tests.create_projects(self.session)

+         tests.create_tokens(self.session)

+         tests.create_tokens_acl(self.session)

+         repo = pagure.lib.query.get_authorized_project(self.session, "test")

+ 

+         # Add a tag

+         pagure.lib.query.new_tag(

+             self.session, "tag-1", "tag-1 description", "#ff0000", repo.id

+         )

+         # Create a pull-request

+         forked_repo = pagure.lib.query.get_authorized_project(

+             self.session, "test"

+         )

+         req = pagure.lib.query.new_pull_request(

+             session=self.session,

+             repo_from=forked_repo,

+             branch_from="master",

+             repo_to=repo,

+             branch_to="master",

+             title="test pull-request",

+             user="pingou",

+         )

+         self.session.commit()

+         self.assertEqual(req.id, 1)

+         self.assertEqual(req.title, "test pull-request")

+ 

+         output = self.app.get("/api/0/test/pull-requests?tags=tag-1")

+         self.assertEqual(output.status_code, 200)

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

+         for k in ["first", "last"]:

+             self.assertIsNotNone(data["pagination"][k])

+             data["pagination"][k] = "http://localhost..."

+         self.assertDictEqual(

+             data,

+             {

+                 "args": {

+                     "assignee": None,

+                     "author": None,

+                     "tags": ["tag-1"],

+                     "page": 1,

+                     "per_page": 20,

+                     "status": True,

+                 },

+                 "pagination": {

+                     "first": "http://localhost...",

+                     "last": "http://localhost...",

+                     "next": None,

+                     "page": 1,

+                     "pages": 0,

+                     "per_page": 20,

+                     "prev": None,

+                 },

+                 "requests": [],

+                 "total_requests": 0,

+             },

+         )

+ 

+         # Tag the PR and try again

+         pagure.lib.query.update_tags(

+             self.session, obj=req, tags=["tag-1"], username="pingou"

+         )

+         self.session.commit()

+ 

+         output = self.app.get("/api/0/test/pull-requests?tags=tag-1")

+         self.assertEqual(output.status_code, 200)

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

+         self.assertEqual(

+             sorted(data.keys()),

+             ["args", "pagination", "requests", "total_requests"],

+         )

+         self.assertDictEqual(

+             data["args"],

+             {

+                 "assignee": None,

+                 "author": None,

+                 "tags": ["tag-1"],

+                 "page": 1,

+                 "per_page": 20,

+                 "status": True,

+             },

+         )

+         self.assertEqual(data["total_requests"], 1)

+ 

+         # Try negative filtering

+         output = self.app.get("/api/0/test/pull-requests?tags=!tag-1")

+         self.assertEqual(output.status_code, 200)

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

+         for k in ["first", "last"]:

+             self.assertIsNotNone(data["pagination"][k])

+             data["pagination"][k] = "http://localhost..."

+         self.assertDictEqual(

+             data,

+             {

+                 "args": {

+                     "assignee": None,

+                     "author": None,

+                     "tags": ["!tag-1"],

+                     "page": 1,

+                     "per_page": 20,

+                     "status": True,

+                 },

+                 "pagination": {

+                     "first": "http://localhost...",

+                     "last": "http://localhost...",

+                     "next": None,

+                     "page": 1,

+                     "pages": 0,

+                     "per_page": 20,

+                     "prev": None,

+                 },

+                 "requests": [],

+                 "total_requests": 0,

+             },

+         )

+ 

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

      def test_api_pull_request_view_pr_disabled(self, send_email):

          """ Test the api_pull_request_view method of the flask api. """

          send_email.return_value = True

@@ -1508,6 +1508,7 @@ 

                      "args": {

                          "assignee": None,

                          "author": None,

+                         "tags": [],

                          "page": 1,

                          "per_page": 20,

                          "status": True,

@@ -1516,6 +1516,189 @@ 

          self.assertEqual(output.status_code, 404)

  

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

+     def test_request_pulls_filters_tags(self, send_email):

+         """Test the requests_pull

+ 

+         i.e Make sure that the results are filtered properly"""

+         send_email.return_value = True

+ 

+         tests.create_projects(self.session)

+         tests.create_projects_git(os.path.join(self.path, "repos"), bare=True)

+ 

+         repo = pagure.lib.query.get_authorized_project(self.session, "test")

+         # Create some tags to play with

+         pagure.lib.query.new_tag(

+             self.session, "tag-1", "tag-1 descripcion", "#ff0000", repo.id

+         )

+         pagure.lib.query.new_tag(

+             self.session, "tag-2", "tag-2 description", "#00ff00", repo.id

+         )

+         pagure.lib.query.new_tag(

+             self.session, "tag-3", "tag-3 description", "#0000ff", repo.id

+         )

+ 

+         fork = pagure.lib.model.Project(

+             user_id=2,

+             name="test",

+             description="test project #1",

+             hook_token="aaabbb",

+             is_fork=True,

+             parent_id=1,

+         )

+         self.session.add(fork)

+         self.session.commit()

+ 

+         # Create PR's to play with

+         # PR-1, tags: tag-1, tag-3

+         req = pagure.lib.query.new_pull_request(

+             session=self.session,

+             repo_to=repo,

+             repo_from=fork,

+             branch_from="feature",

+             branch_to="master",

+             title="First PR",

+             user="pingou",

+             status="Open",

+         )

+         pagure.lib.query.update_tags(

+             self.session, obj=req, tags=["tag-1", "tag-3"], username="pingou"

+         )

+         self.session.commit()

+ 

+         # PR-2, tags: tag-2, tag-3

+         req = pagure.lib.query.new_pull_request(

+             session=self.session,

+             repo_to=repo,

+             repo_from=fork,

+             branch_from="feature",

+             branch_to="master",

+             title="Second PR",

+             user="pingou",

+             status="Open",

+         )

+         pagure.lib.query.update_tags(

+             self.session, obj=req, tags=["tag-2", "tag-3"], username="pingou"

+         )

+         self.session.commit()

+ 

+         # PR-3 closed, tags: tag-1, tag-3

+         req = pagure.lib.query.new_pull_request(

+             session=self.session,

+             repo_to=repo,

+             repo_from=fork,

+             branch_from="feature",

+             branch_to="master",

+             title="Third PR",

+             user="pingou",

+             status="Closed",

+         )

+         pagure.lib.query.update_tags(

+             self.session, obj=req, tags=["tag-1", "tag-3"], username="pingou"

+         )

+         self.session.commit()

+ 

+         # PR-4 closed, tags: tag-1, tag-2

+         req = pagure.lib.query.new_pull_request(

+             session=self.session,

+             repo_to=repo,

+             repo_from=fork,

+             branch_from="feature",

+             branch_to="master",

+             title="Fourth PR",

+             user="pingou",

+             status="Closed",

+         )

+         pagure.lib.query.update_tags(

+             self.session, obj=req, tags=["tag-1", "tag-2"], username="pingou"

+         )

+         self.session.commit()

+ 

+         # filter by 'tag-1'

+         output = self.app.get("/test/pull-requests?tags=tag-1")

+         self.assertEqual(output.status_code, 200)

+         output_text = output.get_data(as_text=True)

+         tr_elements = re.findall(

+             '<div class="request-row list-group-item list-group-item-action ">(.*?)</div><!--end request-row-->',

+             output_text,

+             re.M | re.S,

+         )

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

+         self.assertIn('href="/test/pull-request/1', tr_elements[0])

+ 

+         # filter by '!tag-1'

+         output = self.app.get("/test/pull-requests?tags=!tag-1")

+         self.assertEqual(output.status_code, 200)

+         output_text = output.get_data(as_text=True)

+         tr_elements = re.findall(

+             '<div class="request-row list-group-item list-group-item-action ">(.*?)</div><!--end request-row-->',

+             output_text,

+             re.M | re.S,

+         )

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

+         self.assertIn('href="/test/pull-request/2', tr_elements[0])

+ 

+         # filter by 'tag-2' and 'tag-3'

+         output = self.app.get("/test/pull-requests?tags=tag2&tags=tag-3")

+         self.assertEqual(output.status_code, 200)

+         output_text = output.get_data(as_text=True)

+         tr_elements = re.findall(

+             '<div class="request-row list-group-item list-group-item-action ">(.*?)</div><!--end request-row-->',

+             output_text,

+             re.M | re.S,

+         )

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

+         self.assertIn('href="/test/pull-request/2', tr_elements[0])

+         self.assertIn('href="/test/pull-request/1', tr_elements[1])

+ 

+         # filter by '!tag-3'

+         output = self.app.get("/test/pull-requests?tags=!tag-3")

+         self.assertEqual(output.status_code, 200)

+         output_text = output.get_data(as_text=True)

+         tr_elements = re.findall(

+             '<div class="request-row list-group-item list-group-item-action ">(.*?)</div><!--end request-row-->',

+             output_text,

+             re.M | re.S,

+         )

+         self.assertEqual(0, len(tr_elements))

+ 

+         # filter by tag-2 on Closed prs

+         output = self.app.get("/test/pull-requests?status=Closed&tags=tag-2")

+         self.assertEqual(output.status_code, 200)

+         output_text = output.get_data(as_text=True)

+         tr_elements = re.findall(

+             '<div class="request-row list-group-item list-group-item-action ">(.*?)</div><!--end request-row-->',

+             output_text,

+             re.M | re.S,

+         )

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

+         self.assertIn('href="/test/pull-request/4', tr_elements[0])

+ 

+         # filter by !tag-2 on Closed prs

+         output = self.app.get("/test/pull-requests?status=Closed&tags=!tag-2")

+         self.assertEqual(output.status_code, 200)

+         output_text = output.get_data(as_text=True)

+         tr_elements = re.findall(

+             '<div class="request-row list-group-item list-group-item-action ">(.*?)</div><!--end request-row-->',

+             output_text,

+             re.M | re.S,

+         )

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

+         self.assertIn('href="/test/pull-request/3', tr_elements[0])

+ 

+         # filter by tag-2 on all the prs

+         output = self.app.get("/test/pull-requests?status=all&tags=tag-2")

+         self.assertEqual(output.status_code, 200)

+         output_text = output.get_data(as_text=True)

+         tr_elements = re.findall(

+             '<div class="request-row list-group-item list-group-item-action ">(.*?)</div><!--end request-row-->',

+             output_text,

+             re.M | re.S,

+         )

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

+         self.assertIn('href="/test/pull-request/4', tr_elements[0])

+         self.assertIn('href="/test/pull-request/2', tr_elements[1])

+ 

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

      def test_request_pull_patch(self, send_email):

          """ Test the request_pull_patch endpoint. """

          send_email.return_value = True

If the approach is valid I'll work on tests & api versions =)

rebased onto 13934f568982b141d4b24841324dd5e00eef11b3

4 years ago

pretty please pagure-ci rebuild

4 years ago

@jflory7 the pipeline is broken, the pr itself passes the tests but we don't support (yet) arrow 0.15.x and part of our pipeline is fetching it from pip. Once #4612 or other solution is merged jenkins should be happy

Code looks fine, we'll need some tests for this :)

I wonder how similar this is from the issue filtering and thus if there would be a way to refactor this so we don't have too much code duplication

rebased onto 340baf71bfcf7a8c46eee3b40ba09769a5f2e0b8

4 years ago

rebased onto 162732f4f6640c786583cff98a6a6b84072b13f8

4 years ago

1 new commit added

  • Tag filtering support on api pull requests endpoint
4 years ago

3 new commits added

  • Tag filtering support on api pull requests endpoint
  • unit tests for pull request view tag filtering
  • Tag filtering support on pull requests list view
4 years ago

pretty please pagure-ci rebuild

4 years ago

Let's rebase and get this in :)

rebased onto 056734e758b956adf4272ae7efb61fe088fb31b7

4 years ago

rebased onto ebdf787

4 years ago

Pull-Request has been merged by pingou

4 years ago