@Findus23 opened this Pull Request on March 28th 2021 Member

It seems like no one (apart from @Chardonneaur) used that API in years as it always returns

{
  "result": "error",
  "message": "The period 'never' is not supported. Try any of the following instead: day, week, month, year, range"
}

even if the period is definitely not set to 'never'. It seems like the wrong variable is used here, as $report['period_param'] = $period; is set above.

Review

  • [ ] Functional review done
  • [ ] Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
  • [ ] 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
@sgiehl commented on March 29th 2021 Member

Wondering why there actually is no test that failed because of that.

@Findus23 commented on March 29th 2021 Member
@tsteur commented on March 29th 2021 Member

We're using this API in our app itself eg in the tasks to send every report etc. Also when clicking in the UI on "send report now" so would have assumed this worked before? Just wanting to make sure we won't regress anything

@diosmosis commented on March 29th 2021 Member

@tsteur the task only supplies the report ID, not an override period. I'm writing a quick test for it now.

@diosmosis commented on March 29th 2021 Member

Actually just seeing the angular controller uses the API directly w/ the period param...

@diosmosis commented on March 29th 2021 Member

I see, it happens if the report's email schedule is set to a value that doesn't map to a matomo period type. (We use the correct value when generating the report, but pass the wrong value to the sendReport event; but I don't think it's used much there.)

This Pull Request was closed on March 30th 2021
Powered by GitHub Issue Mirror