PR#4151: Fix the pull-request filtering by status in API calls - pagure

pagure

#4151 Fix the pull-request filtering by status in API calls

Closed 5 years ago by lenkaseg. Opened 5 years ago by lenkaseg.

Unknown source fix_status_filter into master

Fix pull-request filtering by status in API calls

Lenka Segura • 5 years ago

c123eb4

dev-data.py

file modified

+104 -4

		`@@ -355,11 +355,83 @@`
		`repo_to=repo,`
		`branch_to='master',`
		`title='Fixing code for unittest',`
		`- user=username`
		`+ user=username,`
		`+ status="Open"`
		`)`
		`session.commit()`

		`- ######################################`
		`+ repo = pagure.lib.query.get_authorized_project(session, 'test')`
		`+ forked_repo = pagure.lib.query.get_authorized_project(session, 'test')`
		`+ req = pagure.lib.query.new_pull_request(`
		`+ session=session,`
		`+ repo_from=forked_repo,`
		`+ branch_from='master',`
		`+ repo_to=repo,`
		`+ branch_to='master',`
		`+ title='add very nice README',`
		`+ user=username,`
		`+ status="Open"`
		`+ )`
		`+ session.commit()`
		`+`
		`+ repo = pagure.lib.query.get_authorized_project(session, 'test')`
		`+ forked_repo = pagure.lib.query.get_authorized_project(session, 'test')`
		`+ req = pagure.lib.query.new_pull_request(`
		`+ session=session,`
		`+ repo_from=forked_repo,`
		`+ branch_from='master',`
		`+ repo_to=repo,`
		`+ branch_to='master',`
		`+ title='Add README',`
		`+ user=username,`
		`+ status="Closed"`
		`+ )`
		`+ session.commit()`
		`+`
		`+ repo = pagure.lib.query.get_authorized_project(session, 'test')`
		`+ forked_repo = pagure.lib.query.get_authorized_project(session, 'test')`
		`+ req = pagure.lib.query.new_pull_request(`
		`+ session=session,`
		`+ repo_from=forked_repo,`
		`+ branch_from='master',`
		`+ repo_to=repo,`
		`+ branch_to='master',`
		`+ title='Fix some containers',`
		`+ user=username,`
		`+ status="Merged"`
		`+ )`
		`+ session.commit()`
		`+`
		`+ repo = pagure.lib.query.get_authorized_project(session, 'test')`
		`+ forked_repo = pagure.lib.query.get_authorized_project(session, 'test')`
		`+ req = pagure.lib.query.new_pull_request(`
		`+ session=session,`
		`+ repo_from=forked_repo,`
		`+ branch_from='master',`
		`+ repo_to=repo,`
		`+ branch_to='master',`
		`+ title='Fix pull request statuses',`
		`+ user=username,`
		`+ status="Closed"`
		`+ )`
		`+ session.commit()`
		`+`
		`+ repo = pagure.lib.query.get_authorized_project(session, 'test')`
		`+ forked_repo = pagure.lib.query.get_authorized_project(session, 'test')`
		`+ req = pagure.lib.query.new_pull_request(`
		`+ session=session,`
		`+ repo_from=forked_repo,`
		`+ branch_from='master',`
		`+ repo_to=repo,`
		`+ branch_to='master',`
		`+ title='Fixing UI of issue',`
		`+ user=username,`
		`+ status="Merged"`
		`+ )`
		`+ session.commit()`
		`+`
		`+`
		`+ #####################################`
		`# tokens`
		`tests.create_tokens(session, user_id=pingou.id, project_id=project1.id)`

		`@@ -426,12 +498,40 @@`
		`percent=80,`
		`comment="Jenkins build passes",`
		`url=str(pr.id),`
		`- status="Open"`
		`+ status="success"`
		`+ )`
		`+ session.add(item)`
		`+ session.commit()`
		`+`
		`+ pr = pagure.lib.query.get_pull_request_of_user(session, "foo")[1]`
		`+ item = pagure.lib.model.PullRequestFlag(`
		`+ uid="oink oink uid",`
		`+ pull_request_uid=pr.uid,`
		`+ user_id=pingou.id,`
		`+ username=pingou.user,`
		`+ percent=80,`
		`+ comment="Jenkins does not pass",`
		`+ url=str(pr.id),`
		`+ status="failure"`
		`)`
		`session.add(item)`
		`session.commit()`

		`######################################`
		`+ # pull_request_assignee`
		`+ pr = pagure.lib.query.search_pull_requests(session, requestid='1006')`
		`+ pr.assignee_id = pingou.id`
		`+ session.commit()`
		`+`
		`+ pr = pagure.lib.query.search_pull_requests(session, requestid='1007')`
		`+ pr.assignee_id = you.id`
		`+ session.commit()`
		`+`
		`+ pr = pagure.lib.query.search_pull_requests(session, requestid='1004')`
		`+ pr.assignee_id = foo.id`
		`+ session.commit()`
		`+`
		`+ ######################################`
		`# tags_issues`
		`repo = pagure.lib.query.get_authorized_project(session, 'test')`
		`issues = pagure.lib.query.search_issues(session, repo)`
		`@@ -595,7 +695,7 @@`
		`empty_dev_db(meta, eng)`

		`if args.populate or args.all:`
		`- session = create_session(_config['DB_URL'])`
		`+ session = pagure.lib.query.create_session(_config['DB_URL'])`
		`invalid_option = ['pingou', 'bar@pingou.com', 'foo', 'foo@bar.com']`
		`print("")`
		`user_name = six.moves.input(`

