Make submission page's subreddit auto-completion dropdown obey over18 state #747

Closed
wants to merge 1 commit into
from

Projects

None yet

3 participants

@buddydvd
Contributor

This pull request updates submission page's subreddit auto-completion dropdown to not show over18 subreddits if user's over18 pref is false, which mirrors the behavior of subreddit topic recommender bar at http://www.reddit.com/subreddit (See: https://github.com/reddit/reddit/blob/master/r2/r2/controllers/api.py#L3179).

Steps to reproduce:

  1. login to reddit
  2. uncheck the over18 pref checkbox
  3. go to the submission page and type "pics" into the subreddit input text field

Expected result:

The auto-completion dropdown box shows a list of non-over18 subreddits that have "pics" as their prefix.

Actual result:

The auto-completion dropdown shows the following subreddits:

  • pics
  • picsofmenwiththings
  • picsofharlequincats
  • PicsOfHorseDicks [over18]
  • PicsOfCanineVaginas [over18]
  • Picsofryanrunning
  • picsofnathancowley
  • picsofporn [over18]
  • picsofdannyjsleeping
  • PicsOfStonedMexicans

The proposed update makes it so that those subreddits only show up if they have set their account's over18 pref to true.

Lastly, it does so without breaking existing API clients by preserving the previous behavior as the default option for POST_search_reddit_names.

@bsimpson63
Member

Do you expect the only over18 option will be used? I think it might be better to drop that and then simplify things. Also rather than creating multiple columns you could store lists of (srname, over18) tuples and then filter after retrieving the row from cassandra.

@bsimpson63
Member

๐Ÿ’…

@buddydvd
Contributor

Also rather than creating multiple columns you could store lists of (srname, over18) tuples and then filter after retrieving the row from cassandra.

I considered that option but decided against it because the filtered list may produce fewer than 10 results. If that reason is not convincing enough, just let me know, and I'll change it to the way you described.

@buddydvd
Contributor

๐Ÿ’‡

@bsimpson63
Member

I think it's ok to have fewer than 10 results. The unfiltered result is the standard and removing over18 subreddits doesn't get you anything extra.

@buddydvd
Contributor

Okay, done โ€” the new implementation stores a list of (sr.name, sr.over_18) tuples into cassandra.

Also, sorry about all the revert commits. If you prefer rebase, I can do that as well. ๐Ÿ’‡

@andre-d
Contributor
andre-d commented Apr 21, 2013

I think the reverted commits and their reverter commit can be rebased out.

@buddydvd
Contributor

@andre-d: Per your recommendation, I rebased the branch. Though I really wish Github preserved the state before the rebase so no context is lost.

@bsimpson63: I simplified POST_search_reddit_names's over_18 parameter into a boolean type. I hope that makes the code a little bit easier to understand.

@buddydvd
Contributor

I refactored/simplified the code more to minimize the number of line changes.

@bsimpson63
Member

@buddydvd Looks good, I just want to test this a bit before marking it approved.

@bsimpson63
Member

๐ŸŸ

@bsimpson63
Member

Hey sorry to reopen this, but I think popular_searches needs to be changed to include over18 so that the prefetched searches will also obey over18 pref.

๐Ÿ’…

@buddydvd
Contributor

Okay, updated subreddit_search.py's popular_searches method and pages.py's NewLink class to obey over18 pref.

๐Ÿ’‡

@buddydvd
Contributor

๐Ÿ’‡ Corrected the pass/continue mistake (buddydvd@b48485d#commitcomment-3190245)

@bsimpson63
Member
@bsimpson63 bsimpson63 closed this May 13, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment