Add support for hiding and unhiding multiple fullnames

[voussoir]

/u/thorarakis updated hide/unhide endpoints to accept a comma-separated list of fullnames. Add method hide and unhide to the reddit session under ReportMixin.

http://www.reddit.com/r/redditdev/comments/384w9i/hide_and_unhide_endpoints_now_support_comma/

I was hesitant to create the new ReportMixin class, but since all of the other Mixins are scope-based, and hide / unhide are listed under the report scope, I felt it was the right thing to do.

objects.Hideable.hide now points to reddit_session.hide, passing its fullname. objects.Hideable.unhide still point to objects.Hideable.hide with unhide=True.

If you try to give more than 50 fullnames, it returns a very generic HTTPError that doesn't explain the problem. Would you like me to do anything about this, or do you think reddit will add an explanation to the error they return?

Please critique!

[bboe]

Please add the feature to the end of the feature list section. I like to try to keep them grouped by category.

[bboe]

Can you prefix unhide with an underscore as _unhide to indicate that it should not be used directly in this way? This permits us to remove the parameter in the future should we decide to change the implementation without breaking contractual backwards compatibility.

[bboe]

First, thank you for the addition. My in-line comments were very trivial. Aside from those there are two addtions that will be necessary prior to merging your PR.

1. Can you modify the scope rules on the previously existing hide class? It looks like it doesn't currently support the report scope.
2. The code coverage has decreased. Can you add a test similar to the following that verifies for multiple objects they can be hidden and subsequently unhidden? The test probably should appear in the test_authenticated_reddit.py:

praw/tests/test_submission.py

Line 75 in 3e9c99a

@betamax

If you're not sure how to run the tests please ask. My only request in such a situation is to keep some simple notes so we can improve the contributors.md file with instructions necessary to run and write new tests.

Thanks again!

[voussoir]

• Moved line in changes.rst
• Changed unhide to _unhide
• Added a note in the docstring for :param _unhide: that the user should use :meth:unhide instead of setting this directly.
• Applied scope @decorators.restrict_access(scope='report') to objects.Hideable.hide
• Removed urls variable

Anything else?

 

I managed to figure out the tests. Here are the steps a layman like myself might take:

1. Writing the test was easy since I'm familiar with your style guide.
2. Tried running scripts/init_test_environment.py, then realized it's meant for testing a reddit install.
3. Started python, did import tests and then dir(tests), it didn't show anything useful.
4. tests.test_authenticated_reddit gives an AttributeError. I don't know too much about package structure, so I tried to use what I know from using praw. In praw I can do praw.objects, praw.helpers, etc for all of the .py files that are inside the folder with init.py. I assumed this would be the same when using this test package.
5. Figured out that I had to do from tests import test_authenticated_reddit. I'm still not sure why this is different than doing praw.objects.Redditor
6. t = test_authenticated_reddit.AuthenticatedRedditTest() works great.
7. t.test_submission_hide_and_unhide_batch() raises AttributeError, No attribute r.
8. Opened helper.py and realized that self.r is created by PRAWTest.configure(), so I do t.configure().
9. t.test_submission_hide_and_unhide_batch() fails because of my bad programming
10. After fixing the error, the method now returns a Betamax exception mentioning a cassette. I was prepared to give up here, until I realized the cassettes are created automatically, and my method's cassette was broken because of that initial failure. I deleted it and ran again.
11. Test now runs quietly.

Let's see what coveralls thinks.

 

I had flake8 errors in the Travis build. Sorry about that.

 

Okay, more Travis errors. That Cassette error is the same one that I thought I had fixed. What does it mean?

[bboe]

Single item hiding and unhiding will already be tested via the submission_test hide and unhide so it isn't necessary.

[bboe]

Pretty good description of your test approach. This should help you out though. The tests are designed to run via python setup.py test. You can somewhat see that is how travis runs the tests in https://github.com/praw-dev/praw/blob/master/.travis.yml#L20 although travis runs using the coverage wrapped which doesn't do much good locally.

Betamax caches the requests and responses to reddit. All tests that make remote requests should have the betamax decorator. Doing this allows the entire test suite to run in seconds rather than multiple tens of minutes. So the trick is to continuously delete the cassette that is created until (1) the test initially passes, and (2) passes again when you immediately re-run the test. If an unexpected request occurs that's not part of the cassette, then betamax will raise an exception. Repeat until the problem is solved. If you get really stuck feel free to pass it along to me until I can take a look.

Also, in order for these saved requests to work, they must reply exactly in each run, which means tests cannot have any randomness in them. At the moment the file uploading tests fail because the underlying requests library introduces randomness. I'm working on a fix to ignore that specific randomness.

Great work so far! It's very close to being done.

[voussoir]

I don't think [item.refresh() for item in new] is going to be any different from what I have now, right? All it does is use the /comments/b36id link for the submission, and it'll take 5 times as long.

If you want me to use something other than get_new, I would probably start with by_id or get_info, rather than using refresh. Then at least it will be a single call. What are your thoughts?

This new cassette should work. I hope ✓

[bboe]

Looks great thanks!

I don't think [item.refresh() for item in new] is going to be any different from what I have now, right? All it does is use the /comments/b36id link for the submission, and it'll take 5 times as long.

The only difference would be if someone else adds a submission during the period of time the test is running. For speed reasons, it only matters when the cassette is created, subsequent replays is really fast. That's okay though, I'm okay with it as is.

Great job!