rubocop style model self needed but triggering warning

Question

For some reason rubocop is choking on this code I have in my model to properly address accepts_nested_attributes_for to work like find or create. When I tried to remove the self calls it blows up. At this point I am deferring to experts before I shut that darn cop off. Thoughts?

class Job < ActiveRecord::Base
  belongs_to :company
  before_validation :find_company

  accepts_nested_attributes_for :company

  private

  def find_company
    if self.company
      self.company = Company.where(email: self.company.email).first_or_initialize
    end
  end
end

score 0 · Accepted Answer · answered Feb 17 '16 at 07:07

0

This would make Rubocop happy:

def find_company
  self.company = Company.where(email: company.email).first_or_initialize if company
end

P.S. I do not see the logic behind this method - you are checking, if company association is present, and if it is you are assigning association again with this same company.

Method does not make sense - I think you'd be better of deleting it altogether.

If you want to make sure, that company is always present, just check add presence validation:

validates :company, presence: true

answered Feb 17 '16 at 07:07

Andrey Deineko

51,333
10
112
145

When you are using nested attributes you have to account for find or create by http://stackoverflow.com/questions/3579924/accepts-nested-attributes-for-with-find-or-create => Am I missing something here? This did fix rubocop though. – Chris Hough Feb 17 '16 at 07:25
@chrishough anyway, as to rubocop - use `self.attribute` only when you're assigning it, like `self.age = 25; save!`, any other time it will work without `self` – Andrey Deineko Feb 17 '16 at 07:27
@chrishough also regarding the reference you've provided - I did not have time to read it thoroughly, but I bet that deleting the `before_validation` and adding presence validation would work – Andrey Deineko Feb 17 '16 at 07:37
Unfortunately after more testing today, this did not work. I had to switch the method to def find_company existing_company = Company.where(email: company.email) if company self.company = existing_company.first if existing_company.count > 0 end The before_validation is needed because it passing attributes to the other model. If you take that off it will always attempt to create a duplicate record. – Chris Hough Feb 17 '16 at 19:52
you are screwing thing hard with this `before_validation`. I use nested_attributes on daily basis and never had I to set up before_validation, that you claim to be required. But hey, that's your code after all, not mine ;) – Andrey Deineko Feb 17 '16 at 20:00
Question though, not trying to say this works the best, but are your nested attributes creating a new record each time or handling find or create? – Chris Hough Feb 17 '16 at 20:50
@chrishough it is unacceptable, so no way, no. I do not see the reasoning behind nested attributes creating a new record each time something happens. I think you have to just debug and see, what's going on and why, when not having this before validation you allow nested attrs to create new record. May be somewhere in controller, may be your form does that, may be you do not have proper nested model validations - lots of things to check :) – Andrey Deineko Feb 17 '16 at 20:55
maybe I am a bit confused here, but it did create in the past correctly, however, if I tried to create another job with the same company email address it through an error. I only permit companies with unique email addresses. If I removed that it would work, but than the db would have duplicates. everything else checked out. am I crazy here? – Chris Hough Feb 17 '16 at 21:35

score 0 · Answer 2 · answered Feb 18 '16 at 23:14

In order to fix the issue with find or create by and pass through rubocop successfully this variation solved the problem

  private

  def find_company
    existing_company = Company.where(email: company.email) if company
    self.company = existing_company.first if existing_company.count > 0
  end

rubocop style model self needed but triggering warning

2 Answers2