Added REST API for posts #54

noisecapella · 2017-07-20T22:21:03Z

What are the relevant tickets?

Fixes #29

What's this PR do?

Adds a REST API to update, read and create posts
Adds open_discussions/recorder.py to aid in creating cassettes for use in mocking reddit during testing

How should this be manually tested?

Go to /api/v0/channels/<subreddit>/posts/ to view a list of posts or to create a new one. Go to /api/v0/posts/<post_id>/ to view information on a specific post or to update or delete a post.

codecov-io · 2017-07-21T14:30:59Z

Codecov Report

Merging #54 into master will decrease coverage by 0.33%.
The diff coverage is 87.59%.

@@            Coverage Diff             @@
##           master      #54      +/-   ##
==========================================
- Coverage   93.99%   93.65%   -0.34%     
==========================================
  Files          47       49       +2     
  Lines         932     1087     +155     
  Branches       42       54      +12     
==========================================
+ Hits          876     1018     +142     
- Misses         44       58      +14     
+ Partials       12       11       -1

Impacted Files	Coverage Δ
channels/urls.py	`100% <ø> (ø)`	⬆️
open_discussions/recorder.py	`0% <0%> (ø)`
channels/conftest.py	`100% <100%> (ø)`	⬆️
channels/views.py	`100% <100%> (ø)`	⬆️
channels/api.py	`86.66% <100%> (+1.72%)`	⬆️
open_discussions/betamax_config.py	`100% <100%> (ø)`
channels/serializers.py	`98.05% <97.56%> (+2.82%)`	⬆️
static/js/components/PostSummary_test.js	`100% <0%> (ø)`	⬆️
static/js/components/PostList.js	`100% <0%> (ø)`	⬆️
static/js/components/PostList_test.js	`100% <0%> (ø)`	⬆️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 985411f...dc87b48. Read the comment docs.

rhysyngsun · 2017-07-24T16:23:28Z

I'll dig into this more after lunch, but I'm getting this error on some /api/v0/posts/<id> requests:


