From db33c0fb63d50e31751fb580d502de38ee665385 Mon Sep 17 00:00:00 2001 From: Tiago Botelho Date: Fri, 16 Jun 2017 12:28:49 +0100 Subject: Ensures default user limits when external user is unchecked --- app/models/user.rb | 15 ++++++---- ...t-user-limits-when-unchecking-external-user.yml | 4 +++ spec/models/user_spec.rb | 32 ++++++++++++++++++++++ 3 files changed, 45 insertions(+), 6 deletions(-) create mode 100644 changelogs/unreleased/30725-reset-user-limits-when-unchecking-external-user.yml diff --git a/app/models/user.rb b/app/models/user.rb index 5d128e4b390..368c74037f4 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -153,7 +153,7 @@ class User < ActiveRecord::Base after_update :update_emails_with_primary_email, if: ->(user) { user.email_changed? } before_save :ensure_authentication_token, :ensure_incoming_email_token - before_save :ensure_external_user_rights + before_save :ensure_user_rights_and_limits, if: ->(user) { user.external_changed? } after_save :ensure_namespace_correct after_initialize :set_projects_limit after_destroy :post_destroy_hook @@ -1033,11 +1033,14 @@ class User < ActiveRecord::Base super end - def ensure_external_user_rights - return unless external? - - self.can_create_group = false - self.projects_limit = 0 + def ensure_user_rights_and_limits + if external? + self.can_create_group = false + self.projects_limit = 0 + else + self.can_create_group = true + self.projects_limit = current_application_settings.default_projects_limit + end end def signup_domain_valid? diff --git a/changelogs/unreleased/30725-reset-user-limits-when-unchecking-external-user.yml b/changelogs/unreleased/30725-reset-user-limits-when-unchecking-external-user.yml new file mode 100644 index 00000000000..3058404b3f8 --- /dev/null +++ b/changelogs/unreleased/30725-reset-user-limits-when-unchecking-external-user.yml @@ -0,0 +1,4 @@ +--- +title: Ensures default user limits when external user is unchecked +merge_request: 12218 +author: diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index d5bd9946ab6..633f377a382 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -451,6 +451,38 @@ describe User, models: true do end end + describe '#ensure_user_rights_and_limits' do + describe 'with external user' do + let(:user) { create(:user, external: true) } + + it 'receives callback when external changes' do + expect(user).to receive(:ensure_user_rights_and_limits) + + user.update_attributes(external: false) + end + + it 'ensures correct rights and limits for user' do + expect { user.update_attributes(external: false) }.to change { user.can_create_group }.to(true) + .and change { user.projects_limit }.to(current_application_settings.default_projects_limit) + end + end + + describe 'without external user' do + let(:user) { create(:user, external: false) } + + it 'receives callback when external changes' do + expect(user).to receive(:ensure_user_rights_and_limits) + + user.update_attributes(external: true) + end + + it 'ensures correct rights and limits for user' do + expect { user.update_attributes(external: true) }.to change { user.can_create_group }.to(false) + .and change { user.projects_limit }.to(0) + end + end + end + describe 'rss token' do it 'ensures an rss token on read' do user = create(:user, rss_token: nil) -- cgit v1.2.1 From 4449f57ea627aa92e7f011a488c11285465057d2 Mon Sep 17 00:00:00 2001 From: Tiago Botelho Date: Mon, 19 Jun 2017 15:21:25 +0100 Subject: refactors user model validations --- app/models/user.rb | 18 +++++++++--------- spec/models/user_spec.rb | 2 ++ 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 368c74037f4..782c162e1f3 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -139,21 +139,21 @@ class User < ActiveRecord::Base presence: true, uniqueness: { case_sensitive: false } - validate :namespace_uniq, if: ->(user) { user.username_changed? } + validate :namespace_uniq, if: :username_changed? validate :avatar_type, if: ->(user) { user.avatar.present? && user.avatar_changed? } - validate :unique_email, if: ->(user) { user.email_changed? } - validate :owns_notification_email, if: ->(user) { user.notification_email_changed? } - validate :owns_public_email, if: ->(user) { user.public_email_changed? } + validate :unique_email, if: :email_changed? + validate :owns_notification_email, if: :notification_email_changed? + validate :owns_public_email, if: :public_email_changed? validate :signup_domain_valid?, on: :create, if: ->(user) { !user.created_by_id } validates :avatar, file_size: { maximum: 200.kilobytes.to_i } before_validation :sanitize_attrs - before_validation :set_notification_email, if: ->(user) { user.email_changed? } - before_validation :set_public_email, if: ->(user) { user.public_email_changed? } + before_validation :set_notification_email, if: :email_changed? + before_validation :set_public_email, if: :public_email_changed? - after_update :update_emails_with_primary_email, if: ->(user) { user.email_changed? } + after_update :update_emails_with_primary_email, if: :email_changed? before_save :ensure_authentication_token, :ensure_incoming_email_token - before_save :ensure_user_rights_and_limits, if: ->(user) { user.external_changed? } + before_save :ensure_user_rights_and_limits, if: :external_changed? after_save :ensure_namespace_correct after_initialize :set_projects_limit after_destroy :post_destroy_hook @@ -1038,7 +1038,7 @@ class User < ActiveRecord::Base self.can_create_group = false self.projects_limit = 0 else - self.can_create_group = true + self.can_create_group = gitlab_config.default_can_create_group self.projects_limit = current_application_settings.default_projects_limit end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 633f377a382..dbe75819663 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -462,6 +462,8 @@ describe User, models: true do end it 'ensures correct rights and limits for user' do + stub_config_setting(default_can_create_group: true) + expect { user.update_attributes(external: false) }.to change { user.can_create_group }.to(true) .and change { user.projects_limit }.to(current_application_settings.default_projects_limit) end -- cgit v1.2.1