@sgiehl opened this Pull Request on November 27th 2020 Member

Description:

Using one those values actually won't directly pass them to strtotime, as it might have imho unexpected results. last week passed to strtotime for example always returns the monday of the previous week. Instead I guess it would be expected to result in the day 7 days ago. Similar applies to last month and year.

For last month there are still some edge cases on the last day of a month when the previous month didn't have that much days.

E.g. last month on 31th Dezember would return 1st Dezember

fixes #13818

Review

  • [ ] Functional review done
  • [x] Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
  • [x] Security review done see checklist
  • [x] Code review done
  • [x] Tests were added if useful/possible
  • [x] Reviewed for breaking changes
  • [ ] Developer changelog updated if needed
  • [ ] Documentation added if needed
  • [ ] Existing documentation updated if needed
@diosmosis commented on December 1st 2020 Member

@mattab / @tsteur would we need to update documentation anywhere for this?

@diosmosis commented on December 1st 2020 Member

Should these be usable in the API? If so, would it be good to have a system test or two for them?

@diosmosis commented on December 1st 2020 Member

Tested locally, works well, tested comparison + export links. Left one other test related comment, otherwise lgtm

@tsteur commented on December 1st 2020 Member

Yes be great to document here if it's supported in the API https://developer.matomo.org/api-reference/reporting-api

@sgiehl commented on December 1st 2020 Member

@diosmosis it actually wasn't possible to use those as api params. at least the results were incorrect.
I've added a new commit to make that possible. It also fixes the usage when period=range.
Also added some system tests.

This Pull Request was closed on December 10th 2020
Powered by GitHub Issue Mirror