views:

120

answers:

2

Currently my controller lets a user submit muliple "links" at a time. It collects them into an array, creates them for that user, but catches any errors for the User to go back and fix. How can I ignore the creation of any links that already exist for that user? I know that I can use validates_uniqueness_of with a scope for that user, but I'd rather just ignore their creation completely. Here's my controller:

@links = params[:links].values.collect{ |link| current_user.links.create(link) }.reject { |p| p.errors.empty? }

Each link has a url, so I thought about checking if that link.url already exists for that user, but wasn't really sure how, or where, to do that. Should I tack this onto my controller somehow? Or should it be a new method in the model, like as in a before_validation Callback? (Note: these "links" are not nested, but they do belong_to :user.)

So, I'd like to just be able to ignore the creation of these links if possible. Like if a user submits 5 links, but 2 of them already exist for him, then I'd just like for those 2 to be ignored, while the other 3 are created. How should I go about doing this?

Edit: Thanks to Kandada, I'm now using this:

@links = params[:links].values.collect.reject{ |link| current_user.links.exists?(:url=>link[:url])}

@links = @links.collect{ |link| current_user.links.create(link) }.reject { |p| p.errors.empty? }

So I separated the two, to first check if there are any that exist, then to create those that weren't rejected. Is there a better way to do this, like maybe combining the two statements would increase performance? If not, I think I'm pretty satisfied. (thank you again Kandada and j.)

+1  A: 

Reject the existent links before create them:

new_links = params[:links].reject{ |link| current_user.links.exists?(link) }

Somethink like this. Not sure about this code...

j.
Thanks j. That's exactly what I was thinking, but couldn't figure out how to make it fit with my controller above. Can there be 2 "reject" calls the same statement, or should I separate them somehow?
GoodGets
where do you need another `reject`?
j.
because I'm already rejecting those that don't pass validations. Is there a better way to do that than using the code in my controller?
GoodGets
+2  A: 

Try this:

@links = current_user.links.create(params[:links].reject{ |link| 
           current_user.links.exists?(:url=>link[:url]) })

Alternatively you can add an uniqueness check in the Link model for the url attribute.

class Link
  validates_uniqueness_of :url, :scope => [:user_id]
end

In your controller:

@links = current_user.links.create(params[:links])

The result set returned is an array of newly created Link objects. Any links matching the existing links are ignored.

Edit

Here is another way to do this in one pass.

@links = params[:links].map{|link| 
           !current_user.links.exists?(:url=> link[:url]) and
             current_user.links.create(link)}.select{|link| link and 
               link.errors.empty?}

I still think you should get your unique validation working and use this code afterwards:

@links = current_user.links.create(params[:links]).select{|link| 
           link.errors.empty?}

In the latter approach uniqueness validation is done in the model. This ensures link url uniqueness regardless how the link is created.

KandadaBoggu
Hi, Kandada and thanks.Trying your first code, I get an undefined method 'url' for Array. And trying the second way, using the validates_uniqueness_of :url, :scope => :user_id (I added in the user scope), I get a NoMethodError, private method 'gsub' called, which is something I do after_validation. So, I'm not sure what's wrong.
GoodGets
Fixed the bug in the first option.Try again. In order for me to help you with the second part, I need to look at the model code. Post the pastie(http://pastie.org/) link to the code.
KandadaBoggu
No need. That did it. Big thanks Kandada.I actually solved it by calculating the @links twice. Once to see if they exist, then again for those that weren't rejected. (I added my solution code to the question, just in case you know of an even better way). Thank you a Kandada.
GoodGets
I have updated my answer to do it in one pass. Take a look
KandadaBoggu
wow, thank you again Kandada. You just keep coming through. I do have a question though: Are there any benefits to both checking uniqueness and creating in the same pass? I sorta like having the two separated, but didn't know if there would be any performance decreases/increases. Also, are you saying that I should use @links = current_user.links.create(params[:links]).select{|link| link.errors.empty?} instead of @links = @links.collect{ |link| current_user.links.create(link) }.reject { |p| p.errors.empty? } ??? What's really the difference? Finally, thank you again for everything.
GoodGets
You are saving on CPU cycles by doing the processing in one pass. Unless you need a list of existing links and new links there is no benefit having two separate lists. I have amended my answer to explain why I prefer the second approach. Take a look
KandadaBoggu
Just amazing! I seriously can't thank you enough Kandada. I did have to add ".values.collect" before .map (so @links = params[:links].values.collect.map{|link|...) else I get a "symbol as array index" error. Also, you said that you're saving CPU cycles by doing it all in one pass, but I didn't know if this one "big" pass would be more taxing on the server than two smaller ones. Also, combining them all into one pass won't allow me to count the duplicates will it? That's really the only benefit I see in doing it in 2 passes. Either way, I've learned a lot thanks to you. Thank you again.
GoodGets
The array method `collect` and `map` are one and the same. So you should be able to remove the extra collect, i.e. `@links = params[:links].values.map{|link|...}`. I am not sure why you are getting a hash instead of an array for params[:links]. That's something for you to debug :-)
KandadaBoggu
Yeah, you were right. I can remove the extra collect and it works. However, I just noticed that the one "big" pass doesn't like a before_save filter I have (it fills in the link.name if the user left it blank). The one pass throws and error, but splitting them into 2 passes does not. No big deal because I think I was leaning towards splitting them up anyway, so I can get their size and display them in a notice to the user that "X number of links weren't created because of duplicity." Kandada I'm very appreciative and very happy with the help you've given me. A thank you is well deserved.
GoodGets