Add FosterAccount association to AdopterFosterProfile

[ErinClaudio]

🔗 Issue

#449

✍️ Description

📷 Screenshots/Demos

[mononoken]

Hey, Erin!

I took a look at your branch. My main comment that I think is affecting this, but not the direct cause of the error, is that this association seems incorrect:

# app/models/adopter_foster_profile.rb:59
  belongs_to :foster_account, class_name: “User”, foreign_key: :adopter_account_id

I think this should be a simple reference to a FosterAccount. If that is not the intention, then it should definitely be named something else since FosterAccount is a model in the app.

However, that creates another issue. belongs_to associations inherently create presence validations in Rails. That means AdopterFosterProfile would require both an AdopterAccount AND a FosterAccount. I think the intention is for one or the other to be required, not both (correct me if I am wrong).

If the intention is for either an adopter_account OR foster_account, I think a polymorphic association for an account could work here. With that, each profile would have one associated account, of either foster type or adopter type.

However, if the intention is for each Profile to have a reference to both, then we could add optional: true to the foster_account association. That would mean every fosterer would have to be an adopter too though, which I am not sure is intended or not.

[ErinClaudio]

@mononoken , thank you for your feedback. After considering your input and reviewing the ticket, I agree that a user should be either an AdopterAccount, FosterAccount, or StaffAccount, not "And." I will proceed with your suggested changes, including creating a polymorphic association. Thank you. @kasugaijin , I'm double-checking with you before moving forward with these adjustments.

[kasugaijin]

@ErinClaudio So there are cases where a User could be an Adopter and a Fosterer
Can't we just do this?

class AdopterFosterProfile < ApplicationRecord
  belongs_to :location, dependent: :destroy
  belongs_to :adopter_account
  **belongs_to :foster_account**
  accepts_nested_attributes_for :location
  validates_associated :location

[ErinClaudio]

@kasugaijin Yup I will go that route

[mononoken]

@ErinClaudio So there are cases where a User could be an Adopter and a Fosterer Can't we just do this?

class AdopterFosterProfile < ApplicationRecord
  belongs_to :location, dependent: :destroy
  belongs_to :adopter_account
  **belongs_to :foster_account**
  accepts_nested_attributes_for :location
  validates_associated :location

Since this is the case, we do need to make one or both accounts optional too. belongs_to creates a presence validation by default. That means these associations would cause AdopterFosterProfile to be invalid when a User only has an AdopterAccount (and I think this is probably behind the seed file errors you were getting).

...
However, if the intention is for each Profile to have a reference to both, then we could add optional: true to the foster_account association. That would mean every fosterer would have to be an adopter too though, which I am not sure is intended or not.

I'm thinking both accounts should be optional then, and we could write a custom validation to check the presence of at least one account.

[kasugaijin]

You know @mononoken was right, polymorphic might be a better suit here. My suggestion would require custom code and we should avoid that when we can, @ErinClaudio

https://guides.rubyonrails.org/association_basics.html#polymorphic-associations

# Model AdopterAccount
class AdopterAccount < ApplicationRecord
  has_one :adopter_foster_profile, as: :parent
end

# Model FosterAccount
class FosterAccount < ApplicationRecord
  has_one :adopter_foster_profile, as: :parent
end

# Model AdopterFosterProfile
class AdopterFosterProfile < ApplicationRecord
  belongs_to :parent, polymorphic: true
end

[ErinClaudio]

@mononoken @kasugaijin Thank you

[kasugaijin]

@ErinClaudio check out this draft PR I made to help you get the seeds to build - see the video as well for explanation.
#502

[ErinClaudio]

@kasugaijin working on this now with your direction. I will close this PR after I have made the required changes