diff options
| -rw-r--r-- | CHANGELOG | 1 | ||||
| -rw-r--r-- | app/finders/issuable_finder.rb | 114 | ||||
| -rw-r--r-- | app/views/projects/merge_requests/widget/_heading.html.haml | 73 | ||||
| -rw-r--r-- | db/fixtures/development/05_users.rb | 4 | ||||
| -rw-r--r-- | db/fixtures/development/07_milestones.rb | 2 | ||||
| -rw-r--r-- | db/fixtures/development/09_issues.rb | 2 | ||||
| -rw-r--r-- | db/fixtures/development/12_snippets.rb | 2 |
7 files changed, 113 insertions, 85 deletions
diff --git a/CHANGELOG b/CHANGELOG index 278c6978e83..db7b700e685 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -75,6 +75,7 @@ v 8.1.0 (unreleased) - Only render 404 page from /public - Hide passwords from services API (Alex Lossent) - Fix: Images cannot show when projects' path was changed + - Optimize query when filtering on issuables (Zeger-Jan van de Weg) - Fix padding of outdated discussion item. v 8.0.4 diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 97c7e74c294..c407dfc163a 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -53,15 +53,36 @@ class IssuableFinder end end + def project? + params[:project_id].present? + end + def project return @project if defined?(@project) - @project = - if params[:project_id].present? - Project.find(params[:project_id]) - else - nil - end + if project? + @project = Project.find(params[:project_id]) + + unless Ability.abilities.allowed?(current_user, :read_project, @project) + @project = nil + end + else + @project = nil + end + + @project + end + + def projects + return @projects if defined?(@projects) + + if project? + project + elsif current_user && params[:authorized_only].presence && !current_user_related? + current_user.authorized_projects + else + ProjectsFinder.new.execute(current_user) + end end def search @@ -72,7 +93,7 @@ class IssuableFinder params[:milestone_title].present? end - def no_milestones? + def filter_by_no_milestone? milestones? && params[:milestone_title] == Milestone::None.title end @@ -81,12 +102,22 @@ class IssuableFinder @milestones = if milestones? - Milestone.where(title: params[:milestone_title]) + scope = Milestone.where(project_id: projects) + + scope.where(title: params[:milestone_title]) else nil end end + def labels? + params[:label_name].present? + end + + def filter_by_no_label? + labels? && params[:label_name] == Label::None.title + end + def assignee? params[:assignee_id].present? end @@ -120,19 +151,7 @@ class IssuableFinder private def init_collection - table_name = klass.table_name - - if project - if Ability.abilities.allowed?(current_user, :read_project, project) - project.send(table_name) - else - [] - end - elsif current_user && params[:authorized_only].presence && !current_user_related? - klass.of_projects(current_user.authorized_projects).references(:project) - else - klass.of_projects(ProjectsFinder.new.execute(current_user)).references(:project) - end + klass.all end def by_scope(items) @@ -170,7 +189,12 @@ class IssuableFinder end def by_project(items) - items = items.of_projects(project.id) if project + items = + if projects + items.of_projects(projects).references(:project) + else + items.none + end items end @@ -185,18 +209,6 @@ class IssuableFinder items.sort(params[:sort]) end - def by_milestone(items) - if milestones? - if no_milestones? - items = items.where(milestone_id: [-1, nil]) - else - items = items.where(milestone_id: milestones.try(:pluck, :id)) - end - end - - items - end - def by_assignee(items) if assignee? items = items.where(assignee_id: assignee.try(:id)) @@ -213,20 +225,36 @@ class IssuableFinder items end - def by_label(items) - if params[:label_name].present? - if params[:label_name] == Label::None.title - item_ids = LabelLink.where(target_type: klass.name).pluck(:target_id) + def by_milestone(items) + if milestones? + if filter_by_no_milestone? + items = items.where(milestone_id: [-1, nil]) + else + items = items.joins(:milestone).where(milestones: { title: params[:milestone_title] }) + + if projects + items = items.where(milestones: { project_id: projects }) + end + end + end + + items + end - items = items.where('id NOT IN (?)', item_ids) + def by_label(items) + if labels? + if filter_by_no_label? + items = items. + joins("LEFT OUTER JOIN label_links ON label_links.target_type = '#{klass.name}' AND label_links.target_id = #{klass.table_name}.id"). + where(label_links: { id: nil }) else label_names = params[:label_name].split(",") - item_ids = LabelLink.joins(:label). - where('labels.title in (?)', label_names). - where(target_type: klass.name).pluck(:target_id) + items = items.joins(:labels).where(labels: { title: label_names }) - items = items.where(id: item_ids) + if projects + items = items.where(labels: { project_id: projects }) + end end end diff --git a/app/views/projects/merge_requests/widget/_heading.html.haml b/app/views/projects/merge_requests/widget/_heading.html.haml index 68dda1424cf..10efb811939 100644 --- a/app/views/projects/merge_requests/widget/_heading.html.haml +++ b/app/views/projects/merge_requests/widget/_heading.html.haml @@ -1,44 +1,43 @@ -- if @merge_request.has_ci? - - ci_commit = @merge_request.source_project.ci_commit(@merge_request.source_sha) - - if ci_commit - - status = ci_commit.status - .mr-widget-heading - .ci_widget{class: "ci-#{status}"} - = ci_status_icon(ci_commit) +- ci_commit = @merge_request.source_project.ci_commit(@merge_request.source_sha) +- if ci_commit + - status = ci_commit.status + .mr-widget-heading + .ci_widget{class: "ci-#{status}"} + = ci_status_icon(ci_commit) + %span CI build #{status} + for #{@merge_request.last_commit_short_sha}. + %span.ci-coverage + = link_to "View build details", ci_status_path(ci_commit) + +- elsif @merge_request.has_ci? + - # Compatibility with old CI integrations (ex jenkins) when you request status from CI server via AJAX + - # Remove in later versions when services like Jenkins will set CI status via Commit status API + .mr-widget-heading + - [:success, :skipped, :canceled, :failed, :running, :pending].each do |status| + .ci_widget{class: "ci-#{status}", style: "display:none"} + - if status == :success + - status = "passed" + = icon("check-circle") + - else + = icon("circle") %span CI build #{status} for #{@merge_request.last_commit_short_sha}. %span.ci-coverage - = link_to "View build details", ci_status_path(ci_commit) - - - else - - # Compatibility with old CI integrations (ex jenkins) when you request status from CI server via AJAX - - # Remove in later versions when services like Jenkins will set CI status via Commit status API - .mr-widget-heading - - [:success, :skipped, :canceled, :failed, :running, :pending].each do |status| - .ci_widget{class: "ci-#{status}", style: "display:none"} - - if status == :success - - status = "passed" - = icon("check-circle") - - else - = icon("circle") - %span CI build #{status} - for #{@merge_request.last_commit_short_sha}. - %span.ci-coverage - - if ci_build_details_path(@merge_request) - = link_to "View build details", ci_build_details_path(@merge_request), :"data-no-turbolink" => "data-no-turbolink" + - if ci_build_details_path(@merge_request) + = link_to "View build details", ci_build_details_path(@merge_request), :"data-no-turbolink" => "data-no-turbolink" - .ci_widget - = icon("spinner spin") - Checking CI status for #{@merge_request.last_commit_short_sha}… + .ci_widget + = icon("spinner spin") + Checking CI status for #{@merge_request.last_commit_short_sha}… - .ci_widget.ci-not_found{style: "display:none"} - = icon("times-circle") - Could not find CI status for #{@merge_request.last_commit_short_sha}. + .ci_widget.ci-not_found{style: "display:none"} + = icon("times-circle") + Could not find CI status for #{@merge_request.last_commit_short_sha}. - .ci_widget.ci-error{style: "display:none"} - = icon("times-circle") - Could not connect to the CI server. Please check your settings and try again. + .ci_widget.ci-error{style: "display:none"} + = icon("times-circle") + Could not connect to the CI server. Please check your settings and try again. - :coffeescript - $ -> - merge_request_widget.getCiStatus() + :coffeescript + $ -> + merge_request_widget.getCiStatus() diff --git a/db/fixtures/development/05_users.rb b/db/fixtures/development/05_users.rb index 378354efd5a..03da29c4c68 100644 --- a/db/fixtures/development/05_users.rb +++ b/db/fixtures/development/05_users.rb @@ -1,5 +1,5 @@ Gitlab::Seeder.quiet do - (2..20).each do |i| + 20.times do |i| begin User.create!( username: FFaker::Internet.user_name, @@ -15,7 +15,7 @@ Gitlab::Seeder.quiet do end end - (1..5).each do |i| + 5.times do |i| begin User.create!( username: "user#{i}", diff --git a/db/fixtures/development/07_milestones.rb b/db/fixtures/development/07_milestones.rb index a43116829d9..e028ac82ba3 100644 --- a/db/fixtures/development/07_milestones.rb +++ b/db/fixtures/development/07_milestones.rb @@ -1,6 +1,6 @@ Gitlab::Seeder.quiet do Project.all.each do |project| - (1..5).each do |i| + 5.times do |i| milestone_params = { title: "v#{i}.0", description: FFaker::Lorem.sentence, diff --git a/db/fixtures/development/09_issues.rb b/db/fixtures/development/09_issues.rb index c636e96381c..4fa572fca9b 100644 --- a/db/fixtures/development/09_issues.rb +++ b/db/fixtures/development/09_issues.rb @@ -1,6 +1,6 @@ Gitlab::Seeder.quiet do Project.all.each do |project| - (1..10).each do |i| + 10.times do issue_params = { title: FFaker::Lorem.sentence(6), description: FFaker::Lorem.sentence, diff --git a/db/fixtures/development/12_snippets.rb b/db/fixtures/development/12_snippets.rb index 3bd4b442ade..74898544a69 100644 --- a/db/fixtures/development/12_snippets.rb +++ b/db/fixtures/development/12_snippets.rb @@ -22,7 +22,7 @@ class Member < ActiveRecord::Base end eos - (1..50).each do |i| + 50.times do |i| user = User.all.sample PersonalSnippet.seed(:id, [{ |