This PR contains the following changes which allow defining & handling custom period types.
Left 2 comments, otherwise looks good 👍
@tsteur made some changes including using
Plugin\Manager::findComponents instead of DI for the period factory. I think it deserves another review.
Looking at the unit test failures, I'm not sure if it's a good idea to make
Period\Factory::build dependent on plugins for every period type. For example, the AddSegmentByLabelInUTC filter uses it, and will thus become dependent on plugins being loaded to work (which breaks unit tests that use it).
I think ideally, the build function shouldn't be called as much as it is. Since it only creates a period instance from query parameters, I think it should ideally only be called at the beginning of a request. This would be a very large change to make, though.
I see three options to solve this (please suggest others if I'm missing something):
Period\Factory::buildw/ a test to ensure core knows if the custom period factory code breaks. This way plugins are only required to be loaded if a custom period is requested.
Good points. I'd probably go back to 2) like you had it before (core periods in core and not in plugin).
Didn't think of tests. If we decided to keep it in CoreHome, would need to make them integration tests.
No worries @tsteur, will probably be a bit before it gets merged anyway :)