Devise after_sign_in_path_for not working; being ignored when model has validations on: :update

Question

My method is executing, but Devise is not using the return value at all. On the sign in page, it just reloads the page with a 'Signed in successfully' notice. It doesn't redirect to the value returned from the method.

Log

Started POST "/users/sign_in" for 127.0.0.1 at 2018-03-05 22:19:50 -0500
Processing by Users::SessionsController#create as HTML
  Parameters: {"utf8"=>"√", "authenticity_token"=>"tQd5a43StP85oyyCpEmFU8cAkFXdJL2OLpuAK1+sqQC6/rIqcd+fB2iE4RT0RoPKPCqreNBYlv2bxjl9gZFrWg==", "user"=>{"email"=>"[email protected]", "password"=>"[FILTERED]", "remember_me"=>"0"}, "commit"=>"Log in"}
  User Load (2.0ms)  SELECT  "users".* FROM "users" WHERE "users"."email" = $1 ORDER BY "users"."id" ASC LIMIT $2  [["email", "[email protected]"], ["LIMIT", 1]]
   (5.0ms)  BEGIN
  User Exists (3.0ms)  SELECT  1 AS one FROM "users" WHERE "users"."email" = $1 AND ("users"."id" != $2) LIMIT $3  [["email", "[email protected]"], ["id", 23], ["LIMIT", 1]]
  Sector Load (0.0ms)  SELECT "sectors".* FROM "sectors" INNER JOIN "sectors_users" ON "sectors"."id" = "sectors_users"."sector_id" WHERE "sectors_users"."user_id" = $1  [["user_id", 23]]
  Region Load (0.0ms)  SELECT "regions".* FROM "regions" INNER JOIN "regions_users" ON "regions"."id" = "regions_users"."region_id" WHERE "regions_users"."user_id" = $1  [["user_id", 23]]
  Criterium Load (0.0ms)  SELECT "criteria".* FROM "criteria" INNER JOIN "criteria_users" ON "criteria"."id" = "criteria_users"."criterium_id" WHERE "criteria_users"."user_id" = $1  [["user_id", 23]]
  AssetType Load (0.0ms)  SELECT "asset_types".* FROM "asset_types" INNER JOIN "asset_types_users" ON "asset_types"."id" = "asset_types_users"."asset_type_id" WHERE "asset_types_users"."user_id" = $1  [["user_id", 23]]
  Company Load (1.0ms)  SELECT  "companies".* FROM "companies" WHERE "companies"."id" = $1 LIMIT $2  [["id", 42], ["LIMIT", 1]]
   (5.0ms)  ROLLBACK
############### /users/23/edit
  Rendering users/sessions/new.haml within layouts/application
  Rendered users/shared/_links.html.erb (3.0ms)
  Rendered users/sessions/new.haml within layouts/application (251.2ms)
  Rendered layouts/_footer.haml (15.0ms)
Completed 200 OK in 6554ms (Views: 3364.9ms | ActiveRecord: 86.1ms)

Notice it is rendering users/sessions/new.haml instead of the edit page?

Code

class ApplicationController < ActionController::Base
...
  def after_sign_in_path_for(resource)
    logger.debug '############### ' + edit_user_path(resource) if resource.is_a?(User) && resource.signature.blank?
    return edit_user_path resource if resource.is_a?(User) && resource.signature.blank?
    stored_location_for(resource) ||
      if resource.is_a?(User)
        dashboard_path
      elsif resource.is_a?(Facilitator) && resource.name.nil?
        edit_facilitator_path resource
      elsif resource.is_a?(Facilitator)
        facilitator_path resource
      else
        super
      end
  end

I completely commented out the method and it still reloaded the login page.

Started POST "/users/sign_in" for 127.0.0.1 at 2018-03-05 22:25:21 -0500
...
  Rendering users/sessions/new.haml within layouts/application

Devise 4.4.0

Documentation:

https://github.com/plataformatec/devise/wiki/How-To%3A-Redirect-to-a-specific-page-on-successful-sign-in-and-sign-out

http://www.rubydoc.info/github/plataformatec/devise/master/Devise/Controllers/Helpers:after_sign_in_path_for

---

I added

  def after_sign_in_path_for(resource)
    logger.debug '############# ' + resource.errors.full_messages.join(', ')

