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

Jump to bottom

Check if filter limit is -1 when querying log visits #17302

Merged
merged 4 commits into from Mar 9, 2021
Merged

Check if filter limit is -1 when querying log visits #17302

merged 4 commits into from Mar 9, 2021

Conversation

flamisz
Copy link
Contributor

@flamisz flamisz commented Mar 4, 2021 •
edited by sgiehl

Description:

fixes #17237

Review

  • Functional review done
  • Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
  • Security review done see checklist
  • Code review done
  • Tests were added if useful/possible
  • Reviewed for breaking changes
  • Developer changelog updated if needed
  • Documentation added if needed
  • Existing documentation updated if needed

@flamisz flamisz self-assigned this Mar 4, 2021
@flamisz flamisz added 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. labels Mar 4, 2021
@flamisz
Copy link
Contributor Author

flamisz commented Mar 4, 2021

I see some tests are failing now, I'll check those.

@flamisz flamisz marked this pull request as draft March 4, 2021 03:59
sgiehl
sgiehl requested changes Mar 4, 2021
View reviewed changes
plugins/Live/Model.php Outdated Show resolved Hide resolved
@sgiehl sgiehl added this to the 4.3.0 milestone Mar 4, 2021
@flamisz flamisz marked this pull request as ready for review March 7, 2021 21:12
@sgiehl
Copy link
Member

sgiehl commented Mar 8, 2021

The changes are looking good I think, but maybe we could add or adjust a test case that queries Live.getLastVisitsDetails with filter_limit=-1 to ensure it returns the correct values.

@flamisz flamisz removed the Needs Review PRs that need a code review label Mar 9, 2021
@sgiehl
Copy link
Member

sgiehl commented Mar 9, 2021

@flamisz Looks good now. Feel free to merge if you are already done with adding tests.

@flamisz flamisz merged commit a027154 into 4.x-dev Mar 9, 2021
@flamisz flamisz deleted the 17237-filter-limit branch March 9, 2021 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sgiehl sgiehl sgiehl approved these changes

Assignees

@flamisz flamisz

Labels
not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Milestone
4.3.0
Development

Successfully merging this pull request may close these issues.

Unwanted date filter automatically added to Live.getLastVisitsDetails
2 participants
@flamisz @sgiehl