Fix updating shared_runners_minutes_limit; Check effects

made by the API. TODO: Need to do this for CE and all
the API tests as well

lib/api/groups.rb

@@ -128,10 +128,14 @@ module API
         authorize! :admin_group, group
 
         # EE
-        if params[:shared_runners_minutes_limit].to_i !=
-            group.shared_runners_minutes_limit.to_i
+        if params[:shared_runners_minutes_limit]
+          group.shared_runners_minutes_limit =
+            params[:shared_runners_minutes_limit].to_i
+
+          if group.shared_runners_minutes_limit_changed?
             authenticated_as_admin!
           end
+        end
 
         if ::Groups::UpdateService.new(group, current_user, declared_params(include_missing: false)).execute
           present group, with: Entities::GroupDetail, current_user: current_user

spec/requests/api/groups_spec.rb

@@ -275,7 +275,9 @@ describe API::Groups do
 
       # EE
       it 'returns 403 for updating shared_runners_minutes_limit' do
+        expect do
           put api("/groups/#{group1.id}", user1), shared_runners_minutes_limit: 133
+        end.not_to change { group1.shared_runners_minutes_limit }
 
         expect(response).to have_http_status(403)
       end
@@ -283,7 +285,9 @@ describe API::Groups do
       it 'returns 200 if shared_runners_minutes_limit is not changing' do
         group1.update(shared_runners_minutes_limit: 133)
 
+        expect do
           put api("/groups/#{group1.id}", user1), shared_runners_minutes_limit: 133
+        end.not_to change { group1.shared_runners_minutes_limit }
 
         expect(response).to have_http_status(200)
       end
@@ -299,7 +303,10 @@ describe API::Groups do
 
       # EE
       it 'updates the group for shared_runners_minutes_limit' do
+        expect do
           put api("/groups/#{group1.id}", admin), shared_runners_minutes_limit: 133
+        end.to change { group1.reload.shared_runners_minutes_limit }
+          .from(nil).to(133)
 
         expect(response).to have_http_status(200)
         expect(json_response['shared_runners_minutes_limit']).to eq(133)
@@ -509,7 +516,9 @@ describe API::Groups do
           it "does not create a group with shared_runners_minutes_limit" do
             group = attributes_for(:group, { shared_runners_minutes_limit: 133 })
 
+            expect do
               post api("/groups", user3), group
+            end.not_to change { Group.count }
 
             expect(response).to have_http_status(403)
           end
@@ -519,8 +528,13 @@ describe API::Groups do
           it "creates a group with shared_runners_minutes_limit" do
             group = attributes_for(:group, { shared_runners_minutes_limit: 133 })
 
+            expect do
               post api("/groups", admin), group
+            end.to change { Group.count }.by(1)
 
+            created_group = Group.find(json_response['id'])
+
+            expect(created_group.shared_runners_minutes_limit).to eq(133)
             expect(response).to have_http_status(201)
             expect(json_response['shared_runners_minutes_limit']).to eq(133)
           end

spec/requests/api/users_spec.rb

@@ -425,13 +425,15 @@ describe API::Users do
       expect(user.reload.external?).to be_truthy
     end
 
+    # EE
     it "updates shared_runners_minutes_limit" do
+      expect do
         put api("/users/#{user.id}", admin), { shared_runners_minutes_limit: 133 }
+      end.to change { user.reload.shared_runners_minutes_limit }
+        .from(nil).to(133)
 
       expect(response).to have_http_status(200)
-      expect(json_response['shared_runners_minutes_limit'])
-        .to eq(133)
-      expect(user.reload.shared_runners_minutes_limit).to eq(133)
+      expect(json_response['shared_runners_minutes_limit']).to eq(133)
     end
 
     it "does not update admin status" do
@@ -449,12 +451,18 @@ describe API::Users do
 
     context 'when the current user is not an admin' do
       it "is not available" do
+        expect do
           put api("/users/#{user.id}", user), attributes_for(:user)
+        end.not_to change { user.reload.attributes }
+
         expect(response).to have_http_status(403)
       end
 
       it "cannot update their own shared_runners_minutes_limit" do
+        expect do
           put api("/users/#{user.id}", user), { shared_runners_minutes_limit: 133 }
+        end.not_to change { user.reload.shared_runners_minutes_limit }
+
         expect(response).to have_http_status(403)
       end
     end