web_1     | [2017-07-24 16:20:43] ERROR 20 [django.request] exception.py:124 - [8562e466a2eb] - Internal Server Error: /api/v0/posts/i/
web_1     | Traceback (most recent call last):
web_1     |   File "/usr/local/lib/python3.6/site-packages/django/core/handlers/exception.py", line 39, in inner
web_1     |     response = get_response(request)
web_1     |   File "/usr/local/lib/python3.6/site-packages/django/core/handlers/base.py", line 249, in _legacy_get_response
web_1     |     response = self._get_response(request)
web_1     |   File "/usr/local/lib/python3.6/site-packages/django/core/handlers/base.py", line 187, in _get_response
web_1     |     response = self.process_exception_by_middleware(e, request)
web_1     |   File "/usr/local/lib/python3.6/site-packages/django/core/handlers/base.py", line 185, in _get_response
web_1     |     response = wrapped_callback(request, *callback_args, **callback_kwargs)
web_1     |   File "/usr/local/lib/python3.6/site-packages/django/views/decorators/csrf.py", line 58, in wrapped_view
web_1     |     return view_func(*args, **kwargs)
web_1     |   File "/usr/local/lib/python3.6/site-packages/django/views/generic/base.py", line 68, in view
web_1     |     return self.dispatch(request, *args, **kwargs)
web_1     |   File "/usr/local/lib/python3.6/site-packages/rest_framework/views.py", line 477, in dispatch
web_1     |     response = self.handle_exception(exc)
web_1     |   File "/usr/local/lib/python3.6/site-packages/rest_framework/views.py", line 437, in handle_exception
web_1     |     self.raise_uncaught_exception(exc)
web_1     |   File "/usr/local/lib/python3.6/site-packages/rest_framework/views.py", line 474, in dispatch
web_1     |     response = handler(request, *args, **kwargs)
web_1     |   File "/usr/local/lib/python3.6/site-packages/rest_framework/generics.py", line 283, inget
web_1     |     return self.retrieve(request, *args, **kwargs)
web_1     |   File "/usr/local/lib/python3.6/site-packages/rest_framework/mixins.py", line 58, in retrieve
web_1     |     return Response(serializer.data)
web_1     |   File "/usr/local/lib/python3.6/site-packages/rest_framework/serializers.py", line 527,in data
web_1     |     ret = super(Serializer, self).data
web_1     |   File "/usr/local/lib/python3.6/site-packages/rest_framework/serializers.py", line 262,in data
web_1     |     self._data = self.to_representation(self.instance)
web_1     |   File "/usr/local/lib/python3.6/site-packages/rest_framework/serializers.py", line 496,in to_representation
web_1     |     ret[field.field_name] = field.to_representation(attribute)
web_1     |   File "/usr/local/lib/python3.6/site-packages/rest_framework/fields.py", line 1734, in to_representation
web_1     |     return method(value)
web_1     |   File "./channels/serializers.py", line 76, in get_url
web_1     |     return instance.url if not instance.is_self else None
web_1     |   File "/usr/local/lib/python3.6/site-packages/praw/models/reddit/base.py", line 31, in __getattr__
web_1     |     self._fetch()
web_1     |   File "/usr/local/lib/python3.6/site-packages/praw/models/reddit/submission.py", line 142, in _fetch
web_1     |     'sort': self.comment_sort})
web_1     |   File "/usr/local/lib/python3.6/site-packages/praw/reddit.py", line 368, in get
web_1     |     return self._objector.objectify(data)
web_1     |   File "/usr/local/lib/python3.6/site-packages/praw/objector.py", line 105, in objectify
web_1     |     return [self.objectify(item) for item in data]
web_1     |   File "/usr/local/lib/python3.6/site-packages/praw/objector.py", line 105, in <listcomp>
web_1     |     return [self.objectify(item) for item in data]
web_1     |   File "/usr/local/lib/python3.6/site-packages/praw/objector.py", line 108, in objectify
web_1     |     return parser.parse(data['data'], self._reddit)
web_1     |   File "/usr/local/lib/python3.6/site-packages/praw/models/base.py", line 30, in parse
web_1     |     return cls(reddit, _data=data)
web_1     |   File "/usr/local/lib/python3.6/site-packages/praw/models/base.py", line 41, in __init__
web_1     |     setattr(self, attribute, value)
web_1     |   File "/usr/local/lib/python3.6/site-packages/praw/models/listing/listing.py", line 21,in __setattr__
web_1     |     value = self._reddit._objector.objectify(value)
web_1     |   File "/usr/local/lib/python3.6/site-packages/praw/objector.py", line 105, in objectify
web_1     |     return [self.objectify(item) for item in data]
web_1     |   File "/usr/local/lib/python3.6/site-packages/praw/objector.py", line 105, in <listcomp>
web_1     |     return [self.objectify(item) for item in data]
web_1     |   File "/usr/local/lib/python3.6/site-packages/praw/objector.py", line 108, in objectify
web_1     |     return parser.parse(data['data'], self._reddit)
web_1     |   File "/usr/local/lib/python3.6/site-packages/praw/models/base.py", line 30, in parse
web_1     |     return cls(reddit, _data=data)
web_1     |   File "/usr/local/lib/python3.6/site-packages/praw/models/reddit/comment.py", line 58, in __init__
web_1     |     super(Comment, self).__init__(reddit, _data)
web_1     |   File "/usr/local/lib/python3.6/site-packages/praw/models/reddit/base.py", line 46, in __init__
web_1     |     super(RedditBase, self).__init__(reddit, _data)
web_1     |   File "/usr/local/lib/python3.6/site-packages/praw/models/base.py", line 41, in __init__
web_1     |     setattr(self, attribute, value)
web_1     |   File "/usr/local/lib/python3.6/site-packages/praw/models/reddit/comment.py", line 73, in __setattr__
web_1     |     value = self._reddit._objector.objectify(value).children
web_1     |   File "/usr/local/lib/python3.6/site-packages/praw/objector.py", line 108, in objectify
web_1     |     return parser.parse(data['data'], self._reddit)
web_1     |   File "/usr/local/lib/python3.6/site-packages/praw/models/base.py", line 30, in parse
web_1     |     return cls(reddit, _data=data)
web_1     |   File "/usr/local/lib/python3.6/site-packages/praw/models/base.py", line 41, in __init__
web_1     |     setattr(self, attribute, value)
web_1     |   File "/usr/local/lib/python3.6/site-packages/praw/models/listing/listing.py", line 21,in __setattr__
web_1     |     value = self._reddit._objector.objectify(value)
web_1     |   File "/usr/local/lib/python3.6/site-packages/praw/objector.py", line 105, in objectify
web_1     |     return [self.objectify(item) for item in data]
web_1     |   File "/usr/local/lib/python3.6/site-packages/praw/objector.py", line 105, in <listcomp>
web_1     |     return [self.objectify(item) for item in data]
web_1     |   File "/usr/local/lib/python3.6/site-packages/praw/objector.py", line 108, in objectify
web_1     |     return parser.parse(data['data'], self._reddit)
web_1     |   File "/usr/local/lib/python3.6/site-packages/praw/models/base.py", line 30, in parse
web_1     |     return cls(reddit, _data=data)
web_1     |   File "/usr/local/lib/python3.6/site-packages/praw/models/reddit/comment.py", line 58, in __init__
web_1     |     super(Comment, self).__init__(reddit, _data)
web_1     |   File "/usr/local/lib/python3.6/site-packages/praw/models/reddit/base.py", line 46, in __init__
web_1     |     super(RedditBase, self).__init__(reddit, _data)
web_1     |   File "/usr/local/lib/python3.6/site-packages/praw/models/base.py", line 41, in __init__
web_1     |     setattr(self, attribute, value)
web_1     |   File "/usr/local/lib/python3.6/site-packages/praw/models/reddit/comment.py", line 68, in __setattr__
web_1     |     value = Redditor.from_data(self._reddit, value)
web_1     |   File "/usr/local/lib/python3.6/site-packages/praw/models/reddit/redditor.py", line 21,in from_data
web_1     |     return cls(reddit, data)
web_1     |   File "/usr/local/lib/python3.6/site-packages/praw/models/reddit/redditor.py", line 31,in __init__
web_1     |     raise TypeError('Either `name` or `_data` must be provided.')
web_1     | TypeError: Either `name` or `_data` must be provided.

