pagure

Clone

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

file modified
+72 -24 
@@ -140,25 +140,36 @@ 
 

  

          GET /api/0/group/some_group_name?projects=1&acl=commit
 

  

+     ::
 

+ 

+         GET /api/0/group/some_group_name?page=1&per_page=50
 

+ 

      Input
 

      ^^^^^
 

  

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

-     | Key              | Type    | Optionality  | Description                 |
 

-     +==================+=========+==============+=============================+
 

-     | ``group name``   | str     | Mandatory    | The name of the group to    |
 

-     |                  |         |              | retrieve information about. |
 

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

-     | ``projects``     | bool    | Optional     | Specifies whether to include|
 

-     |                  |         |              | projects in the data        |
 

-     |                  |         |              | returned.                   |
 

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

-     | ``acl``          | str     | Optional     | Filter the project returned |
 

-     |                  |         |              | (if any) to those where the |
 

-     |                  |         |              | has the specified ACL level.|
 

-     |                  |         |              | Can be any of: ``admin``,   |
 

-     |                  |         |              | ``commit`` or ``ticket``.   |
 

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

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

+     | Key                   | Type    | Optionality  | Description                 |
 

+     +=======================+=========+==============+=============================+
 

+     | ``group name``        | str     | Mandatory    | The name of the group to    |
 

+     |                       |         |              | retrieve information about. |
 

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

+     | ``projects``          | bool    | Optional     | Specifies whether to include|
 

+     |                       |         |              | projects in the data        |
 

+     |                       |         |              | returned.                   |
 

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

+     | ``acl``               | str     | Optional     | Filter the project returned |
 

+     |                       |         |              | (if any) to those where the |
 

+     |                       |         |              | has the specified ACL level.|
 

+     |                       |         |              | Can be any of: ``admin``,   |
 

+     |                       |         |              | ``commit`` or ``ticket``.   |
 

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

+     | ``page``              | int     | Optional     | Specifies which page to     |
 

+     |                       |         |              | return (defaults to: 1)     |
 

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

+     | ``per_page``          | int     | Optional     | The number of projects      |
 

+     |                       |         |              | to return per page.         |
 

+     |                       |         |              | The maximum is 100.         |
 

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

  

  

      Sample response
 
@@ -179,6 +190,15 @@ 
 

            "description": "Some Group",
 

            "display_name": "Some Group",
 

            "group_type": "user",
 

+           "pagination": {
 

+             "first": "http://127.0.0.1:5000/api/0/group/some_group_name?per_page=2&page=1",
 

+             "last": "http://127.0.0.1:5000/api/0/group/some_group_name?per_page=2&page=2",
 

+             "next": "http://127.0.0.1:5000/api/0/group/some_group_name?per_page=2&page=2",
 

+             "page": 1,
 

+             "pages": 2,
 

+             "per_page": 2,
 

+             "prev": null
 

+           },
 

            "members": [
 

              "user1",
 

              "user2"
 
@@ -206,6 +226,19 @@ 
 

              "user2"
 

            ],
 

            "name": "some_group_name",
 

+           "total_projects": 1000,
 

+           "pagination": {
 

+             "first":
 

+                 "http://127.0.0.1:5000/api/0/group/some_group_name?per_page=2&projects=1&page=1",
 

+             "last":
 

+                 "http://127.0.0.1:5000/api/0/group/some_group_name?per_page=2&projects=1&page=500",
 

+             "next":
 

+                 "http://127.0.0.1:5000/api/0/group/some_group_name?per_page=2&projects=1&page=2",
 

+             "page": 1,
 

+             "pages": 500,
 

+             "per_page": 2,
 

+             "prev": null
 

+           },
 

            "projects": [],
 

          }
 

  
@@ -228,15 +261,30 @@ 
 

          raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOGROUP)
 

  

      output = group.to_json(public=(not pagure.utils.api_authenticated()))
 

-     if projects and not acl:
 

-         output["projects"] = [
 

-             project.to_json(public=True) for project in group.projects
 

-         ]
 

-     elif projects and acl:
 

+ 

+     if projects:
 

+         # Prepare pagination data for projects
 

+         if not acl:
 

+             group_projects = group.projects
 

+         elif acl:
 

+             group_projects = [
 

+                 pg.project for pg in group.projects_groups if pg.access in acl
 

+             ]
 

+         page = get_page()
 

+         per_page = get_per_page()
 

+         projects_cnt = len(group_projects)
 

+         pagination_metadata = pagure.lib.query.get_pagination_metadata(
 

+             flask.request, page, per_page, projects_cnt
 

+         )
 

+         query_start = (page - 1) * per_page
 

+         query_limit = per_page
 

+         page_projects = group_projects[query_start:query_limit]
 

+ 

+         output["total_projects"] = projects_cnt
 

+         output["pagination"] = pagination_metadata
 

+ 

          output["projects"] = [
 

-             pg.project.to_json(public=True)
 

-             for pg in group.projects_groups
 

-             if pg.access in acl
 

+             project.to_json(public=True) for project in page_projects
 

          ]
 

      jsonout = flask.jsonify(output)
 

      jsonout.status_code = 200

file modified
+98 -5 
@@ -286,6 +286,15 @@ 
 

              "date_created": "1492020239",
 

              "group_type": "user",
 

              "name": "some_group",
 

