Skip to content
This repository has been archived by the owner on Nov 9, 2017. It is now read-only.

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

Conversation

buddydvd
Copy link
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
Copy link
Contributor

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
Copy link
Contributor

💅

@buddydvd
Copy link
Contributor Author

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
Copy link
Contributor Author

💇

@bsimpson63
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

andre-d commented Apr 21, 2013

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

@buddydvd
Copy link
Contributor Author

@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
Copy link
Contributor Author

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

@bsimpson63
Copy link
Contributor

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

@bsimpson63
Copy link
Contributor

🐟

@bsimpson63
Copy link
Contributor

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
Copy link
Contributor Author

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

💇

@buddydvd
Copy link
Contributor Author

💇 Corrected the pass/continue mistake (buddydvd@b48485d#commitcomment-3190245)

@bsimpson63
Copy link
Contributor

54d5bec

@bsimpson63 bsimpson63 closed this May 13, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants