This service object allows users without a session to create a comment, and register OR login at the same time.
If a user is signed out he must enter an email and a password along with the comment.
The service checks if a user exists using the email, then attempts to authenticate the user. Otherwise, it creates a new user.
Three code smells:
- User errors are added to the comment. There is no user object in the view, just a params hash with that name.
- If user validation fails, comment validation does not trigger, meaning if the comment was also blank they need another round trip to find out. Probably not significant though.
- User data is not repopulated on validation failure.
Question: I'm proud of writing a nifty object like that, but it stills feels a little 'wrong-ish' if there is such a word. Is there a better way to handle this type of scenario?
class CommentService
def self.by(params, user)
if user
new params, user
else
new params, find_or_new(params)
end
end
def self.find_or_new(params)
User.find_by(email: params[:user][:email]) || User.new
end
attr_reader :params, :comment, :user
def initialize(params, user)
@params, @user = params, user
@comment = Comment.new(comment_params)
end
def save
if user.new_record?
self.user.attributes = user_params
user.save ? persist : add_user_errors
else
if params[:user].present?
user.authenticate(params[:user][:password]) ? persist : add_auth_error
else
persist
end
end
end
def post
@post ||= Post.find(params[:post_id])
end
private
def add_user_errors
user.errors.each do |k,v|
comment.errors.add(k, v)
end
false
end
def add_auth_error
comment.errors[:base] << "Invalid email or password."
false
end
def user_params
params.require(:user).permit(:name, :email, :password)
end
def comment_params
params.require(:comment).permit(:text)
end
def persist
comment.author = user
comment.post = post
comment.save
end
end
Controller code:
def create
service = CommentService.by(params, current_user)
@post = service.post
if service.save
redirect_to @post
else
@comment = service.comment
render 'new'
end
end
View code:
= form_for [@post, @comment] do |f|
= f.text_area :comment
/ No user object here. Just tossing a user hash into the params.
- unless signed_in?
= fields_for :user do |uf|
= uf.text_field :email
= uf.password_field :password
= f.submit
1 Answer 1
My opinions:
- The class methods
by
and others are unnecessary. - The service object should be initialized by feeding objects instead of params.
- Build a guest user is not job of CommentService.
Then code like these:
# Controller
def create
user = current_user || build_guest_user
@comment = user.new(params[:comment])
service = CommentService.new(@comment, user)
if service.save
if user.guest?
redirect_to 'guest_user_sign_in_or_register'
else
redirect :back, notice: "Comment created"
end
else
# blah blah
end
end
# ApplicationController
def build_guest_user
# your logic here
end
# CommentService
def initialize(comment, user)
@comment = comment
@user = user
end
def save
if @comment.save
# extra service logic here
else
false
end
end
Add
Mohamad, think about this, when a visitor hit a "Post Comment" button, he will reach create
.
If he's logged in, that's fine.
If he not logged in, he will be treated as a guest user, no matter registered or not. -- Then treated by build_guest_user
method.
Then, after comment saved, the controller can judge if he is a guest user or not. If not, nothing to do. If he is, an Ajax template will be rendered back asking his signing in or register.
If he signed in, the comment under the guest will be moved to his user account. And this guest user will be deleted, guest session be replaced by user session.
If he register, same as above.
If he refuse to either sign in or register, the guest user and his comment will be kept there.
-
\$\begingroup\$ Thanks. But how would you handle logging in for existing users? In other words, users who are already registered but not signed in. \$\endgroup\$Mohamad– Mohamad2013年06月20日 14:23:52 +00:00Commented Jun 20, 2013 at 14:23
-
\$\begingroup\$ Not just that, how do we handle an existing user with a login failure? \$\endgroup\$Mohamad– Mohamad2013年06月20日 14:26:57 +00:00Commented Jun 20, 2013 at 14:26
-
\$\begingroup\$ @Mohamad, check my updates. \$\endgroup\$Billy Chan– Billy Chan2013年06月20日 14:34:34 +00:00Commented Jun 20, 2013 at 14:34
-
\$\begingroup\$ @Mohamad, I revised some. The judging guest work is better for controller in my opinion. If no other things needed for comment, service object seems not necessary for this case. \$\endgroup\$Billy Chan– Billy Chan2013年06月20日 14:43:59 +00:00Commented Jun 20, 2013 at 14:43
-
\$\begingroup\$ I tend to agree with you in principle, although this requires creating the notion of a 'guest user' with special validation and additional logic. You will also need to track the comment id in the session to reassign it if the user signs in. I does add a fair amount of complexity. \$\endgroup\$Mohamad– Mohamad2013年06月20日 17:13:59 +00:00Commented Jun 20, 2013 at 17:13