Skip to content

Admin user can merge two articles from edit page#25

Open
emgord wants to merge 34 commits intoAda-C4:masterfrom
emgord:master
Open

Admin user can merge two articles from edit page#25
emgord wants to merge 34 commits intoAda-C4:masterfrom
emgord:master

Conversation

@emgord
Copy link
Copy Markdown

@emgord emgord commented Mar 30, 2016

New feature allows admin to merge two articles from the edit page. Contributors do not have access to merge articles.

emgord added 30 commits March 28, 2016 11:06
def merge
id = params[:id]
merge_id = params[:merge_with][:merge_id]
if current_user.admin?
Copy link
Copy Markdown

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

Copy link
Copy Markdown
Author

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.

Comment thread app/models/article.rb
end

def merge_with(other_article_id)
Article.find(other_article_id).comments.each do |comment|
Copy link
Copy Markdown

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants