-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added Rest APIs to add/remove contributors in a channel #140
Conversation
Codecov Report
@@ Coverage Diff @@
## master #140 +/- ##
==========================================
+ Coverage 96.22% 96.55% +0.33%
==========================================
Files 117 118 +1
Lines 2806 3077 +271
Branches 66 70 +4
==========================================
+ Hits 2700 2971 +271
Misses 96 96
Partials 10 10
Continue to review full report at Codecov.
|
c7a7cf8
to
ee2e4e3
Compare
Functionality looks great |
channels/api.py
Outdated
channel = self.get_channel(channel_name) | ||
if contributor_name in channel.moderator(): | ||
raise exceptions.RemoveUserException( | ||
"User {} is a moderator of the channel{}".format(contributor_name, channel_name)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a space between channel
and {}
?
channels/urls.py
Outdated
@@ -20,6 +22,16 @@ | |||
name='post-list', | |||
), | |||
url( | |||
r'^api/v0/channels/(?P<channel_name>[A-Za-z0-9_]+)/contributors/$', | |||
ContributorListView.as_view(), | |||
name='contributors-list', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you change this to contributor-list
and the below to contributor-detail
, for consistency with the other URL names?
contributor = UserFactory.create() | ||
client = api.Api(client_user) | ||
client.remove_contributor(contributor.username, 'foo_channel_name') | ||
mock_client.subreddit.return_value.contributor.remove.assert_called_once_with(contributor) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this only tests if the method was called, but it does not test if the user was removed from the list of contributors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well... the assumption is that if the library is called and it does not return an error, the user has been removed.
Actually the problem is even more complicated, given that praw
will not return errors if the user is not a contributor, for example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great
10c67f6
to
bc45935
Compare
What are the relevant tickets?
closes #132
What's this PR do?
Adds a REST API to add and remove contributors in a channel
How should this be manually tested?
if you disable the
JwtIsStaffOrReadonlyPermission
in the views, you can use the django rest framework UI to perform the requests and you can go tohttp://reddit.local/r/<channel_name>/about/contributors/
and verify that the action you perform via rest API correspond to what you see in the UI