CODEWITHSUNDEEP
ruby-on-railsruby-on-rails-3
Josh Smith
Josh Smith

Reputation: 15028

Create logic in one controller accessing two models

I have three models, Artwork, ArtworkTag, and Tag, where ArtworkTag represents an m:n relation between Artwork and Tag. Tag only stores the tag name, i.e. :tag.

When creating tags from ArtworkTagsController, it's necessary to first see if the :tag exists in the tags table, create it if not, and then use the tag id to make the relation in ArtworkTag.

Below you can see that I'm calling Tag directly from this controller, which I don't think is the right practice.

How should I be handling this situation with the proper separation of concerns?

def create
  tag = params[:artwork_tag][:tag].downcase
  @tag = Tag.find_by_tag(tag)

  if @tag.blank?
    @tag = Tag.new(:tag => tag)
    @tag.save
  end

  artwork_id = params[:artwork_tag][:artwork_id]
  user_id = params[:artwork_tag][:user_id]

  artwork_tag = {
    "tag_id" => @tag.id,
    "artwork_id" => artwork_id,
    "user_id" => user_id
  }

  @artwork_tag = ArtworkTag.new(:tag_id => @tag.id, :artwork_id => artwork_id, :user_id => user_id)
  @artwork_tag.save
  respond_to do |format|
    format.json { render :json => {
      "id" => @artwork_tag.id, "tag" => @tag.tag, "artwork_id" => artwork_id },
      :status => :created }
  end
end

Upvotes: 0

Views: 77

Answers (2)

Jason Swett
Jason Swett

Reputation: 45094

I would do something like this:

def create
  artwork_tag = ArtworkTag.create(
    :tag => Tag.find_or_create_by_tag(params[:artwork_tag][:tag].downcase),
    :artwork_id => params[:artwork_tag][:artwork_id],
    :user_id => params[:artwork_tag][:user_id]
  )

  respond_to do |format|
    format.json { render :json => { :artwork_tag => artwork_tag } }
  end
end

Upvotes: 1

Thanh
Thanh

Reputation: 8604

I'm not sure I understand your question, but you can use first_or_create method to check if record is already exists or not, if not it will create new record. You can replace this:

@tag = Tag.find_by_tag(tag)

if @tag.blank?
  @tag = Tag.new(:tag => tag)
  @tag.save
end

with only one row: (I'm not use instance variable because i think you don't need use it in view, if you need just change it to @tag and in params to tag_id: @tag.id)

tag = Tag.where(tag: tag).first_or_create()

I see you defined artwork_tag, but why you aren't use it? I think you can replace:

artwork_tag = {
  "tag_id" => @tag.id,
  "artwork_id" => artwork_id,
  "user_id" => user_id
}

@artwork_tag = ArtworkTag.new(:tag_id => @tag.id, :artwork_id => artwork_id, :user_id => user_id)
@artwork_tag.save

with this code: 

params = { artwork_tag: { 
                          tag_id: tag.id, 
                          artwork_id: artwork_id, 
                          user_id: user_id  } }
@artwork_tag = ArtworkTag.create(params[:artwork_tag])

Upvotes: 0

Related Questions