#5025 Add pagination to group API
Merged by pingou. Opened by zlopez.
zlopez/pagure group_api_pagination  into  master

Download 5025.patch

This PR adds pagination for the group API call if projects are
requested.

Fixes issue https://pagure.io/fedora-infrastructure/issue/9399

Signed-off-by: Michal Konečný mkonecny@redhat.com

rebased onto 44dc764a7ce73b461ec2f22de3875284cfbee6df

Black and flake8 seem unhappy:

15:46:08  stdout: 
15:46:08  --- /pagure/pagure/api/group.py   2020-11-05 15:22:25.307802 +0000
15:46:08  +++ /pagure/pagure/api/group.py   2020-11-05 15:45:19.965541 +0000
15:46:08  @@ -265,11 +265,13 @@
15:46:08       if projects:
15:46:08           # Prepare pagination data for projects
15:46:08           if not acl:
15:46:08               group_projects = group.projects
15:46:08           elif acl:
15:46:08  -            group_projects = [pg.project for pg in group.projects_groups if pg.access in acl]
15:46:08  +            group_projects = [
15:46:08  +                pg.project for pg in group.projects_groups if pg.access in acl
15:46:08  +            ]
15:46:08           page = get_page()
15:46:08           per_page = get_per_page()
15:46:08           projects_cnt = len(group_projects)
15:46:08           projects_pagination_metadata = pagure.lib.query.get_pagination_metadata(
15:46:08               flask.request, page, per_page, projects_cnt
15:46:08  --- /pagure/tests/test_pagure_flask_api_group.py  2020-11-05 15:22:25.307802 +0000
15:46:08  +++ /pagure/tests/test_pagure_flask_api_group.py  2020-11-05 15:45:35.565205 +0000
15:46:08  @@ -349,16 +349,16 @@
15:46:08           self.assertDictEqual(data, exp)
15:46:08   
15:46:08           output = self.app.get(
15:46:08               "/api/0/group/some_group?projects=1&acl=admin", headers=headers
15:46:08           )
15:46:08  -        exp["pagination"]["first"] = (
15:46:08  -            "http://localhost/api/0/group/some_group?per_page=20&projects=1&acl=admin&page=1"
15:46:08  -        )
15:46:08  -        exp["pagination"]["last"] = (
15:46:08  -            "http://localhost/api/0/group/some_group?per_page=20&projects=1&acl=admin&page=1"
15:46:08  -        )
15:46:08  +        exp["pagination"][
15:46:08  +            "first"
15:46:08  +        ] = "http://localhost/api/0/group/some_group?per_page=20&projects=1&acl=admin&page=1"
15:46:08  +        exp["pagination"][
15:46:08  +            "last"
15:46:08  +        ] = "http://localhost/api/0/group/some_group?per_page=20&projects=1&acl=admin&page=1"
15:46:08           data = json.loads(output.get_data(as_text=True))
15:46:08           data["date_created"] = "1492020239"
15:46:08           projects = []
15:46:08           for p in data["projects"]:
15:46:08               p["date_created"] = "1492020239"
15:46:08  (b'/pagure/pagure/api/group.py:270:80: E501 line too long (93 > 79 characters)\n/pagure/pagure/api/group.py:274:80: E501 line too long (80 > 79 characters)\n', None)

rebased onto f966873df7fddd3315a0261bc1845bdd73e0bfaa

I tried to run black on my machine and it reformated only the /pagure/api/group.py. Which is strange, according to the jenkins output, I have the same version of black.

I see, I forgot about the line length.

rebased onto 9c90d71ac8c1fa541bb2aca24f063d36cc63dd77

rebased onto 7b329088c093ffff8a3a14c438b1b127d3913055

pretty please pagure-ci rebuild

rebased onto 21e7ed390b7715f98b4aa693c0e8b276a8273d9a

rebased onto 5a2f30ec6a7745716dedccf91ca78c030a161a74

It looks pretty nice. The one thing I wonder is: we're effectively breaking backward compatibility with this change. Would we have a way to make it optional? Should we?

I'm not sure what's the best way here :(

The old requests should still work, the page and per_page have default values, however the response will be different. I could change it to make it optional, if page argument is provided. Would this work?

I think we should not make pagination optional, because the current state can knock out a Pagure server. So I'm good with this PR as-is.

:thumbsup:

rebased onto 72edfd8e10ea12bdebac9cff7a5b80d9710bc453

pretty please pagure-ci rebuild

rebased onto 93bdbd22f0f31ba20e04553c90d2f719645583b0

Alright, as much as I'm not happy about breaking backward compatibility, I'm inclined to do it here and we'll need to be careful to announce it in the release doc.

Pull-Request has been merged by pingou

Metadata