From 3d3b46f344748420a638f8a66745d974fa2c2748 Mon Sep 17 00:00:00 2001 From: blackst0ne Date: Wed, 4 Apr 2018 09:19:47 +0000 Subject: [Rails5] Rename `sort` methods to `sort_by_attribute` --- app/models/user.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/models/user.rb') diff --git a/app/models/user.rb b/app/models/user.rb index f934b654225..ba51595e6a3 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -256,7 +256,7 @@ class User < ActiveRecord::Base end end - def sort(method) + def sort_by_attribute(method) order_method = method || 'id_desc' case order_method.to_s -- cgit v1.2.1 From 7de250fb81fb24f8e905cfb330072206c4a8a40a Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 5 Apr 2018 11:14:26 +0200 Subject: Ensure internal users (ghost, support bot) get assigned a namespace --- app/models/user.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'app/models/user.rb') diff --git a/app/models/user.rb b/app/models/user.rb index ba51595e6a3..7b6857a0d34 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -164,12 +164,15 @@ class User < ActiveRecord::Base before_validation :sanitize_attrs before_validation :set_notification_email, if: :email_changed? + before_save :set_notification_email, if: :email_changed? # in case validation is skipped before_validation :set_public_email, if: :public_email_changed? + before_save :set_public_email, if: :public_email_changed? # in case validation is skipped before_save :ensure_incoming_email_token before_save :ensure_user_rights_and_limits, if: ->(user) { user.new_record? || user.external_changed? } before_save :skip_reconfirmation!, if: ->(user) { user.email_changed? && user.read_only_attribute?(:email) } before_save :check_for_verified_email, if: ->(user) { user.email_changed? && !user.new_record? } before_validation :ensure_namespace_correct + before_save :ensure_namespace_correct # in case validation is skipped after_validation :set_username_errors after_update :username_changed_hook, if: :username_changed? after_destroy :post_destroy_hook @@ -408,7 +411,6 @@ class User < ActiveRecord::Base unique_internal(where(ghost: true), 'ghost', email) do |u| u.bio = 'This is a "Ghost User", created to hold all issues authored by users that have since been deleted. This user cannot be removed.' u.name = 'Ghost User' - u.notification_email = email end end end -- cgit v1.2.1 From b099c6ff9fcbd81beb75fd12bff8d1d5b9c16083 Mon Sep 17 00:00:00 2001 From: m b Date: Fri, 23 Mar 2018 00:09:26 -0500 Subject: Deleting a MR you are assigned to should decrements counter The merge request counter in the UI was not decreasing when a merge request was deleting. This was just due to the cache not being refreshed on a delete action. fixes: https://gitlab.com/gitlab-org/gitlab-ce/issues/44458 --- app/models/user.rb | 5 ----- 1 file changed, 5 deletions(-) (limited to 'app/models/user.rb') diff --git a/app/models/user.rb b/app/models/user.rb index 7b6857a0d34..ce56b39b1c8 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1046,11 +1046,6 @@ class User < ActiveRecord::Base end end - def update_cache_counts - assigned_open_merge_requests_count(force: true) - assigned_open_issues_count(force: true) - end - def update_todos_count_cache todos_done_count(force: true) todos_pending_count(force: true) -- cgit v1.2.1 From 29b0a90c208f29606a05d1391a72b9ff7ff843b1 Mon Sep 17 00:00:00 2001 From: Andreas Brandl Date: Wed, 4 Apr 2018 17:14:19 +0200 Subject: Cache personal projects count. Closes #37462. --- app/models/user.rb | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) (limited to 'app/models/user.rb') diff --git a/app/models/user.rb b/app/models/user.rb index ce56b39b1c8..a14aefc61d2 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -700,10 +700,6 @@ class User < ActiveRecord::Base projects_limit - personal_projects_count end - def personal_projects_count - @personal_projects_count ||= personal_projects.count - end - def recent_push(project = nil) service = Users::LastPushEventService.new(self) @@ -1046,6 +1042,18 @@ class User < ActiveRecord::Base end end + def personal_projects_count(force: false) + Rails.cache.fetch(['users', id, 'personal_projects_count'], force: force, expires_in: 24.hours, raw: true) do + personal_projects.count + end.to_i + end + + def update_cache_counts + assigned_open_merge_requests_count(force: true) + assigned_open_issues_count(force: true) + personal_projects_count(force: true) + end + def update_todos_count_cache todos_done_count(force: true) todos_pending_count(force: true) @@ -1056,6 +1064,7 @@ class User < ActiveRecord::Base invalidate_merge_request_cache_counts invalidate_todos_done_count invalidate_todos_pending_count + invalidate_personal_projects_count end def invalidate_issue_cache_counts @@ -1074,6 +1083,10 @@ class User < ActiveRecord::Base Rails.cache.delete(['users', id, 'todos_pending_count']) end + def invalidate_personal_projects_count + Rails.cache.delete(['users', id, 'personal_projects_count']) + end + # This is copied from Devise::Models::Lockable#valid_for_authentication?, as our auth # flow means we don't call that automatically (and can't conveniently do so). # -- cgit v1.2.1 From fa46b19ddb82851523fabaea2fca4660c181db89 Mon Sep 17 00:00:00 2001 From: Andreas Brandl Date: Thu, 5 Apr 2018 15:48:18 +0200 Subject: Remove unused method. --- app/models/user.rb | 6 ------ 1 file changed, 6 deletions(-) (limited to 'app/models/user.rb') diff --git a/app/models/user.rb b/app/models/user.rb index a14aefc61d2..2b95be3f888 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1048,12 +1048,6 @@ class User < ActiveRecord::Base end.to_i end - def update_cache_counts - assigned_open_merge_requests_count(force: true) - assigned_open_issues_count(force: true) - personal_projects_count(force: true) - end - def update_todos_count_cache todos_done_count(force: true) todos_pending_count(force: true) -- cgit v1.2.1 From 0e78c2e9c925d180a443d132658691adf18f26a1 Mon Sep 17 00:00:00 2001 From: Dylan Griffith Date: Tue, 27 Mar 2018 16:31:43 +1100 Subject: Allow group owner to enable runners from subgroups (#41981) --- app/models/user.rb | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) (limited to 'app/models/user.rb') diff --git a/app/models/user.rb b/app/models/user.rb index 2b95be3f888..01ca1446376 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -955,6 +955,10 @@ class User < ActiveRecord::Base Gitlab::GroupHierarchy.new(owned_and_master_groups).base_and_descendants end + def manageable_group_projects + Project.where(namespace: manageable_groups) + end + def namespaces namespace_ids = groups.pluck(:id) namespace_ids.push(namespace.id) @@ -1205,12 +1209,15 @@ class User < ActiveRecord::Base end def ci_projects_union - scope = { access_level: [Gitlab::Access::MASTER, Gitlab::Access::OWNER] } - groups = groups_projects.where(members: scope) - other = projects.where(members: scope) + manageable_other_projects = projects.where(members: { + access_level: [Gitlab::Access::MASTER, Gitlab::Access::OWNER] + }) - Gitlab::SQL::Union.new([personal_projects.select(:id), groups.select(:id), - other.select(:id)]) + Gitlab::SQL::Union.new([ + manageable_group_projects.select(:id), + personal_projects.select(:id), + manageable_other_projects.select(:id) + ]) end # Added according to https://github.com/plataformatec/devise/blob/7df57d5081f9884849ca15e4fde179ef164a575f/README.md#activejob-integration -- cgit v1.2.1 From 742d23a5d0f61c9939d42605762b6fc073ae4f22 Mon Sep 17 00:00:00 2001 From: Dylan Griffith Date: Thu, 5 Apr 2018 13:12:21 +1000 Subject: Use project_authorizations instead of members to calculate manageable CI projects to speed up query (#41981) --- app/models/user.rb | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) (limited to 'app/models/user.rb') diff --git a/app/models/user.rb b/app/models/user.rb index 01ca1446376..25441d2b68f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -997,7 +997,7 @@ class User < ActiveRecord::Base def ci_authorized_runners @ci_authorized_runners ||= begin runner_ids = Ci::RunnerProject - .where("ci_runner_projects.project_id IN (#{ci_projects_union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection + .where(project: authorized_projects(Gitlab::Access::MASTER)) .select(:runner_id) Ci::Runner.specific.where(id: runner_ids) end @@ -1208,18 +1208,6 @@ class User < ActiveRecord::Base ], remove_duplicates: false) end - def ci_projects_union - manageable_other_projects = projects.where(members: { - access_level: [Gitlab::Access::MASTER, Gitlab::Access::OWNER] - }) - - Gitlab::SQL::Union.new([ - manageable_group_projects.select(:id), - personal_projects.select(:id), - manageable_other_projects.select(:id) - ]) - end - # Added according to https://github.com/plataformatec/devise/blob/7df57d5081f9884849ca15e4fde179ef164a575f/README.md#activejob-integration def send_devise_notification(notification, *args) return true unless can?(:receive_notifications) -- cgit v1.2.1 From f45b8888fc1a2694b8e2da64512137ab54a09a66 Mon Sep 17 00:00:00 2001 From: Dylan Griffith Date: Sat, 7 Apr 2018 07:31:52 +1000 Subject: Remove unused User#manageable_group_projects (#41981) --- app/models/user.rb | 4 ---- 1 file changed, 4 deletions(-) (limited to 'app/models/user.rb') diff --git a/app/models/user.rb b/app/models/user.rb index 25441d2b68f..42bb27d4753 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -955,10 +955,6 @@ class User < ActiveRecord::Base Gitlab::GroupHierarchy.new(owned_and_master_groups).base_and_descendants end - def manageable_group_projects - Project.where(namespace: manageable_groups) - end - def namespaces namespace_ids = groups.pluck(:id) namespace_ids.push(namespace.id) -- cgit v1.2.1 From 20fdbbe86a6cffbf467f08d50a0d8ef0f5c87f50 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Thu, 5 Apr 2018 13:19:24 +0200 Subject: Use Goldiloader for handling N+1 queries Goldiloader (https://github.com/salsify/goldiloader) can eager load associations automatically. This removes the need for adding "includes" calls in a variety of different places. This also comes with the added benefit of not having to eager load data if it's not used. --- app/models/user.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'app/models/user.rb') diff --git a/app/models/user.rb b/app/models/user.rb index 2b95be3f888..c7e1dfaf595 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -96,9 +96,9 @@ class User < ActiveRecord::Base # Groups has_many :members has_many :group_members, -> { where(requested_at: nil) }, source: 'GroupMember' - has_many :groups, through: :group_members - has_many :owned_groups, -> { where members: { access_level: Gitlab::Access::OWNER } }, through: :group_members, source: :group - has_many :masters_groups, -> { where members: { access_level: Gitlab::Access::MASTER } }, through: :group_members, source: :group + has_many :groups, -> { auto_include(false) }, through: :group_members + has_many :owned_groups, -> { where(members: { access_level: Gitlab::Access::OWNER }).auto_include(false) }, through: :group_members, source: :group + has_many :masters_groups, -> { where(members: { access_level: Gitlab::Access::MASTER }).auto_include(false) }, through: :group_members, source: :group # Projects has_many :groups_projects, through: :groups, source: :projects -- cgit v1.2.1 From 4ef3e3491e2ecc34e7f4de1221d5ad7b8b4a1e24 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Mon, 9 Apr 2018 12:19:18 +0100 Subject: Add cop for has_many :through without disabled autoloading Goldiloader is great, but has several issues with has_many :through relations: * https://github.com/salsify/goldiloader/issues/12 * https://github.com/salsify/goldiloader/issues/14 * https://github.com/salsify/goldiloader/issues/18 Rather than try to figure out which applies in each case, we should just do the drudge work of manually disabling autoloading for all relations of this type. We can always use regular preloading for specific cases, but this way we avoid generating invalid queries through Goldiloader's magic. --- app/models/user.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'app/models/user.rb') diff --git a/app/models/user.rb b/app/models/user.rb index c7e1dfaf595..fc87ada7447 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -101,18 +101,18 @@ class User < ActiveRecord::Base has_many :masters_groups, -> { where(members: { access_level: Gitlab::Access::MASTER }).auto_include(false) }, through: :group_members, source: :group # Projects - has_many :groups_projects, through: :groups, source: :projects - has_many :personal_projects, through: :namespace, source: :projects + has_many :groups_projects, -> { auto_include(false) }, through: :groups, source: :projects + has_many :personal_projects, -> { auto_include(false) }, through: :namespace, source: :projects has_many :project_members, -> { where(requested_at: nil) } - has_many :projects, through: :project_members - has_many :created_projects, foreign_key: :creator_id, class_name: 'Project' + has_many :projects, -> { auto_include(false) }, through: :project_members + has_many :created_projects, foreign_key: :creator_id, class_name: 'Project' has_many :users_star_projects, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent - has_many :starred_projects, through: :users_star_projects, source: :project + has_many :starred_projects, -> { auto_include(false) }, through: :users_star_projects, source: :project has_many :project_authorizations - has_many :authorized_projects, through: :project_authorizations, source: :project + has_many :authorized_projects, -> { auto_include(false) }, through: :project_authorizations, source: :project has_many :user_interacted_projects - has_many :project_interactions, through: :user_interacted_projects, source: :project, class_name: 'Project' + has_many :project_interactions, -> { auto_include(false) }, through: :user_interacted_projects, source: :project, class_name: 'Project' has_many :snippets, dependent: :destroy, foreign_key: :author_id # rubocop:disable Cop/ActiveRecordDependent has_many :notes, dependent: :destroy, foreign_key: :author_id # rubocop:disable Cop/ActiveRecordDependent @@ -132,7 +132,7 @@ class User < ActiveRecord::Base has_many :triggers, dependent: :destroy, class_name: 'Ci::Trigger', foreign_key: :owner_id # rubocop:disable Cop/ActiveRecordDependent has_many :issue_assignees - has_many :assigned_issues, class_name: "Issue", through: :issue_assignees, source: :issue + has_many :assigned_issues, -> { auto_include(false) }, class_name: "Issue", through: :issue_assignees, source: :issue has_many :assigned_merge_requests, dependent: :nullify, foreign_key: :assignee_id, class_name: "MergeRequest" # rubocop:disable Cop/ActiveRecordDependent has_many :custom_attributes, class_name: 'UserCustomAttribute' -- cgit v1.2.1