Managing users and profiles in Rails

I am building a Rails marketplace application using TDD. I would like to get advice on the way in which I have built the User associated Profile associations and way in which these are then handled in the controllers.

User model

class User  { uniq }, :through => :watches, dependent: :destroy

  # Include default devise modules. Others available are:

  acts_as_messageable

  def mailboxer_email(object)
    email
  end

  def self.search(search)
    where("email ILIKE ?", "%#{search}%") 
  end

  after_create :build_profile

end

Profile model

class Profile  { :medium => "300x300>", :thumb => "100x100>" }, :default_url => "/images/:style/missing.png"
  validates_attachment_content_type :avatar, :content_type => /\Aimage\/.*\Z/
end

Users controller

class UsersController < ApplicationController
  def index
    @users = User.all
    if params[:search]
      @users = User.search(params[:search]).order("created_at DESC")
    else
      @users = User.all.order('created_at DESC')
    end
  end

  def show
    @user = User.find(params[:id])
  end

  def watchlist
    @watched_listings = current_user.watched_listings.all
  end
end

Profiles controller

```
class ProfilesController < ApplicationController

before_filter :authenticate_user!, :only [:edit, :update]
before_filter :correct_user, :only [:edit, :update]

def show
@profile = Profile.find_by(user_id: params[:user_id])
end

def edit
@profile = Profile.find_by user_id: current_user.id
end

def update
@profile = Profile.find_by user_id: current_user.id
if @profile.update(profile_params)
flash[:notices] = ["Your profile was successfully updated"]
render 'show'
else
flash[:notices] = ["Your profile could not be updated"]
render 'edit'
end
end

private

def profile_params
params.require(:profile).permit(:city, :country, :avatar)
end

def correct_user
@profile = Profile.find_by(user_id: params[:user_id])
redir

Here's my thoughts.
Profile Model

I'm not sure that the separate profile is pulling enough weight to justify its existence. It seems that a lot of this code could be avoided by collapsing it into the User model. I appreciate that the User and Profile can be two separate entities but it looks like it'll be much easier to keep them in one model until they grow too large to be viable, or there comes a time when a Profile has a separate lifecycle from a User.

Which then avoids your question around SRP altogether :)

But if we were to keep them separate, I don't think it would be a practical violation of SRP because your application likely depends on a profile always existing for a user. So having asserted that User should always have a Profile, you are then faced with the choice of leaving the creation in the hands of the controller, or the model. I favour leaving it in the hands of the model because the model is responsible for modelling the relationships between different business objects.
Finding the Profile and Validating Access

You could replace the find methods with a before_action that uses the user's relationship with the profile like so:

before_action :get_profile, only: [:edit, :update]

def get_profile
   @profile = current_user.profile
end

This will both reduce the amount of code required to select the profile, and ensure that the user can only access their profile to edit/update it.

However, at that point it doesn't make sense to have the profile edit and update actions available with a user ID. They're essentially now singular resources and should be handled accordingly in your routes.

You ask whether find by user_id or not is a good choice. It depends on whether the person accessing that route will be accessing it from the context of its User, or the Profile owner.
current_user?

I don't believe this code will work. You are passing it only Profile objects, and comparing it to the current_user method which returns a User object (or null if not logged in, if memory serves). This should always return false. You either need to call profile.user before passing it to the method, or have that method make the call.

StackExchange Code Review Q#112277, answer score: 2