fix(api): add all_pages param to allow passing `all` to APIfix/all-pages-consumes-all-param

8 files changed, 133 insertions, 60 deletions

diff --git a/docs/api-usage.rst b/docs/api-usage.rst
index e911664..3bf4354 100644
--- a/docs/api-usage.rst
+++ b/docs/api-usage.rst
@@ -190,6 +190,8 @@ a project (the previous example used 2 API calls):
    project = gl.projects.get(1, lazy=True)  # no API call
    project.star()  # API call
 
+.. _pagination:
+
 Pagination
 ==========
 
@@ -205,11 +207,16 @@ listing methods support the ``page`` and ``per_page`` parameters:
    The first page is page 1, not page 0.
 
 By default GitLab does not return the complete list of items. Use the ``all``
-parameter to get all the items when using listing methods:
+parameter to get all the items when using listing methods. Alternatively, if
+the endpoint also accepts an ``all`` parameter itself, you can use the
+``all_pages`` parameter for listing methods to avoid this conflict:
 
 .. code-block:: python
 
    all_groups = gl.groups.list(all=True)
+   # or
+   all_groups = gl.groups.list(all_pages=True)
+
    all_owned_projects = gl.projects.list(owned=True, all=True)
 
 You can define the ``per_page`` value globally to avoid passing it to every
diff --git a/docs/faq.rst b/docs/faq.rst
index 0f914ed..834420d 100644
--- a/docs/faq.rst
+++ b/docs/faq.rst
@@ -36,3 +36,8 @@ I get an ``AttributeError`` when accessing attributes after ``save()`` or ``refr
     You are most likely trying to access an attribute that was not returned
     by the server on the second request. Please look at the documentation in
     :ref:`object_attributes` to see how to avoid this.
+
+I passed ``all=True`` (or ``--all`` via the CLI) to the API and I still cannot see all items returned.
+    In some cases, API endpoints take an ``all`` parameter that conflicts with python-gitlab's
+    own ``all`` used with pagination. Use ``all_pages=True`` (or ``--all-pages`` via the CLI) along
+    with ``all=True`` to really fetch all items in this case. See :ref:`pagination` for more details.
diff --git a/docs/gl_objects/commits.rst b/docs/gl_objects/commits.rst
index a1d878c..550658b 100644
--- a/docs/gl_objects/commits.rst
+++ b/docs/gl_objects/commits.rst
@@ -30,7 +30,12 @@ results::
 .. note::
 
    The available ``all`` listing argument conflicts with the python-gitlab
-   argument. Use ``query_parameters`` to avoid the conflict::
+   argument. Use the ``all_pages`` argument to return all paginated items
+   if you need to pass ``all`` to the API:
+
+        commits = project.commits.list(all_pages=True, all=True)
+
+   Alternatively, use ``query_parameters`` to specify the parameters::
 
        commits = project.commits.list(all=True,
                                       query_parameters={'ref_name': 'my_branch'})
diff --git a/gitlab/client.py b/gitlab/client.py
index 1fcda1e..e42350e 100644
--- a/gitlab/client.py
+++ b/gitlab/client.py
@@ -652,9 +652,9 @@ class Gitlab(object):
         Returns:
             list: A list of the objects returned by the server. If `as_list` is
             False and no pagination-related arguments (`page`, `per_page`,
-            `all`) are defined then a GitlabList object (generator) is returned
-            instead. This object will make API calls when needed to fetch the
-            next items from the server.
+            `all`/`all_pages`) are defined then a GitlabList object (generator) is
+            returned instead. This object will make API calls when needed to fetch
+            the next items from the server.
 
         Raises:
             GitlabHttpError: When the return code is not 2xx
@@ -665,7 +665,12 @@ class Gitlab(object):
         # In case we want to change the default behavior at some point
         as_list = True if as_list is None else as_list
 
-        get_all = kwargs.pop("all", False)
+        # Provide an "all_pages" param for endpoints that also take "all" as param.
+        get_all = kwargs.pop("all_pages", None)
+
+        if get_all is None:
+            get_all = kwargs.pop("all", False)
+
         url = self._build_url(path)
 
         page = kwargs.get("page")
diff --git a/gitlab/v4/cli.py b/gitlab/v4/cli.py
index 42b94aa..b8f40db 100644
--- a/gitlab/v4/cli.py
+++ b/gitlab/v4/cli.py
@@ -152,7 +152,18 @@ def _populate_sub_parser_by_class(cls, sub_parser):
 
             sub_parser_action.add_argument("--page", required=False)
             sub_parser_action.add_argument("--per-page", required=False)
-            sub_parser_action.add_argument("--all", required=False, action="store_true")
+            sub_parser_action.add_argument(
+                "--all",
+                required=False,
+                action="store_true",
+                help="Return all items from the server, without pagination.",
+            )
+            sub_parser_action.add_argument(
+                "--all-pages",
+                required=False,
+                action="store_true",
+                help="Same as --all. Use when you need to pass `all` to the GitLab API.",
+            )
 
         if action_name == "delete":
             if cls._id_attr is not None:
