-
Notifications
You must be signed in to change notification settings - Fork 20
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
Admin user can merge two articles from edit page #25
base: master
Are you sure you want to change the base?
Conversation
… added to the view
def merge | ||
id = params[:id] | ||
merge_id = params[:merge_with][:merge_id] | ||
if current_user.admin? |
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.
Creating these local variables first is nice, but if the user isn't an admin, they aren't needed to i would suggest moving them into the first conditional
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.
Great point. I made the change.
@@ -416,6 +416,17 @@ def access_by?(user) | |||
user.admin? || user_id == user.id | |||
end | |||
|
|||
def merge_with(other_article_id) | |||
Article.find(other_article_id).comments.each do |comment| |
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.
There is a lot of method chaining going on here. Is there a better way to get the comments associated without iterating through each one?
New feature allows admin to merge two articles from the edit page. Contributors do not have access to merge articles.