diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-05-03 03:10:40 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-05-03 03:10:40 +0300 |
commit | 86b7e30423820a05997039fd2af8aa2b0063953b (patch) | |
tree | 43f7f163f103a408bb4863880b84771c8a681748 | |
parent | 9db443551f7d97a4b676a79e14a1f1bbd9d5f2e8 (diff) |
Add latest changes from gitlab-org/gitlab@master
-rw-r--r-- | .rubocop_manual_todo.yml | 3 | ||||
-rw-r--r-- | app/models/environment.rb | 4 | ||||
-rw-r--r-- | changelogs/unreleased/fix-environment-ar-relationship-cause-massive-preloading.yml | 5 | ||||
-rw-r--r-- | changelogs/unreleased/issue-220040-fix-rails-savebang-users-model.yml | 5 | ||||
-rw-r--r-- | spec/models/user_preference_spec.rb | 2 | ||||
-rw-r--r-- | spec/models/user_spec.rb | 57 | ||||
-rw-r--r-- | spec/models/user_status_spec.rb | 2 |
7 files changed, 44 insertions, 34 deletions
diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index 1438814728f..b07a75e33c5 100644 --- a/.rubocop_manual_todo.yml +++ b/.rubocop_manual_todo.yml @@ -219,9 +219,6 @@ Rails/SaveBang: - 'spec/models/service_spec.rb' - 'spec/models/snippet_spec.rb' - 'spec/models/upload_spec.rb' - - 'spec/models/user_preference_spec.rb' - - 'spec/models/user_spec.rb' - - 'spec/models/user_status_spec.rb' Rails/TimeZone: Enabled: true diff --git a/app/models/environment.rb b/app/models/environment.rb index 6dff8e4f5f8..1c7fc6442bb 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -24,13 +24,13 @@ class Environment < ApplicationRecord has_many :self_managed_prometheus_alert_events, inverse_of: :environment has_many :alert_management_alerts, class_name: 'AlertManagement::Alert', inverse_of: :environment - has_one :last_deployment, -> { success.order('deployments.id DESC') }, class_name: 'Deployment', inverse_of: :environment + has_one :last_deployment, -> { success.distinct_on_environment }, class_name: 'Deployment', inverse_of: :environment has_one :last_deployable, through: :last_deployment, source: 'deployable', source_type: 'CommitStatus' has_one :last_pipeline, through: :last_deployable, source: 'pipeline' has_one :last_visible_deployment, -> { visible.distinct_on_environment }, inverse_of: :environment, class_name: 'Deployment' has_one :last_visible_deployable, through: :last_visible_deployment, source: 'deployable', source_type: 'CommitStatus' has_one :last_visible_pipeline, through: :last_visible_deployable, source: 'pipeline' - has_one :upcoming_deployment, -> { running.order('deployments.id DESC') }, class_name: 'Deployment', inverse_of: :environment + has_one :upcoming_deployment, -> { running.distinct_on_environment }, class_name: 'Deployment', inverse_of: :environment has_one :latest_opened_most_severe_alert, -> { order_severity_with_open_prometheus_alert }, class_name: 'AlertManagement::Alert', inverse_of: :environment before_validation :nullify_external_url diff --git a/changelogs/unreleased/fix-environment-ar-relationship-cause-massive-preloading.yml b/changelogs/unreleased/fix-environment-ar-relationship-cause-massive-preloading.yml new file mode 100644 index 00000000000..7702aa398a6 --- /dev/null +++ b/changelogs/unreleased/fix-environment-ar-relationship-cause-massive-preloading.yml @@ -0,0 +1,5 @@ +--- +title: Fix EnvironmentSerializer preloads unrelated pipelines/builds +merge_request: 60562 +author: +type: fixed diff --git a/changelogs/unreleased/issue-220040-fix-rails-savebang-users-model.yml b/changelogs/unreleased/issue-220040-fix-rails-savebang-users-model.yml new file mode 100644 index 00000000000..e2a4c5d3dd3 --- /dev/null +++ b/changelogs/unreleased/issue-220040-fix-rails-savebang-users-model.yml @@ -0,0 +1,5 @@ +--- +title: Fix Rails/SaveBang Rubocop offenses for user related models +merge_request: 57901 +author: Huzaifa Iftikhar @huzaifaiftikhar +type: other diff --git a/spec/models/user_preference_spec.rb b/spec/models/user_preference_spec.rb index 27ddaea763d..5806f123871 100644 --- a/spec/models/user_preference_spec.rb +++ b/spec/models/user_preference_spec.rb @@ -61,7 +61,7 @@ RSpec.describe UserPreference do describe 'sort_by preferences' do shared_examples_for 'a sort_by preference' do it 'allows nil sort fields' do - user_preference.update(attribute => nil) + user_preference.update!(attribute => nil) expect(user_preference).to be_valid end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index bd27eba9ac7..07031f16410 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -136,7 +136,7 @@ RSpec.describe User do it 'creates `user_detail` when `bio` is first updated' do user = create(:user) - expect { user.update(bio: 'my bio') }.to change { user.user_detail.persisted? }.from(false).to(true) + expect { user.update!(bio: 'my bio') }.to change { user.user_detail.persisted? }.from(false).to(true) end end @@ -659,9 +659,10 @@ RSpec.describe User do it 'does not accept not verified emails' do email = create(:email) user = email.user - user.update(notification_email: email.email) + user.notification_email = email.email expect(user).to be_invalid + expect(user.errors[:notification_email]).to include('is not an email you own') end end @@ -669,7 +670,7 @@ RSpec.describe User do it 'accepts verified emails' do email = create(:email, :confirmed, email: 'test@test.com') user = email.user - user.update(public_email: email.email) + user.notification_email = email.email expect(user).to be_valid end @@ -677,9 +678,10 @@ RSpec.describe User do it 'does not accept not verified emails' do email = create(:email) user = email.user - user.update(public_email: email.email) + user.public_email = email.email expect(user).to be_invalid + expect(user.errors[:public_email]).to include('is not an email you own') end end @@ -1293,7 +1295,7 @@ RSpec.describe User do let(:secondary) { create(:email, :confirmed, email: 'secondary@example.com', user: user) } before do - user.emails.create(email_attrs) + user.emails.create!(email_attrs) user.tap { |u| u.update!(notification_email: email_attrs[:email]) }.reload end @@ -1447,7 +1449,7 @@ RSpec.describe User do let!(:accessible_deploy_keys_project) { create(:deploy_keys_project, project: project) } before do - public_deploy_keys_project.deploy_key.update(public: true) + public_deploy_keys_project.deploy_key.update!(public: true) project.add_developer(user) end @@ -1537,13 +1539,13 @@ RSpec.describe User do it 'receives callback when external changes' do expect(user).to receive(:ensure_user_rights_and_limits) - user.update(external: false) + user.update!(external: false) end it 'ensures correct rights and limits for user' do stub_config_setting(default_can_create_group: true) - expect { user.update(external: false) }.to change { user.can_create_group }.to(true) + expect { user.update!(external: false) }.to change { user.can_create_group }.to(true) .and change { user.projects_limit }.to(Gitlab::CurrentSettings.default_projects_limit) end end @@ -1554,11 +1556,11 @@ RSpec.describe User do it 'receives callback when external changes' do expect(user).to receive(:ensure_user_rights_and_limits) - user.update(external: true) + user.update!(external: true) end it 'ensures correct rights and limits for user' do - expect { user.update(external: true) }.to change { user.can_create_group }.to(false) + expect { user.update!(external: true) }.to change { user.can_create_group }.to(false) .and change { user.projects_limit }.to(0) end end @@ -2454,7 +2456,7 @@ RSpec.describe User do end context 'with a redirect route matching the given path' do - let!(:redirect_route) { user.namespace.redirect_routes.create(path: 'foo') } + let!(:redirect_route) { user.namespace.redirect_routes.create!(path: 'foo') } context 'without the follow_redirects option' do it 'returns nil' do @@ -2555,7 +2557,7 @@ RSpec.describe User do expect(Gitlab::AvatarCache).to receive(:delete_by_email).with(*user.verified_emails) - user.update(avatar: fixture_file_upload('spec/fixtures/dk.png')) + user.update!(avatar: fixture_file_upload('spec/fixtures/dk.png')) end end @@ -2869,12 +2871,12 @@ RSpec.describe User do expect(user.starred?(project1)).to be_truthy expect(user.starred?(project2)).to be_truthy - star1.destroy + star1.destroy! expect(user.starred?(project1)).to be_falsey expect(user.starred?(project2)).to be_truthy - star2.destroy + star2.destroy! expect(user.starred?(project1)).to be_falsey expect(user.starred?(project2)).to be_falsey @@ -3424,7 +3426,7 @@ RSpec.describe User do expect(user.authorized_projects).to include(project) - member.destroy + member.destroy! expect(user.authorized_projects).not_to include(project) end @@ -3449,7 +3451,7 @@ RSpec.describe User do expect(user2.authorized_projects).to include(project) - project.destroy + project.destroy! expect(user2.authorized_projects).not_to include(project) end @@ -3463,7 +3465,7 @@ RSpec.describe User do expect(user.authorized_projects).to include(project) - group.destroy + group.destroy! expect(user.authorized_projects).not_to include(project) end @@ -4463,9 +4465,10 @@ RSpec.describe User do end it 'adds the namespace errors to the user' do - user.update(username: new_username) + user.username = new_username - expect(user.errors.full_messages.first).to eq('A user, alias, or group already exists with that username.') + expect(user).to be_invalid + expect(user.errors[:base]).to include('A user, alias, or group already exists with that username.') end end end @@ -5356,21 +5359,21 @@ RSpec.describe User do with_them do context 'when state was changed' do - subject { user.update(attributes) } + subject { user.update!(attributes) } include_examples 'update highest role with exclusive lease' end end context 'when state was not changed' do - subject { user.update(email: 'newmail@example.com') } + subject { user.update!(email: 'newmail@example.com') } include_examples 'does not update the highest role' end end describe 'destroy user' do - subject { user.destroy } + subject { user.destroy! } include_examples 'does not update the highest role' end @@ -5392,7 +5395,7 @@ RSpec.describe User do context 'when user is a ghost user' do before do - user.update(user_type: :ghost) + user.update!(user_type: :ghost) end it { is_expected.to be false } @@ -5410,7 +5413,7 @@ RSpec.describe User do with_them do before do - user.update(user_type: user_type) + user.update!(user_type: user_type) end it { is_expected.to be expected_result } @@ -5433,7 +5436,7 @@ RSpec.describe User do context 'when user is an internal user' do before do - user.update(user_type: :ghost) + user.update!(user_type: :ghost) end it { is_expected.to be :forbidden } @@ -5467,7 +5470,7 @@ RSpec.describe User do context 'when user is an internal user' do before do - user.update(user_type: 'alert_bot') + user.update!(user_type: 'alert_bot') end it_behaves_like 'does not require password to be present' @@ -5475,7 +5478,7 @@ RSpec.describe User do context 'when user is a project bot user' do before do - user.update(user_type: 'project_bot') + user.update!(user_type: 'project_bot') end it_behaves_like 'does not require password to be present' diff --git a/spec/models/user_status_spec.rb b/spec/models/user_status_spec.rb index 51dd91149cc..87d1fa14aca 100644 --- a/spec/models/user_status_spec.rb +++ b/spec/models/user_status_spec.rb @@ -15,7 +15,7 @@ RSpec.describe UserStatus do it 'is expected to be deleted when the user is deleted' do status = create(:user_status) - expect { status.user.destroy }.to change { described_class.count }.from(1).to(0) + expect { status.user.destroy! }.to change { described_class.count }.from(1).to(0) end describe '#clear_status_after=' do |