Skip to content
New issue

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

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

Already on GitHub? # to your account

before trying to invalidate parent period, make sure period is enabled in INI config #17874

Merged
merged 3 commits into from
Aug 12, 2021

Conversation

diosmosis
Copy link
Member

@diosmosis diosmosis commented Aug 10, 2021

Description:

Fixes #17872

This has the consequence where if a week period is disabled, but the month period is enabled, the month will not archive. Which I think is expected since the month period depends on the week.

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

@diosmosis diosmosis added not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Needs Review PRs that need a code review labels Aug 10, 2021
@diosmosis diosmosis added this to the 4.5.0 milestone Aug 10, 2021
@sgiehl
Copy link
Member

sgiehl commented Aug 11, 2021

This has the consequence where if a week period is disabled, but the month period is enabled, the month will not archive. Which I think is expected since the month period depends on the week.

Not sure if that's expected by the user. In theory we could also calculate the month archives with the daily ones. Should we maybe add a hint in the config, that disabling a period might break the archiving for bigger periods?

@diosmosis diosmosis merged commit 429e7bc into 4.x-dev Aug 12, 2021
@diosmosis diosmosis deleted the 17872-fix branch August 12, 2021 16:41
# for free to join this conversation on GitHub. Already have an account? # 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.

If the period is not enabled in config "The period is not supported" message is shown in cron archiving
2 participants