Accept '/r/foo' everywhere 'foo' works

It's annoying to use the common subreddit syntax of `/r/foo` and be told you
can't do that.  We've already been normalizing this in a few places, but now we
should be accepting `/r/foo` or `r/foo` everywhere and silently stripping that
to just plain ol' `foo` for the rest of the code.

Here are the validators I found doing some sort of subreddit name check, and
where they're used:

- VSubredditName
    * multi.py
        + PUT_multi_subreddit - add a subreddit to a multi
- VAvailableSubredditName
    * api.py
        + POST_site_admin - create or configure a subreddit
- VMultiPath - already requires /r/
    * multi.py a bunch of places.
- VSubredditList - already doing this
- VSRByName
    * preferences.py - stylesheets everywhere
    * api.py
        + POST_compose - when sending a PM from a subreddit
    * multi.py
        + GET_list_sr_multis - getting a subreddit's multis
        + GET_multi_subreddit - get data about a subreddit in a multi
        + DELETE_multi_subreddit - remove a subreddit from a multi
- VSRByNames
    * api.py
        + GET_subreddit_recommendations
        + POST_rec_feedback - recommender feedback

One important thing to note: we don't want to just modify
`Subreddit.is_valid_name()` because that's used in lower-level code, like when
creating a `Subreddit` object, and that could cause all sorts of problems.
This commit is contained in:
xiongchiamiov
2015-04-10 16:22:28 -07:00
parent 54c1bfd52d
commit 9350508dbf
2 changed files with 54 additions and 17 deletions

View File

@@ -688,6 +688,8 @@ class VSubredditName(VRequired):
self.allow_language_srs = allow_language_srs
def run(self, name):
if name:
name = sr_path_rx.sub('\g<name>', name.strip())
valid_name = Subreddit.is_valid_name(
name, allow_language_srs=self.allow_language_srs)
if not valid_name:
@@ -716,6 +718,7 @@ class VSRByName(Validator):
if not sr_name:
self.set_error(errors.BAD_SR_NAME, code=400)
else:
sr_name = sr_path_rx.sub('\g<name>', sr_name.strip())
try:
sr = Subreddit._by_name(sr_name)
return sr
@@ -741,7 +744,8 @@ class VSRByNames(Validator):
def run(self, sr_names_csv):
if sr_names_csv:
sr_names = [s.strip() for s in sr_names_csv.split(',')]
sr_names = [sr_path_rx.sub('\g<name>', s.strip())
for s in sr_names_csv.split(',')]
return Subreddit._by_name(sr_names)
elif self.required:
self.set_error(errors.BAD_SR_NAME, code=400)

View File

@@ -28,10 +28,53 @@ stage_for_paste()
from pylons import c
from r2.lib.errors import errors, ErrorSet
from r2.lib.validator import ValidEmail
from r2.lib.validator import VSubredditName, ValidEmail
class TestValidEmail(unittest.TestCase):
class ValidatorTests(unittest.TestCase):
def _test_failure(self, input, error):
"""Helper for testing bad inputs."""
self.validator.run(input)
self.assertTrue(self.validator.has_errors)
self.assertTrue(c.errors.get((error, None)))
def _test_success(self, input, assertEqual=True):
result = self.validator.run(input)
self.assertFalse(self.validator.has_errors)
self.assertEqual(len(c.errors), 0)
if assertEqual:
self.assertEqual(result, input)
return result
class TestVSubredditName(ValidatorTests):
def setUp(self):
# Reset the validator state and errors before every test.
self.validator = VSubredditName(None)
c.errors = ErrorSet()
def _test_failure(self, input, error=errors.BAD_SR_NAME):
super(TestVSubredditName, self)._test_failure(input, error)
# Most of this validator's logic is already covered in `IsValidNameTest`.
def test_slash_r_slash(self):
result = self._test_success('/r/foo', assertEqual=False)
self.assertEqual(result, 'foo')
def test_r_slash(self):
result = self._test_success('r/foo', assertEqual=False)
self.assertEqual(result, 'foo')
def test_two_prefixes(self):
self._test_failure('/r/r/foo')
def test_slash_not_prefix(self):
self._test_failure('foo/r/')
class TestValidEmail(ValidatorTests):
"""Lightly test email address ("addr-spec") validation against RFC 2822.
http://www.faqs.org/rfcs/rfc2822.html
@@ -42,22 +85,12 @@ class TestValidEmail(unittest.TestCase):
c.errors = ErrorSet()
def test_valid_emails(self):
def test(email):
result = self.validator.run(email)
self.assertEqual(result, email)
self.assertFalse(self.validator.has_errors)
self.assertEqual(len(c.errors), 0)
test('test@example.com')
test('test@example.co.uk')
test('test+foo@example.com')
self._test_success('test@example.com')
self._test_success('test@example.co.uk')
self._test_success('test+foo@example.com')
def _test_failure(self, email, error=errors.BAD_EMAIL):
"""Helper for testing bad emails."""
result = self.validator.run(email)
self.assertEqual(result, None)
self.assertTrue(self.validator.has_errors)
self.assertTrue(c.errors.get((error, None)))
super(TestValidEmail, self)._test_failure(email, error)
def test_blank_email(self):
self._test_failure('', errors.NO_EMAIL)