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

Use Request::processRequest in UserId::isUsedInSite. #13875

Merged
merged 2 commits into from Dec 18, 2018

Conversation

diosmosis
Copy link
Member

No description provided.

@diosmosis diosmosis added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Dec 18, 2018
@diosmosis diosmosis added this to the 3.8.0 milestone Dec 18, 2018
@diosmosis diosmosis added the Needs Review PRs that need a code review label Dec 18, 2018
'columns' => 'nb_users',
'idSite' => $idSite,
'period' => $period,
'date' => $date,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does segment need to be set to false or this automatically happens because of $defaultRequest = []? Never really sure what happens if the system maybe picks it up somehow... Maybe doesn't hurt to mention segment => false? Eg the segment string might be read from $_SERVER['QUERY_STRING] here https://github.com/matomo-org/matomo/blob/3.8.0-b5/core/API/Request.php#L512-L518 because of https://github.com/matomo-org/matomo/blob/3.8.0-b5/core/API/Request.php#L592-L594 etc

Otherwise looks good to merge 👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Segments are handled very strangely in matomo.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes!!! 100% strangely handled.

@diosmosis diosmosis merged commit 0a2d0ea into 3.x-dev Dec 18, 2018
@diosmosis diosmosis deleted the user-id-api-process-request branch December 18, 2018 23:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants