Merge pull request #4290 from rubyforgood/revert-4257-serve-most-rece…

…nt-role

Revert "feat(#3345): serve last user role on login"

app/controllers/application_controller.rb

@@ -37,6 +37,7 @@ def current_role
     return nil unless current_user
 
     @role = Role.find_by(id: session[:current_role]) || UsersRole.current_role_for(current_user)
+    session[:current_role] = @role&.id
 
     @role
   end

app/controllers/users/sessions_controller.rb

@@ -17,8 +17,7 @@ def new
   # POST /resource/sign_in
   def create
     super
-    session[:current_role] ||= UsersRole.current_role_for(current_user)&.id
-    UsersRole.set_last_role_for(current_user, @role)
+    session[:current_role] = UsersRole.current_role_for(current_user)&.id
   end
 
   # DELETE /resource/sign_out

app/controllers/users_controller.rb

@@ -17,7 +17,6 @@ def switch_to_role
     end
 
     @role = role
-    UsersRole.set_last_role_for(current_user, role)
     session[:current_role] = params[:role_id]
     redirect_to dashboard_path_from_current_role
   end

app/models/user.rb

@@ -30,7 +30,6 @@
 #  created_at             :datetime         not null
 #  updated_at             :datetime         not null
 #  invited_by_id          :integer
-#  last_role_id           :bigint
 #  organization_id        :integer
 #  partner_id             :bigint
 #
@@ -43,10 +42,6 @@ class User < ApplicationRecord
 
   has_one :organization_role_join, class_name: "UsersRole", dependent: :destroy
   has_one :organization_role, through: :organization_role_join, class_name: "Role", source: :role
-
-  belongs_to :last_role_join, class_name: "UsersRole", optional: true, inverse_of: :user, foreign_key: :last_role_id
-  has_one :last_role, through: :last_role_join, class_name: "Role", source: :role
-
   accepts_nested_attributes_for :organization_role_join
   has_one :organization, through: :organization_role, source: :resource, source_type: "Organization"
   has_many :organizations, through: :roles, source: :resource, source_type: "Organization"

app/models/users_role.rb

@@ -10,18 +10,12 @@ class UsersRole < ApplicationRecord
   has_paper_trail
   belongs_to :user
   belongs_to :role
-  has_one :last_user, class_name: "User", foreign_key: :last_role_id, inverse_of: :last_role, dependent: :nullify
 
   accepts_nested_attributes_for :user
 
-  validates :role, uniqueness: {scope: :user}
-
   # @param user [User]
   # @return [Role,nil]
   def self.current_role_for(user)
-    return nil if user.nil?
-    return user.last_role if user.last_role
-
     role_order = [Role::SUPER_ADMIN, Role::ORG_ADMIN, Role::ORG_USER, Role::PARTNER]
     role_order.each do |role|
       found_role = user&.roles&.find { |r| r.name.to_sym == role }
@@ -30,13 +24,4 @@ def self.current_role_for(user)
 
     nil
   end
-
-  # @param user [User]
-  # @param role [Role]
-  def self.set_last_role_for(user, role)
-    users_role = UsersRole.find_by(user: user, role: role)
-    return if users_role.nil?
-
-    user.update(last_role_id: users_role.id)
-  end
 end

app/services/remove_role_service.rb

@@ -18,7 +18,6 @@ def self.call(user_id:, role_id: nil, resource_type: nil, resource_id: nil)
     end
 
     user_role.destroy
-
     if user_role.role.name.to_sym == Role::ORG_USER # they can't be an admin if they're not a user
       admin_role = Role.find_by(resource_id: user_role.role.resource_id, name: Role::ORG_ADMIN)
       if admin_role

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.0].define(version: 2024_04_05_154448) do
+ActiveRecord::Schema[7.0].define(version: 2024_04_02_230156) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
@@ -810,14 +810,12 @@
     t.string "provider"
     t.string "uid"
     t.bigint "partner_id"
