Commit 215a01f63ccdc085f75a48f6f7ab6f2b15b5852c
1 parent
81092c01
Exists in
master
and in
4 other branches
move notes login to one controller
Showing
16 changed files
with
56 additions
and
47 deletions
Show diff stats
app/assets/javascripts/note.js
| 1 | var NoteList = { | 1 | var NoteList = { |
| 2 | 2 | ||
| 3 | +notes_path: null, | ||
| 4 | +target_params: null, | ||
| 5 | +target_id: 0, | ||
| 6 | +target_type: null, | ||
| 3 | first_id: 0, | 7 | first_id: 0, |
| 4 | last_id: 0, | 8 | last_id: 0, |
| 5 | -resource_name: null, | ||
| 6 | disable:false, | 9 | disable:false, |
| 7 | 10 | ||
| 8 | init: | 11 | init: |
| 9 | - function(resource_name, first_id, last_id) { | ||
| 10 | - this.resource_name = resource_name; | ||
| 11 | - this.first_id = first_id; | ||
| 12 | - this.last_id = last_id; | 12 | + function(tid, tt, path) { |
| 13 | + this.notes_path = path + ".js"; | ||
| 14 | + this.target_id = tid; | ||
| 15 | + this.target_type = tt; | ||
| 16 | + this.target_params = "&target_type=" + this.target_type + "&target_id=" + this.target_id | ||
| 17 | + this.refresh(); | ||
| 13 | this.initRefresh(); | 18 | this.initRefresh(); |
| 14 | this.initLoadMore(); | 19 | this.initLoadMore(); |
| 15 | }, | 20 | }, |
| @@ -19,8 +24,8 @@ getOld: | @@ -19,8 +24,8 @@ getOld: | ||
| 19 | $('.loading').show(); | 24 | $('.loading').show(); |
| 20 | $.ajax({ | 25 | $.ajax({ |
| 21 | type: "GET", | 26 | type: "GET", |
| 22 | - url: location.href, | ||
| 23 | - data: "first_id=" + this.first_id, | 27 | + url: this.notes_path, |
| 28 | + data: "first_id=" + this.first_id + this.target_params, | ||
| 24 | complete: function(){ $('.loading').hide()}, | 29 | complete: function(){ $('.loading').hide()}, |
| 25 | dataType: "script"}); | 30 | dataType: "script"}); |
| 26 | }, | 31 | }, |
| @@ -56,8 +61,8 @@ getNew: | @@ -56,8 +61,8 @@ getNew: | ||
| 56 | // refersh notes list | 61 | // refersh notes list |
| 57 | $.ajax({ | 62 | $.ajax({ |
| 58 | type: "GET", | 63 | type: "GET", |
| 59 | - url: location.href, | ||
| 60 | - data: "last_id=" + this.last_id, | 64 | + url: this.notes_path, |
| 65 | + data: "last_id=" + this.last_id + this.target_params, | ||
| 61 | dataType: "script"}); | 66 | dataType: "script"}); |
| 62 | }, | 67 | }, |
| 63 | 68 | ||
| @@ -66,8 +71,8 @@ refresh: | @@ -66,8 +71,8 @@ refresh: | ||
| 66 | // refersh notes list | 71 | // refersh notes list |
| 67 | $.ajax({ | 72 | $.ajax({ |
| 68 | type: "GET", | 73 | type: "GET", |
| 69 | - url: location.href, | ||
| 70 | - data: "first_id=" + this.first_id + "&last_id=" + this.last_id, | 74 | + url: this.notes_path, |
| 75 | + data: "first_id=" + this.first_id + "&last_id=" + this.last_id + this.target_params, | ||
| 71 | dataType: "script"}); | 76 | dataType: "script"}); |
| 72 | }, | 77 | }, |
| 73 | 78 |
app/controllers/application_controller.rb
| @@ -95,18 +95,6 @@ class ApplicationController < ActionController::Base | @@ -95,18 +95,6 @@ class ApplicationController < ActionController::Base | ||
| 95 | redirect_to @project unless @project.repo_exists? && @project.has_commits? | 95 | redirect_to @project unless @project.repo_exists? && @project.has_commits? |
| 96 | end | 96 | end |
| 97 | 97 | ||
| 98 | - def respond_with_notes | ||
| 99 | - if params[:last_id] && params[:first_id] | ||
| 100 | - @notes = @notes.where("id >= ?", params[:first_id]) | ||
| 101 | - elsif params[:last_id] | ||
| 102 | - @notes = @notes.where("id > ?", params[:last_id]) | ||
| 103 | - elsif params[:first_id] | ||
| 104 | - @notes = @notes.where("id < ?", params[:first_id]) | ||
| 105 | - else | ||
| 106 | - nil | ||
| 107 | - end | ||
| 108 | - end | ||
| 109 | - | ||
| 110 | def no_cache_headers | 98 | def no_cache_headers |
| 111 | response.headers["Cache-Control"] = "no-cache, no-store, max-age=0, must-revalidate" | 99 | response.headers["Cache-Control"] = "no-cache, no-store, max-age=0, must-revalidate" |
| 112 | response.headers["Pragma"] = "no-cache" | 100 | response.headers["Pragma"] = "no-cache" |
app/controllers/commits_controller.rb
| @@ -29,16 +29,9 @@ class CommitsController < ApplicationController | @@ -29,16 +29,9 @@ class CommitsController < ApplicationController | ||
| 29 | 29 | ||
| 30 | git_not_found! and return unless @commit | 30 | git_not_found! and return unless @commit |
| 31 | 31 | ||
| 32 | - @notes = project.commit_notes(@commit).fresh.limit(20) | ||
| 33 | @note = @project.build_commit_note(@commit) | 32 | @note = @project.build_commit_note(@commit) |
| 34 | - | ||
| 35 | @comments_allowed = true | 33 | @comments_allowed = true |
| 36 | @line_notes = project.commit_line_notes(@commit) | 34 | @line_notes = project.commit_line_notes(@commit) |
| 37 | - | ||
| 38 | - respond_to do |format| | ||
| 39 | - format.html | ||
| 40 | - format.js { respond_with_notes } | ||
| 41 | - end | ||
| 42 | end | 35 | end |
| 43 | 36 | ||
| 44 | def compare | 37 | def compare |
app/controllers/issues_controller.rb
| @@ -49,7 +49,6 @@ class IssuesController < ApplicationController | @@ -49,7 +49,6 @@ class IssuesController < ApplicationController | ||
| 49 | end | 49 | end |
| 50 | 50 | ||
| 51 | def show | 51 | def show |
| 52 | - @notes = @issue.notes.inc_author.order("created_at DESC").limit(20) | ||
| 53 | @note = @project.notes.new(:noteable => @issue) | 52 | @note = @project.notes.new(:noteable => @issue) |
| 54 | 53 | ||
| 55 | @commits = if @issue.branch_name && @project.repo.heads.map(&:name).include?(@issue.branch_name) | 54 | @commits = if @issue.branch_name && @project.repo.heads.map(&:name).include?(@issue.branch_name) |
| @@ -61,7 +60,7 @@ class IssuesController < ApplicationController | @@ -61,7 +60,7 @@ class IssuesController < ApplicationController | ||
| 61 | 60 | ||
| 62 | respond_to do |format| | 61 | respond_to do |format| |
| 63 | format.html | 62 | format.html |
| 64 | - format.js { respond_with_notes } | 63 | + format.js |
| 65 | end | 64 | end |
| 66 | end | 65 | end |
| 67 | 66 |
app/controllers/merge_requests_controller.rb
| @@ -39,7 +39,6 @@ class MergeRequestsController < ApplicationController | @@ -39,7 +39,6 @@ class MergeRequestsController < ApplicationController | ||
| 39 | git_not_found! and return | 39 | git_not_found! and return |
| 40 | end | 40 | end |
| 41 | 41 | ||
| 42 | - @notes = @merge_request.notes.inc_author.order("created_at DESC").limit(20) | ||
| 43 | @note = @project.notes.new(:noteable => @merge_request) | 42 | @note = @project.notes.new(:noteable => @merge_request) |
| 44 | 43 | ||
| 45 | @commits = @project.repo. | 44 | @commits = @project.repo. |
| @@ -52,7 +51,7 @@ class MergeRequestsController < ApplicationController | @@ -52,7 +51,7 @@ class MergeRequestsController < ApplicationController | ||
| 52 | 51 | ||
| 53 | respond_to do |format| | 52 | respond_to do |format| |
| 54 | format.html | 53 | format.html |
| 55 | - format.js { respond_with_notes } | 54 | + format.js |
| 56 | end | 55 | end |
| 57 | end | 56 | end |
| 58 | 57 |
app/controllers/notes_controller.rb
| @@ -9,6 +9,23 @@ class NotesController < ApplicationController | @@ -9,6 +9,23 @@ class NotesController < ApplicationController | ||
| 9 | 9 | ||
| 10 | respond_to :js | 10 | respond_to :js |
| 11 | 11 | ||
| 12 | + def index | ||
| 13 | + @notes = case params[:target_type] | ||
| 14 | + when "commit" | ||
| 15 | + then project.commit_notes(project.commit((params[:target_id]))).fresh.limit(20) | ||
| 16 | + when "wall" | ||
| 17 | + then project.common_notes.order("created_at DESC").fresh.limit(20) | ||
| 18 | + when "issue" | ||
| 19 | + then project.issues.find(params[:target_id]).notes.inc_author.order("created_at DESC").limit(20) | ||
| 20 | + when "merge_request" | ||
| 21 | + then project.merge_requests.find(params[:target_id]).notes.inc_author.order("created_at DESC").limit(20) | ||
| 22 | + end | ||
| 23 | + | ||
| 24 | + respond_to do |format| | ||
| 25 | + format.js { respond_with_notes } | ||
| 26 | + end | ||
| 27 | + end | ||
| 28 | + | ||
| 12 | def create | 29 | def create |
| 13 | @note = @project.notes.new(params[:note]) | 30 | @note = @project.notes.new(params[:note]) |
| 14 | @note.author = current_user | 31 | @note.author = current_user |
| @@ -34,4 +51,17 @@ class NotesController < ApplicationController | @@ -34,4 +51,17 @@ class NotesController < ApplicationController | ||
| 34 | end | 51 | end |
| 35 | end | 52 | end |
| 36 | 53 | ||
| 54 | + protected | ||
| 55 | + | ||
| 56 | + def respond_with_notes | ||
| 57 | + if params[:last_id] && params[:first_id] | ||
| 58 | + @notes = @notes.where("id >= ?", params[:first_id]) | ||
| 59 | + elsif params[:last_id] | ||
| 60 | + @notes = @notes.where("id > ?", params[:last_id]) | ||
| 61 | + elsif params[:first_id] | ||
| 62 | + @notes = @notes.where("id < ?", params[:first_id]) | ||
| 63 | + else | ||
| 64 | + nil | ||
| 65 | + end | ||
| 66 | + end | ||
| 37 | end | 67 | end |
app/controllers/projects_controller.rb
| @@ -82,14 +82,10 @@ class ProjectsController < ApplicationController | @@ -82,14 +82,10 @@ class ProjectsController < ApplicationController | ||
| 82 | 82 | ||
| 83 | def wall | 83 | def wall |
| 84 | return render_404 unless @project.wall_enabled | 84 | return render_404 unless @project.wall_enabled |
| 85 | - | ||
| 86 | @note = Note.new | 85 | @note = Note.new |
| 87 | - @notes = @project.common_notes.order("created_at DESC") | ||
| 88 | - @notes = @notes.fresh.limit(20) | ||
| 89 | 86 | ||
| 90 | respond_to do |format| | 87 | respond_to do |format| |
| 91 | format.html | 88 | format.html |
| 92 | - format.js { respond_with_notes } | ||
| 93 | end | 89 | end |
| 94 | end | 90 | end |
| 95 | 91 |
app/views/commits/show.html.haml
| @@ -21,7 +21,7 @@ | @@ -21,7 +21,7 @@ | ||
| 21 | %p.cgray | 21 | %p.cgray |
| 22 | Showing #{pluralize(@commit.diffs.count, "changed file")} | 22 | Showing #{pluralize(@commit.diffs.count, "changed file")} |
| 23 | = render "commits/diffs", :diffs => @commit.diffs | 23 | = render "commits/diffs", :diffs => @commit.diffs |
| 24 | -= render "notes/notes" | 24 | += render "notes/notes", :tid => @commit.id, :tt => "commit" |
| 25 | = render "notes/per_line_form" | 25 | = render "notes/per_line_form" |
| 26 | 26 | ||
| 27 | 27 |
app/views/issues/show.html.haml
app/views/issues/show.js.haml
| @@ -1 +0,0 @@ | @@ -1 +0,0 @@ | ||
| 1 | -= render "notes/load" |
app/views/merge_requests/show.html.haml
| @@ -63,7 +63,7 @@ | @@ -63,7 +63,7 @@ | ||
| 63 | %img{:src => "/assets/ajax-loader-facebook.gif", :class => "dashboard-loader"} | 63 | %img{:src => "/assets/ajax-loader-facebook.gif", :class => "dashboard-loader"} |
| 64 | 64 | ||
| 65 | .merge-request-notes | 65 | .merge-request-notes |
| 66 | - .merge_request_notes= render "notes/notes" | 66 | + .merge_request_notes= render "notes/notes", :tid => @merge_request.id, :tt => "merge_request" |
| 67 | .loading{ :style => "display:none;"} | 67 | .loading{ :style => "display:none;"} |
| 68 | %center= image_tag "ajax-loader.gif" | 68 | %center= image_tag "ajax-loader.gif" |
| 69 | .clear | 69 | .clear |
app/views/notes/_notes.html.haml
| @@ -2,7 +2,7 @@ | @@ -2,7 +2,7 @@ | ||
| 2 | = render "notes/form" | 2 | = render "notes/form" |
| 3 | .clear | 3 | .clear |
| 4 | %hr | 4 | %hr |
| 5 | -%ul#notes-list= render "notes/notes_list" | 5 | +%ul#notes-list |
| 6 | 6 | ||
| 7 | :javascript | 7 | :javascript |
| 8 | $('.delete-note').live('ajax:success', function() { | 8 | $('.delete-note').live('ajax:success', function() { |
| @@ -22,5 +22,5 @@ | @@ -22,5 +22,5 @@ | ||
| 22 | $('.attach_holder').show(); | 22 | $('.attach_holder').show(); |
| 23 | }); | 23 | }); |
| 24 | 24 | ||
| 25 | - NoteList.init("wall", #{@notes.last.try(:id) || 0}, #{@notes.first.try(:id) || 0}); | 25 | + NoteList.init("#{tid}", "#{tt}", "#{project_notes_path(@project)}"); |
| 26 | }); | 26 | }); |
| @@ -0,0 +1 @@ | @@ -0,0 +1 @@ | ||
| 1 | += render "notes/load" |
app/views/projects/wall.html.haml
app/views/projects/wall.js.haml
| @@ -1 +0,0 @@ | @@ -1 +0,0 @@ | ||
| 1 | -= render "notes/load" |
config/routes.rb
| @@ -121,7 +121,7 @@ Gitlab::Application.routes.draw do | @@ -121,7 +121,7 @@ Gitlab::Application.routes.draw do | ||
| 121 | get :search | 121 | get :search |
| 122 | end | 122 | end |
| 123 | end | 123 | end |
| 124 | - resources :notes, :only => [:create, :destroy] | 124 | + resources :notes, :only => [:index, :create, :destroy] |
| 125 | end | 125 | end |
| 126 | root :to => "projects#index" | 126 | root :to => "projects#index" |
| 127 | end | 127 | end |