diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-10-21 07:08:36 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-10-21 07:08:36 +0000 |
commit | 48aff82709769b098321c738f3444b9bdaa694c6 (patch) | |
tree | e00c7c43e2d9b603a5a6af576b1685e400410dee /app/finders | |
parent | 879f5329ee916a948223f8f43d77fba4da6cd028 (diff) | |
download | gitlab-ce-48aff82709769b098321c738f3444b9bdaa694c6.tar.gz |
Add latest changes from gitlab-org/gitlab@13-5-stable-eev13.5.0-rc42
Diffstat (limited to 'app/finders')
-rw-r--r-- | app/finders/README.md | 11 | ||||
-rw-r--r-- | app/finders/alert_management/alerts_finder.rb | 14 | ||||
-rw-r--r-- | app/finders/ci/jobs_finder.rb | 4 | ||||
-rw-r--r-- | app/finders/concerns/time_frame_filter.rb | 7 | ||||
-rw-r--r-- | app/finders/environment_names_finder.rb | 47 | ||||
-rw-r--r-- | app/finders/group_labels_finder.rb | 29 | ||||
-rw-r--r-- | app/finders/group_members_finder.rb | 28 | ||||
-rw-r--r-- | app/finders/groups_finder.rb | 12 | ||||
-rw-r--r-- | app/finders/issuable_finder.rb | 18 | ||||
-rw-r--r-- | app/finders/keys_finder.rb | 2 | ||||
-rw-r--r-- | app/finders/merge_requests/by_approvals_finder.rb | 93 | ||||
-rw-r--r-- | app/finders/merge_requests_finder.rb | 63 | ||||
-rw-r--r-- | app/finders/milestones_finder.rb | 3 | ||||
-rw-r--r-- | app/finders/packages/generic/package_finder.rb | 22 | ||||
-rw-r--r-- | app/finders/projects_finder.rb | 13 | ||||
-rw-r--r-- | app/finders/releases_finder.rb | 10 |
16 files changed, 315 insertions, 61 deletions
diff --git a/app/finders/README.md b/app/finders/README.md index 1a1c69dea38..52f7378c484 100644 --- a/app/finders/README.md +++ b/app/finders/README.md @@ -1,10 +1,11 @@ # Finders -This type of classes responsible for collection items based on different conditions. -To prevent lookup methods in models like this: +These types of classes are responsible for retrieving collection items based on different conditions. +They prevent lookup methods in models like this: + ```ruby -class Project +class Project < ApplicationRecord def issues_for_user_filtered_by(user, filter) # A lot of logic not related to project model itself end @@ -13,10 +14,10 @@ end issues = project.issues_for_user_filtered_by(user, params) ``` -Better use this: +The GitLab approach is to use a Finder: ```ruby issues = IssuesFinder.new(project, user, filter).execute ``` -It will help keep models thiner. +It will help keep models thinner. diff --git a/app/finders/alert_management/alerts_finder.rb b/app/finders/alert_management/alerts_finder.rb index cb35be43c15..1d6f790af31 100644 --- a/app/finders/alert_management/alerts_finder.rb +++ b/app/finders/alert_management/alerts_finder.rb @@ -2,8 +2,8 @@ module AlertManagement class AlertsFinder - # @return [Hash<Integer,Integer>] Mapping of status id to count - # ex) { 0: 6, ...etc } + # @return [Hash<Symbol,Integer>] Mapping of status id to count + # ex) { triggered: 6, ...etc } def self.counts_by_status(current_user, project, params = {}) new(current_user, project, params).execute.counts_by_status end @@ -19,8 +19,10 @@ module AlertManagement collection = project.alert_management_alerts collection = by_status(collection) - collection = by_search(collection) collection = by_iid(collection) + collection = by_assignee(collection) + collection = by_search(collection) + sort(collection) end @@ -35,7 +37,7 @@ module AlertManagement end def by_status(collection) - values = AlertManagement::Alert::STATUSES.values & Array(params[:status]) + values = AlertManagement::Alert.status_names & Array(params[:status]) values.present? ? collection.for_status(values) : collection end @@ -48,6 +50,10 @@ module AlertManagement params[:sort] ? collection.sort_by_attribute(params[:sort]) : collection end + def by_assignee(collection) + params[:assignee_username].present? ? collection.for_assignee_username(params[:assignee_username]) : collection + end + def authorized? Ability.allowed?(current_user, :read_alert_management_alert, project) end diff --git a/app/finders/ci/jobs_finder.rb b/app/finders/ci/jobs_finder.rb index 8515b77ec0b..40c610f8209 100644 --- a/app/finders/ci/jobs_finder.rb +++ b/app/finders/ci/jobs_finder.rb @@ -25,7 +25,7 @@ module Ci attr_reader :current_user, :pipeline, :project, :params, :type def init_collection - if Feature.enabled?(:ci_jobs_finder_refactor) + if Feature.enabled?(:ci_jobs_finder_refactor, default_enabled: true) pipeline_jobs || project_jobs || all_jobs else project ? project_builds : all_jobs @@ -59,7 +59,7 @@ module Ci end def filter_by_scope(builds) - if Feature.enabled?(:ci_jobs_finder_refactor) + if Feature.enabled?(:ci_jobs_finder_refactor, default_enabled: true) return filter_by_statuses!(params[:scope], builds) if params[:scope].is_a?(Array) end diff --git a/app/finders/concerns/time_frame_filter.rb b/app/finders/concerns/time_frame_filter.rb index e0baba25b64..d1ebed730f6 100644 --- a/app/finders/concerns/time_frame_filter.rb +++ b/app/finders/concerns/time_frame_filter.rb @@ -11,4 +11,11 @@ module TimeFrameFilter rescue ArgumentError items end + + def containing_date(items) + return items unless params[:containing_date] + + date = params[:containing_date].to_date + items.within_timeframe(date, date) + end end diff --git a/app/finders/environment_names_finder.rb b/app/finders/environment_names_finder.rb new file mode 100644 index 00000000000..a92998921c7 --- /dev/null +++ b/app/finders/environment_names_finder.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +# Finder for obtaining the unique environment names of a project or group. +# +# This finder exists so that the merge requests "environments" filter can be +# populated with a unique list of environment names. If we retrieve _just_ the +# environments, duplicates may be present (e.g. multiple projects in a group +# having a "staging" environment). +# +# In addition, this finder only produces unfoldered environments. We do this +# because when searching for environments we want to exclude review app +# environments. +class EnvironmentNamesFinder + attr_reader :project_or_group, :current_user + + def initialize(project_or_group, current_user) + @project_or_group = project_or_group + @current_user = current_user + end + + def execute + all_environments.unfoldered.order_by_name.pluck_unique_names + end + + def all_environments + if project_or_group.is_a?(Namespace) + namespace_environments + else + project_environments + end + end + + def namespace_environments + projects = + project_or_group.all_projects.public_or_visible_to_user(current_user) + + Environment.for_project(projects) + end + + def project_environments + if current_user.can?(:read_environment, project_or_group) + project_or_group.environments + else + Environment.none + end + end +end diff --git a/app/finders/group_labels_finder.rb b/app/finders/group_labels_finder.rb deleted file mode 100644 index a668a0f0fae..00000000000 --- a/app/finders/group_labels_finder.rb +++ /dev/null @@ -1,29 +0,0 @@ -# frozen_string_literal: true - -class GroupLabelsFinder - attr_reader :current_user, :group, :params - - def initialize(current_user, group, params = {}) - @current_user = current_user - @group = group - @params = params - end - - def execute - group.labels - .optionally_subscribed_by(subscriber_id) - .optionally_search(params[:search]) - .order_by(params[:sort]) - .page(params[:page]) - end - - private - - def subscriber_id - current_user&.id if subscribed? - end - - def subscribed? - params[:subscribed] == 'true' - end -end diff --git a/app/finders/group_members_finder.rb b/app/finders/group_members_finder.rb index ce0d52ad97a..09283f061c0 100644 --- a/app/finders/group_members_finder.rb +++ b/app/finders/group_members_finder.rb @@ -17,9 +17,8 @@ class GroupMembersFinder < UnionFinder @params = params end - # rubocop: disable CodeReuse/ActiveRecord def execute(include_relations: [:inherited, :direct]) - group_members = group.members + group_members = group_members_list relations = [] return group_members if include_relations == [:direct] @@ -27,17 +26,13 @@ class GroupMembersFinder < UnionFinder relations << group_members if include_relations.include?(:direct) if include_relations.include?(:inherited) && group.parent - parents_members = GroupMember.non_request.non_minimal_access - .where(source_id: group.ancestors.select(:id)) - .where.not(user_id: group.users.select(:id)) + parents_members = relation_group_members(group.ancestors) relations << parents_members end if include_relations.include?(:descendants) - descendant_members = GroupMember.non_request.non_minimal_access - .where(source_id: group.descendants.select(:id)) - .where.not(user_id: group.users.select(:id)) + descendant_members = relation_group_members(group.descendants) relations << descendant_members end @@ -47,7 +42,6 @@ class GroupMembersFinder < UnionFinder members = find_union(relations, GroupMember) filter_members(members) end - # rubocop: enable CodeReuse/ActiveRecord private @@ -67,6 +61,22 @@ class GroupMembersFinder < UnionFinder def can_manage_members Ability.allowed?(user, :admin_group_member, group) end + + def group_members_list + group.members + end + + def relation_group_members(relation) + all_group_members(relation).non_minimal_access + end + + # rubocop: disable CodeReuse/ActiveRecord + def all_group_members(relation) + GroupMember.non_request + .where(source_id: relation.select(:id)) + .where.not(user_id: group.users.select(:id)) + end + # rubocop: enable CodeReuse/ActiveRecord end GroupMembersFinder.prepend_if_ee('EE::GroupMembersFinder') diff --git a/app/finders/groups_finder.rb b/app/finders/groups_finder.rb index 54715557399..4b6b2716c64 100644 --- a/app/finders/groups_finder.rb +++ b/app/finders/groups_finder.rb @@ -12,6 +12,8 @@ # all_available: boolean (defaults to true) # min_access_level: integer # exclude_group_ids: array of integers +# include_parent_descendants: boolean (defaults to false) - includes descendant groups when +# filtering by parent. The parent param must be present. # # Users with full private access can see all groups. The `owned` and `parent` # params can be used to restrict the groups that are returned. @@ -84,7 +86,11 @@ class GroupsFinder < UnionFinder def by_parent(groups) return groups unless params[:parent] - groups.where(parent: params[:parent]) + if include_parent_descendants? + groups.id_in(params[:parent].descendants) + else + groups.where(parent: params[:parent]) + end end # rubocop: enable CodeReuse/ActiveRecord @@ -100,6 +106,10 @@ class GroupsFinder < UnionFinder params.fetch(:all_available, true) end + def include_parent_descendants? + params.fetch(:include_parent_descendants, false) + end + def min_access_level? current_user && params[:min_access_level].present? end diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index f13dc8c2451..9c4aecedd93 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -102,7 +102,7 @@ class IssuableFinder items = filter_items(items) # Let's see if we have to negate anything - items = filter_negated_items(items) + items = filter_negated_items(items) if should_filter_negated_args? # This has to be last as we use a CTE as an optimization fence # for counts by passing the force_cte param and passing the @@ -134,13 +134,15 @@ class IssuableFinder by_my_reaction_emoji(items) end - # Negates all params found in `negatable_params` - def filter_negated_items(items) - return items unless Feature.enabled?(:not_issuable_queries, params.group || params.project, default_enabled: true) + def should_filter_negated_args? + return false unless Feature.enabled?(:not_issuable_queries, params.group || params.project, default_enabled: true) # API endpoints send in `nil` values so we test if there are any non-nil - return items unless not_params.present? && not_params.values.any? + not_params.present? && not_params.values.any? + end + # Negates all params found in `negatable_params` + def filter_negated_items(items) items = by_negated_author(items) items = by_negated_assignee(items) items = by_negated_label(items) @@ -151,7 +153,11 @@ class IssuableFinder end def row_count - Gitlab::IssuablesCountForState.new(self).for_state_or_opened(params[:state]) + fast_fail = Feature.enabled?(:soft_fail_count_by_state, params.group || params.project) + + Gitlab::IssuablesCountForState + .new(self, nil, fast_fail: fast_fail) + .for_state_or_opened(params[:state]) end # We often get counts for each state by running a query per state, and diff --git a/app/finders/keys_finder.rb b/app/finders/keys_finder.rb index e7e78d71a58..9c357e12205 100644 --- a/app/finders/keys_finder.rb +++ b/app/finders/keys_finder.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true class KeysFinder + delegate :find, :find_by_id, to: :execute + InvalidFingerprint = Class.new(StandardError) GitLabAccessDeniedError = Class.new(StandardError) diff --git a/app/finders/merge_requests/by_approvals_finder.rb b/app/finders/merge_requests/by_approvals_finder.rb new file mode 100644 index 00000000000..e6ab1467f06 --- /dev/null +++ b/app/finders/merge_requests/by_approvals_finder.rb @@ -0,0 +1,93 @@ +# frozen_string_literal: true + +module MergeRequests + # Used to filter MergeRequest collections by approvers + class ByApprovalsFinder + attr_reader :usernames, :ids + + # We apply a limitation to the amount of elements that can be part of the filter condition + MAX_FILTER_ELEMENTS = 5 + + # Initialize the finder + # + # @param [Array<String>] usernames + # @param [Array<Integers>] ids + def initialize(usernames, ids) + # rubocop:disable CodeReuse/ActiveRecord + @usernames = Array(usernames).map(&:to_s).uniq.take(MAX_FILTER_ELEMENTS) + @ids = Array(ids).uniq.take(MAX_FILTER_ELEMENTS) + # rubocop:enable CodeReuse/ActiveRecord + end + + # Filter MergeRequest collections by approvers + # + # @param [ActiveRecord::Relation] items the activerecord relation + def execute(items) + if by_no_approvals? + without_approvals(items) + elsif by_any_approvals? + with_any_approvals(items) + elsif ids.present? + find_approved_by_ids(items) + elsif usernames.present? + find_approved_by_names(items) + else + items + end + end + + private + + # Is param using special condition: "None" ? + # + # @return [Boolean] whether special condition "None" is being used + def by_no_approvals? + includes_special_label?(IssuableFinder::Params::FILTER_NONE) + end + + # Is param using special condition: "Any" ? + # + # @return [Boolean] whether special condition "Any" is being used + def by_any_approvals? + includes_special_label?(IssuableFinder::Params::FILTER_ANY) + end + + # Check if we have the special label in ids or usernames field + # + # @param [String] label the special label + # @return [Boolean] whether ids or usernames includes the special label + def includes_special_label?(label) + ids.first.to_s.downcase == label || usernames.map(&:downcase).include?(label) + end + + # Merge Requests without any approval + # + # @param [ActiveRecord::Relation] items + def without_approvals(items) + items.without_approvals + end + + # Merge Requests with any number of approvals + # + # @param [ActiveRecord::Relation] items the activerecord relation + def with_any_approvals(items) + items.select_from_union([ + items.with_approvals + ]) + end + + # Merge Requests approved by given usernames + # + # @param [ActiveRecord::Relation] items the activerecord relation + def find_approved_by_names(items) + items.approved_by_users_with_usernames(*usernames) + end + + # Merge Requests approved by given user IDs + # + # @param [ActiveRecord::Relation] items the activerecord relation + def find_approved_by_ids(items) + items.approved_by_users_with_ids(*ids) + end + end +end diff --git a/app/finders/merge_requests_finder.rb b/app/finders/merge_requests_finder.rb index 37da29b32ff..c998de75ab2 100644 --- a/app/finders/merge_requests_finder.rb +++ b/app/finders/merge_requests_finder.rb @@ -33,7 +33,21 @@ class MergeRequestsFinder < IssuableFinder include MergedAtFilter def self.scalar_params - @scalar_params ||= super + [:wip, :draft, :target_branch, :merged_after, :merged_before] + @scalar_params ||= super + [ + :approved_by_ids, + :deployed_after, + :deployed_before, + :draft, + :environment, + :merged_after, + :merged_before, + :target_branch, + :wip + ] + end + + def self.array_params + @array_params ||= super.merge(approved_by_usernames: []) end def klass @@ -42,11 +56,13 @@ class MergeRequestsFinder < IssuableFinder def filter_items(_items) items = by_commit(super) - items = by_deployment(items) items = by_source_branch(items) items = by_draft(items) items = by_target_branch(items) items = by_merged_at(items) + items = by_approvals(items) + items = by_deployments(items) + by_source_project_id(items) end @@ -80,17 +96,21 @@ class MergeRequestsFinder < IssuableFinder items.where(target_branch: target_branch) end + # rubocop: enable CodeReuse/ActiveRecord def source_project_id @source_project_id ||= params[:source_project_id].presence end + # rubocop: disable CodeReuse/ActiveRecord def by_source_project_id(items) return items unless source_project_id items.where(source_project_id: source_project_id) end + # rubocop: enable CodeReuse/ActiveRecord + # rubocop: disable CodeReuse/ActiveRecord def by_draft(items) draft_param = params[:draft] || params[:wip] @@ -102,6 +122,7 @@ class MergeRequestsFinder < IssuableFinder items end end + # rubocop: enable CodeReuse/ActiveRecord # WIP is deprecated in favor of Draft. Currently both options are supported def wip_match(table) @@ -121,16 +142,54 @@ class MergeRequestsFinder < IssuableFinder .or(table[:title].matches('(Draft)%')) end + # rubocop: disable CodeReuse/ActiveRecord def by_deployment(items) return items unless deployment_id items.includes(:deployment_merge_requests) .where(deployment_merge_requests: { deployment_id: deployment_id }) end + # rubocop: enable CodeReuse/ActiveRecord def deployment_id @deployment_id ||= params[:deployment_id].presence end + + # Filter by merge requests that had been approved by specific users + # rubocop: disable CodeReuse/Finder + def by_approvals(items) + MergeRequests::ByApprovalsFinder + .new(params[:approved_by_usernames], params[:approved_by_ids]) + .execute(items) + end + # rubocop: enable CodeReuse/Finder + + def by_deployments(items) + # Until this feature flag is enabled permanently, we retain the old + # filtering behaviour/code. + return by_deployment(items) unless Feature.enabled?(:deployment_filters) + + env = params[:environment] + before = params[:deployed_before] + after = params[:deployed_after] + id = params[:deployment_id] + + return items if !env && !before && !after && !id + + # Each filter depends on the same JOIN+WHERE. To prevent this JOIN+WHERE + # from being duplicated for every filter, we only produce it once. The + # filter methods in turn expect the JOIN+WHERE to already be present. + # + # This approach ensures that query performance doesn't degrade as the number + # of deployment related filters increases. + deploys = DeploymentMergeRequest.join_deployments_for_merge_requests + deploys = deploys.by_deployment_id(id) if id + deploys = deploys.deployed_to(env) if env + deploys = deploys.deployed_before(before) if before + deploys = deploys.deployed_after(after) if after + + items.where_exists(deploys) + end end MergeRequestsFinder.prepend_if_ee('EE::MergeRequestsFinder') diff --git a/app/finders/milestones_finder.rb b/app/finders/milestones_finder.rb index 16e59b31b36..5d2a54ac979 100644 --- a/app/finders/milestones_finder.rb +++ b/app/finders/milestones_finder.rb @@ -9,6 +9,8 @@ # order - Orders by field default due date asc. # title - filter by title. # state - filters by state. +# start_date & end_date - filters by timeframe (see TimeFrameFilter) +# containing_date - filters by point in time (see TimeFrameFilter) class MilestonesFinder include FinderMethods @@ -28,6 +30,7 @@ class MilestonesFinder items = by_search_title(items) items = by_state(items) items = by_timeframe(items) + items = containing_date(items) order(items) end diff --git a/app/finders/packages/generic/package_finder.rb b/app/finders/packages/generic/package_finder.rb new file mode 100644 index 00000000000..3a260e11fa3 --- /dev/null +++ b/app/finders/packages/generic/package_finder.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +module Packages + module Generic + class PackageFinder + def initialize(project) + @project = project + end + + def execute!(package_name, package_version) + project + .packages + .generic + .by_name_and_version!(package_name, package_version) + end + + private + + attr_reader :project + end + end +end diff --git a/app/finders/projects_finder.rb b/app/finders/projects_finder.rb index 471029c1ef9..14b84d0bfa6 100644 --- a/app/finders/projects_finder.rb +++ b/app/finders/projects_finder.rb @@ -50,7 +50,12 @@ class ProjectsFinder < UnionFinder use_cte = params.delete(:use_cte) collection = Project.wrap_with_cte(collection) if use_cte collection = filter_projects(collection) - sort(collection) + + if params[:sort] == 'similarity' && params[:search] && Feature.enabled?(:project_finder_similarity_sort) + collection.sorted_by_similarity_desc(params[:search]) + else + sort(collection) + end end private @@ -209,7 +214,11 @@ class ProjectsFinder < UnionFinder end def sort(items) - params[:sort].present? ? items.sort_by_attribute(params[:sort]) : items.projects_order_id_desc + if params[:sort].present? + items.sort_by_attribute(params[:sort]) + else + items.projects_order_id_desc + end end def by_archived(projects) diff --git a/app/finders/releases_finder.rb b/app/finders/releases_finder.rb index e961ad4c0ca..da72178169e 100644 --- a/app/finders/releases_finder.rb +++ b/app/finders/releases_finder.rb @@ -9,6 +9,9 @@ class ReleasesFinder @parent = parent @current_user = current_user @params = params + + params[:order_by] ||= 'released_at' + params[:sort] ||= 'desc' end def execute(preload: true) @@ -17,7 +20,8 @@ class ReleasesFinder releases = get_releases releases = by_tag(releases) releases = releases.preloaded if preload - releases.sorted + releases = order_releases(releases) + releases end private @@ -57,4 +61,8 @@ class ReleasesFinder releases.where(tag: params[:tag]) end # rubocop: enable CodeReuse/ActiveRecord + + def order_releases(releases) + releases.sort_by_attribute("#{params[:order_by]}_#{params[:sort]}") + end end |