1
\$\begingroup\$

I have a JSON API where a user can update a post and related details to that post. To simplify the process the user just fills in one hash and we don't expose our architecture. What I did here works but I would like to know if their is a better way. This smells to me, but I don't know why...

def update
    params['blog_post'] = params.delete 'post'
    @blog_post.update_attributes blog_post_params
    update_blog_post_groups if params['blog_post']['group_ids']
    params['blog_post_details'] = params.delete 'blog_post'
    @blog_post.blog_post_details.update_attributes blog_post_details_params
    render :blog_post, status: 200
end

blog_post_params and blog_post_details_params are strong_parameter methods.

It seems that I should use first : accepts_nested_attributes_for

\$\endgroup\$
  • \$\begingroup\$ can you please paste hash? \$\endgroup\$ – Vrushali Pawar Aug 31 '15 at 13:55
1
\$\begingroup\$

This could be refactored into a service object that does the manipulations. Your controller would still manipulate the Hash, but it would provide the service object the hash values it needs:

class BlogPostUpdateService
  def initialize(blog_post_params, blog_post_details_params)
    @blog_post_params = blog_post_params
    @blog_post_details_params = blog_post_details_params
  end

  def execute(blog_post)
    blog_post.update_attributes @blog_post_params
    blog_post.blog_post_details.update_attributes @blog_post_details_params
    update_blog_post_groups blog_post if @blog_post_params['group_ids']
    blog_post
  end

private
  def update_blog_post_groups(blog_post)
    # ...
  end
end

All of the logic for updating the blog post would be contained within the service object. Abstracting your internal architecture should be handled by the controller, which then delegates to the service for the actual modifications:

def update
  # ... abstract away your internal schema
  update_service = BlogPostUpdateService.new(blog_post_params, blog_post_details_params)
  update_service.execute @blog_post
  render :blog_post, status: 200
end

I'm also not clear why you are deleting 'post' and saving at as 'blog_post', but then later deleting 'blog_post' and saving it as 'blog_post_details'. This seems like you are jumping through a lot of hoops and I'm not sure what benefit it has, other than being really confusing.

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.