PR#2494: Make the API returns only open issues by default. - pagure

pagure

#2494 Make the API returns only open issues by default.

Merged 6 years ago by pingou. Opened 6 years ago by cverna.

cverna/pagure fix_2468 into master

Make the API returns only open issues by default.

Clement Verna • 6 years ago

f7b10e1

pagure/api/issue.py

file modified

		`@@ -437,6 +437,8 @@`
		`params.update({'closed': True})`
		`else:`
		`params.update({'status': status})`
		`+ else:`
		`+ params.update({'status': 'Open'})`

		`updated_after = None`
		`if since:`

tests/test_pagure_flask_api_issue.py

file modified

+14 -12

		`@@ -45,7 +45,7 @@`
		`"milestone": "",`
		`"priority": None,`
		`"private": True,`
		`- "status": "Open",`
		`+ "status": "Closed",`
		`"tags": [],`
		`"title": "Test issue",`
		`"user": {`
		`@@ -298,7 +298,6 @@`

		`pagure.APP.config['TICKETS_FOLDER'] = None`

		`-`
		`def test_api_new_issue(self):`
		`""" Test the api_new_issue method of the flask api. """`
		`tests.create_projects(self.session)`
		`@@ -848,7 +847,8 @@`
		`user='pingou',`
		`ticketfolder=None,`
		`private=True,`
		`- milestone=""`
		`+ milestone="",`
		`+ status="Closed"`
		`)`
		`self.session.commit()`
		`self.assertEqual(msg.title, 'Test issue')`
		`@@ -943,8 +943,8 @@`
		`"status": None,`
		`"tags": []`
		`},`
		`- "issues": FULL_ISSUE_LIST,`
		`- "total_issues": 9`
		`+ "issues": FULL_ISSUE_LIST[1:],`
		`+ "total_issues": 8`
		`}`
		`)`
		`headers = {'Authorization': 'token aaabbbccc'}`
		`@@ -1012,8 +1012,8 @@`
		`"status": None,`
		`"tags": []`
		`},`
		`- "issues": FULL_ISSUE_LIST,`
		`- "total_issues": 9`
		`+ "issues": FULL_ISSUE_LIST[1:],`
		`+ "total_issues": 8`
pingou commented 6 years ago Does it make sense to adjust one of these to return the full list?
cverna commented 6 years ago It used to return the full list because there was no `Closed` status issues in the `FULL_ISSUE_LIST`. I have change on issue status to `Closed` so that we can test the case below.
pingou commented 6 years ago Yes but would it make sense for one of the two tests we have here to include all tickets, open and closed?
cverna commented 6 years ago There is already one test for that with `?status=All` https://pagure.io/pagure/blob/master/f/tests/test_pagure_flask_api_issue.py#_1065
		`}`
		`)`

		`@@ -1021,21 +1021,23 @@`
		`output = self.app.get('/api/0/test/issues?status=Closed', headers=headers)`
		`self.assertEqual(output.status_code, 200)`
		`data = json.loads(output.data)`
		`+ data['issues'][0]['date_created'] = '1431414800'`
		`+ data['issues'][0]['last_updated'] = '1431414800'`
		`self.assertDictEqual(`
		`data,`
		`{`
		`"args": {`
		`"assignee": None,`
		`"author": None,`
		`- 'milestones': [],`
		`- 'no_stones': None,`
		`- 'priority': None,`
		`+ "milestones": [],`
		`+ "no_stones": None,`
		`+ "priority": None,`
		`"since": None,`
		`"status": "Closed",`
		`"tags": []`
		`},`
		`- "total_issues": 0,`
		`- "issues": []`
		`+ "issues": [FULL_ISSUE_LIST[0]],`
		`+ "total_issues": 1,`
pingou commented 6 years ago wow, `?status=closed` was not returning anything before, gloups :s
		`}`
		`)`

cverna commented 6 years ago

Signed-off-by: Clement Verna cverna@tutanota.com

cverna commented 6 years ago

FIx for #2468

rebased

6 years ago

pingou commented on line 46 of tests/test_pagure_flask_api_issue.py 6 years ago

Does it make sense to adjust one of these to return the full list?

pingou commented on line 75 of tests/test_pagure_flask_api_issue.py 6 years ago

wow, ?status=closed was not returning anything before, gloups :s

pingou commented 6 years ago

One question, otherwise looks good

cverna commented on line 46 of tests/test_pagure_flask_api_issue.py 6 years ago

It used to return the full list because there was no Closed status issues in the FULL_ISSUE_LIST. I have change on issue status to Closed so that we can test the case below.