Address MB to Bytes convertion on update/create services

app/controllers/admin/application_settings_controller.rb

@@ -5,7 +5,9 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
   end
 
   def update
-    if @application_setting.update_attributes(application_setting_params)
+    result = ::ApplicationSettings::UpdateService.new(@application_setting, current_user, application_setting_params).execute
+
+    if result[:status] == :success
       redirect_to admin_application_settings_path,
         notice: 'Application settings saved successfully'
     else

app/services/application_settings/base_service.rb

+module ApplicationSettings
+  class BaseService < ::BaseService
+    attr_accessor :application_setting, :current_user, :params
+
+    def initialize(application_setting, user, params = {})
+      @application_setting, @current_user, @params = application_setting, user, params.dup
+    end
+  end
+end

app/services/application_settings/update_service.rb

+module ApplicationSettings
+  class UpdateService < ApplicationSettings::BaseService
+    def execute
+      # Repository size limit comes as MB from the view
+      assign_repository_size_limit_as_bytes(application_setting)
+
+      if application_setting.update(params)
+        success
+      else
+        error('Application settings could not be updated')
+      end
+    end
+  end
+end

app/services/base_service.rb

@@ -46,6 +46,13 @@ class BaseService
 
   private
 
+  def assign_repository_size_limit_as_bytes(model)
+    repository_size_limit = @params.delete(:repository_size_limit)
+    new_value = repository_size_limit.to_i.megabytes if repository_size_limit.present?
+
+    model.repository_size_limit = new_value
+  end
+
   def error(message, http_status = nil)
     result = {
       message: message,

app/services/groups/create_service.rb

@@ -12,6 +12,9 @@ module Groups
         return @group
       end
 
+      # Repository size limit comes as MB from the view
+      assign_repository_size_limit_as_bytes(@group)
+
       if @group.parent && !can?(current_user, :admin_group, @group.parent)
         @group.parent = nil
         @group.errors.add(:parent_id, 'manage access required to create subgroup')

app/services/groups/update_service.rb

@@ -12,6 +12,9 @@ module Groups
         end
       end
 
+      # Repository size limit comes as MB from the view
+      assign_repository_size_limit_as_bytes(group)
+
       group.assign_attributes(params)
 
       begin

app/services/projects/create_service.rb

@@ -22,6 +22,9 @@ module Projects
         return @project
       end
 
+      # Repository size limit comes as MB from the view
+      assign_repository_size_limit_as_bytes(@project)
+
       # Set project name from path
       if @project.name.present? && @project.path.present?
         # if both name and path set - everything is ok

app/services/projects/update_service.rb

@@ -13,6 +13,9 @@ module Projects
         end
       end
 
+      # Repository size limit comes as MB from the view
+      assign_repository_size_limit_as_bytes(project)
+
       new_branch = params.delete(:default_branch)
       new_repository_storage = params.delete(:repository_storage)
 

spec/controllers/admin/application_settings_controller_spec.rb

@@ -6,6 +6,36 @@ describe Admin::ApplicationSettingsController do
   let(:admin) { create(:admin) }
   let(:user) { create(:user)}
 
+  describe 'PUT #update' do
+    before do
+      sign_in(admin)
+    end
+
+    context 'with valid params' do
+      subject { put :update, application_setting: { repository_size_limit: '100' } }
+
+      it 'redirect to application settings page' do
+        is_expected.to redirect_to(admin_application_settings_path)
+      end
+
+      it 'set flash notice' do
+        is_expected.to set_flash[:notice].to('Application settings saved successfully')
+      end
+    end
+
+    context 'with invalid params' do
+      subject! { put :update, application_setting: { repository_size_limit: '-100' } }
+
+      it 'render show template' do
+        is_expected.to render_template(:show)
+      end
+
+      it 'assigned @application_settings has errors' do
+        expect(assigns(:application_setting).errors[:repository_size_limit]).to be_present
+      end
+    end
+  end
+
   describe 'GET #usage_data with no access' do
     before do
       sign_in(user)

spec/models/ee/group_spec.rb

@@ -100,7 +100,7 @@ describe Group, models: true do
     it 'returns the value set locally' do
       group.update_attribute(:repository_size_limit, 75)
 
-      expect(group.actual_size_limit).to eq(75.megabytes)
+      expect(group.actual_size_limit).to eq(75)
     end
   end
 

spec/models/project_spec.rb

@@ -573,13 +573,13 @@ describe Project, models: true do
         group = create(:group, repository_size_limit: 100)
         project.update_attribute(:namespace_id, group.id)
 
-        expect(project.actual_size_limit).to eq(100.megabytes)
+        expect(project.actual_size_limit).to eq(100)
       end
 
       it 'returns the value set locally' do
         project.update_attribute(:repository_size_limit, 75)
 
-        expect(project.actual_size_limit).to eq(75.megabytes)
+        expect(project.actual_size_limit).to eq(75)
       end
     end
 

spec/services/application_settings/update_service_spec.rb

+require 'spec_helper'
+
+describe ApplicationSettings::UpdateService, services: true do
+  let(:user)    { create(:user) }
+  let(:setting) { ApplicationSetting.create_from_defaults }
+  let(:service) { described_class.new(setting, user, opts) }
+
+  describe '#execute' do
+    context 'common params' do
+      let(:opts) { { home_page_url: 'http://foo.bar' } }
+
+      it 'properly updates settings with given params' do
+        service.execute
+
+        expect(setting.home_page_url).to eql(opts[:home_page_url])
+      end
+    end
+
+    context 'with valid params' do
+      let(:opts) { { repository_size_limit: '100' } }
+
+      it 'returns success params' do
+        result = service.execute
+
+        expect(result).to eql(status: :success)
+      end
+    end
+
+    context 'with invalid params' do
+      let(:opts) { { repository_size_limit: '-100' } }
+
+      it 'returns error params' do
+        result = service.execute
+
+        expect(result).to eql(message: "Application settings could not be updated", status: :error)
+      end
+    end
+
+    context 'repository_size_limit assignment as Bytes' do
+      let(:service) { described_class.new(setting, user, opts) }
+
+      context 'when param present' do
+        let(:opts) { { repository_size_limit: '100' } }
+
+        it 'converts from MB to Bytes' do
+          service.execute
+
+          expect(setting.reload.repository_size_limit).to eql(100 * 1024 * 1024)
+        end
+      end
+
+      context 'when param not present' do
+        let(:opts) { { repository_size_limit: '' } }
+
+        it 'does not update due to invalidity' do
+          service.execute
+
+          expect(setting.reload.repository_size_limit).to be_zero
+        end
+
+        it 'assign nil value' do
+          service.execute
+
+          expect(setting.repository_size_limit).to be_nil
+        end
+      end
+    end
+  end
+end

spec/services/groups/create_service_spec.rb

@@ -40,4 +40,29 @@ describe Groups::CreateService, '#execute', services: true do
       end
     end
   end
+
+  context 'repository_size_limit assignment as Bytes' do
+    let(:admin_user) { create(:user, admin: true) }
+    let(:service) { described_class.new(admin_user, group_params.merge(opts)) }
+
+    context 'when param present' do
+      let(:opts) { { repository_size_limit: '100' } }
+
+      it 'assign repository_size_limit as Bytes' do
+        group = service.execute
+
+        expect(group.repository_size_limit).to eql(100 * 1024 * 1024)
+      end
+    end
+
+    context 'when param not present' do
+      let(:opts) { { repository_size_limit: '' } }
+
+      it 'assign nil value' do
+        group = service.execute
+
+        expect(group.repository_size_limit).to be_nil
+      end
+    end
+  end
 end

spec/services/groups/update_service_spec.rb

@@ -38,6 +38,31 @@ describe Groups::UpdateService, services: true do
     end
   end
 
+  context 'repository_size_limit assignment as Bytes' do
+    let(:group) { create(:group, :public, repository_size_limit: 0) }
+    let(:service) { described_class.new(group, user, opts) }
+
+    context 'when param present' do
+      let(:opts) { { repository_size_limit: '100' } }
+
+      it 'converts from MB to Bytes' do
+        service.execute
+
+        expect(group.reload.repository_size_limit).to eql(100 * 1024 * 1024)
+      end
+    end
+
+    context 'when param not present' do
+      let(:opts) { { repository_size_limit: '' } }
+
+      it 'assign nil value' do
+        service.execute
+
+        expect(group.reload.repository_size_limit).to be_nil
+      end
+    end
+  end
+
   context "unauthorized visibility_level validation" do
     let!(:service) { described_class.new(internal_group, user, visibility_level: 99) }
     before do

spec/services/projects/create_service_spec.rb

@@ -98,6 +98,30 @@ describe Projects::CreateService, '#execute', services: true do
     end
   end
 
+  context 'repository_size_limit assignment as Bytes' do
+    let(:admin_user) { create(:user, admin: true) }
+
+    context 'when param present' do
+      let(:opts) { { repository_size_limit: '100' } }
+
+      it 'assign repository_size_limit as Bytes' do
+        project = create_project(admin_user, opts)
+
+        expect(project.repository_size_limit).to eql(100 * 1024 * 1024)
+      end
+    end
+
+    context 'when param not present' do
+      let(:opts) { { repository_size_limit: '' } }
+
+      it 'assign nil value' do
+        project = create_project(admin_user, opts)
+
+        expect(project.repository_size_limit).to be_nil
+      end
+    end
+  end
+
   context 'restricted visibility level' do
     before do
       stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC])

spec/services/projects/update_service_spec.rb

@@ -127,6 +127,31 @@ describe Projects::UpdateService, services: true do
     end
   end
 
+  context 'repository_size_limit assignment as Bytes' do
+    let(:admin_user) { create(:user, admin: true) }
+    let(:project) { create(:empty_project, repository_size_limit: 0) }
+
+    context 'when param present' do
+      let(:opts) { { repository_size_limit: '100' } }
+
+      it 'converts from MB to Bytes' do
+        update_project(project, admin_user, opts)
+
+        expect(project.reload.repository_size_limit).to eql(100 * 1024 * 1024)
+      end
+    end
+
+    context 'when param not present' do
+      let(:opts) { { repository_size_limit: '' } }
+
+      it 'assign nil value' do
+        update_project(project, admin_user, opts)
+
+        expect(project.reload.repository_size_limit).to be_nil
+      end
+    end
+  end
+
   it 'returns an error result when record cannot be updated' do
     result = update_project(project, admin, { name: 'foo&bar' })