@peterhashair opened this Pull Request on June 13th 2022 Contributor

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

@bx80 commented on June 13th 2022 Contributor

@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:

@peterhashair commented on June 13th 2022 Contributor

@bx80 yes, I add a flag in to the evolution, when the lastN is remember force the reload, otherwise, it won't reload.

@peterhashair commented on June 14th 2022 Contributor

@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.

@bx80 commented on June 15th 2022 Contributor

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:

This Pull Request was closed on June 15th 2022
Powered by GitHub Issue Mirror