replace org_user deactivation with removal of org_user role

app/controllers/organizations_controller.rb

@@ -65,18 +65,17 @@ def demote_to_user
     end
   end
 
-  def deactivate_user
-    user = User.with_discarded.find_by!(id: params[:user_id])
-    raise ActiveRecord::RecordNotFound unless user.has_role?(Role::ORG_USER, current_organization)
-    user.discard!
-    redirect_to user_update_redirect_path, notice: "User has been deactivated."
-  end
-
-  def reactivate_user
-    user = User.with_discarded.find_by!(id: params[:user_id])
+  def remove_user
+    user = User.find(params[:user_id])
     raise ActiveRecord::RecordNotFound unless user.has_role?(Role::ORG_USER, current_organization)
-    user.undiscard!
-    redirect_to user_update_redirect_path, notice: "User has been reactivated."
+    begin
+      RemoveRoleService.call(user_id: params[:user_id],
+        resource_type: Role::ORG_USER,
+        resource_id: current_organization.id)
+      redirect_to user_update_redirect_path, notice: "User has been removed!"
+    rescue => e
+      redirect_back(fallback_location: organization_path, alert: e.message)
+    end
   end
 
   private

app/controllers/partners/users_controller.rb

@@ -23,6 +23,7 @@ def update
       end
     end
 
+    # partner user creation
     def create
       user = UserInviteService.invite(name: user_params[:name],
         email: user_params[:email],

app/views/users/_organization_user.html.erb

@@ -1,9 +1,6 @@
 <tr>
   <td>
     <%= user.display_name %>
-    <% if user.discarded? %>
-      <span class="badge badge-danger bg-danger">Deactivated</span>
-    <% end %>
   </td>
   <td><%= user.email %></td>
   <td><%= user.kind %> </td>
@@ -18,30 +15,25 @@
           <span class="caret"></span>
         </button>
         <ul class="dropdown-menu">
-          <% if user.kept? %>
-            <li>
-              <%=
-                  edit_button_to(
-                    promote_to_org_admin_organization_path(user_id: user.id),
-                    {text: 'Make admin'},
-                    {method: :post, rel: "nofollow", data: {confirm: 'This will promote the user to admin status. Are you sure that you want to submit this?', size: 'xs'}}
-                  )
-              %>
-            </li>
-            <li>
-              <%= deactivate_button_to deactivate_user_organization_path(user_id: user.id),
-                  {id: dom_id(user), method: :post, class: 'deactivate', rel: "nofollow", data: {confirm: 'This will deactivate the user. Are you sure that you want to submit this?', size: 'xs'}} %>
-            </li>
-          <% else %>
-            <%= reactivate_button_to reactivate_user_organization_path(user_id: user.id),
-                {id: dom_id(user), method: :post, class: 'reactivate', rel: "nofollow", data: {confirm: 'This will reactivate the user. Are you sure that you want to submit this?', size: 'xs'}} %>
-          <% end %>
+          <li>
+            <%=
+              edit_button_to(
+                promote_to_org_admin_organization_path(user_id: user.id),
+                {text: 'Promote to Admin'},
+                {method: :post, rel: "nofollow", data: {confirm: 'This will promote the user to admin status. Are you sure that you want to submit this?', size: 'xs'}}
+              )
+            %>
+          </li>
+          <li>
+            <%= delete_button_to remove_user_organization_path(user_id: user.id),
+              {id: dom_id(user), method: :post, class: 'deactivate', text: "Remove User", rel: "nofollow", data: {confirm: 'This will revoke the user\'s organization permissions. Are you sure that you want to submit this?', size: 'xs'}} %>
+          </li>
         </ul>
       </div>
     <% end %>
     <% if current_user.has_role?(Role::ORG_ADMIN, current_organization) && user.has_role?(Role::ORG_ADMIN, current_organization) %>
       <%= edit_button_to demote_to_user_organization_path(user_id: user.id),
-                 {text: 'Make User'},
+                 {text: 'Demote to User'},
                  {method: :post, rel: "nofollow", data: {confirm: 'This will demote the admin to user status. Are you sure that you want to submit this?', size: 'xs'}} unless user.id == current_user.id %>
     <% end %>
   </td>

config/initializers/devise.rb

@@ -326,3 +326,10 @@
   # so you need to do it manually. For the users scope, it would be:
   # config.omniauth_path_prefix = '/my_engine/users/auth'
 end
+
+Warden::Manager.after_set_user do |user, auth, opts|
+  if user.roles.empty?
+    auth.logout
+    throw(:warden)
+  end
+end

config/routes.rb

@@ -89,8 +89,7 @@ def set_up_flipper
   resource :organization, path: :manage, only: %i(edit update) do
     collection do
       post :invite_user
-      put :deactivate_user
-      put :reactivate_user
+      post :remove_user
       post :resend_user_invitation
       post :promote_to_org_admin
       post :demote_to_user

db/migrate/20240624185108_reactivate_users_and_remove_org_user_roles.rb

@@ -0,0 +1,15 @@
+class ReactivateUsersAndRemoveOrgUserRoles < ActiveRecord::Migration[7.1]
+  def up
+    users = User.unscoped.where.not(discarded_at: nil)
+    users.each do |user|
+      user.transaction do
+        user.update!(discarded_at: nil)
+        user.roles.delete_all
+      end
+    end
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.1].define(version: 2024_06_01_155348) do
+ActiveRecord::Schema[7.1].define(version: 2024_06_24_185108) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 