+             "pagination": {
 

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

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

+                 "next": None,
 

+                 "page": 1,
 

+                 "pages": 1,
 

+                 "per_page": 20,
 

+                 "prev": None,
 

+             },
 

              "projects": [
 

                  {
 

                      "access_groups": {
 
@@ -327,9 +336,14 @@ 
 

                      },
 

                  }
 

              ],
 

+             "total_projects": 1,
 

          }
 

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

          data["date_created"] = "1492020239"
 

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

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

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

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

          projects = []
 

          for p in data["projects"]:
 

              p["date_created"] = "1492020239"
 
@@ -338,13 +352,22 @@ 
 

          data["projects"] = projects
 

          self.assertDictEqual(data, exp)
 

  

-         output2 = self.app.get(
 

+         output = self.app.get(
 

              "/api/0/group/some_group?projects=1&acl=admin", headers=headers
 

          )
 

-         self.assertListEqual(
 

-             output.get_data(as_text=True).split("\n"),
 

-             output2.get_data(as_text=True).split("\n"),
 

-         )
 

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

+         data["date_created"] = "1492020239"
 

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

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

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

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

+         projects = []
 

+         for p in data["projects"]:
 

+             p["date_created"] = "1492020239"
 

+             p["date_modified"] = "1492020239"
 

+             projects.append(p)
 

+         data["projects"] = projects
 

+         self.assertDictEqual(data, exp)
 

  

      def test_api_view_group_w_projects_and_acl_commit(self):
 

          """
 
@@ -366,6 +389,15 @@ 
 

              "date_created": "1492020239",
 

              "group_type": "user",
 

              "name": "some_group",
 

+             "pagination": {
 

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

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

+                 "next": None,
 

+                 "page": 1,
 

+                 "pages": 1,
 

+                 "per_page": 20,
 

+                 "prev": None,
 

+             },
 

              "projects": [
 

                  {
 

                      "access_groups": {
 
@@ -407,9 +439,14 @@ 
 

                      },
 

                  }
 

              ],
 

+             "total_projects": 1,
 

          }
 

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

          data["date_created"] = "1492020239"
 

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

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

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

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

          projects = []
 

          for p in data["projects"]:
 

              p["date_created"] = "1492020239"
 
@@ -438,6 +475,15 @@ 
 

              "date_created": "1492020239",
 

              "group_type": "user",
 

              "name": "some_group",
 

+             "pagination": {
 

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

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

+                 "next": None,
 

+                 "page": 1,
 

+                 "pages": 1,
 

+                 "per_page": 20,
 

+                 "prev": None,
 

+             },
 

              "projects": [
 

                  {
 

                      "access_groups": {
 
@@ -479,9 +525,14 @@ 
 

                      },
 

                  }
 

              ],
 

+             "total_projects": 1,
 

          }
 

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

          data["date_created"] = "1492020239"
 

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

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

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

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

          projects = []
 

          for p in data["projects"]:
 

              p["date_created"] = "1492020239"
 
@@ -522,10 +573,24 @@ 
 

              "date_created": "1492020239",
 

              "group_type": "user",
 

              "name": "some_group",
 

+             "pagination": {
 

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

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

+                 "next": None,
 

+                 "page": 1,
 

+                 "pages": 0,
 

+                 "per_page": 20,
 

+                 "prev": None,
 

+             },
 

              "projects": [],
 

+             "total_projects": 0,
 

          }
 

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

          data["date_created"] = "1492020239"
 

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

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

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

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

          self.assertDictEqual(data, exp)
 

  

      def test_api_view_group_w_projects_and_acl_commit_no_project(self):
 
@@ -560,10 +625,24 @@ 
 

              "date_created": "1492020239",
 

              "group_type": "user",
 

              "name": "some_group",
 

+             "pagination": {
 

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

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

+                 "next": None,
 

+                 "page": 1,
 

+                 "pages": 0,
 

+                 "per_page": 20,
 

+                 "prev": None,
 

+             },
 

              "projects": [],
 

+             "total_projects": 0,
 

          }
 

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

          data["date_created"] = "1492020239"
 

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

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

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

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

          self.assertDictEqual(data, exp)
 

  

      def test_api_view_group_w_projects_and_acl_ticket_no_project(self):
 
@@ -596,10 +675,24 @@ 
 

              "date_created": "1492020239",
 

              "group_type": "user",
 

              "name": "rel-eng",
 

+             "pagination": {
 

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

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

+                 "next": None,
 

+                 "page": 1,
 

+                 "pages": 0,
 

+                 "per_page": 20,
 

+                 "prev": None,
 

+             },
 

              "projects": [],
 

+             "total_projects": 0,
 

          }
 

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

          data["date_created"] = "1492020239"
 

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

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

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

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

          self.assertDictEqual(data, exp)

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
3 years ago

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
3 years ago

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
3 years ago

rebased onto 7b329088c093ffff8a3a14c438b1b127d3913055
3 years ago

pretty please pagure-ci rebuild
3 years ago

rebased onto 21e7ed390b7715f98b4aa693c0e8b276a8273d9a
3 years ago

rebased onto 5a2f30ec6a7745716dedccf91ca78c030a161a74
3 years ago

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.
Edited 3 years ago by ngompa

:thumbsup:

rebased onto 72edfd8e10ea12bdebac9cff7a5b80d9710bc453
3 years ago

pretty please pagure-ci rebuild
3 years ago

rebased onto 93bdbd2
3 years ago

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
3 years ago
Changes Summary 2
+72 -24
file changed
pagure/api/group.py
+98 -5
file changed
tests/test_pagure_flask_api_group.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.