Merge branch '292238-fix-user-availability-update' into 'master'

Fix user status update when not_set availability is given

See merge request gitlab-org/gitlab!49592

app/services/users/set_status_service.rb

@@ -14,10 +14,10 @@ module Users
     def execute
       return false unless can?(current_user, :update_user_status, target_user)
 
-      if params[:emoji].present? || params[:message].present? || params[:availability].present?
-        set_status
-      else
+      if status_cleared?
         remove_status
+      else
+        set_status
       end
     end
 
@@ -25,8 +25,7 @@ module Users
 
     def set_status
       params[:emoji] = UserStatus::DEFAULT_EMOJI if params[:emoji].blank?
-      params.delete(:availability) if params[:availability].blank?
-      return false if params[:availability].present? && UserStatus.availabilities.keys.exclude?(params[:availability])
+      params[:availability] = UserStatus.availabilities[:not_set] unless new_user_availability
 
       user_status.update(params)
     end
@@ -38,5 +37,15 @@ module Users
     def user_status
       target_user.status || target_user.build_status
     end
+
+    def status_cleared?
+      params[:emoji].blank? &&
+        params[:message].blank? &&
+        (new_user_availability.blank? || new_user_availability == UserStatus.availabilities[:not_set])
+    end
+
+    def new_user_availability
+      UserStatus.availabilities[params[:availability]]
+    end
   end
 end

spec/services/users/set_status_service_spec.rb

@@ -31,19 +31,28 @@ RSpec.describe Users::SetStatusService do
         expect(service.execute).to be(true)
       end
 
+      context 'when setting availability to not_set' do
+        before do
+          params[:availability] = 'not_set'
+
+          create(:user_status, user: current_user, availability: 'busy')
+        end
+
+        it 'updates the availability' do
+          expect { service.execute }.to change { current_user.status.availability }.from('busy').to('not_set')
+        end
+      end
+
       context 'when the given availability value is not valid' do
-        let(:params) { { availability: 'not a valid value' } }
+        before do
+          params[:availability] = 'not a valid value'
+        end
 
         it 'does not update the status' do
           user_status = create(:user_status, user: current_user)
 
           expect { service.execute }.not_to change { user_status.reload }
         end
-
-        it 'returns false' do
-          create(:user_status, user: current_user)
-          expect(service.execute).to be(false)
-        end
       end
 
       context 'for another user' do
@@ -69,6 +78,7 @@ RSpec.describe Users::SetStatusService do
     context 'without params' do
       let(:params) { {} }
 
+      shared_examples 'removes user status record' do
         it 'deletes the status' do
           status = create(:user_status, user: current_user)
 
@@ -76,5 +86,14 @@ RSpec.describe Users::SetStatusService do
             .to change { current_user.reload.status }.from(status).to(nil)
         end
       end
+
+      it_behaves_like 'removes user status record'
+
+      context 'when not_set is given for availability' do
+        let(:params) { { availability: 'not_set' } }
+
+        it_behaves_like 'removes user status record'
+      end
+    end
   end
 end