spec/factories/users.rb

@@ -77,6 +77,10 @@
       end
     end
 
+    trait :no_roles do
+      organization { nil }
+    end
+
     trait :deactivated do
       discarded_at { Time.zone.now }
     end

spec/factories/users_roles.rb

@@ -0,0 +1,14 @@
+# == Schema Information
+#
+# Table name: users_roles
+#
+#  id      :bigint           not null, primary key
+#  role_id :bigint
+#  user_id :bigint
+#
+FactoryBot.define do
+  factory :users_role do
+    user
+    role
+  end
+end

spec/requests/organization_requests_spec.rb

@@ -109,28 +109,28 @@
       end
     end
 
-    describe "PUT #deactivate_user" do
-      subject { put deactivate_user_organization_path(user_id: user.id) }
+    describe "POST #remove_user" do
+      subject { post remove_user_organization_path(user_id: user.id) }
 
-      it "redirect after update" do
-        subject
-        expect(response).to redirect_to(organization_path)
-      end
-      it "deactivates the user" do
-        expect { subject }.to change { user.reload.discarded_at }.to be_present
+      context "when user is org user" do
+        it "redirects after update" do
+          subject
+          expect(response).to redirect_to(organization_path)
+        end
+
+        it "removes the org user role" do
+          expect { subject }.to change { user.has_role?(Role::ORG_USER, organization) }.from(true).to(false)
+        end
       end
-    end
 
-    describe "PUT #reactivate_user" do
-      subject { put reactivate_user_organization_path(user_id: user.id) }
-      before { user.discard! }
+      context "when user is not an org user" do
+        let(:user) { create(:user, organization: create(:organization)) }
 
-      it "redirect after update" do
-        subject
-        expect(response).to redirect_to(organization_path)
-      end
-      it "reactivates the user" do
-        expect { subject }.to change { user.reload.discarded_at }.to be_nil
+        it 'raises an error' do
+          subject
+
+          expect(response).to be_not_found
+        end
       end
     end
 

spec/requests/partners/user_requests_spec.rb

@@ -7,6 +7,7 @@
   before do
     sign_in(partner_user)
   end
+
   describe "GET #edit" do
     it "successfully loads the page" do
       get edit_partners_user_path(partner_user)
@@ -28,4 +29,59 @@
       expect(partner_user.name).to eq "New name"
     end
   end