pagure/api/fork.py

file modified

+1 -1

		`@@ -164,7 +164,7 @@`

		`status_text = ("%s" % status).lower()`
		`requests = []`
		`- if status_text in ["0", "false", "closed"]:`
		`+ if status_text in ["0", "false"]:`
		`requests = pagure.lib.query.search_pull_requests(`
		`flask.g.session,`
		`project_id=repo.id,`

pagure/lib/query.py

file modified

+3 -3

		`@@ -45,7 +45,7 @@`
		`import sqlalchemy.schema`

		`from six.moves.urllib_parse import urlparse, urlencode, parse_qsl`
		`- from sqlalchemy import func`
		`+ from sqlalchemy import func, create_engine`
		`from sqlalchemy import asc, desc`
		`from sqlalchemy.orm import aliased`
		`from sqlalchemy.orm import sessionmaker`
		`@@ -162,14 +162,14 @@`
		`if db_url is None:`
		`raise ValueError("First call to create_session needs db_url")`
		`if db_url.startswith("postgres"): # pragma: no cover`
		`- engine = sqlalchemy.create_engine(`
		`+ engine = create_engine(`
pingou commented 5 years ago Why this change?
		`db_url,`
		`echo=debug,`
		`pool_recycle=pool_recycle,`
		`client_encoding="utf8",`
		`)`
		`else: # pragma: no cover`
		`- engine = sqlalchemy.create_engine(`
		`+ engine = create_engine(`
		`db_url, echo=debug, pool_recycle=pool_recycle`
		`)`

pagure/ui/filters.py

file modified

+1 -1

		`@@ -762,7 +762,7 @@`
		`def flag_to_label(flag):`
		`""" For a given flag return the bootstrap label to use`
		`"""`
		`- return pagure_config["FLAG_STATUSES_LABELS"][flag.status.lower()]`
		`+ return pagure_config["FLAG_STATUSES_LABELS"][flag.status]`
pingou commented 5 years ago Why this change?


		`@UI_NS.app_template_filter("join_prefix")`

lenkaseg commented 5 years ago

When filtering with status=Closed, both Closed and Merged pull requests were returned. Now status=Closed returns only closed PRs and status=Merged returns only merged ones.

Added more pull requests to the dev-data.py. (So I could test it on something.)

Fixed the mistake of pr status flag in dev-data.pyand the import in query.py (for some reason it was failing for me)

ngompa commented 5 years ago

Shouldn't we have a test that ensures this behavior remains this way?

Otherwise, LGTM. :thumbsup:

lenkaseg commented 5 years ago

It seems the tests already exist in test_pagure_flask_api_user.py

rebased onto c123eb4

5 years ago

lenkaseg commented 5 years ago

I added some more data to dev-data.py: more pull requests and assignees of pull requests.

pingou commented on line 15 of pagure/lib/query.py 5 years ago

Why this change?

pingou commented on line 6 of pagure/ui/filters.py 5 years ago

Why this change?

pingou commented 5 years ago

The changes to dev-data.py should be in a different commit, it is not related to fixing the filtering in the API.

I'm also not seeing any new tests, if we're fixing something, we should add a test that fails without the fix and passes with it (does that make sense?)

lenkaseg commented 5 years ago

Why this change?

I was getting an import error, I have no idea why. This fixed it.

lenkaseg commented 5 years ago

The changes to dev-data.py should be in a different commit, it is not related to fixing the filtering in the API.

Ok. I will split it.

I'm also not seeing any new tests, if we're fixing something, we should add a test that fails without the fix and passes with it (does that make sense?)

Ok, I'll try.

lenkaseg commented 5 years ago

i made a new PR for this. Closing this one.

Pull-Request has been closed by lenkaseg

5 years ago