ResultPager fetchall on apps/installations incorrect result

[acrobat]

See https://platform.github.community/t/pagination-differs-in-installations-api/1862

The result is an array with a count field and and subarray of all installations but we merge result when using the fetchAll method

We need to check if this is something we need/can fix in code or if we should add some docs about this issue

[bdelbasso]

FYI I'm using a CustomResultPager as a workaround for this problem.

<?php

namespace AppBundle\Github;

use Github\Api\ApiInterface;
use Github\Client;
use Github\ResultPager;

/**
 * Workaround for:
 *
 * https://platform.github.community/t/pagination-differs-in-installations-api/1862
 */
class CustomResultPager extends ResultPager
{

    private $key;

    public function __construct(Client $client, string $key)
    {
        parent::__construct($client);
        $this->key = $key;
    }

    public function fetchAll(ApiInterface $api, $method, array $parameters = array())
    {
        // get the perPage from the api
        $perPage = $api->getPerPage();

        // set parameters per_page to GitHub max to minimize number of requests
        $api->setPerPage(100);

        $result = $this->decode($this->callApi($api, $method, $parameters));
        $this->postFetch();

        while ($this->hasNext()) {
            $next = $this->fetchNext();

            $result = array_merge($result, $this->decode($next));
        }

        // restore the perPage
        $api->setPerPage($perPage);

        return $result;
    }

    protected function decode(array $result) {
        if(! isset($result[$this->key])) {
            throw new \Exception("No key $this->key, got: ".array_keys($result));
        }
        return $result[$this->key];
    }

}

I see two solutions to address this issue here:

1. Include this class in php-github-api and document when to use it.

2. Modify the existing ResultPager to merge differently when the key totalCount is present. This would be transparent to the user but more hack-ish.

@acrobat @Nyholm What do you think?

[Daniel15]

My workaround is very similar to @bdelbasso's. I instead implemented it as a separate method:

class ResultPagerWithCustomField extends ResultPager {
  public function fetchAllUsingField(
    ApiInterface $api,
    string $method,
    string $field,
    array $parameters = array()
  ) {
    // ...
  }
}

I think it'd be good to include this functionality in the existing ResultPager