Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Reduce the number of queries used to build ministers index links #9418

Merged
merged 1 commit into from
Sep 6, 2024

Conversation

ryanb-gds
Copy link
Contributor

@ryanb-gds ryanb-gds commented Sep 5, 2024

The ministers index presenter was not leveraging eager loading and consequently was running huge numbers of queries. This has the potential to cause performance issues in the Sidekiq queues, as indeed was recently the case with the embassies index presenter.

We also remove the unused govspeak helper from the presenter.

I've tested this in integration and it produces exactly the same list of links as the current presenter.

@ryanb-gds ryanb-gds force-pushed the reduce-ministers-index-queries branch from 769cdd3 to 27f7a96 Compare September 5, 2024 08:00
The ministers index presenter was not leveraging eager loading and consequently was running huge numbers of queries. This has the potential to cause performance issues in the Sidekiq queues, as indeed was recently the case with the embassies index presenter.

We also move the govspeak helper from the main presenter to the reshuffle presenter because the main presenter doesn't use any of the methods it provides.
@ryanb-gds ryanb-gds force-pushed the reduce-ministers-index-queries branch from 27f7a96 to 615905c Compare September 5, 2024 08:41
Copy link
Contributor

@dnkrj dnkrj left a comment

Choose a reason for hiding this comment

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

Any way that we could a regression test for this?

@ryanb-gds
Copy link
Contributor Author

Any way that we could a regression test for this?

There's this existing one: https://github.com/alphagov/whitehall/blob/main/test/unit/app/presenters/publishing_api/ministers_index_presenter_test.rb

@ryanb-gds ryanb-gds merged commit 8bc504b into main Sep 6, 2024
19 checks passed
@ryanb-gds ryanb-gds deleted the reduce-ministers-index-queries branch September 6, 2024 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants