Skip to content

Commit

Permalink
fix ready_for_l10n and replace all N+1 queries
Browse files Browse the repository at this point in the history
  • Loading branch information
@escattone
escattone committed Jan 10, 2025
1 parent b434bae commit 1df8b53
Show file tree
Hide file tree
Showing 2 changed files with 119 additions and 28 deletions.
90 changes: 68 additions & 22 deletions kitsune/dashboards/readouts.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

from django.conf import settings
from django.contrib.postgres.aggregates import StringAgg
from django.db.models import Count, Exists, F, Max, OuterRef, Q, Subquery
from django.db.models import Case, Count, Exists, F, Max, OuterRef, Q, Subquery, When
from django.db.models.functions import Coalesce
from django.template.loader import render_to_string
from django.utils.translation import gettext as _
Expand Down Expand Up @@ -135,20 +135,76 @@ def kb_overview_rows(user=None, mode=None, max=None, locale=None, product=None,
if mode is None:
mode = LAST_30_DAYS

docs = Document.objects.visible(
user, locale=settings.WIKI_DEFAULT_LANGUAGE, is_archived=False
).exclude(html__startswith=REDIRECT_HTML)
docs = (
Document.objects.visible(user, locale=settings.WIKI_DEFAULT_LANGUAGE, is_archived=False)
.exclude(html__startswith=REDIRECT_HTML)
.select_related("current_revision")
)

if product:
docs = docs.filter(products__in=[product])

if category:
docs = docs.filter(category__in=[category])

docs = docs.annotate(num_visits=get_visits_subquery(period=mode)).order_by(
F("num_visits").desc(nulls_last=True), "title"
docs = docs.annotate(
num_visits=get_visits_subquery(period=mode),
ready_for_l10n=Case(
When(
Q(latest_localizable_revision__isnull=False)
& ~Exists(
Revision.objects.filter(
document=OuterRef("pk"),
is_approved=True,
is_ready_for_localization=False,
significance__gt=TYPO_SIGNIFICANCE,
id__gt=F("document__latest_localizable_revision__id"),
)
),
then=True,
),
default=False,
),
unapproved_revision_comment=Subquery(
Revision.objects.filter(
document=OuterRef("pk"),
reviewed=None,
)
.filter(
Q(document__current_revision__isnull=True)
| Q(id__gt=F("document__current_revision__id"))
)
.order_by("created")[:1]
.values("comment")
),
)

if locale != settings.WIKI_DEFAULT_LANGUAGE:
transdoc_subquery = Document.objects.filter(
locale=locale,
is_archived=False,
parent=OuterRef("pk"),
current_revision__isnull=False,
)
docs = docs.annotate(
transdoc_exists=Exists(transdoc_subquery),
transdoc_current_revision_based_on_id=Subquery(
transdoc_subquery.values("current_revision__based_on__id")
),
).annotate(
transdoc_is_outdated=Exists(
Revision.objects.filter(
document=OuterRef("pk"),
is_approved=True,
is_ready_for_localization=True,
significance__gte=MEDIUM_SIGNIFICANCE,
id__gt=OuterRef("transdoc_current_revision_based_on_id"),
)
),
)

docs = docs.order_by(F("num_visits").desc(nulls_last=True), "title")

if max:
docs = docs[:max]

Expand All @@ -164,9 +220,7 @@ def kb_overview_rows(user=None, mode=None, max=None, locale=None, product=None,
),
"title": d.title,
"num_visits": d.num_visits,
"ready_for_l10n": d.revisions.filter(
is_approved=True, is_ready_for_localization=True
).exists(),
"ready_for_l10n": d.ready_for_l10n,
}

if d.current_revision:
Expand All @@ -178,23 +232,15 @@ def kb_overview_rows(user=None, mode=None, max=None, locale=None, product=None,
if "expiry_date" in data and data["expiry_date"]:
data["stale"] = data["expiry_date"] < datetime.now()

# Check L10N status
if d.current_revision:
unapproved_revs = d.revisions.filter(reviewed=None, id__gt=d.current_revision.id)[:1]
else:
unapproved_revs = d.revisions.all()

if unapproved_revs.count():
data["revision_comment"] = unapproved_revs[0].comment
else:
if d.unapproved_revision_comment is None:
data["latest_revision"] = True
else:
data["revision_comment"] = d.unapproved_revision_comment

# Get the translated doc
if locale != settings.WIKI_DEFAULT_LANGUAGE:
transdoc = d.translations.filter(locale=locale, is_archived=False).first()

if transdoc:
data["needs_update"] = transdoc.is_outdated()
if d.transdoc_exists:
data["needs_update"] = d.transdoc_is_outdated
else: # For en-US we show the needs_changes comment.
data["needs_update"] = d.needs_change
data["needs_update_comment"] = d.needs_change_comment
Expand Down
57 changes: 51 additions & 6 deletions kitsune/dashboards/tests/test_readouts.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,19 +83,64 @@ def test_unapproved_articles(self):
self.assertEqual(2, len(kb_overview_rows(user=user1)))

def test_ready_for_l10n(self):
d = DocumentFactory()
r = RevisionFactory(document=d)
d.current_revision = r
d.save()
d1 = DocumentFactory()
ApprovedRevisionFactory(document=d1, significance=MAJOR_SIGNIFICANCE)
d2 = DocumentFactory()
rev1_d2 = RevisionFactory(document=d2, significance=None)

WikiDocumentVisits.objects.create(document=d1, visits=20, period=LAST_30_DAYS)
WikiDocumentVisits.objects.create(document=d2, visits=10, period=LAST_30_DAYS)

data = kb_overview_rows(user=UserFactory(is_staff=True, is_superuser=True))
self.assertEqual(2, len(data))
self.assertEqual(False, data[0]["ready_for_l10n"])
self.assertEqual(False, data[1]["ready_for_l10n"])

ApprovedRevisionFactory(
document=d1, is_ready_for_localization=True, significance=MEDIUM_SIGNIFICANCE
)
rev1_d2.is_approved = True
rev1_d2.significance = MAJOR_SIGNIFICANCE
rev1_d2.save()
data = kb_overview_rows()
self.assertEqual(True, data[0]["ready_for_l10n"])
self.assertEqual(False, data[1]["ready_for_l10n"])

ApprovedRevisionFactory(document=d1, significance=TYPO_SIGNIFICANCE)
ApprovedRevisionFactory(
document=d2, is_ready_for_localization=True, significance=MEDIUM_SIGNIFICANCE
)
data = kb_overview_rows()
self.assertEqual(True, data[0]["ready_for_l10n"])
self.assertEqual(True, data[1]["ready_for_l10n"])

ApprovedRevisionFactory(document=d1, significance=MEDIUM_SIGNIFICANCE)
ApprovedRevisionFactory(document=d2, significance=TYPO_SIGNIFICANCE)
data = kb_overview_rows()
self.assertEqual(1, len(data))
self.assertEqual(False, data[0]["ready_for_l10n"])
self.assertEqual(True, data[1]["ready_for_l10n"])

ApprovedRevisionFactory(document=d, is_ready_for_localization=True)
ApprovedRevisionFactory(
document=d1, is_ready_for_localization=True, significance=MAJOR_SIGNIFICANCE
)
ApprovedRevisionFactory(document=d2, significance=MAJOR_SIGNIFICANCE)
data = kb_overview_rows()
self.assertEqual(True, data[0]["ready_for_l10n"])
self.assertEqual(False, data[1]["ready_for_l10n"])

ApprovedRevisionFactory(document=d1, significance=TYPO_SIGNIFICANCE)
ApprovedRevisionFactory(document=d2, significance=TYPO_SIGNIFICANCE)
data = kb_overview_rows()
self.assertEqual(True, data[0]["ready_for_l10n"])
self.assertEqual(False, data[1]["ready_for_l10n"])

ApprovedRevisionFactory(document=d1, significance=MEDIUM_SIGNIFICANCE)
ApprovedRevisionFactory(
document=d2, is_ready_for_localization=True, significance=MEDIUM_SIGNIFICANCE
)
data = kb_overview_rows()
self.assertEqual(False, data[0]["ready_for_l10n"])
self.assertEqual(True, data[1]["ready_for_l10n"])

def test_filter_by_category(self):
ApprovedRevisionFactory(document__category=CATEGORIES[1][0])
Expand Down

0 comments on commit 1df8b53

Please sign in to comment.