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

[statistics] Recruitment graph optimization #9196

Merged
merged 4 commits into from
Apr 16, 2024

Conversation

driusan
Copy link
Collaborator

@driusan driusan commented Apr 5, 2024

The recruitment graphs on the dashboard provided by the statistics module are not efficient. They run one SQL query per month per year per site for each graph that is displayed. This is highly inefficient for large projects.

This replaces the single queries in the inner loops with a single group-by query which it processes into the same format.

The result should be significantly faster load times on the dashboard when the statistics module is loaded.

Testing instructions (if applicable)

  1. Load the LORIS dashboard with and without this change. The load time should be significantly faster for on-going projects with a lot of sites or spread over a long amount of time. The results returned should be identical.

The recruitment graphs on the dashboard provided by the
statistics module are not efficient. They run one SQL query
per month per year per site for each graph that is displayed.
This is highly inefficient for large projects.

This replaces the single queries in the inner loops with a
single group-by query which it processes into the same format.

The result should be significantly faster load times on the dashboard
when the statistics module is loaded.
@CamilleBeau CamilleBeau self-requested a review April 11, 2024 14:09
@CamilleBeau CamilleBeau self-assigned this Apr 11, 2024
Copy link
Contributor

@CamilleBeau CamilleBeau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passing manual tests, just a couple small comments on furthering optimization / reducing code clutter

modules/statistics/php/charts.class.inc Show resolved Hide resolved
modules/statistics/php/widgets.class.inc Outdated Show resolved Hide resolved
@CamilleBeau CamilleBeau added the Passed Manual Tests PR has undergone proper testing by at least one peer label Apr 11, 2024
@driusan
Copy link
Collaborator Author

driusan commented Apr 16, 2024

@CamilleBeau what's the state of this? It still says changes requested but I think they're all addressed

@CamilleBeau
Copy link
Contributor

@driusan Looks good!

@driusan driusan merged commit 65ad4b5 into aces:main Apr 16, 2024
28 checks passed
@ridz1208 ridz1208 added this to the 26.0.0 milestone Jun 6, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Passed Manual Tests PR has undergone proper testing by at least one peer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants