Handle edgecase where a user cannot have all roles removed.

When updating a user, I cannot remove the last role from that user. This
is due to an assumption that was made that the form still submitted the
param `spree_role_ids` with an empty array if none of the checkboxes
were checked.

This assumption was false. We have to add behavior around whether or not
the roles can be cleared if that parameter is not present which is not
ideal as explained in the comment in users_controller#set_roles.

backend/app/controllers/spree/admin/users_controller.rb

@@ -159,9 +159,16 @@ def load_stock_locations
         end
 
         def set_roles
-          return unless user_params[:spree_role_ids]
-
-          @user.spree_roles = Spree::Role.where(id: user_params[:spree_role_ids])
+          # FIXME: user_params permits the roles that can be set, if spree_role_ids is set.
+          # when submitting a user with no roles, the param is not present. Because users can be updated
+          # with some users being able to set roles, and some users not being able to set roles, we have to check
+          # if the roles should be cleared, or unchanged again here. The roles form should probably hit a seperate
+          # action or controller to remedy this.
+          if user_params[:spree_role_ids]
+            @user.spree_roles = Spree::Role.where(id: user_params[:spree_role_ids])
+          elsif can?(:manage, Spree::Role)
+            @user.spree_roles = []
+          end
         end
 
         def set_stock_locations

backend/spec/controllers/spree/admin/users_controller_spec.rb

@@ -77,7 +77,12 @@
     context "when the user can manage roles" do
       it "can set roles" do
         expect(mock_user).to receive(:spree_roles=).with([dummy_role])
-        spree_post :create, { user: { spree_role_ids: [dummy_role.id] } }
+        spree_post :create, { id: mock_user.id, user: { first_name: "Bob", spree_role_ids: [dummy_role.id] } }
+      end
+
+      it "can clear roles" do
+        expect(mock_user).to receive(:spree_roles=).with([])
+        spree_post :create, { id: mock_user.id, user: { first_name: "Bob"}  }
       end
     end
 
@@ -90,6 +95,16 @@
         expect(mock_user).to_not receive(:spree_roles=)
         spree_post :create, { user: { spree_role_ids: [dummy_role.id] } }
       end
+
+      it "cannot set roles" do
+        expect(mock_user).to_not receive(:spree_roles=)
+        spree_post :create, { id: mock_user.id, user: { first_name: "Bob", spree_role_ids: [dummy_role.id] } }
+      end
+
+      it "cannot clear roles" do
+        expect(mock_user).to_not receive(:spree_roles=)
+        spree_post :create, { id: mock_user.id, user: { first_name: "Bob"}  }
+      end
     end
 
     it "can create a shipping_address" do
@@ -125,7 +140,12 @@
     context "when the user can manage roles" do
       it "can set roles" do
         expect(mock_user).to receive(:spree_roles=).with([dummy_role])
-        spree_put :update, { id: mock_user.id, user: { spree_role_ids: [dummy_role.id] } }
+        spree_put :update, { id: mock_user.id, user: { first_name: "Bob", spree_role_ids: [dummy_role.id] } }
+      end
+
+      it "can clear roles" do
+        expect(mock_user).to receive(:spree_roles=).with([])
+        spree_put :update, { id: mock_user.id, user: { first_name: "Bob"}  }
       end
     end
 
@@ -136,7 +156,12 @@
 
       it "cannot set roles" do
         expect(mock_user).to_not receive(:spree_roles=)
-        spree_put :update, { id: mock_user.id, user: { spree_role_ids: [dummy_role.id] } }
+        spree_put :update, { id: mock_user.id, user: { first_name: "Bob", spree_role_ids: [dummy_role.id] } }
+      end
+
+      it "cannot clear roles" do
+        expect(mock_user).to_not receive(:spree_roles=)
+        spree_put :update, { id: mock_user.id, user: { first_name: "Bob" }  }
       end
     end