Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unwanted date filter automatically added to Live.getLastVisitsDetails #17237

Closed
davidjmann opened this issue Feb 19, 2021 · 1 comment · Fixed by #17302
Closed

Unwanted date filter automatically added to Live.getLastVisitsDetails #17237

davidjmann opened this issue Feb 19, 2021 · 1 comment · Fixed by #17302
Assignees
Labels
Regression Indicates a feature used to work in a certain way but it no longer does even though it should.
Milestone

Comments

@davidjmann
Copy link

I've been working on migrating a system from Piwik 3 to Matomo 4; as part of this, I've been investigating why an API call which worked on the old system isn't working on the new one.

In brief, our system is making the following call:
/index.php?module=API&method=Live.getLastVisitsDetails&idSite=1&token_auth=xxxxxx&filter_limit=-1&segment=visitId%3D%3D12345678

I.e. we're requesting the details of a specific visit from a specific site.

However, on the new system, this was returning zero results.

Having dug into the code, I can see what's triggering this. In brief, it's because we've set the filter_limit to -1.

// plugins/Live/API.php
    private function loadLastVisitsDetailsFromDatabase($idSite, $period, $date, $segment = false, $offset = 0, $limit = 100, $minTimestamp = false, $filterSortOrder = false, $visitorId = false)
    {
        $model = new Model();
        [$data, $hasMoreVisits] = $model->queryLogVisits($idSite, $period, $date, $segment, $offset, $limit, $visitorId, $minTimestamp, $filterSortOrder, true);
        return $this->makeVisitorTableFromArray($data, $hasMoreVisits);
    }

// plugins/Live/Model.php
    public function queryLogVisits($idSite, $period, $date, $segment, $offset, $limit, $visitorId, $minTimestamp, $filterSortOrder, $checkforMoreEntries = false)
    {
        // to check for more entries increase the limit by one, but cut off the last entry before returning the result
        if ($checkforMoreEntries) {
            $limit++;
        }

        // If no other filter, only look at the last 24 hours of stats
        if (empty($visitorId)
            && empty($limit)
            && empty($offset)
            && empty($period)
            && empty($date)
        ) {
            $period = 'day';
            $date = 'yesterdaySameTime';
        }

I.e, loadLastVisitsDetailsFromDatabase() is always hardcoding $checkforMoreEntries to true when it calls queryLogVisits(). And that means that queryLogVisits() increments $limit from -1 to 0, which then triggers the "last 24 hours" rule.

For now, and for this specific scenario, I can fix things by setting filter_limit to 1 (which will then be converted to 2 within queryLogVisits), since we're looking for a specific visit and therefore will only ever get one result back.

However, this feels like a logic issue; the documentation (https://developer.matomo.org/api-reference/reporting-api) still states that setting filter_limit to -1 is valid, and if the caller has requested an unlimited number of results, there shouldn't be any additional filters being applied!

It feels like the fix should be to check what value is set for filter_limit before setting $checkforMoreEntries.

@tsteur tsteur added the Regression Indicates a feature used to work in a certain way but it no longer does even though it should. label Feb 21, 2021
@tsteur tsteur added this to the 4.2.0 milestone Feb 21, 2021
@tsteur
Copy link
Member

tsteur commented Feb 21, 2021

Thanks for creating this issue and investigating the root cause @davidjmann This sounds like a regression so we will look into this soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Regression Indicates a feature used to work in a certain way but it no longer does even though it should.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants