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

For non-logged-in web users, hide over18 links in under18 subreddits #479

Closed
wants to merge 1 commit into from

Conversation

talklittle
Copy link
Contributor

specifically when visiting non-nsfw subreddits like /r/pics
it was showing NSFW links

@bboe
Copy link
Contributor

bboe commented Jul 14, 2012

I believe this was part of something I changed. When I discussed with the admins, the conclusion was rather than trying to have the equivalent of "do you want to view over 18 content", the NSFW content should be displayed for non-logged in users.

If content is hidden, then there needs to be a way to indicate to the API users that hidden content can be made available by setting a cookie or something. Maybe a new "warning" type needs to be sent along with the response to indicate that some results are filtered if you feel strongly that unauthenticated users should be filtered.

@bboe
Copy link
Contributor

bboe commented Jul 14, 2012

Though on second thought, NSFW links on non-explicit subreddits wasn't something I considered. Nevertheless, I still think there should be some indication to the API clients that results are being filtered.

@talklittle
Copy link
Contributor Author

Hey thanks @bboe. I'm familiar with the change you made to make NSFW content available to API clients. This pull request is not affecting that work you did. For API clients it will still by default show all the NSFW content. (That's the condition that you added a few code lines above this change, which checks for API clients.) If you pass the query string parameter "obey_over18=true" then this logic will also apply to API clients. (as of pull request GH-452)

The logic I changed here is mainly affecting web clients, not API clients, unless the API client passes obey_over18=true.

@talklittle
Copy link
Contributor Author

Changed pull request title to reflect my previous comment.

@bboe
Copy link
Contributor

bboe commented Jul 14, 2012

@talklittle Perfect :) I usually forget exactly how I implemented things, so the diff in this case didn't really tell me too much. Thanks for the clarification.

@spladug
Copy link
Contributor

spladug commented Jul 15, 2012

I love the more-readable version of that conditional you've got going there. Those is_ variables are great.

There's a lot going on here, so please take what I say with a huge grain of salt and correct me if I'm wrong, but I think the logic could be drastically simplified by using more fundamental building blocks. Let's start with user_is_over18 = (c.over18) and item_is_over18 = bool(wrapped.over18) or bool(wrapped.subreddit.over18). Then the conditional could become: if item_is_over18 and not user_is_over18: return False, right?

A logged out user without an over18 cookie is under18. They cannot get into an over18 subreddit without the over18 interstitial that would then add the over18 cookie if they accept (therefore making them over18). A logged in user without the over18 pref set would be in the same boat. A logged in user with the pref would be over18 and none of this would apply. This should also deal with multireddits transparently as they're marked under18 themselves but the user would also be under18 and so the links would be filtered out like currently.

I can't think of anything wrong with this but I feel like I must be missing something. :)

@talklittle
Copy link
Contributor Author

@spladug I'm all for simplicity! I think your suggestion is good. It does make sense to user c.over18 as the building block, because this way a logged-out user even has a (clumsy) way of viewing over18 links in under18 reddits- they can say Yes on the interstitial on an over18 reddit, then once they have the cookie, it unlocks the over18 links on the under18 reddits as well. With my originally proposed change, it would never show over18 links in under18 reddits to logged-out users.

@talklittle
Copy link
Contributor Author

Github weirdness, not sure why that commit was added to this old pull request.

@talklittle talklittle closed this Mar 28, 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