And did discover validation errors like

 ############# Title can't be blank, Country can't be blank, Signature can't be blank, ...

But it does show the notice

Signed in successfully.

And I do have a session and can navigate elsewhere. My validations are on: :update.

  validates :email, :name, :title, :phone, :address1, :city, :state, :zip, :country, :type, :signature, presence: true, on: :update

This should not cause log in behavior errors.

---

I commented all validations on the model and it does work, but this is highly unusual! Validations should not affect login behavior. There has to be a workaround.

Started POST "/users/sign_in" for 127.0.0.1 at 2018-03-05 23:11:43 -0500
  SQL (15.0ms)  UPDATE "users" SET "current_sign_in_at" = $1, "last_sign_in_at" = $2, "current_sign_in_ip" = $3, "sign_in_count" = $4, "updated_at" = $5 WHERE "users"."id" = $6  [["current_sign_in_at", "2018-03-06 04:11:44.225501"], ["last_sign_in_at", "2017-11-09 01:22:28.245231"], ["current_sign_in_ip", "127.0.0.1/32"], ["sign_in_count", 6], ["updated_at", "2018-03-06 04:11:44.230506"], ["id", 23]]
Redirected to http://localhost:3000/users/23/edit
Completed 302 Found in 2183ms (ActiveRecord: 48.0ms)

Perfective

Maybe throw some logger.debug statements in the conditional block so you know where the code is going? – Silsby

Note: i don't know if this is your issue, but I note that ruby can sometimes get confused about parentheses. I'd use edit_user_path(resource) in case it's getting confused as to where to put the if. eg maybe it's doing something like: edit_user_path(resource if resource.is_a?(User) && resource.signature.blank?) Secondly: I find the ROLLBACK a bit odd... why does it seem to be failing to load the user from the db? – Sponge

@CD-RUM If you see the results of the debug statements, you can see that the conditional results to true and so it returns immediately after the last debug statement. – Perfective

@TarynEast That's not valid Ruby if the parenthesis are that way. I added them to the argument anyways but it didn't help. I removed some SQL statements from the log. It does a User Load, a BEGIN transaction, User Exists, then loads several relations, then ROLLBACK. I don't know why either. Probably because Devise is setting a last login time or something and saving the record and it's validating? – Perfective

Rollback, though, cancels the save. It can be interpreted as valid ruby, and return a nil if the conditional: resource.is_a?(User) && resource.signature.blank? fails. It's definitely not good ruby... definitely wouldn't be clear ruby and definitely not the intended ruby... thus why being specific helps. Good to know you actually tried it and it isn't the problem though - we can rule that edge case out. Debugging consists of ruling out the possible causes of failure one by one until you find the actual issue(s) :) – Sponge

Put a puts after the return edit_user-path line... and see if we are returning this path at this point... for all we know, the bug could be not one in this method but somewhere else... Do you have a check/redirect on edit_user-path for example? Can you show us your routes file for users? – Sponge

This is a bug with Devise 4.4. #4742. gem 'devise', '~> 4.3.0' – Perfective

you have the standard devise routes? can we see the output of rake routes | grep devise or grep users or grep sessions – Herrmann

also I would try to remove all this code and reproduce a simple test case for this, because I believe most of the code included in this question is not relevant for your issue – Herrmann

@FabrizioBertoglio did mention that I did comment the method and it still failed. I completely commented out the method and it still reloaded the login page. Here are routes for Users and sessions and registrations. gist.github.com/starrychloe/dd3aede7357653301b16e28afe7c2215 A lot of other routes unrelated left out. – Perfective

Answer 1

As you only want your validations on update, I guess that you only need them for a specific form, since your users are still valid even without this validations. In that case I would use a so called form object, that does the on update validations for you and remove the on update validations on your user model. In that case your validations don't affect other parts of your app.

Here is a good guide on how to do that with just using ActiveModel.

Example:

app/models/user.rb

class User < ApplicationRecord
  # remove the validations here
end

app/forms/user_edit_form.rb

