diff --git a/CHANGELOG b/CHANGELOG index 82967cbe0b006509c5085d572e0a7e2eb35cf6b0..553bfb02823a1c41d688e8df21872db8acf1b840 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -6,6 +6,7 @@ v 7.12.0 (unreleased) - Fix Zen Mode not closing with ESC key (Stan Hu) - Allow HipChat API version to be blank and default to v2 (Stan Hu) - Add file attachment support in Milestone description (Stan Hu) + - Set milestone on new issue when creating issue from index with milestone filter active. - Add web hook support for note events (Stan Hu) - Disable "New Issue" and "New Merge Request" buttons when features are disabled in project settings (Stan Hu) - Remove Rack Attack monkey patches and bump to version 4.3.0 (Stan Hu) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 8ce881c741423864f6f8d943c8578ad0905bf90b..e5da94b2327f639bc87a41e982410b4e126a12b4 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -289,14 +289,14 @@ class ApplicationController < ActionController::Base def get_issues_collection set_filters_params - issues = IssuesFinder.new.execute(current_user, @filter_params) - issues + @issuable_finder = IssuesFinder.new(current_user, @filter_params) + @issuable_finder.execute end def get_merge_requests_collection set_filters_params - merge_requests = MergeRequestsFinder.new.execute(current_user, @filter_params) - merge_requests + @issuable_finder = MergeRequestsFinder.new(current_user, @filter_params) + @issuable_finder.execute end def github_import_enabled? diff --git a/app/finders/README.md b/app/finders/README.md index 1f46518d23096345c48c3e11dadb860607b6cb14..1a1c69dea38c8edcb98bc98f8f65b7b3f5f2397d 100644 --- a/app/finders/README.md +++ b/app/finders/README.md @@ -16,7 +16,7 @@ issues = project.issues_for_user_filtered_by(user, params) Better use this: ```ruby -issues = IssuesFinder.new.execute(project, user, filter) +issues = IssuesFinder.new(project, user, filter).execute ``` It will help keep models thiner. diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index e658e1411595442bae810fa34b58ead875ebb876..0bed2115dc7addae67eebd62b5474fbbc2d09ddc 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -23,10 +23,12 @@ class IssuableFinder attr_accessor :current_user, :params - def execute(current_user, params) + def initialize(current_user, params) @current_user = current_user @params = params + end + def execute items = init_collection items = by_scope(items) items = by_state(items) @@ -40,6 +42,77 @@ class IssuableFinder items = sort(items) end + def group + return @group if defined?(@group) + + @group = + if params[:group_id].present? + Group.find(params[:group_id]) + else + nil + end + end + + def project + return @project if defined?(@project) + + @project = + if params[:project_id].present? + Project.find(params[:project_id]) + else + nil + end + end + + def search + params[:search].presence + end + + def milestones? + params[:milestone_title].present? + end + + def milestones + return @milestones if defined?(@milestones) + + @milestones = + if milestones? && params[:milestone_title] != NONE + Milestone.where(title: params[:milestone_title]) + else + nil + end + end + + def assignee? + params[:assignee_id].present? + end + + def assignee + return @assignee if defined?(@assignee) + + @assignee = + if assignee? && params[:assignee_id] != NONE + User.find(params[:assignee_id]) + else + nil + end + end + + def author? + params[:author_id].present? + end + + def author + return @author if defined?(@author) + + @author = + if author? && params[:author_id] != NONE + User.find(params[:author_id]) + else + nil + end + end + private def init_collection @@ -89,25 +162,19 @@ class IssuableFinder end def by_group(items) - if params[:group_id].present? - items = items.of_group(Group.find(params[:group_id])) - end + items = items.of_group(group) if group items end def by_project(items) - if params[:project_id].present? - items = items.of_projects(params[:project_id]) - end + items = items.of_projects(project.id) if project items end def by_search(items) - if params[:search].present? - items = items.search(params[:search]) - end + items = items.search(search) if search items end @@ -117,25 +184,24 @@ class IssuableFinder end def by_milestone(items) - if params[:milestone_title].present? - milestone_ids = (params[:milestone_title] == NONE ? nil : Milestone.where(title: params[:milestone_title]).pluck(:id)) - items = items.where(milestone_id: milestone_ids) + if milestones? + items = items.where(milestone_id: milestones.try(:pluck, :id)) end items end def by_assignee(items) - if params[:assignee_id].present? - items = items.where(assignee_id: (params[:assignee_id] == NONE ? nil : params[:assignee_id])) + if assignee? + items = items.where(assignee_id: assignee.try(:id)) end items end def by_author(items) - if params[:author_id].present? - items = items.where(author_id: (params[:author_id] == NONE ? nil : params[:author_id])) + if author? + items = items.where(author_id: author.try(:id)) end items @@ -155,10 +221,6 @@ class IssuableFinder items end - def project - Project.where(id: params[:project_id]).first if params[:project_id].present? - end - def current_user_related? params[:scope] == 'created-by-me' || params[:scope] == 'authored' || params[:scope] == 'assigned-to-me' end diff --git a/app/views/projects/issues/index.html.haml b/app/views/projects/issues/index.html.haml index a378b37f4a8a4b46fea182dd9f73f4559ae34fdf..1d5597602d1908835b7a43643149add635502d67 100644 --- a/app/views/projects/issues/index.html.haml +++ b/app/views/projects/issues/index.html.haml @@ -14,7 +14,7 @@ = render 'shared/issuable_search_form', path: namespace_project_issues_path(@project.namespace, @project) - if can? current_user, :write_issue, @project - = link_to new_namespace_project_issue_path(@project.namespace, @project, issue: { assignee_id: params[:assignee_id], milestone_id: params[:milestone_id]}), class: "btn btn-new pull-left", title: "New Issue", id: "new_issue_link" do + = link_to new_namespace_project_issue_path(@project.namespace, @project, issue: { assignee_id: @issuable_finder.assignee.try(:id), milestone_id: @issuable_finder.milestones.try(:first).try(:id) }), class: "btn btn-new pull-left", title: "New Issue", id: "new_issue_link" do %i.fa.fa-plus New Issue diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb index 69bac387d201cd4ba12ee17eda7dcb89a748ab84..db20b23f87d55b62909ff9859b75e270b58e4e95 100644 --- a/spec/finders/issues_finder_spec.rb +++ b/spec/finders/issues_finder_spec.rb @@ -26,37 +26,37 @@ describe IssuesFinder do context 'scope: all' do it 'should filter by all' do params = { scope: "all", state: 'opened' } - issues = IssuesFinder.new.execute(user, params) + issues = IssuesFinder.new(user, params).execute expect(issues.size).to eq(3) end it 'should filter by assignee id' do params = { scope: "all", assignee_id: user.id, state: 'opened' } - issues = IssuesFinder.new.execute(user, params) + issues = IssuesFinder.new(user, params).execute expect(issues.size).to eq(2) end it 'should filter by author id' do params = { scope: "all", author_id: user2.id, state: 'opened' } - issues = IssuesFinder.new.execute(user, params) + issues = IssuesFinder.new(user, params).execute expect(issues).to eq([issue3]) end it 'should filter by milestone id' do params = { scope: "all", milestone_title: milestone.title, state: 'opened' } - issues = IssuesFinder.new.execute(user, params) + issues = IssuesFinder.new(user, params).execute expect(issues).to eq([issue1]) end it 'should be empty for unauthorized user' do params = { scope: "all", state: 'opened' } - issues = IssuesFinder.new.execute(nil, params) + issues = IssuesFinder.new(nil, params).execute expect(issues.size).to be_zero end it 'should not include unauthorized issues' do params = { scope: "all", state: 'opened' } - issues = IssuesFinder.new.execute(user2, params) + issues = IssuesFinder.new(user2, params).execute expect(issues.size).to eq(2) expect(issues).not_to include(issue1) expect(issues).to include(issue2) @@ -67,13 +67,13 @@ describe IssuesFinder do context 'personal scope' do it 'should filter by assignee' do params = { scope: "assigned-to-me", state: 'opened' } - issues = IssuesFinder.new.execute(user, params) + issues = IssuesFinder.new(user, params).execute expect(issues.size).to eq(2) end it 'should filter by project' do params = { scope: "assigned-to-me", state: 'opened', project_id: project1.id } - issues = IssuesFinder.new.execute(user, params) + issues = IssuesFinder.new(user, params).execute expect(issues.size).to eq(1) end end diff --git a/spec/finders/merge_requests_finder_spec.rb b/spec/finders/merge_requests_finder_spec.rb index 8536377a7f079b534fd8ea9e728ee728e003ce86..bc385fd0d691c68820214a50d0c9ec293d46f9c8 100644 --- a/spec/finders/merge_requests_finder_spec.rb +++ b/spec/finders/merge_requests_finder_spec.rb @@ -20,13 +20,13 @@ describe MergeRequestsFinder do describe "#execute" do it 'should filter by scope' do params = { scope: 'authored', state: 'opened' } - merge_requests = MergeRequestsFinder.new.execute(user, params) + merge_requests = MergeRequestsFinder.new(user, params).execute expect(merge_requests.size).to eq(2) end it 'should filter by project' do params = { project_id: project1.id, scope: 'authored', state: 'opened' } - merge_requests = MergeRequestsFinder.new.execute(user, params) + merge_requests = MergeRequestsFinder.new(user, params).execute expect(merge_requests.size).to eq(1) end end