diff options
| -rw-r--r-- | CHANGELOG | 2 | ||||
| -rw-r--r-- | app/controllers/application_controller.rb | 8 | ||||
| -rw-r--r-- | app/finders/README.md | 2 | ||||
| -rw-r--r-- | app/finders/issuable_finder.rb | 104 | ||||
| -rw-r--r-- | app/models/ability.rb | 2 | ||||
| -rw-r--r-- | app/views/dashboard/milestones/_milestone.html.haml | 4 | ||||
| -rw-r--r-- | app/views/dashboard/milestones/show.html.haml | 3 | ||||
| -rw-r--r-- | app/views/groups/milestones/_milestone.html.haml | 4 | ||||
| -rw-r--r-- | app/views/groups/milestones/show.html.haml | 3 | ||||
| -rw-r--r-- | app/views/projects/issues/index.html.haml | 2 | ||||
| -rw-r--r-- | app/views/projects/milestones/_milestone.html.haml | 4 | ||||
| -rw-r--r-- | app/views/projects/milestones/show.html.haml | 2 | ||||
| -rw-r--r-- | features/project/project.feature | 5 | ||||
| -rw-r--r-- | features/steps/groups.rb | 4 | ||||
| -rw-r--r-- | features/steps/project/project.rb | 4 | ||||
| -rw-r--r-- | features/steps/shared/project.rb | 5 | ||||
| -rw-r--r-- | spec/finders/issues_finder_spec.rb | 16 | ||||
| -rw-r--r-- | spec/finders/merge_requests_finder_spec.rb | 4 |
18 files changed, 131 insertions, 47 deletions
diff --git a/CHANGELOG b/CHANGELOG index 437420841ce..46a052e1a19 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -6,6 +6,8 @@ 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) + - Fix milestone "Browse Issues" button. + - 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/models/ability.rb b/app/models/ability.rb index e166b4197fd..4e6c60dc8ca 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -115,7 +115,7 @@ class Ability end unless project.snippets_enabled - rules -= named_abilities('snippet') + rules -= named_abilities('project_snippet') end unless project.wiki_enabled diff --git a/app/views/dashboard/milestones/_milestone.html.haml b/app/views/dashboard/milestones/_milestone.html.haml index 21e730bb7ff..d6f3e029a38 100644 --- a/app/views/dashboard/milestones/_milestone.html.haml +++ b/app/views/dashboard/milestones/_milestone.html.haml @@ -3,10 +3,10 @@ = link_to_gfm truncate(milestone.title, length: 100), dashboard_milestone_path(milestone.safe_title, title: milestone.title) .row .col-sm-6 - = link_to dashboard_milestone_path(milestone.safe_title, title: milestone.title) do + = link_to issues_dashboard_path(milestone_title: milestone.title) do = pluralize milestone.issue_count, 'Issue' - = link_to dashboard_milestone_path(milestone.safe_title, title: milestone.title) do + = link_to merge_requests_dashboard_path(milestone_title: milestone.title) do = pluralize milestone.merge_requests_count, 'Merge Request' %span.light #{milestone.percent_complete}% complete diff --git a/app/views/dashboard/milestones/show.html.haml b/app/views/dashboard/milestones/show.html.haml index 24f0bcb60d5..0d204ced7ea 100644 --- a/app/views/dashboard/milestones/show.html.haml +++ b/app/views/dashboard/milestones/show.html.haml @@ -56,6 +56,9 @@ Participants %span.badge= @dashboard_milestone.participants.count + .pull-right + = link_to 'Browse Issues', issues_dashboard_path(milestone_title: @dashboard_milestone.title), class: "btn edit-milestone-link btn-grouped" + .tab-content .tab-pane.active#tab-issues .row diff --git a/app/views/groups/milestones/_milestone.html.haml b/app/views/groups/milestones/_milestone.html.haml index 30093d2d05d..ba30e6e07c6 100644 --- a/app/views/groups/milestones/_milestone.html.haml +++ b/app/views/groups/milestones/_milestone.html.haml @@ -9,10 +9,10 @@ = link_to_gfm truncate(milestone.title, length: 100), group_milestone_path(@group, milestone.safe_title, title: milestone.title) .row .col-sm-6 - = link_to group_milestone_path(@group, milestone.safe_title, title: milestone.title) do + = link_to issues_group_path(@group, milestone_title: milestone.title) do = pluralize milestone.issue_count, 'Issue' - = link_to group_milestone_path(@group, milestone.safe_title, title: milestone.title) do + = link_to merge_requests_group_path(@group, milestone_title: milestone.title) do = pluralize milestone.merge_requests_count, 'Merge Request' %span.light #{milestone.percent_complete}% complete diff --git a/app/views/groups/milestones/show.html.haml b/app/views/groups/milestones/show.html.haml index 6c41cd6b9e4..8f2decb851f 100644 --- a/app/views/groups/milestones/show.html.haml +++ b/app/views/groups/milestones/show.html.haml @@ -62,6 +62,9 @@ Participants %span.badge= @group_milestone.participants.count + .pull-right + = link_to 'Browse Issues', issues_group_path(@group, milestone_title: @group_milestone.title), class: "btn edit-milestone-link btn-grouped" + .tab-content .tab-pane.active#tab-issues .row 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/app/views/projects/milestones/_milestone.html.haml b/app/views/projects/milestones/_milestone.html.haml index 62360158ff9..14a0580f966 100644 --- a/app/views/projects/milestones/_milestone.html.haml +++ b/app/views/projects/milestones/_milestone.html.haml @@ -13,10 +13,10 @@ = milestone.expires_at .row .col-sm-6 - = link_to namespace_project_issues_path(milestone.project.namespace, milestone.project, milestone_id: milestone.id) do + = link_to namespace_project_issues_path(milestone.project.namespace, milestone.project, milestone_title: milestone.title) do = pluralize milestone.issues.count, 'Issue' - = link_to namespace_project_merge_requests_path(milestone.project.namespace, milestone.project, milestone_id: milestone.id) do + = link_to namespace_project_merge_requests_path(milestone.project.namespace, milestone.project, milestone_title: milestone.title) do = pluralize milestone.merge_requests.count, 'Merge Request' %span.light #{milestone.percent_complete}% complete diff --git a/app/views/projects/milestones/show.html.haml b/app/views/projects/milestones/show.html.haml index ee2139e75f6..417eaa1b09d 100644 --- a/app/views/projects/milestones/show.html.haml +++ b/app/views/projects/milestones/show.html.haml @@ -67,7 +67,7 @@ %i.fa.fa-plus New Issue - if can?(current_user, :read_issue, @project) - = link_to 'Browse Issues', namespace_project_issues_path(@milestone.project.namespace, @milestone.project, milestone_id: @milestone.id), class: "btn edit-milestone-link btn-grouped" + = link_to 'Browse Issues', namespace_project_issues_path(@milestone.project.namespace, @milestone.project, milestone_title: @milestone.title), class: "btn edit-milestone-link btn-grouped" .tab-content .tab-pane.active#tab-issues diff --git a/features/project/project.feature b/features/project/project.feature index ef11bceed11..56ae5c78d01 100644 --- a/features/project/project.feature +++ b/features/project/project.feature @@ -68,3 +68,8 @@ Feature: Project When I visit project "Shop" page Then I should not see "New Issue" button And I should not see "New Merge Request" button + + Scenario: I should not see Project snippets + Given I disable snippets in project + When I visit project "Shop" page + Then I should not see "Snippets" button diff --git a/features/steps/groups.rb b/features/steps/groups.rb index 228b83e5fd0..84348d1709a 100644 --- a/features/steps/groups.rb +++ b/features/steps/groups.rb @@ -203,8 +203,8 @@ class Spinach::Features::Groups < Spinach::FeatureSteps step 'I should see group milestones index page with milestones' do page.should have_content('Version 7.2') page.should have_content('GL-113') - page.should have_link('2 Issues', href: group_milestone_path("owned", "version-7-2", title: "Version 7.2")) - page.should have_link('3 Merge Requests', href: group_milestone_path("owned", "gl-113", title: "GL-113")) + page.should have_link('2 Issues', href: issues_group_path("owned", milestone_title: "Version 7.2")) + page.should have_link('3 Merge Requests', href: merge_requests_group_path("owned", milestone_title: "GL-113")) end step 'I click on one group milestone' do diff --git a/features/steps/project/project.rb b/features/steps/project/project.rb index 93fea693f89..fcc15aacc21 100644 --- a/features/steps/project/project.rb +++ b/features/steps/project/project.rb @@ -110,4 +110,8 @@ class Spinach::Features::Project < Spinach::FeatureSteps step 'I should not see "New Merge Request" button' do page.should_not have_link 'New Merge Request' end + + step 'I should not see "Snippets" button' do + page.should_not have_link 'Snippets' + end end diff --git a/features/steps/shared/project.rb b/features/steps/shared/project.rb index 24136fe421c..3059c4ee041 100644 --- a/features/steps/shared/project.rb +++ b/features/steps/shared/project.rb @@ -14,6 +14,11 @@ module SharedProject @project.team << [@user, :master] end + step 'I disable snippets in project' do + @project.snippets_enabled = false + @project.save + end + step 'I disable issues and merge requests in project' do @project.issues_enabled = false @project.merge_requests_enabled = false 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 |