mirror of
https://github.com/reddit-archive/reddit.git
synced 2026-04-27 03:00:12 -04:00
Comment sort preference: dual-write to migrate
We ran the numbers on how many people have set some sort of sorting preference (this includes *anything* using `VMenu`, since the field is serialized in the database), and it's much more than I anticipated. Per some discussion, we decided we should make a good attempt to migrate over users' sort preferences so as to not needlessly annoy them. While we could write a migration script that iterates through all users with this option set, that would take a long time and probably affect a lot of long-disused accounts. Instead we're opting to dual-write the new and old preferences whenever they're updated through the preferences page (the new way). Additionally, we're checking for inconsistencies between those two preferences on every view of a comments page, and setting the new preference to the value of the old preference if any difference is found. That allows us to ship quickly while still invisibly migrating the preference for active users - even those who are only viewing comment threads, not changing the sort. This is likely going to lead to an increase in queries as we start to deploy it, but it should be ok if we just take it slow. After we've reached some point at which the number of accounts migrating over is low enough we no longer care, we can rip out this commit.
This commit is contained in:
@@ -196,7 +196,7 @@ class FrontController(RedditController):
|
||||
comment=VCommentID('comment',
|
||||
docs={"comment": "(optional) ID36 of a comment"}),
|
||||
context=VInt('context', min=0, max=8),
|
||||
sort=VOneOf('sort', CommentSortMenu.visible_options()),
|
||||
sort=VTransitionaryMenu('controller'),
|
||||
limit=VInt('limit',
|
||||
docs={"limit": "(optional) an integer"}),
|
||||
depth=VInt('depth',
|
||||
|
||||
@@ -19,6 +19,8 @@
|
||||
# All portions of the code written by reddit are Copyright (c) 2006-2015 reddit
|
||||
# Inc. All Rights Reserved.
|
||||
###############################################################################
|
||||
from copy import copy
|
||||
|
||||
from pylons import g
|
||||
from r2.lib.menus import CommentSortMenu
|
||||
from r2.lib.validator.validator import (
|
||||
@@ -82,6 +84,14 @@ PREFS_VALIDATORS = dict(
|
||||
def set_prefs(user, prefs):
|
||||
for k, v in prefs.iteritems():
|
||||
setattr(user, k, v)
|
||||
if k == 'pref_default_comment_sort':
|
||||
# We have to do this copy-modify-assign shenanigans because if we
|
||||
# just assign directly into `c.user.sort_options`, `Thing` doesn't
|
||||
# know what happened and will wipe out our changes on save.
|
||||
sort_options = copy(user.sort_options)
|
||||
sort_options['front_sort'] = v
|
||||
user.sort_options = sort_options
|
||||
g.stats.simple_event('default_comment_sort.changed_in_prefs')
|
||||
|
||||
|
||||
def filter_prefs(prefs, user):
|
||||
|
||||
@@ -38,6 +38,7 @@ from r2.lib.souptest import SoupError, SoupUnsupportedEntityError
|
||||
from r2.lib.template_helpers import add_sr
|
||||
from r2.lib.jsonresponse import JQueryResponse, JsonResponse
|
||||
from r2.lib.log import log_text
|
||||
from r2.lib.menus import CommentSortMenu
|
||||
from r2.lib.permissions import ModeratorPermissionSet
|
||||
from r2.models import *
|
||||
from r2.models.promo import Location
|
||||
@@ -1760,7 +1761,6 @@ class VColor(Validator):
|
||||
|
||||
|
||||
class VMenu(Validator):
|
||||
|
||||
def __init__(self, param, menu_cls, remember = True, **kw):
|
||||
self.nav = menu_cls
|
||||
self.remember = remember
|
||||
@@ -1798,6 +1798,34 @@ class VMenu(Validator):
|
||||
}
|
||||
|
||||
|
||||
class VTransitionaryMenu(VMenu):
|
||||
"""A temporary version of VMenu helping to transition to a new preference."""
|
||||
def __init__(self, param):
|
||||
# Our logic down below only makes sense for comment sorts, so let's
|
||||
# hard-code that in as the menu.
|
||||
VMenu.__init__(self, param, CommentSortMenu, remember=False)
|
||||
|
||||
def run(self, sort, where):
|
||||
old_user_pref = c.user.sort_options.get('front_sort')
|
||||
new_user_pref = c.user.pref_default_comment_sort
|
||||
|
||||
if c.user_is_loggedin and old_user_pref != new_user_pref:
|
||||
# Even on view, if we catch an inconsistency between the
|
||||
# preferences, we want to update them to match. This allows us to
|
||||
# transition over to the new one without having to wait for people
|
||||
# to change their sort.
|
||||
c.user.pref_default_comment_sort = old_user_pref
|
||||
c.user._commit()
|
||||
|
||||
g.stats.simple_event('default_comment_sort.synchronized')
|
||||
|
||||
# Was a valid sort provided?
|
||||
if sort not in self.nav._options:
|
||||
sort = c.user.default_comment_sort # Includes fallbacks
|
||||
|
||||
return sort
|
||||
|
||||
|
||||
class VRatelimit(Validator):
|
||||
def __init__(self, rate_user = False, rate_ip = False,
|
||||
prefix = 'rate_', error = errors.RATELIMIT, *a, **kw):
|
||||
|
||||
@@ -82,7 +82,7 @@ class Account(Thing):
|
||||
pref_min_comment_score = -4,
|
||||
pref_num_comments = g.num_comments,
|
||||
pref_highlight_controversial=False,
|
||||
pref_default_comment_sort = 'confidence',
|
||||
pref_default_comment_sort = None,
|
||||
pref_lang = g.lang,
|
||||
pref_content_langs = (g.lang,),
|
||||
pref_over_18 = False,
|
||||
@@ -749,6 +749,17 @@ class Account(Thing):
|
||||
return (self.has_gold_subscription or
|
||||
(self.pref_creddit_autorenew and self.gold_creddits > 0))
|
||||
|
||||
@property
|
||||
def default_comment_sort(self):
|
||||
if self.pref_default_comment_sort:
|
||||
return self.pref_default_comment_sort
|
||||
|
||||
old_sort_pref = self.sort_options.get('front_sort')
|
||||
if old_sort_pref:
|
||||
return old_sort_pref
|
||||
|
||||
return 'confidence'
|
||||
|
||||
|
||||
class FakeAccount(Account):
|
||||
_nodb = True
|
||||
|
||||
@@ -60,7 +60,7 @@
|
||||
<% menu = CommentSortMenu() %>
|
||||
<select name="default_comment_sort">
|
||||
%for sort in menu.visible_options():
|
||||
<option ${'selected="selected"' if sort == c.user.pref_default_comment_sort else ""}
|
||||
<option ${'selected="selected"' if sort == c.user.default_comment_sort else ""}
|
||||
value="${sort}">
|
||||
${menu.make_title(sort)}
|
||||
</option>
|
||||
|
||||
Reference in New Issue
Block a user