From 0a31efb57768345e2b3350a493021a26b54994a3 Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Tue, 7 Feb 2017 18:02:02 +0100 Subject: Remove query parameters from notes polling endpoint to make caching easier --- app/assets/javascripts/notes.js | 2 +- app/controllers/projects/notes_controller.rb | 7 +++++- .../projects/notes/_notes_with_form.html.haml | 2 +- config/routes/project.rb | 4 +++- spec/controllers/projects/notes_controller_spec.rb | 27 ++++++++++++++++++++++ spec/routing/project_routing_spec.rb | 18 +++++++++++---- 6 files changed, 52 insertions(+), 8 deletions(-) diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 47fa0f2eb96..df7a7d2a459 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -198,7 +198,7 @@ require('./task_list'); this.refreshing = true; return $.ajax({ url: this.notes_url, - data: "last_fetched_at=" + this.last_fetched_at, + headers: { "X-Last-Fetched-At": this.last_fetched_at }, dataType: "json", success: (function(_this) { return function(data) { diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb index 5cf3a7f593b..d00177e7612 100644 --- a/app/controllers/projects/notes_controller.rb +++ b/app/controllers/projects/notes_controller.rb @@ -211,6 +211,11 @@ class Projects::NotesController < Projects::ApplicationController end def find_current_user_notes - @notes = NotesFinder.new(project, current_user, params).execute.inc_author + @notes = NotesFinder.new(project, current_user, params.merge(last_fetched_at: last_fetched_at)) + .execute.inc_author + end + + def last_fetched_at + request.headers['X-Last-Fetched-At'] end end diff --git a/app/views/projects/notes/_notes_with_form.html.haml b/app/views/projects/notes/_notes_with_form.html.haml index 08c73d94a09..90a150aa74c 100644 --- a/app/views/projects/notes/_notes_with_form.html.haml +++ b/app/views/projects/notes/_notes_with_form.html.haml @@ -23,4 +23,4 @@ to post a comment :javascript - var notes = new Notes("#{namespace_project_notes_path(namespace_id: @project.namespace, project_id: @project, target_id: @noteable.id, target_type: @noteable.class.name.underscore)}", #{@notes.map(&:id).to_json}, #{Time.now.to_i}, "#{diff_view}") + var notes = new Notes("#{namespace_project_noteable_notes_path(namespace_id: @project.namespace, project_id: @project, target_id: @noteable.id, target_type: @noteable.class.name.underscore)}", #{@notes.map(&:id).to_json}, #{Time.now.to_i}, "#{diff_view}") diff --git a/config/routes/project.rb b/config/routes/project.rb index 84f123ff717..a9f95c09bef 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -265,7 +265,7 @@ constraints(ProjectUrlConstrainer.new) do resources :group_links, only: [:index, :create, :update, :destroy], constraints: { id: /\d+/ } - resources :notes, only: [:index, :create, :destroy, :update], concerns: :awardable, constraints: { id: /\d+/ } do + resources :notes, only: [:create, :destroy, :update], concerns: :awardable, constraints: { id: /\d+/ } do member do delete :delete_attachment post :resolve @@ -273,6 +273,8 @@ constraints(ProjectUrlConstrainer.new) do end end + get 'noteable/:target_type/:target_id/notes' => 'notes#index', as: 'noteable_notes' + resources :boards, only: [:index, :show] do scope module: :boards do resources :issues, only: [:index, :update] diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index dc597202050..d80780b1d90 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -200,4 +200,31 @@ describe Projects::NotesController do end end end + + describe 'GET index' do + let(:last_fetched_at) { '1487756246' } + let(:request_params) do + { + namespace_id: project.namespace, + project_id: project, + target_type: 'issue', + target_id: issue.id + } + end + + before do + sign_in(user) + project.team << [user, :developer] + end + + it 'passes last_fetched_at from headers to NotesFinder' do + request.headers['X-Last-Fetched-At'] = last_fetched_at + + expect(NotesFinder).to receive(:new) + .with(anything, anything, hash_including(last_fetched_at: last_fetched_at)) + .and_call_original + + get :index, request_params + end + end end diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index a5bc62ef6c2..d31f1bdfb7c 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -431,12 +431,22 @@ describe 'project routing' do end end - # project_notes GET /:project_id/notes(.:format) notes#index - # POST /:project_id/notes(.:format) notes#create - # project_note DELETE /:project_id/notes/:id(.:format) notes#destroy + # project_noteable_notes GET /:project_id/noteable/:target_type/:target_id/notes notes#index + # POST /:project_id/notes(.:format) notes#create + # project_note DELETE /:project_id/notes/:id(.:format) notes#destroy describe Projects::NotesController, 'routing' do + it 'to #index' do + expect(get('/gitlab/gitlabhq/noteable/issue/1/notes')).to route_to( + 'projects/notes#index', + namespace_id: 'gitlab', + project_id: 'gitlabhq', + target_type: 'issue', + target_id: '1' + ) + end + it_behaves_like 'RESTful project resources' do - let(:actions) { [:index, :create, :destroy] } + let(:actions) { [:create, :destroy] } let(:controller) { 'notes' } end end -- cgit v1.2.3