VisualAppeal / Matomo-PHP-API

PHP wrapper for the Matomo API.
MIT License
95 stars 33 forks source link

Missing parameter in function getSiteSettings() #54

Closed Guilhem-Lalanne closed 1 year ago

Guilhem-Lalanne commented 1 year ago

Hi,

It seems that the method getSiteSettings() is missing an argument siteId: https://github.com/VisualAppeal/Matomo-PHP-API/blob/5b8d5351c25f17172bcf7563b6a56a67caa60d00/src/Matomo.php#L4322C14-L4322C14

Here is the error when using the function: PHP Fatal error: Uncaught VisualAppeal\InvalidResponseException: Please specify a value for 'idSite'

Regards,

Guilhem-Lalanne commented 1 year ago

I think that I understand the problem: it seems that you consider the idSite to always be the id of the Matomo instance itself:

private function _parseUrl(string $method, array $params = [])
    {
        $params = [
            'module' => 'API',
            'method' => $method,
            'token_auth' => $this->_token,
            'idSite' => $this->_siteId,
            'period' => $this->_period,
            'format' => $this->_format,
            'language' => $this->_language,
            'filter_limit' => $this->_filter_limit
        ] + $params;

But it should not be the case.

Here, the idSite parameters are mixed together, and therefore the idSite that I want is never passed as an argument, even if I change the function getSiteSettings() to accept 1 arg.

thelfensdrfer commented 1 year ago

Hi,

yes, every method in the Matomo instance uses the same site ID. Otherwise every method has to include several parameters which otherwise can be set on an instance basis, e.g. the time period, format, language, etc. But you can always create more instances of the class:

$myFirstSite = new Matomo('http://stats.example.org', 'my_access_token', 1, Matomo::FORMAT_JSON);
$mySecondSite = new Matomo('http://stats.example.org', 'my_access_token', 1621, Matomo::FORMAT_JSON);

var_dump($mySecondSite->getSiteSettings());

Or, before calling the method, you change the site ID:

$onlyOneInstance = new Matomo('http://stats.example.org', 'my_access_token', 1, Matomo::FORMAT_JSON);
$onlyOneInstance = $onlyOneInstance->setSiteId(1621);

var_dump($onlyOneInstance->getSiteSettings());

$onlyOneInstance = $onlyOneInstance->setSiteId(1);
var_dump($onlyOneInstance->getUniqueVisitors());
Guilhem-Lalanne commented 1 year ago

Hi @thelfensdrfer ,

Thank you for your answer. Although I find it unintuitive, I can understand your choice of implementation.

I close the issue, thanks again for your time.

thelfensdrfer commented 1 year ago

No problem, but the alternative would be that every one of the ~300 methods would look like this:

public function getKeywordsFromSearchEngineId(
        int $idSubtable,
        string $segment = '',
        array $optional = [],
        // The following parameters are normally set in the constructor
        string $site,
        string $token,
        int $siteId = null,
        string $format = self::FORMAT_JSON,
        string $period = self::PERIOD_DAY,
        string $date = self::DATE_YESTERDAY,
        string $rangeStart = '',
        string $rangeEnd = null,
    ): mixed {

Which then would override the parameters in the constructor, if set. Or what would your alternative look like?