Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

[bug 674835] Make dash readouts consider all approved revs between a …

…translation and the latest ready-for-l10n rev when computing max significance.

Also, factor up some repetition of convenience methods in test_readouts.
  • Loading branch information...
commit e0f8e9a852bc6e14f62326281da385dc0b78cfed 1 parent 417c216
@erikrose erikrose authored
Showing with 102 additions and 33 deletions.
  1. +20 −16 apps/dashboards/readouts.py
  2. +82 −17 apps/dashboards/tests/test_readouts.py
View
36 apps/dashboards/readouts.py
@@ -264,15 +264,16 @@ def _most_visited_query_and_params(self, max, extra_where=''):
return (
'SELECT engdoc.slug, engdoc.title, transdoc.slug, '
'transdoc.title, dashboards_wikidocumentvisits.visits, '
- # The most significant ready-for-localization, approved change to
- # the English article since the English revision the current
- # translated revision is based on:
+ # The most significant approved change to the English article
+ # between {the English revision the current translated revision is
+ # based on} and {the latest ready-for-localization revision}:
'(SELECT MAX(engrev.significance) '
'FROM wiki_revision engrev, wiki_revision transrev '
- 'WHERE engrev.is_ready_for_localization '
+ 'WHERE engrev.is_approved '
'AND transrev.id=transdoc.current_revision_id '
'AND engrev.document_id=transdoc.parent_id '
'AND engrev.id>transrev.based_on_id '
+ 'AND engrev.id<=engdoc.latest_localizable_revision_id'
'), '
# Whether there are any unreviewed revs of the translation made
# since the current one:
@@ -412,6 +413,7 @@ def _query_and_params(self, max):
return ('SELECT transdoc.slug, transdoc.title, engrev.reviewed, '
'dashboards_wikidocumentvisits.visits '
'FROM wiki_document transdoc '
+ 'INNER JOIN wiki_document engdoc ON transdoc.parent_id=engdoc.id '
'INNER JOIN wiki_revision engrev ON engrev.id='
# The oldest English rev to have an approved, ready-for-
# localization level-30 change since the translated doc had an
@@ -430,24 +432,26 @@ def _query_and_params(self, max):
'AND wiki_revision.significance>=%s '
'AND %s='
# Completely filter out outer selections where 30 is not the
- # max signif of english revisions since trans was last
+ # max signif of approved English revisions since trans was last
# approved. Other maxes will be shown by other readouts.
# Optimize: try "30 IN" if MySQL's statistics gatherer is
# stupid/nonexistent; the inner query should be able to bail
# out early. [Ed: No effect on EXPLAIN on admittedly light test
# corpus.]
'(SELECT MAX(engsince.significance) '
- 'FROM wiki_revision engsince '
- 'WHERE engsince.document_id=transdoc.parent_id '
- # Assumes that any ready (and therefor approved) revision
- # became the current revision at some point: we don't let the
- # user go back and approve revisions older than the latest
- # approved one.
- 'AND engsince.is_ready_for_localization '
- 'AND engsince.id>'
- # The English revision the current translation's based on:
- '(SELECT based_on_id FROM wiki_revision basedonrev '
- 'WHERE basedonrev.id=transdoc.current_revision_id) '
+ 'FROM wiki_revision engsince '
+ 'WHERE engsince.document_id=transdoc.parent_id '
+ # Assumes that any approved revision became the current
+ # revision at some point: we don't let the user go back and
+ # approve revisions older than the latest approved one.
+ 'AND engsince.is_approved '
+ # Consider revisions between the one the last translation
+ # was based on and the latest ready-for-l10n one.
+ 'AND engsince.id>'
+ # The English revision the current translation's based on:
+ '(SELECT based_on_id FROM wiki_revision basedonrev '
+ 'WHERE basedonrev.id=transdoc.current_revision_id) '
+ 'AND engsince.id<=engdoc.latest_localizable_revision_id'
')'
') '
# Join up the visits table for stats:
View
99 apps/dashboards/tests/test_readouts.py
@@ -2,7 +2,7 @@
from nose.tools import eq_
-from dashboards.readouts import (UnreviewedReadout,
+from dashboards.readouts import (UnreviewedReadout, OutOfDateReadout,
TemplateTranslationsReadout,
MostVisitedTranslationsReadout)
from sumo.tests import TestCase
@@ -14,18 +14,27 @@ class MockRequest(object):
locale = 'de' # Same locale as translated_revision uses by default
-class UnreviewedChangesTests(TestCase):
+class ReadoutTestCase(TestCase):
+ """Test case for one readout. Provides some convenience methods."""
+
+ def row(self):
+ """Return first row shown by the readout this class tests."""
+ return self.readout(MockRequest()).rows()[0]
+
+ def titles(self):
+ """Return the titles shown by the Unreviewed Changes readout."""
+ return [row['title'] for row in
+ self.readout(MockRequest()).rows()]
+
+
+class UnreviewedChangesTests(ReadoutTestCase):
"""Tests for the Unreviewed Changes readout
I'm not trying to cover every slice of the Venn diagram--just the tricky
bits.
"""
- @staticmethod
- def titles():
- """Return the titles shown by the Unreviewed Changes readout."""
- return [row['title'] for row in
- UnreviewedReadout(MockRequest()).rows()]
+ readout = UnreviewedReadout
def test_unrevieweds_after_current(self):
"""Show the unreviewed revisions with later creation dates than the
@@ -48,17 +57,14 @@ def test_rejected_newer_than_current(self):
assert rejected.document.title not in self.titles()
-class MostVisitedTranslationsTests(TestCase):
+class MostVisitedTranslationsTests(ReadoutTestCase):
"""Tests for the Most Visited Translations readout
This is an especially tricky readout, since it effectively implements a
superset of all other readouts' status discriminators.
"""
- @staticmethod
- def row():
- """Return first row shown by the Most Visited Translations readout."""
- return MostVisitedTranslationsReadout(MockRequest()).rows()[0]
+ readout = MostVisitedTranslationsReadout
def test_unreviewed(self):
"""Assert articles in need of review are labeled as such."""
@@ -118,14 +124,73 @@ def test_spam(self):
r = revision(is_approved=False, save=True)
eq_([], MostVisitedTranslationsReadout(MockRequest()).rows())
+ def test_consider_max_significance(self):
+ """When determining how significantly an article has changed since
+ translation, use the max significance of the approved revisions, not
+ just that of the latest ready-to-localize one."""
+ translation = translated_revision(is_approved=True, save=True)
+ revision(document=translation.document.parent,
+ is_approved=True,
+ is_ready_for_localization=False, # should still count
+ significance=MAJOR_SIGNIFICANCE,
+ save=True)
+ revision(document=translation.document.parent,
+ is_approved=True,
+ is_ready_for_localization=True,
+ significance=MEDIUM_SIGNIFICANCE,
+ save=True)
+ row = self.row()
+ eq_(row['title'], translation.document.title)
+ eq_(unicode(row['status']), 'Immediate Update Needed')
+
+ def test_consider_only_approved_significances(self):
+ """Consider only approved significances when computing the max."""
+ translation = translated_revision(is_approved=True, save=True)
+ revision(document=translation.document.parent,
+ is_approved=False, # shouldn't count
+ is_ready_for_localization=False,
+ significance=MAJOR_SIGNIFICANCE,
+ save=True)
+ revision(document=translation.document.parent,
+ is_approved=True,
+ is_ready_for_localization=True,
+ significance=MEDIUM_SIGNIFICANCE,
+ save=True)
+ row = self.row()
+ eq_(row['title'], translation.document.title)
+ # This should show as medium significance, because the revision with
+ # major significance is unapproved:
+ eq_(unicode(row['status']), 'Update Needed')
+
+
+class OutOfDateTests(ReadoutTestCase):
+ """Tests for OutOfDateReadout and, by dint of factoring,
+ NeedingUpdatesReadout."""
+
+ readout = OutOfDateReadout
+
+ def test_consider_max_significance(self):
+ """When determining how significantly an article has changed since
+ translation, use the max significance of the approved revisions, not
+ just that of the latest ready-to-localize one."""
+ translation = translated_revision(is_approved=True, save=True)
+ revision(document=translation.document.parent,
+ is_approved=True,
+ is_ready_for_localization=False, # should still count
+ significance=MAJOR_SIGNIFICANCE,
+ save=True)
+ revision(document=translation.document.parent,
+ is_approved=True,
+ is_ready_for_localization=True,
+ significance=MEDIUM_SIGNIFICANCE,
+ save=True)
+ eq_([translation.document.title], self.titles())
+
-class TemplateTranslationsTests(TestCase):
+class TemplateTranslationsTests(ReadoutTestCase):
"""Tests for the Template Translations readout"""
- @staticmethod
- def row():
- """Return first row shown by the Template Translations readout."""
- return TemplateTranslationsReadout(MockRequest()).rows()[0]
+ readout = TemplateTranslationsReadout
def test_not_template(self):
"""Documents that are not templates shouldn't show up in the list."""
Please sign in to comment.
Something went wrong with that request. Please try again.