From 45e4727f97034f719b4fb2a061fd626f545db968 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Mon, 25 May 2015 13:36:28 +0200 Subject: [PATCH] Set milestone on new issue when creating issue from index with milestone filter active. --- CHANGELOG | 1 + app/controllers/application_controller.rb | 8 +- app/finders/README.md | 2 +- app/finders/issuable_finder.rb | 104 ++++++++++++++++----- app/views/projects/issues/index.html.haml | 2 +- spec/finders/issues_finder_spec.rb | 16 ++-- spec/finders/merge_requests_finder_spec.rb | 4 +- 7 files changed, 100 insertions(+), 37 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 82967cbe0b0..553bfb02823 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 8ce881c7414..e5da94b2327 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 1f46518d230..1a1c69dea38 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 e658e141159..0bed2115dc7 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 a378b37f4a8..1d5597602d1 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 69bac387d20..db20b23f87d 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 8536377a7f0..bc385fd0d69 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