diff --git a/tools/functional/api/test_repository.py b/tools/functional/api/test_repository.py
index 7ba84ea..b4ba799 100644
--- a/tools/functional/api/test_repository.py
+++ b/tools/functional/api/test_repository.py
@@ -71,6 +71,26 @@ def test_create_commit(project):
     assert isinstance(commit.merge_requests(), list)
 
 
+def test_list_all_commits(project):
+    data = {
+        "branch": "new-branch",
+        "start_branch": "master",
+        "commit_message": "New commit on new branch",
+        "actions": [
+            {"action": "create", "file_path": "new-file", "content": "new content"}
+        ],
+    }
+    commit = project.commits.create(data)
+
+    commits = project.commits.list(all=True)
+    assert commit not in commits
+
+    # Listing commits on other branches requires `all` parameter passed to the API
+    all_commits = project.commits.list(all_pages=True, all=True)
+    assert commit in all_commits
+    assert len(all_commits) > len(commits)
+
+
 def test_create_commit_status(project):
     commit = project.commits.list()[0]
     size = len(commit.statuses.list())
diff --git a/tools/functional/cli/test_cli_repository.py b/tools/functional/cli/test_cli_repository.py
new file mode 100644
index 0000000..825d5ba
--- /dev/null
+++ b/tools/functional/cli/test_cli_repository.py
@@ -0,0 +1,73 @@
+def test_project_create_file(gitlab_cli, project):
+    file_path = "README"
+    branch = "master"
+    content = "CONTENT"
+    commit_message = "Initial commit"
+
+    cmd = [
+        "project-file",
+        "create",
+        "--project-id",
+        project.id,
+        "--file-path",
+        file_path,
+        "--branch",
+        branch,
+        "--content",
+        content,
+        "--commit-message",
+        commit_message,
+    ]
+    ret = gitlab_cli(cmd)
+
+    assert ret.success
+
+
+def test_list_all_commits(gitlab_cli, project):
+    data = {
+        "branch": "new-branch",
+        "start_branch": "master",
+        "commit_message": "New commit on new branch",
+        "actions": [
+            {"action": "create", "file_path": "new-file", "content": "new content"}
+        ],
+    }
+    commit = project.commits.create(data)
+
+    cmd = ["project-commit", "list", "--project-id", project.id, "--all-pages"]
+    ret = gitlab_cli(cmd)
+    assert commit.id not in ret.stdout
+
+    # Listing commits on other branches requires `all` parameter passed to the API
+    cmd = ["project-commit", "list", "--project-id", project.id, "--all-pages", "--all"]
+    ret_all = gitlab_cli(cmd)
+    assert commit.id in ret_all.stdout
+    assert len(ret_all.stdout) > len(ret.stdout)
+
+
+def test_revert_commit(gitlab_cli, project):
+    commit = project.commits.list()[0]
+
+    cmd = [
+        "project-commit",
+        "revert",
+        "--project-id",
+        project.id,
+        "--id",
+        commit.id,
+        "--branch",
+        "master",
+    ]
+    ret = gitlab_cli(cmd)
+
+    assert ret.success
+
+
+def test_get_commit_signature_not_found(gitlab_cli, project):
+    commit = project.commits.list()[0]
+
+    cmd = ["project-commit", "signature", "--project-id", project.id, "--id", commit.id]
+    ret = gitlab_cli(cmd)
+
+    assert not ret.success
+    assert "404 Signature Not Found" in ret.stderr
diff --git a/tools/functional/cli/test_cli_v4.py b/tools/functional/cli/test_cli_v4.py
index a63c1b1..55482d2 100644
--- a/tools/functional/cli/test_cli_v4.py
+++ b/tools/functional/cli/test_cli_v4.py
@@ -123,31 +123,6 @@ def test_list_user_memberships(gitlab_cli, user):
     assert ret.success
 
 
-def test_project_create_file(gitlab_cli, project):
-    file_path = "README"
-    branch = "master"
-    content = "CONTENT"
-    commit_message = "Initial commit"
-
-    cmd = [
-        "project-file",
-        "create",
-        "--project-id",
-        project.id,
-        "--file-path",
-        file_path,
-        "--branch",
-        branch,
-        "--content",
-        content,
-        "--commit-message",
-        commit_message,
-    ]
-    ret = gitlab_cli(cmd)
-
-    assert ret.success
-
-
 def test_create_project_issue(gitlab_cli, project):
     title = "my issue"
     description = "my issue description"
@@ -249,34 +224,6 @@ def test_accept_request_merge(gitlab_cli, project):
     assert ret.success
 
 
-def test_revert_commit(gitlab_cli, project):
-    commit = project.commits.list()[0]
-
-    cmd = [
-        "project-commit",
-        "revert",
-        "--project-id",
-        project.id,
-        "--id",
-        commit.id,
-        "--branch",
-        "master",
-    ]
-    ret = gitlab_cli(cmd)
-
-    assert ret.success
-
-
-def test_get_commit_signature_not_found(gitlab_cli, project):
-    commit = project.commits.list()[0]
-
-    cmd = ["project-commit", "signature", "--project-id", project.id, "--id", commit.id]
-    ret = gitlab_cli(cmd)
-
-    assert not ret.success
-    assert "404 Signature Not Found" in ret.stderr
-
-
 def test_create_project_label(gitlab_cli, project):
     name = "prjlabel1"
     description = "prjlabel1 description"