Commit 679d0d6d760b850e27c13f3ce0f812b8b081df7f
1 parent
63fe042d
Exists in
master
and in
4 other branches
Context refactoring. Move Issues list, Search logic to context
Showing
17 changed files
with
144 additions
and
127 deletions
Show diff stats
app/contexts/commit_load.rb
| ... | ... | @@ -1,33 +0,0 @@ |
| 1 | -class CommitLoad < BaseContext | |
| 2 | - def execute | |
| 3 | - result = { | |
| 4 | - commit: nil, | |
| 5 | - suppress_diff: false, | |
| 6 | - line_notes: [], | |
| 7 | - notes_count: 0, | |
| 8 | - note: nil, | |
| 9 | - status: :ok | |
| 10 | - } | |
| 11 | - | |
| 12 | - commit = project.commit(params[:id]) | |
| 13 | - | |
| 14 | - if commit | |
| 15 | - commit = CommitDecorator.decorate(commit) | |
| 16 | - line_notes = project.commit_line_notes(commit) | |
| 17 | - | |
| 18 | - result[:commit] = commit | |
| 19 | - result[:note] = project.build_commit_note(commit) | |
| 20 | - result[:line_notes] = line_notes | |
| 21 | - result[:notes_count] = line_notes.count + project.commit_notes(commit).count | |
| 22 | - | |
| 23 | - begin | |
| 24 | - result[:suppress_diff] = true if commit.diffs.size > 200 && !params[:force_show_diff] | |
| 25 | - rescue Grit::Git::GitTimeout | |
| 26 | - result[:suppress_diff] = true | |
| 27 | - result[:status] = :huge_commit | |
| 28 | - end | |
| 29 | - end | |
| 30 | - | |
| 31 | - result | |
| 32 | - end | |
| 33 | -end |
| ... | ... | @@ -0,0 +1,33 @@ |
| 1 | +class CommitLoadContext < BaseContext | |
| 2 | + def execute | |
| 3 | + result = { | |
| 4 | + commit: nil, | |
| 5 | + suppress_diff: false, | |
| 6 | + line_notes: [], | |
| 7 | + notes_count: 0, | |
| 8 | + note: nil, | |
| 9 | + status: :ok | |
| 10 | + } | |
| 11 | + | |
| 12 | + commit = project.commit(params[:id]) | |
| 13 | + | |
| 14 | + if commit | |
| 15 | + commit = CommitDecorator.decorate(commit) | |
| 16 | + line_notes = project.commit_line_notes(commit) | |
| 17 | + | |
| 18 | + result[:commit] = commit | |
| 19 | + result[:note] = project.build_commit_note(commit) | |
| 20 | + result[:line_notes] = line_notes | |
| 21 | + result[:notes_count] = line_notes.count + project.commit_notes(commit).count | |
| 22 | + | |
| 23 | + begin | |
| 24 | + result[:suppress_diff] = true if commit.diffs.size > 200 && !params[:force_show_diff] | |
| 25 | + rescue Grit::Git::GitTimeout | |
| 26 | + result[:suppress_diff] = true | |
| 27 | + result[:status] = :huge_commit | |
| 28 | + end | |
| 29 | + end | |
| 30 | + | |
| 31 | + result | |
| 32 | + end | |
| 33 | +end | ... | ... |
| ... | ... | @@ -0,0 +1,29 @@ |
| 1 | +class IssuesListContext < BaseContext | |
| 2 | + include IssuesHelper | |
| 3 | + | |
| 4 | + attr_accessor :issues | |
| 5 | + | |
| 6 | + def execute | |
| 7 | + @issues = case params[:f] | |
| 8 | + when issues_filter[:all] then @project.issues | |
| 9 | + when issues_filter[:closed] then @project.issues.closed | |
| 10 | + when issues_filter[:to_me] then @project.issues.opened.assigned(current_user) | |
| 11 | + else @project.issues.opened | |
| 12 | + end | |
| 13 | + | |
| 14 | + @issues = @issues.tagged_with(params[:label_name]) if params[:label_name].present? | |
| 15 | + @issues = @issues.includes(:author, :project).order("updated_at") | |
| 16 | + | |
| 17 | + # Filter by specific assignee_id (or lack thereof)? | |
| 18 | + if params[:assignee_id].present? | |
| 19 | + @issues = @issues.where(assignee_id: (params[:assignee_id] == '0' ? nil : params[:assignee_id])) | |
| 20 | + end | |
| 21 | + | |
| 22 | + # Filter by specific milestone_id (or lack thereof)? | |
| 23 | + if params[:milestone_id].present? | |
| 24 | + @issues = @issues.where(milestone_id: (params[:milestone_id] == '0' ? nil : params[:milestone_id])) | |
| 25 | + end | |
| 26 | + | |
| 27 | + @issues | |
| 28 | + end | |
| 29 | +end | ... | ... |
app/contexts/merge_requests_load.rb
| ... | ... | @@ -1,16 +0,0 @@ |
| 1 | -class MergeRequestsLoad < BaseContext | |
| 2 | - def execute | |
| 3 | - type = params[:f] | |
| 4 | - | |
| 5 | - merge_requests = project.merge_requests | |
| 6 | - | |
| 7 | - merge_requests = case type | |
| 8 | - when 'all' then merge_requests | |
| 9 | - when 'closed' then merge_requests.closed | |
| 10 | - when 'assigned-to-me' then merge_requests.opened.assigned(current_user) | |
| 11 | - else merge_requests.opened | |
| 12 | - end.page(params[:page]).per(20) | |
| 13 | - | |
| 14 | - merge_requests.includes(:author, :project).order("closed, created_at desc") | |
| 15 | - end | |
| 16 | -end |
| ... | ... | @@ -0,0 +1,16 @@ |
| 1 | +class MergeRequestsLoadContext < BaseContext | |
| 2 | + def execute | |
| 3 | + type = params[:f] | |
| 4 | + | |
| 5 | + merge_requests = project.merge_requests | |
| 6 | + | |
| 7 | + merge_requests = case type | |
| 8 | + when 'all' then merge_requests | |
| 9 | + when 'closed' then merge_requests.closed | |
| 10 | + when 'assigned-to-me' then merge_requests.opened.assigned(current_user) | |
| 11 | + else merge_requests.opened | |
| 12 | + end.page(params[:page]).per(20) | |
| 13 | + | |
| 14 | + merge_requests.includes(:author, :project).order("closed, created_at desc") | |
| 15 | + end | |
| 16 | +end | ... | ... |
| ... | ... | @@ -0,0 +1,27 @@ |
| 1 | +class SearchContext | |
| 2 | + attr_accessor :project_ids, :params | |
| 3 | + | |
| 4 | + def initialize(project_ids, params) | |
| 5 | + @project_ids, @params = project_ids, params.dup | |
| 6 | + end | |
| 7 | + | |
| 8 | + def execute | |
| 9 | + query = params[:search] | |
| 10 | + | |
| 11 | + return result unless query.present? | |
| 12 | + | |
| 13 | + result[:projects] = Project.where(id: project_ids).search(query).limit(10) | |
| 14 | + result[:merge_requests] = MergeRequest.where(project_id: project_ids).search(query).limit(10) | |
| 15 | + result[:issues] = Issue.where(project_id: project_ids).search(query).limit(10) | |
| 16 | + result | |
| 17 | + end | |
| 18 | + | |
| 19 | + def result | |
| 20 | + @result ||= { | |
| 21 | + projects: [], | |
| 22 | + merge_requests: [], | |
| 23 | + issues: [] | |
| 24 | + } | |
| 25 | + end | |
| 26 | +end | |
| 27 | + | ... | ... |
app/controllers/commit_controller.rb
| ... | ... | @@ -8,7 +8,7 @@ class CommitController < ProjectResourceController |
| 8 | 8 | before_filter :require_non_empty_project |
| 9 | 9 | |
| 10 | 10 | def show |
| 11 | - result = CommitLoad.new(project, current_user, params).execute | |
| 11 | + result = CommitLoadContext.new(project, current_user, params).execute | |
| 12 | 12 | |
| 13 | 13 | @commit = result[:commit] |
| 14 | 14 | git_not_found! unless @commit | ... | ... |
app/controllers/dashboard_controller.rb
| ... | ... | @@ -6,7 +6,7 @@ class DashboardController < ApplicationController |
| 6 | 6 | @projects = current_user.projects_with_events |
| 7 | 7 | @projects = @projects.page(params[:page]).per(20) |
| 8 | 8 | |
| 9 | - @events = Event.recent_for_user(current_user).limit(20).offset(params[:offset] || 0) | |
| 9 | + @events = Event.in_projects(current_user.project_ids).limit(20).offset(params[:offset] || 0) | |
| 10 | 10 | @last_push = current_user.recent_push |
| 11 | 11 | |
| 12 | 12 | respond_to do |format| |
| ... | ... | @@ -19,14 +19,14 @@ class DashboardController < ApplicationController |
| 19 | 19 | # Get authored or assigned open merge requests |
| 20 | 20 | def merge_requests |
| 21 | 21 | @projects = current_user.projects.all |
| 22 | - @merge_requests = current_user.cared_merge_requests.order("created_at DESC").page(params[:page]).per(20) | |
| 22 | + @merge_requests = current_user.cared_merge_requests.recent.page(params[:page]).per(20) | |
| 23 | 23 | end |
| 24 | 24 | |
| 25 | 25 | # Get only assigned issues |
| 26 | 26 | def issues |
| 27 | 27 | @projects = current_user.projects.all |
| 28 | 28 | @user = current_user |
| 29 | - @issues = current_user.assigned_issues.opened.order("created_at DESC").page(params[:page]).per(20) | |
| 29 | + @issues = current_user.assigned_issues.opened.recent.page(params[:page]).per(20) | |
| 30 | 30 | @issues = @issues.includes(:author, :project) |
| 31 | 31 | |
| 32 | 32 | respond_to do |format| | ... | ... |
app/controllers/groups_controller.rb
| ... | ... | @@ -6,10 +6,7 @@ class GroupsController < ApplicationController |
| 6 | 6 | before_filter :projects |
| 7 | 7 | |
| 8 | 8 | def show |
| 9 | - @events = Event.where(project_id: project_ids). | |
| 10 | - order('id DESC'). | |
| 11 | - limit(20).offset(params[:offset] || 0) | |
| 12 | - | |
| 9 | + @events = Event.in_projects(project_ids).limit(20).offset(params[:offset] || 0) | |
| 13 | 10 | @last_push = current_user.recent_push |
| 14 | 11 | |
| 15 | 12 | respond_to do |format| |
| ... | ... | @@ -22,14 +19,14 @@ class GroupsController < ApplicationController |
| 22 | 19 | # Get authored or assigned open merge requests |
| 23 | 20 | def merge_requests |
| 24 | 21 | @merge_requests = current_user.cared_merge_requests |
| 25 | - @merge_requests = @merge_requests.of_group(@group).order("created_at DESC").page(params[:page]).per(20) | |
| 22 | + @merge_requests = @merge_requests.of_group(@group).recent.page(params[:page]).per(20) | |
| 26 | 23 | end |
| 27 | 24 | |
| 28 | 25 | # Get only assigned issues |
| 29 | 26 | def issues |
| 30 | 27 | @user = current_user |
| 31 | 28 | @issues = current_user.assigned_issues.opened |
| 32 | - @issues = @issues.of_group(@group).order("created_at DESC").page(params[:page]).per(20) | |
| 29 | + @issues = @issues.of_group(@group).recent.page(params[:page]).per(20) | |
| 33 | 30 | @issues = @issues.includes(:author, :project) |
| 34 | 31 | |
| 35 | 32 | respond_to do |format| |
| ... | ... | @@ -39,16 +36,11 @@ class GroupsController < ApplicationController |
| 39 | 36 | end |
| 40 | 37 | |
| 41 | 38 | def search |
| 42 | - query = params[:search] | |
| 43 | - | |
| 44 | - @merge_requests = [] | |
| 45 | - @issues = [] | |
| 39 | + result = SearchContext.new(project_ids, params).execute | |
| 46 | 40 | |
| 47 | - if query.present? | |
| 48 | - @projects = @projects.search(query).limit(10) | |
| 49 | - @merge_requests = MergeRequest.where(project_id: project_ids).search(query).limit(10) | |
| 50 | - @issues = Issue.where(project_id: project_ids).search(query).limit(10) | |
| 51 | - end | |
| 41 | + @projects = result[:projects] | |
| 42 | + @merge_requests = result[:merge_requests] | |
| 43 | + @issues = result[:issues] | |
| 52 | 44 | end |
| 53 | 45 | |
| 54 | 46 | def people | ... | ... |
app/controllers/issues_controller.rb
| 1 | 1 | class IssuesController < ProjectResourceController |
| 2 | 2 | before_filter :module_enabled |
| 3 | 3 | before_filter :issue, only: [:edit, :update, :destroy, :show] |
| 4 | - helper_method :issues_filter | |
| 5 | 4 | |
| 6 | 5 | # Allow read any issue |
| 7 | 6 | before_filter :authorize_read_issue! |
| ... | ... | @@ -19,7 +18,6 @@ class IssuesController < ProjectResourceController |
| 19 | 18 | |
| 20 | 19 | def index |
| 21 | 20 | @issues = issues_filtered |
| 22 | - | |
| 23 | 21 | @issues = @issues.page(params[:page]).per(20) |
| 24 | 22 | |
| 25 | 23 | respond_to do |format| |
| ... | ... | @@ -54,7 +52,7 @@ class IssuesController < ProjectResourceController |
| 54 | 52 | |
| 55 | 53 | respond_to do |format| |
| 56 | 54 | format.html do |
| 57 | - if @issue.valid? | |
| 55 | + if @issue.valid? | |
| 58 | 56 | redirect_to project_issue_path(@project, @issue) |
| 59 | 57 | else |
| 60 | 58 | render :new |
| ... | ... | @@ -69,7 +67,7 @@ class IssuesController < ProjectResourceController |
| 69 | 67 | |
| 70 | 68 | respond_to do |format| |
| 71 | 69 | format.js |
| 72 | - format.html do | |
| 70 | + format.html do | |
| 73 | 71 | if @issue.valid? |
| 74 | 72 | redirect_to [@project, @issue] |
| 75 | 73 | else |
| ... | ... | @@ -134,35 +132,6 @@ class IssuesController < ProjectResourceController |
| 134 | 132 | end |
| 135 | 133 | |
| 136 | 134 | def issues_filtered |
| 137 | - @issues = case params[:f] | |
| 138 | - when issues_filter[:all] then @project.issues | |
| 139 | - when issues_filter[:closed] then @project.issues.closed | |
| 140 | - when issues_filter[:to_me] then @project.issues.opened.assigned(current_user) | |
| 141 | - else @project.issues.opened | |
| 142 | - end | |
| 143 | - | |
| 144 | - @issues = @issues.tagged_with(params[:label_name]) if params[:label_name].present? | |
| 145 | - @issues = @issues.includes(:author, :project).order("updated_at") | |
| 146 | - | |
| 147 | - # Filter by specific assignee_id (or lack thereof)? | |
| 148 | - if params[:assignee_id].present? | |
| 149 | - @issues = @issues.where(assignee_id: (params[:assignee_id] == '0' ? nil : params[:assignee_id])) | |
| 150 | - end | |
| 151 | - | |
| 152 | - # Filter by specific milestone_id (or lack thereof)? | |
| 153 | - if params[:milestone_id].present? | |
| 154 | - @issues = @issues.where(milestone_id: (params[:milestone_id] == '0' ? nil : params[:milestone_id])) | |
| 155 | - end | |
| 156 | - | |
| 157 | - @issues | |
| 158 | - end | |
| 159 | - | |
| 160 | - def issues_filter | |
| 161 | - { | |
| 162 | - all: "all", | |
| 163 | - closed: "closed", | |
| 164 | - to_me: "assigned-to-me", | |
| 165 | - open: "open" | |
| 166 | - } | |
| 135 | + @issues = IssuesListContext.new(project, current_user, params).execute | |
| 167 | 136 | end |
| 168 | 137 | end | ... | ... |
app/controllers/merge_requests_controller.rb
| ... | ... | @@ -18,7 +18,7 @@ class MergeRequestsController < ProjectResourceController |
| 18 | 18 | |
| 19 | 19 | |
| 20 | 20 | def index |
| 21 | - @merge_requests = MergeRequestsLoad.new(project, current_user, params).execute | |
| 21 | + @merge_requests = MergeRequestsLoadContext.new(project, current_user, params).execute | |
| 22 | 22 | end |
| 23 | 23 | |
| 24 | 24 | def show |
| ... | ... | @@ -55,7 +55,7 @@ class MergeRequestsController < ProjectResourceController |
| 55 | 55 | @merge_request.reload_code |
| 56 | 56 | redirect_to [@project, @merge_request], notice: 'Merge request was successfully created.' |
| 57 | 57 | else |
| 58 | - render action: "new" | |
| 58 | + render action: "new" | |
| 59 | 59 | end |
| 60 | 60 | end |
| 61 | 61 | |
| ... | ... | @@ -70,7 +70,7 @@ class MergeRequestsController < ProjectResourceController |
| 70 | 70 | end |
| 71 | 71 | |
| 72 | 72 | def automerge_check |
| 73 | - if @merge_request.unchecked? | |
| 73 | + if @merge_request.unchecked? | |
| 74 | 74 | @merge_request.check_if_can_be_merged |
| 75 | 75 | end |
| 76 | 76 | render json: {state: @merge_request.human_state} |
| ... | ... | @@ -125,7 +125,7 @@ class MergeRequestsController < ProjectResourceController |
| 125 | 125 | |
| 126 | 126 | def validates_merge_request |
| 127 | 127 | # Show git not found page if target branch doesnt exist |
| 128 | - return git_not_found! unless @project.repo.heads.map(&:name).include?(@merge_request.target_branch) | |
| 128 | + return git_not_found! unless @project.repo.heads.map(&:name).include?(@merge_request.target_branch) | |
| 129 | 129 | |
| 130 | 130 | # Show git not found page if source branch doesnt exist |
| 131 | 131 | # and there is no saved commits between source & target branch |
| ... | ... | @@ -136,7 +136,7 @@ class MergeRequestsController < ProjectResourceController |
| 136 | 136 | # Build a note object for comment form |
| 137 | 137 | @note = @project.notes.new(noteable: @merge_request) |
| 138 | 138 | |
| 139 | - # Get commits from repository | |
| 139 | + # Get commits from repository | |
| 140 | 140 | # or from cache if already merged |
| 141 | 141 | @commits = @merge_request.commits |
| 142 | 142 | @commits = CommitDecorator.decorate(@commits) | ... | ... |
app/controllers/milestones_controller.rb
app/controllers/refs_controller.rb
| ... | ... | @@ -9,9 +9,9 @@ class RefsController < ProjectResourceController |
| 9 | 9 | before_filter :ref |
| 10 | 10 | before_filter :define_tree_vars, only: [:blob, :logs_tree] |
| 11 | 11 | |
| 12 | - def switch | |
| 13 | - respond_to do |format| | |
| 14 | - format.html do | |
| 12 | + def switch | |
| 13 | + respond_to do |format| | |
| 14 | + format.html do | |
| 15 | 15 | new_path = if params[:destination] == "tree" |
| 16 | 16 | project_tree_path(@project, @ref) |
| 17 | 17 | else | ... | ... |
app/controllers/search_controller.rb
| 1 | 1 | class SearchController < ApplicationController |
| 2 | 2 | def show |
| 3 | - query = params[:search] | |
| 3 | + result = SearchContext.new(current_user.project_ids, params).execute | |
| 4 | 4 | |
| 5 | - @projects = [] | |
| 6 | - @merge_requests = [] | |
| 7 | - @issues = [] | |
| 8 | - | |
| 9 | - if query.present? | |
| 10 | - @projects = current_user.projects.search(query).limit(10) | |
| 11 | - @merge_requests = MergeRequest.where(project_id: current_user.project_ids).search(query).limit(10) | |
| 12 | - @issues = Issue.where(project_id: current_user.project_ids).search(query).limit(10) | |
| 13 | - end | |
| 5 | + @projects = result[:projects] | |
| 6 | + @merge_requests = result[:merge_requests] | |
| 7 | + @issues = result[:issues] | |
| 14 | 8 | end |
| 15 | 9 | end | ... | ... |
app/helpers/issues_helper.rb
| ... | ... | @@ -43,4 +43,13 @@ module IssuesHelper |
| 43 | 43 | # Milestone uses :title, Issue uses :name |
| 44 | 44 | OpenStruct.new(id: 0, title: 'Unspecified', name: 'Unassigned') |
| 45 | 45 | end |
| 46 | + | |
| 47 | + def issues_filter | |
| 48 | + { | |
| 49 | + all: "all", | |
| 50 | + closed: "closed", | |
| 51 | + to_me: "assigned-to-me", | |
| 52 | + open: "open" | |
| 53 | + } | |
| 54 | + end | |
| 46 | 55 | end | ... | ... |
app/models/event.rb
| ... | ... | @@ -30,6 +30,7 @@ class Event < ActiveRecord::Base |
| 30 | 30 | # Scopes |
| 31 | 31 | scope :recent, order("created_at DESC") |
| 32 | 32 | scope :code_push, where(action: Pushed) |
| 33 | + scope :in_projects, ->(project_ids) { where(project_id: project_ids).recent } | |
| 33 | 34 | |
| 34 | 35 | class << self |
| 35 | 36 | def determine_action(record) |
| ... | ... | @@ -39,10 +40,6 @@ class Event < ActiveRecord::Base |
| 39 | 40 | Event::Commented |
| 40 | 41 | end |
| 41 | 42 | end |
| 42 | - | |
| 43 | - def recent_for_user user | |
| 44 | - where(project_id: user.projects.map(&:id)).recent | |
| 45 | - end | |
| 46 | 43 | end |
| 47 | 44 | |
| 48 | 45 | # Next events currently enabled for system | ... | ... |
app/roles/issue_commonality.rb
| 1 | -# Contains common functionality | |
| 2 | -# shared between Issues and MergeRequests | |
| 1 | +# Contains common functionality shared between Issues and MergeRequests | |
| 3 | 2 | module IssueCommonality |
| 4 | 3 | extend ActiveSupport::Concern |
| 5 | 4 | |
| ... | ... | @@ -18,6 +17,7 @@ module IssueCommonality |
| 18 | 17 | scope :closed, where(closed: true) |
| 19 | 18 | scope :of_group, ->(group) { where(project_id: group.project_ids) } |
| 20 | 19 | scope :assigned, ->(u) { where(assignee_id: u.id)} |
| 20 | + scope :recent, order("created_at DESC") | |
| 21 | 21 | |
| 22 | 22 | delegate :name, |
| 23 | 23 | :email, | ... | ... |