@peterhashair
Description:
Fixes: #18781
reload table if the default date changes. that may cause performance issue
Please include a description of this change and which issue it fixes. If no issue exists yet please include context and what problem it solves.
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
- [ ] Wording review done
- [ ] 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
@bx80
@peterhashair I think the desired behavior should be to use the configured defaultLastN period unless there is a remembered lastN from a previous selection.
It seems this is almost happening except that the lastN dropdown value is initialized as 90 days but the data table is loading data for 30 days. I think the initial data table load should match the value of the lastN dropdown (90 days in this case).
Forcing a reload of the data table when it already exists is going probably to be bad for performance as this will apply to every visualization in every circumstance so big data tables may be reloaded multiple times when they don't need to be.
It might be worth checking where both the evolution lastN dropdown and data table load are getting their initial values. Whatever logic is setting the lastN dropdown to 90days should also be instructing the data table initial load to be 90days too. It would be better to get the data table to load the correct lastN first time rather than having it load the wrong values and then do a second load for the correct values. Hope that makes some sense! :slightly_smiling_face:
@bx80 yes, I add a flag in to the evolution, when the lastN is remember force the reload, otherwise, it won't reload.
@bx80 fixed it, found how to load the data, and put it onto the first API loads. but it seems like the PHPCS started to fail on all my PRs.
This is works well for me, nice to catch the cached parameters and get the table loading the correct data first time :+1:
Other than the array key validation tweak, it should be good to go :slightly_smiling_face: