MozillaFoundation / foundation.mozilla.org

Mozilla Foundation website
https://foundation.mozilla.org
Mozilla Public License 2.0
388 stars 152 forks source link

ResearchDetailPage - add test for N+1 queries #10554

Open danielfmiranda opened 1 year ago

danielfmiranda commented 1 year ago

Description

While we have fixed the N+1 query issue in #10418, and have confirmed so by using the django-debug-toolbar's query tracking tool we have been having trouble getting the testing suite to accurately count the number of queries that are occurring when we call the get_research_authors() method. We should spend some more time to investigate why this is the case, and add a working test to the testing suite.

(Link to comment thread showing investigation)

This is the test we had been using while working on #10418:

    def test_get_research_authors_avoids_n1_queries(self) -> None:
        """
        Checking that the method does not suffer from an N+1 query issue.
        """
        author_profiles = []
        detail_page = detail_page_factory.ResearchDetailPageFactory(parent=self.library_page, research_authors=[])

        for _ in range(10):
            research_author_relation = relations_factory.ResearchAuthorRelationFactory(
                research_detail_page=detail_page
            )
            author_profiles.append(research_author_relation.author_profile)

        # Though there are 10 linked research authors, there should be minimal queries.
        with self.assertNumQueries(2):
            research_authors = detail_page.get_research_authors()

        self.assertEqual(len(research_authors), 10)
        self.assertCountEqual(author_profiles, research_authors)

┆Issue is synchronized with this Jira Task

data-sync-user commented 1 month ago

➤ Simon Acosta Torres commented:

Daniel Miranda Do you feel this is still something that the team can handle at some point this year or should we either remove or lower the priority to P3?

data-sync-user commented 1 month ago

➤ Daniel Miranda commented:

Hi Simon Acosta Torres ! This is something we should still do but I would vote that this is OK to move to a P3 priority as its not really affecting anything publicly.