-    t.bigint "last_role_id"
     t.index ["discarded_at"], name: "index_users_on_discarded_at"
     t.index ["email"], name: "index_users_on_email", unique: true
     t.index ["invitation_token"], name: "index_users_on_invitation_token", unique: true
     t.index ["invitations_count"], name: "index_users_on_invitations_count"
     t.index ["invited_by_id"], name: "index_users_on_invited_by_id"
     t.index ["invited_by_type", "invited_by_id"], name: "index_users_on_invited_by_type_and_invited_by_id"
-    t.index ["last_role_id"], name: "index_users_on_last_role_id"
     t.index ["partner_id"], name: "index_users_on_partner_id"
     t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true
   end
@@ -896,5 +894,4 @@
   add_foreign_key "requests", "distributions"
   add_foreign_key "requests", "organizations"
   add_foreign_key "requests", "partners"
-  add_foreign_key "users", "users_roles", column: "last_role_id", on_delete: :nullify
 end

spec/factories/users.rb

@@ -30,7 +30,6 @@
 #  created_at             :datetime         not null
 #  updated_at             :datetime         not null
 #  invited_by_id          :integer
-#  last_role_id           :bigint
 #  organization_id        :integer
 #  partner_id             :bigint
 #

spec/models/user_spec.rb

@@ -30,7 +30,6 @@
 #  created_at             :datetime         not null
 #  updated_at             :datetime         not null
 #  invited_by_id          :integer
-#  last_role_id           :bigint
 #  organization_id        :integer
 #  partner_id             :bigint
 #

spec/models/users_role_spec.rb

@@ -8,66 +8,31 @@
 #
 RSpec.describe UsersRole, type: :model do
   describe "#current_role_for" do
-    context "when last_role is nil" do
-      context "for org user" do
-        it "should return org user" do
-          user = create(:user)
-          expect(UsersRole.current_role_for(user).name).to eq("org_user")
-        end
-      end
-
-      context "for org admin" do
-        it "should return org admin" do
-          user = create(:organization_admin)
-          expect(UsersRole.current_role_for(user).name).to eq("org_admin")
-        end
-      end
-
-      context "for super admin" do
-        it "should return super admin" do
-          user = create(:super_admin)
-          expect(UsersRole.current_role_for(user).name).to eq("super_admin")
-        end
-      end
-
-      context "for partner user" do
-        it "should return partner user" do
-          user = create(:partner_user)
-          expect(UsersRole.current_role_for(user).name).to eq("partner")
-        end
+    context "for org user" do
+      it "should return org user" do
+        user = FactoryBot.create(:user)
+        expect(described_class.current_role_for(user).name).to eq("org_user")
       end
     end
-    context "when last_role is not nil" do
-      it "should return last role" do
-        user = create(:partner_user)
 
-        UsersRole.set_last_role_for(user, user.roles.last)
-
-        user.add_role(Role::ORG_ADMIN, create(:organization))
-        expect(UsersRole.current_role_for(user).name).to eq("partner")
+    context "for org admin" do
+      it "should return org admin" do
+        user = FactoryBot.create(:organization_admin)
+        expect(described_class.current_role_for(user).name).to eq("org_admin")
       end
     end
-  end
-
-  describe "#set_last_role_for" do
-    context "when user has the role" do
-      it "should set last role" do
-        user = create(:partner_user)
-        role = user.roles.first
 
-        UsersRole.set_last_role_for(user, role)
-
-        expect(user.last_role).to eq(role)
+    context "for super admin" do
+      it "should return super admin" do
+        user = FactoryBot.create(:super_admin)
+        expect(described_class.current_role_for(user).name).to eq("super_admin")
       end
     end
 
-    context "when user does not have the role" do
-      it "should not set last role" do
-        user = create(:partner_user)
-        role = Role.find_by(name: "org_user")
-
-        UsersRole.set_last_role_for(user, role)
-        expect(user.last_role).to eq(nil)
+    context "for partner user" do
+      it "should return partner user" do
+        user = FactoryBot.create(:partner_user)
+        expect(described_class.current_role_for(user).name).to eq("partner")
       end
     end
   end

spec/requests/sessions_requests_spec.rb

@@ -19,89 +19,4 @@
       expect(response).to redirect_to(dashboard_url(@organization))
     end
   end
