launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/bug-770576 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #770576 in Launchpad itself: "pageid: scope not working for RootObject:+login"
  https://bugs.launchpad.net/launchpad/+bug/770576

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-770576/+merge/59155

StormFeatureRuleSource.getAllRulesAsTuples orders by (flag, scope, -priority). This means that flags with the 'default' scope will take precedence over just about anything else (eg. 'team:', 'pageid:', etc.). We want to sort by -priority first, only using scope as a tiebreaker.
-- 
https://code.launchpad.net/~wgrant/launchpad/bug-770576/+merge/59155
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bug-770576 into lp:launchpad.

=== modified file 'lib/lp/services/features/rulesource.py'
--- lib/lp/services/features/rulesource.py	2011-03-23 16:28:51 +0000
+++ lib/lp/services/features/rulesource.py	2011-04-27 02:16:28 +0000
@@ -130,8 +130,8 @@
                 .find(FeatureFlag)
                 .order_by(
                     FeatureFlag.flag,
-                    FeatureFlag.scope,
-                    Desc(FeatureFlag.priority)))
+                    Desc(FeatureFlag.priority),
+                    FeatureFlag.scope))
         for r in rs:
             yield Rule(str(r.flag), str(r.scope), r.priority, r.value)
 

=== modified file 'lib/lp/services/features/tests/test_flags.py'
--- lib/lp/services/features/tests/test_flags.py	2011-03-29 00:11:57 +0000
+++ lib/lp/services/features/tests/test_flags.py	2011-04-27 02:16:28 +0000
@@ -165,6 +165,7 @@
 
 test_rules_list = [
     (notification_name, 'beta_user', 100, notification_value),
+    ('ui.icing', 'normal_user', 500, u'5.0'),
     ('ui.icing', 'beta_user', 300, u'4.0'),
     ('ui.icing', 'default', 100, u'3.0'),
     ]
@@ -187,6 +188,7 @@
         self.assertEquals(
             """\
 %s\tbeta_user\t100\t%s
+ui.icing\tnormal_user\t500\t5.0
 ui.icing\tbeta_user\t300\t4.0
 ui.icing\tdefault\t100\t3.0
 """ % (notification_name, notification_value),