class UserEditForm
    include ActiveModel::Model

    ATTRIBUTES = :email, :name, :title, :phone, 
                 :address1, :city, :state, :zip, 
                 :country, :type, :signature
    attr_accessor *ATTRIBUTES

    validates *ATTRIBUTES, presence: true

    def update(user)
      if valid?
        user.update(self.attributes)
      end
    end

    def self.for_user(user)
      new(user.slice(*ATTRIBUTES)
    end
  end

users_controller.rb

class UsersController
  def edit
    @user = User.find(params[:id])
    @user_edit_form = UserEditForm.for_user(@user)
  end

  def update
    @user = User.find(params[:id])
    @user_edit_form = UserEditForm.new(user_update_params).update(@user)
    if @user_edit_form.errors?
      render :edit
    else 
      redirect_to user_path(@user)
    end
  end

 def user_update_params
    # ...
 end
end

edit.html.erb

<%= form_for @user_edit_form, url: user_path(@user), method: :patch do |f| %>
  # ...

  <%= f.submit %>
<% end %>

Alternative

An alternative could be to add a virtual attribute to the model and run your validations conditionally in the user controller.

class User < ApplicationRecord
  attr_accessor :profile_complete

  with_options if: -> { profile_complete } do
    validates :email, :name, :title, :phone, :address1, :city, :state, :zip, :country, :type, :signature, presence: true
  end
end

users_controller.rb

class UsersController < ApplicationController
  def update
    @user = User.find(params[:id])
    @user.profile_complete = true 
    if @user.update(user_update_params)
      redirect_to @user
    else 
      render :edit
    end

    # ...
  end
end

Note: Instead of using a virtual attribute (attr_accessor) you could also use a real DB attribute, so you can also actually know which users filled out their profile completely.

Alternative 2

In some other projects I also used state machine gems (there are a couple e.g. aasm or statemachines-activerecord) to do somehing similar. Some of the state machine gems even support having validations only for certain states or transisions.

Galbreath

OK but that example doesn't show how to only run validations on: :update and only for a single form. And you said "remove validations on the model" but the example does use validations on the model. Having a user register with a simple Name/Email/Password and then filling in a 100 question profile later is standard practice on the web. No use is going to register if they had to fill in a giant questionnaire. – Perfective

The example does not use validations on the model, but uses an extra ActiveModel class that sort of wraps the user model and only is used in that one controller. I will update my post with an example. – Galbreath

I can give an example for the other approach as well if you wish. – Galbreath

Wow that's certainly clever, and I suppose it will work, but it seems entirely unnecessary, over complicated, and anti-DRY. I have like 50 attributes/relations on user, custom setters, and about a dozen validations. That would compare to a single line fix in Devise to use #update_columns instead of save(validate: true). (Not to mention Devise :trackable is now broken as it enables a user to login with a session without saving their IP/timestamp. Not to mention it leaves broken Rails on: :update.) – Perfective

Added another example for the alternative approach, which is simpler and maybe more what you are looking for. But keep in mind, when rails applications grow big, the first approach might be the right way to go, since you want to keep your models free from form logic (which actually belongs to the View in MVC rather than the Model. With form objects you can encapsulate the View Logic away from you models. Might look less DRY first, but there are complete gems like trailblazers reform (trailblazer.to/gems/reform) that help you go down this approach. – Galbreath

Answer 2

You might need conditional validation on your model. Something like this:

 validates :email, :name, :title, :phone, :address1, :city, :state, :zip, :country, :type, :signature, presence: true, on: :update, unless: Proc.new {|user| user.current_sign_in_at.present? }

Devise will update sign_in_at whenever sign_in happens. Which will trigger update action and related validations.

Also Documentation said the allow_nil: true instruct the model to validate the fields ONLY if it exists on the submitted form.

Sponger

NoMethodError (undefined method 'sign_in_at' for #<User:0x000000125f19e8> Did you mean? sign_in_count) – Perfective

whatever attribute which is update on devise login. Edited to current_sign_in_at – Ferriferous

current_sign_in_at | timestamp without time zone is a database field. It's always present and never blank (except new unconfirmed users). That would render the validations a NO-OP (no operation; like commented out). That would allow the user to edit their profile and leave out required information. – Perfective

I mean if current_sign_in_at only presents in updating attributes. Not entire object. – Ferriferous

Answer 3

Check this documentation https://github.com/plataformatec/devise/wiki/How-To:-redirect-to-a-specific-page-on-successful-sign-in. They have clearly mentioned when you will go in loop and solution for it. Check Preventing redirect loops section in above doc.

Sputnik

Not getting loops. That's for PasswordController and RegistrationController anyways. It's a bug in Devise. See last comment under question. – Perfective