Remove query parameters from notes polling endpoint to make caching easier

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