-
-  describe "POST #create" do
-    let(:partner_user) { create(:partner_user) }
-    let(:partner_role) { partner_user.roles.first }
-    let(:organization) { create(:organization) }
-
-    context "when an org_admin with a last used partner role" do
-      before do
-        partner_user.add_role(Role::ORG_ADMIN, organization)
-        UsersRole.set_last_role_for(partner_user, partner_role)
-      end
-
-      it "signs in with partner role" do
-        post user_session_path, params: {user: {email: partner_user.email, password: "password!"}}
-        get root_path
-        expect(response).to redirect_to(partners_dashboard_url)
-      end
-    end
-
-    context "when last used role is revoked" do
-      before do
-        UsersRole.set_last_role_for(partner_user, partner_role)
-        partner_role.destroy
-      end
-
-      it "does not sign in as partner_user" do
-        post user_session_path, params: {user: {email: partner_user.email, password: "password!"}}
-        get root_path
-        expect(response).to_not redirect_to(partners_dashboard_url)
-      end
-    end
-
-    context "when partner_role saved in session" do
-      before do
-        post user_session_path, params: {user: {email: partner_user.email, password: "password!"}}
-        get root_path
-      end
-
-      context "when org_admin role is added and no last_role exists" do
-        before { partner_user.add_role(Role::ORG_ADMIN, organization) }
-
-        it "prefers session role" do
-          post user_session_path, params: {user: {email: partner_user.email, password: "password!"}}
-          get root_path
-
-          expect(response).to redirect_to(partners_dashboard_url)
-        end
-
-        context "when session role is revoked" do
-          before { partner_role.destroy }
-
-          it "prefers org_admin" do
-            post user_session_path, params: {user: {email: partner_user.email, password: "password!"}}
-            get root_path
-
-            expect(response).to redirect_to(dashboard_url(organization))
-          end
-        end
-      end
-
-      context "when org_admin role is added, no last_role exists" do
-        it "prefers partner" do
-          partner_user.add_role(Role::ORG_ADMIN, organization)
-
-          post user_session_path, params: {user: {email: partner_user.email, password: "password!"}}
-          get root_path
-
-          expect(response).to redirect_to(partners_dashboard_url)
-        end
-      end
-    end
-
-    context "without a previously used role" do
-      before do
-        partner_user.add_role(Role::ORG_ADMIN, organization)
-        expect(partner_user.last_role).to be_nil
-      end
-
-      it "signs in as org_admin role" do
-        post user_session_path, params: {user: {email: partner_user.email, password: "password!"}}
-        get root_path
-        expect(response).to redirect_to(dashboard_url(organization))
-      end
-    end
-  end
 end

spec/requests/users_requests_spec.rb

@@ -69,17 +69,6 @@
         # avoid it
         expect(response).to redirect_to(partners_dashboard_path(organization_name: @organization.to_param))
       end
-
-      it "should set last_role to partner" do
-        @user.add_role(Role::PARTNER, partner)
-
-        partner_role = @user.roles.find { |r| r.name == Role::PARTNER.to_s }
-
-        expect do
-          get switch_to_role_users_path(@organization,
-            role_id: partner_role.id)
-        end.to change(@user, :last_role).from(nil).to(partner_role)
-      end
     end
 
     context "without a partner role" do

spec/services/remove_role_service_spec.rb

@@ -18,17 +18,6 @@
         expect(user.reload.has_role?(:org_admin, org)).to eq(false)
       end
 
-      it "should remove last role if users role is removed" do
-        AddRoleService.call(user_id: user.id, resource_type: "org_user", resource_id: org.id)
-        UsersRole.set_last_role_for(user, role)
-        expect(user.last_role).to eq(role)
-
-        described_class.call(user_id: user.id, role_id: role.id)
-
-        expect(user.reload.has_role?(:org_user, org)).to eq(false)
-        expect(user.reload.last_role).to eq(nil)
-      end
-
       it "should work with a type and ID instead of role ID" do
         AddRoleService.call(user_id: user.id, resource_type: "org_user", resource_id: org.id)
         described_class.call(user_id: user.id, resource_type: "org_user", resource_id: org.id)