github/reddit
1
1
Fork 0
mirror of https://github.com/reddit-archive/reddit.git synced 2026-01-25 06:48:01 -05:00
Code Issues Packages Projects Releases Wiki Activity

Convert SimpleGetMenu to SimplePostMenu.

Browse Source 
Fixes issue 118.

The original approach to menus/navbuttons would commit changes to user
preferences as a side effect of validation during a GET request. The new
approach now is to add a POST handler with the same exact validation that
just trivially redirects to the GET url. NavButton links are now hidden
form submits, and the VMenu validator will now only commit preference
changes on POST requests.
This commit is contained in:
Logan Hanks
2011-07-07 16:30:38 -07:00
parent 826f004ef8
commit f4b7dc641f
10 changed files with 107 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
r2/r2/controllers/front.py
View File

@@ -162,6 +162,17 @@ class FrontController(RedditController):
return old_visits
@validate(article = VLink('article'),
comment = VCommentID('comment'),
context = VInt('context', min = 0, max = 8),
sort = VMenu('controller', CommentSortMenu),
limit = VInt('limit'),
depth = VInt('depth'))
def POST_comments(self, article, comment, context, sort, limit, depth):
# VMenu validator will save the value of sort before we reach this
# point. Now just redirect to GET mode.
return self.redirect(request.fullpath + query_string(dict(sort=sort)))
@validate(article = VLink('article'),
comment = VCommentID('comment'),
context = VInt('context', min = 0, max = 8),

15
r2/r2/controllers/listingcontroller.py
View File

@@ -379,6 +379,12 @@ class NewController(ListingController):
else:
return c.site.get_links('new', 'all')
@validate(sort = VMenu('controller', NewMenu))
def POST_listing(self, sort, **env):
# VMenu validator will save the value of sort before we reach this
# point. Now just redirect to GET mode.
return self.redirect(request.fullpath + query_string(dict(sort=sort)))
@validate(sort = VMenu('controller', NewMenu))
def GET_listing(self, sort, **env):
self.sort = sort
@@ -405,6 +411,15 @@ class BrowseController(ListingController):
def query(self):
return c.site.get_links(self.sort, self.time)
# TODO: this is a hack with sort.
@validate(sort = VOneOf('sort', ('top', 'controversial')),
time = VMenu('where', ControversyTimeMenu))
def POST_listing(self, sort, time, **env):
# VMenu validator will save the value of time before we reach this
# point. Now just redirect to GET mode.
return self.redirect(
request.fullpath + query_string(dict(sort=sort, time=time)))
# TODO: this is a hack with sort.
@validate(sort = VOneOf('sort', ('top', 'controversial')),
time = VMenu('where', ControversyTimeMenu))

9
r2/r2/controllers/validator/validator.py
View File

@@ -1018,12 +1018,12 @@ class VMenu(Validator):
def __init__(self, param, menu_cls, remember = True, **kw):
self.nav = menu_cls
self.remember = remember
param = (menu_cls.get_param, param)
param = (menu_cls.name, param)
Validator.__init__(self, param, **kw)
def run(self, sort, where):
if self.remember:
pref = "%s_%s" % (where, self.nav.get_param)
pref = "%s_%s" % (where, self.nav.name)
user_prefs = copy(c.user.sort_options) if c.user else {}
user_pref = user_prefs.get(pref)
@@ -1035,8 +1035,9 @@ class VMenu(Validator):
if sort not in self.nav.options:
sort = self.nav.default
# commit the sort if changed
if self.remember and c.user_is_loggedin and sort != user_pref:
# commit the sort if changed and if this is a POST request
if (self.remember and c.user_is_loggedin and sort != user_pref
and request.method.upper() == 'POST'):
user_prefs[pref] = sort
c.user.sort_options = user_prefs
user = c.user

49
r2/r2/lib/menus.py
View File

@@ -195,6 +195,8 @@ class NavMenu(Styled):
a dropdown from a flatlist, while the optional _class, and _id attributes
can be used to set individualized CSS."""
use_post = True
def __init__(self, options, default = None, title = '', type = "dropdown",
base_path = '', separator = '|', **kw):
self.options = options
@@ -270,8 +272,11 @@ class NavButton(Styled):
p = {}
base_path = ("%s/%s/" % (base_path, self.dest)).replace('//', '/')
self.action_params = p
self.bare_path = _force_unicode(base_path.replace('//', '/')).lower()
self.bare_path = self.bare_path.rstrip('/')
self.base_path = base_path
# append the query string
base_path += query_string(p)
@@ -349,8 +354,6 @@ class NamedButton(NavButton):
except KeyError:
return NavButton.selected_title(self)
class JsButton(NavButton):
"""A button which fires a JS event and thus has no path and cannot
be in the 'selected' state"""
@@ -369,24 +372,26 @@ class PageNameNav(Styled):
subreddit name, the page name, etc.)"""
pass
class SimpleGetMenu(NavMenu):
class SimplePostMenu(NavMenu):
"""Parent class of menus used for sorting and time sensitivity of
results. More specifically, defines a type of menu which changes
the url by adding a GET parameter with name 'get_param' and which
defaults to 'default' (both of which are class-level parameters).
results. Defines a type of menu that uses hidden forms to POST the user's
selection to a handler that may commit the user's choice as a preference
change before redirecting to a URL that also includes the user's choice.
If other user's load this URL, they won't affect their own preferences, but
the given choice will apply for that page load.
The value of the GET parameter must be one of the entries in
The value of the POST/GET parameter must be one of the entries in
'cls.options'. This parameter is also used to construct the list
of NavButtons contained in this Menu instance. The goal here is
to have a menu object which 'out of the box' is self validating."""
options = []
get_param = ''
name = ''
title = ''
default = None
type = 'lightdrop'
def __init__(self, **kw):
buttons = [NavButton(self.make_title(n), n, opt = self.get_param)
buttons = [NavButton(self.make_title(n), n, opt=self.name, style='post')
for n in self.options]
kw['default'] = kw.get('default', self.default)
kw['base_path'] = kw.get('base_path') or request.path
@@ -400,15 +405,15 @@ class SimpleGetMenu(NavMenu):
"""Converts the opt into a DB-esque operator used for sorting results"""
return None
class SortMenu(SimpleGetMenu):
class SortMenu(SimplePostMenu):
"""The default sort menu."""
get_param = 'sort'
name = 'sort'
default = 'hot'
options = ('hot', 'new', 'top', 'old', 'controversial')
def __init__(self, **kw):
kw['title'] = _("sorted by")
SimpleGetMenu.__init__(self, **kw)
SimplePostMenu.__init__(self, **kw)
@classmethod
def operator(self, sort):
@@ -449,15 +454,15 @@ class RecSortMenu(SortMenu):
default = 'new'
options = ('hot', 'new', 'top', 'controversial', 'relevance')
class NewMenu(SimpleGetMenu):
get_param = 'sort'
class NewMenu(SimplePostMenu):
name = 'sort'
default = 'rising'
options = ('new', 'rising')
type = 'flatlist'
def __init__(self, **kw):
kw['title'] = ""
SimpleGetMenu.__init__(self, **kw)
SimplePostMenu.__init__(self, **kw)
@classmethod
def operator(self, sort):
@@ -465,29 +470,29 @@ class NewMenu(SimpleGetMenu):
return operators.desc('_date')
class KindMenu(SimpleGetMenu):
get_param = 'kind'
class KindMenu(SimplePostMenu):
name = 'kind'
default = 'all'
options = ('links', 'comments', 'messages', 'all')
def __init__(self, **kw):
kw['title'] = _("kind")
SimpleGetMenu.__init__(self, **kw)
SimplePostMenu.__init__(self, **kw)
def make_title(self, attr):
if attr == "all":
return _("all")
return menu[attr]
class TimeMenu(SimpleGetMenu):
class TimeMenu(SimplePostMenu):
"""Menu for setting the time interval of the listing (from 'hour' to 'all')"""
get_param = 't'
name = 't'
default = 'all'
options = ('hour', 'day', 'week', 'month', 'year', 'all')
def __init__(self, **kw):
kw['title'] = _("links from")
SimpleGetMenu.__init__(self, **kw)
SimplePostMenu.__init__(self, **kw)
@classmethod
def operator(self, time):
@@ -500,6 +505,8 @@ class ControversyTimeMenu(TimeMenu):
default = 'day'
class SubredditMenu(NavMenu):
use_post = False
def find_selected(self):
"""Always return False so the title is always displayed"""
return None

4
r2/r2/public/static/css/compact.css
View File

@@ -422,7 +422,7 @@ body[orient="landscape"] > #topbar > h1 {
text-overflow: ellipsis;
overflow: hidden; }
.subtoolbar > ul > li > a {
.subtoolbar > ul > li a {
color: #4c566c;
font-weight: bold;
text-decoration: none;
@@ -433,7 +433,7 @@ body[orient="landscape"] > #topbar > h1 {
text-overflow: ellipsis;
overflow: hidden; }
.subtoolbar > ul > li.selected > a {
.subtoolbar > ul > li.selected a {
background-color: #7f7f7f;
background: -moz-linear-gradient(-90deg, #dddddd, #aaaaaa);
background: -webkit-gradient(linear, 0% 0%, 0% 100%, from(#dddddd), to(#aaaaaa));

4
r2/r2/public/static/css/compact.scss
View File

@@ -395,7 +395,7 @@ body[orient="landscape"] > #topbar > h1 {
text-overflow: ellipsis;
overflow: hidden;
}
.subtoolbar > ul > li > a {
.subtoolbar > ul > li a {
color: rgb(76, 86, 108);
font-weight: bold;
text-decoration: none;
@@ -405,7 +405,7 @@ body[orient="landscape"] > #topbar > h1 {
text-overflow: ellipsis;
overflow: hidden;
}
.subtoolbar > ul > li.selected > a {
.subtoolbar > ul > li.selected a {
@include vertical_gradient(#ddd, #aaa);
/* no color-stop outside of webkit */
background: -webkit-gradient(linear, 0% 0%, 0% 100%, from(#ddd), color-stop(.4,#ddd), to(#aaa));

8
r2/r2/templates/navbutton.compact
View File

@@ -20,7 +20,7 @@
## CondeNet, Inc. All Rights Reserved.
################################################################################
<%namespace file="utils.html" import="plain_link" />
<%namespace file="utils.html" import="plain_link, post_link" />
<%def name="plain()">
${plain_link(thing.title,
@@ -36,4 +36,8 @@
onclick = thing.onclick)}
</%def>
<%def name="post()">
${post_link(thing.title, thing.base_path, thing.path, thing.action_params,
_sr_path=sr_path, nocname=nocname, target=thing.target,
_class=thing.css_class, _id=thing._id)}
</%def>

9
r2/r2/templates/navbutton.html
View File

@@ -20,7 +20,7 @@
## CondeNet, Inc. All Rights Reserved.
################################################################################
<%namespace file="utils.html" import="plain_link" />
<%namespace file="utils.html" import="plain_link, post_link" />
<%def name="plain()">
${plain_link(thing.selected_title() if thing.selected else thing.title,
@@ -36,4 +36,9 @@
onclick = thing.onclick)}
</%def>
<%def name="post()">
${post_link(thing.selected_title() if thing.selected else thing.title,
thing.base_path, thing.path, thing.action_params,
_sr_path=thing.sr_path, nocname=thing.nocname,
target=thing.target, _class=thing.css_class, _id=thing._id)}
</%def>

12
r2/r2/templates/navmenu.html
View File

@@ -20,7 +20,7 @@
## CondeNet, Inc. All Rights Reserved.
################################################################################
<%namespace file="utils.html" import="plain_link, text_with_links, img_link, separator"/>
<%namespace file="utils.html" import="plain_link, post_link, text_with_links, img_link, separator"/>
<%def name="dropdown()">
## caching comment:
@@ -47,8 +47,14 @@
<div class="drop-choices ${css_class}">
%for option in thing:
%if option != thing.selected:
${plain_link(option.title, option.path, _sr_path = option.sr_path,
_class = "choice" + " " + option.css_class)}
%if thing.use_post:
${post_link(option.title, option.base_path, option.path,
option.action_params, _sr_path=option.sr_path,
_class="choice " + option.css_class)}
%else:
${plain_link(option.title, option.path, _sr_path = option.sr_path,
_class = "choice" + " " + option.css_class)}
%endif
%endif
%endfor
</div>

23
r2/r2/templates/utils.html
View File

@@ -44,7 +44,6 @@
</%def>
## thing should be global
<%def name="_id(arg)">
id="${arg}_${thing and thing._fullname or ''}"
@@ -137,6 +136,28 @@ ${first_defined(kw[1:])}
${unsafe((fmt % link) if fmt else link)}
</%def>
<%def name="post_link(link_text, post_path, redir_path, params, _sr_path=True,
nocname=False, fmt='', target='', **kw)">
<%
action = add_sr(post_path, sr_path=_sr_path, nocname=nocname)
href = add_sr(redir_path, sr_path=_sr_path, nocname=nocname)
if (not target or target == '_parent') and c.cname:
target = '_top'
if c.cname and redir_path.startswith('http://'):
target = '_top'
if target:
kw['target'] = target
onclick = "$(this).parent().submit(); return false;"
link = _a_buffered(link_text, href=href, onclick=onclick)
%>
<form method="POST" action="${action}">
%for k, v in params.iteritems():
<input type="hidden" name="${k}" value="${v}">
%endfor
${unsafe((fmt % link) if fmt else link)}
</form>
</%def>
<%def name="text_with_links(txt, _sr_path = False, nocname=False, **kw)">
<%