rhysyngsun · 2017-07-24T15:48:26Z

open_discussions/betamax_config.py

+Betamax.register_request_matcher(CustomBodyMatcher)
+
+
+with Betamax.configure() as config:


I'm a little wary of module-level code like this, but it looks like that the only way to do this unless we want to pass betamax configured sessions around, right?

My main concern is this code somehow getting triggered by some inadvertent import path down the road. Maybe we could move this into a function here and store in a global INITIALIZED = True and invoke the method in other methods/fixtures as we need them? That way execution of this code is explicit.

I think we can probably move this into the fixture function

You import it in recorder.py too so I wasn't sure about suggesting that.

rhysyngsun · 2017-07-24T16:29:23Z

channels/serializers.py

+    text = WriteableSerializerMethodField(allow_null=True)
+    title = serializers.CharField()
+    upvoted = WriteableSerializerMethodField()
+    downvoted = WriteableSerializerMethodField()


I don't think we necessarily need this, but these aren't currently writable on the create. I think adding a comment here would suffice.

The user is allowed to write them in the update though. What would the comment be for?

You can pass them in create but they do nothing.

I'll implement this for create, it seems like more consistent behavior to have it work in both cases

Sounds fine to me.

rhysyngsun · 2017-07-24T16:30:28Z

channels/serializers.py

+            **kwargs
+        )
+
+    def update(self, instance, validated_data):


Should it be an error to provide True for both downvote and upvote in the same request?

Yes, it's taken care of in validate though

I missed that, ignore this then.

rhysyngsun · 2017-07-24T16:33:20Z

channels/serializers.py

+        upvote = validated_data.get('upvoted')
+
+        if "text" in validated_data:
+            instance = api.update_post(post_id=post_id, text=validated_data['text'])


It's possible to provide url at the same time as text, which will cause this to raise a ValueError, (gives a 400 and stacktrace page in debug mode). Probably worth validating url wasn't provided before this because it's an invalid request anyway.

Also begs the question whether we need validation code twice (here and in api.py)?

Yeah I'm not sure where the validation best fits, maybe it will become clearer in a future PR. I'll add the url check

rhysyngsun · 2017-07-24T16:34:22Z

Other than the exception and 400 error mentioned above this tested functional.

noisecapella · 2017-07-24T16:39:39Z

