From ebdf78720c07c075cbfaeaab7f3d07983d328459 Mon Sep 17 00:00:00 2001 From: Julen Landa Alustiza Date: Oct 07 2019 10:45:15 +0000 Subject: [PATCH 1/3] Tag filtering support on pull requests list view --- diff --git a/pagure/lib/query.py b/pagure/lib/query.py index 008e731..a4a3c49 100644 --- a/pagure/lib/query.py +++ b/pagure/lib/query.py @@ -3210,6 +3210,7 @@ def search_pull_requests( status=None, author=None, assignee=None, + tags=None, count=False, offset=None, limit=None, @@ -3285,6 +3286,57 @@ def search_pull_requests( 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("*", "%") diff --git a/pagure/templates/requests.html b/pagure/templates/requests.html index 927ba87..45a23f4 100644 --- a/pagure/templates/requests.html +++ b/pagure/templates/requests.html @@ -11,6 +11,20 @@ {% block header %} + {% endblock %} {% block repo %} @@ -133,6 +147,20 @@ repo=repo.name ) }}" method="GET"> +
+ +
+ +
+
+ +
+
+
@@ -309,6 +337,25 @@ $(document).ready(function() { $('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', diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py index ca3e25a..b6834a8 100644 --- a/pagure/ui/fork.py +++ b/pagure/ui/fork.py @@ -69,6 +69,8 @@ def request_pulls(repo, username=None, namespace=None): """ 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 @@ def request_pulls(repo, username=None, namespace=None): 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 @@ def request_pulls(repo, username=None, namespace=None): 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 @@ def request_pulls(repo, username=None, namespace=None): 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 @@ def request_pulls(repo, username=None, namespace=None): 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 @@ def request_pulls(repo, username=None, namespace=None): status="Open", assignee=assignee, author=author, + tags=tags, search_pattern=search_pattern, count=True, ) @@ -161,6 +168,7 @@ def request_pulls(repo, username=None, namespace=None): status="Merged", assignee=assignee, author=author, + tags=tags, search_pattern=search_pattern, count=True, ) @@ -171,6 +179,7 @@ def request_pulls(repo, username=None, namespace=None): status="Closed", assignee=assignee, author=author, + tags=tags, search_pattern=search_pattern, count=True, ) @@ -193,11 +202,15 @@ def request_pulls(repo, username=None, namespace=None): 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, From 9b852bf9059d1dd950d2ad048974bee1cc0cd647 Mon Sep 17 00:00:00 2001 From: Julen Landa Alustiza Date: Oct 07 2019 10:45:15 +0000 Subject: [PATCH 2/3] unit tests for pull request view tag filtering --- diff --git a/tests/test_pagure_flask_ui_fork.py b/tests/test_pagure_flask_ui_fork.py index a7da7b2..1723dc1 100644 --- a/tests/test_pagure_flask_ui_fork.py +++ b/tests/test_pagure_flask_ui_fork.py @@ -1516,6 +1516,189 @@ class PagureFlaskForktests(tests.Modeltests): 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( + '
(.*?)
', + 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( + '
(.*?)
', + 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( + '
(.*?)
', + 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( + '
(.*?)
', + 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( + '
(.*?)
', + 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( + '
(.*?)
', + 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( + '
(.*?)
', + 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 From 49aa66f53b23eb2975f75c2873c837d22a7cf71b Mon Sep 17 00:00:00 2001 From: Julen Landa Alustiza Date: Oct 07 2019 10:45:15 +0000 Subject: [PATCH 3/3] Tag filtering support on api pull requests endpoint --- diff --git a/pagure/api/fork.py b/pagure/api/fork.py index b028771..e1330f4 100644 --- a/pagure/api/fork.py +++ b/pagure/api/fork.py @@ -87,6 +87,13 @@ def api_pull_request_views(repo, username=None, namespace=None): | ``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 @@ def api_pull_request_views(repo, username=None, namespace=None): "args": { "assignee": null, "author": null, - "status": true + "status": true, + "tags": null }, "total_requests": 1, "requests": [ @@ -154,6 +162,8 @@ def api_pull_request_views(repo, username=None, namespace=None): 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 @@ def api_pull_request_views(repo, username=None, namespace=None): status=False, assignee=assignee, author=author, + tags=tags, ) elif status_text == "all": @@ -173,6 +184,7 @@ def api_pull_request_views(repo, username=None, namespace=None): status=None, assignee=assignee, author=author, + tags=tags, ) else: @@ -182,6 +194,7 @@ def api_pull_request_views(repo, username=None, namespace=None): assignee=assignee, author=author, status=status, + tags=tags, ) page = get_page() @@ -201,7 +214,12 @@ def api_pull_request_views(repo, username=None, namespace=None): "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 diff --git a/tests/test_pagure_flask_api_fork.py b/tests/test_pagure_flask_api_fork.py index 8f1c9f3..feb03fa 100644 --- a/tests/test_pagure_flask_api_fork.py +++ b/tests/test_pagure_flask_api_fork.py @@ -123,6 +123,7 @@ class PagureFlaskApiForktests(tests.Modeltests): "args": { "assignee": None, "author": None, + "tags": [], "page": 1, "per_page": 20, "status": "closed", @@ -158,6 +159,7 @@ class PagureFlaskApiForktests(tests.Modeltests): { "assignee": None, "author": None, + "tags": [], "page": 1, "per_page": 20, "status": "closed", @@ -301,6 +303,7 @@ class PagureFlaskApiForktests(tests.Modeltests): { "assignee": None, "author": None, + "tags": [], "page": 1, "per_page": 20, "status": "all", @@ -325,6 +328,7 @@ class PagureFlaskApiForktests(tests.Modeltests): { "assignee": None, "author": None, + "tags": [], "page": 1, "per_page": 20, "status": "all", @@ -386,6 +390,7 @@ class PagureFlaskApiForktests(tests.Modeltests): "args": { "assignee": None, "author": None, + "tags": [], "page": 1, "per_page": 20, "status": True, @@ -514,6 +519,126 @@ class PagureFlaskApiForktests(tests.Modeltests): 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 diff --git a/tests/test_pagure_flask_api_ui_private_repo.py b/tests/test_pagure_flask_api_ui_private_repo.py index b495f0b..6d6451b 100644 --- a/tests/test_pagure_flask_api_ui_private_repo.py +++ b/tests/test_pagure_flask_api_ui_private_repo.py @@ -1508,6 +1508,7 @@ class PagurePrivateRepotest(tests.Modeltests): "args": { "assignee": None, "author": None, + "tags": [], "page": 1, "per_page": 20, "status": True,