+
+  describe "POST #create" do
+    let(:params) do
+      {user: {name: "New User", email: "new_partner_email@example.com"}}
+    end
+
+    it "creates a new user" do
+      post partners_users_path, params: params
+
+      aggregate_failures do
+        expect(response.request.flash[:success]).to eq "You have invited New User to join your organization!"
+        expect(response).to redirect_to(partners_users_path)
+      end
+    end
+
+    context "when the user already exists" do
+      let(:organization) { create(:organization) }
+      let(:org_user) { create(:user, name: "Existing User", organization: organization) }
+
+      let(:params) do
+        {user: {name: org_user.name, email: org_user.email}}
+      end
+
+      it "succeeds and adds additional role to user" do
+        post partners_users_path, params: params
+
+        aggregate_failures do
+          expect(response.request.flash[:success]).to eq "You have invited Existing User to join your organization!"
+          expect(response).to redirect_to(partners_users_path)
+          expect(org_user.has_role?(Role::PARTNER, partner)).to be true
+          expect(org_user.has_role?(Role::ORG_USER, organization)).to be true
+        end
+      end
+
+      context "when org user had role removed" do
+        before do
+          sign_in create(:organization_admin, organization: organization)
+          post remove_user_organization_path(user_id: org_user.id)
+          sign_in(partner_user)
+        end
+
+        it "adds role to user" do
+          expect(org_user.has_role?(Role::ORG_USER, organization)).to be false
+
+          post partners_users_path, params: params
+
+          aggregate_failures do
+            expect(response.request.flash[:success]).to eq "You have invited Existing User to join your organization!"
+            expect(response).to redirect_to(partners_users_path)
+            expect(org_user.has_role?(Role::PARTNER, partner)).to be true
+          end
+        end
+      end
+    end
+  end
 end

spec/system/log_in_system_spec.rb

@@ -10,9 +10,23 @@
     end
   end
 
+  describe "User with no roles" do
+    before do
+      create(:user, :no_roles, email: "no_role_user@example.com")
+    end
+
+    it "should not allow the user to log in" do
+      visit "/users/sign_in"
+      fill_in "user_email", with: "no_role_user@example.com"
+      fill_in "user_password", with: DEFAULT_USER_PASSWORD
+      find('input[name="commit"]').click
+      expect(page).to have_content("You need to sign in before continuing.")
+    end
+  end
+
   describe "Deactivated user" do
     before do
-      create(:user, :deactivated, email: "deactivated@exmaple.com")
+      create(:user, :deactivated, email: "deactivated@example.com")
     end
 
     it "should not allow the user to log in" do

spec/system/organization_system_spec.rb

@@ -158,31 +158,19 @@
     it "can see 'Make user' button for admins" do
       create(:organization_admin)
       visit organization_path
-      expect(page.find(".table.border")).to have_content "Make User"
+      expect(page.find(".table.border")).to have_content "Demote to User"
     end
 
-    it "can deactivate a user in the organization" do
-      user = create(:user, name: "User to be deactivated")
+    it "can remove a user from the organization" do
+      user = create(:user, name: "User to be deactivated", organization: organization)
       visit organization_path
       accept_confirm do
         click_button dom_id(user, "dropdownMenu")
-        click_link dom_id(user)
+        click_link "Remove User"
       end
 
-      expect(page).to have_content("User has been deactivated")
-      expect(user.reload.discarded_at).to be_present
-    end
-
-    it "can re-activate a user in the organization" do
-      user = create(:user, :deactivated)
-      visit organization_path
-      accept_confirm do
-        click_button dom_id(user, "dropdownMenu")
-        click_link dom_id(user)
-      end
-
-      expect(page).to have_content("User has been reactivated")
-      expect(user.reload.discarded_at).to be_nil
+      expect(page).to have_content("User has been removed!")
+      expect(user.has_role?(Role::ORG_USER)).to be false
     end
   end
 end