The post which caused the error, was it created with the script which generates fake data for reddit?

rhysyngsun · 2017-07-24T16:40:30Z

Yes it was the fake data. Should we just ignore those?

noisecapella · 2017-07-24T16:44:46Z

I don't know, it's their fake data script so you would think it would be valid fake data. I would feel better if we did some investigation on why it was happening

noisecapella · 2017-07-25T16:15:13Z

All comments should be addressed

noisecapella · 2017-07-25T18:01:58Z

Just updated this to rename author to author_id and remove downvoted

noisecapella added the Work in Progress label Jul 20, 2017

noisecapella force-pushed the gs/posts_rest_api branch from db7cf40 to e1108de Compare July 21, 2017 20:24

noisecapella added Needs Review and removed Work in Progress labels Jul 21, 2017

rhysyngsun self-assigned this Jul 24, 2017

rhysyngsun requested changes Jul 24, 2017

View reviewed changes

rhysyngsun added Waiting on Author and removed Needs Review labels Jul 24, 2017

pdpinch mentioned this pull request Jul 24, 2017

As a logged in user I should be able to create a post in a chosen channel #17

Closed

3 tasks

George Schneeloch added 5 commits July 25, 2017 10:44

Added REST API for posts

b0dfbd5

Update cassettes

744f15f

Tests

79f34e7

Move initialization code into setup_betamax

3dbb985

WIP

7e8476a

noisecapella force-pushed the gs/posts_rest_api branch from eff9bfc to 7e8476a Compare July 25, 2017 14:44

George Schneeloch added 3 commits July 25, 2017 11:56

Fix tests

e966b54

Add channel_name

848ca54

Add validation for url

27c2fbb

noisecapella added Needs Review and removed Waiting on Author labels Jul 25, 2017

George Schneeloch added 2 commits July 25, 2017 13:48

Rename author to author_id

75a7bde

Remove downvote

4d4319b

George Schneeloch added 2 commits July 25, 2017 14:02

Remove unused cassette

6e86d0b

Remove unused requirement

dc87b48

noisecapella mentioned this pull request Jul 25, 2017

Human readable cassettes #59

Merged

rhysyngsun approved these changes Jul 25, 2017

View reviewed changes

rhysyngsun added Waiting on Author and removed Needs Review labels Jul 25, 2017

noisecapella merged commit 80e02e6 into master Jul 25, 2017

noisecapella deleted the gs/posts_rest_api branch July 25, 2017 20:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added REST API for posts #54

Added REST API for posts #54

noisecapella commented Jul 20, 2017 •

edited

codecov-io commented Jul 21, 2017 •

edited

rhysyngsun commented Jul 24, 2017

rhysyngsun Jul 24, 2017

noisecapella Jul 24, 2017

rhysyngsun Jul 24, 2017

rhysyngsun Jul 24, 2017

noisecapella Jul 24, 2017

rhysyngsun Jul 24, 2017

noisecapella Jul 24, 2017

rhysyngsun Jul 24, 2017

rhysyngsun Jul 24, 2017

noisecapella Jul 24, 2017

rhysyngsun Jul 24, 2017

rhysyngsun Jul 24, 2017

noisecapella Jul 25, 2017

rhysyngsun commented Jul 24, 2017

noisecapella commented Jul 24, 2017

rhysyngsun commented Jul 24, 2017

noisecapella commented Jul 24, 2017

noisecapella commented Jul 25, 2017

noisecapella commented Jul 25, 2017

		Betamax.register_request_matcher(CustomBodyMatcher)


		with Betamax.configure() as config:

Added REST API for posts #54

Added REST API for posts #54

Conversation

noisecapella commented Jul 20, 2017 • edited

What are the relevant tickets?

What's this PR do?

How should this be manually tested?

codecov-io commented Jul 21, 2017 • edited

Codecov Report

rhysyngsun commented Jul 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rhysyngsun commented Jul 24, 2017

noisecapella commented Jul 24, 2017

rhysyngsun commented Jul 24, 2017

noisecapella commented Jul 24, 2017

noisecapella commented Jul 25, 2017

noisecapella commented Jul 25, 2017

noisecapella commented Jul 20, 2017 •

edited

codecov-io commented Jul 21, 2017 •

edited