Note subscriptions

app/abilities/api_capability.rb

@@ -9,6 +9,7 @@ def initialize(token)
 
       if user&.active?
         can [:create, :comment, :close, :reopen], Note if scope?(token, :write_notes)
+        can [:create, :destroy], NoteSubscription if scope?(token, :write_notes)
         can [:show, :data], Trace if scope?(token, :read_gpx)
         can [:create, :update, :destroy], Trace if scope?(token, :write_gpx)
         can [:details], User if scope?(token, :read_prefs)

app/assets/javascripts/index/note.js

@@ -37,31 +37,35 @@ OSM.Note = function (map) {
   };
 
   function initialize(path, id) {
-    content.find("button[type=submit]").on("click", function (e) {
+    content.find("button[name]").on("click", function (e) {
       e.preventDefault();
       var data = $(e.target).data();
-      var form = e.target.form;
-
-      $(form).find("button[type=submit]").prop("disabled", true);
-
-      $.ajax({
+      var name = $(e.target).attr("name");
+      var ajaxSettings = {
         url: data.url,
         type: data.method,
         oauth: true,
-        data: { text: $(form.text).val() },
-        success: function () {
-          OSM.loadSidebarContent(path, function () {
+        success: () => {
+          OSM.loadSidebarContent(path, () => {
             initialize(path, id);
             moveToNote();
           });
         },
-        error: function (xhr) {
-          $(form).find("#comment-error")
+        error: updateButtons
+      };
+
+      if (name !== "subscribe" && name !== "unsubscribe") {
+        ajaxSettings.data = { text: $("textarea").val() };
+        ajaxSettings.error = (xhr) => {
+          content.find("#comment-error")
             .text(xhr.responseText)
             .prop("hidden", false);
-          updateButtons(form);
-        }
-      });
+          updateButtons();
+        };
+      }
+
+      content.find("button[name]").prop("disabled", true);
+      $.ajax(ajaxSettings);
     });
 
     content.find("textarea").on("input", function (e) {
@@ -82,14 +86,16 @@ OSM.Note = function (map) {
     }
   }
 
-  function updateButtons(form) {
-    $(form).find("button[type=submit]").prop("disabled", false);
-    if ($(form.text).val() === "") {
-      $(form.close).text($(form.close).data("defaultActionText"));
-      $(form.comment).prop("disabled", true);
+  function updateButtons() {
+    var resolveButton = content.find("button[name='close']");
+    var commentButton = content.find("button[name='comment']");
+
+    content.find("button[name]").prop("disabled", false);
+    if (content.find("textarea").val() === "") {
+      resolveButton.text(resolveButton.data("defaultActionText"));
+      commentButton.prop("disabled", true);
     } else {
-      $(form.close).text($(form.close).data("commentActionText"));
-      $(form.comment).prop("disabled", false);
+      resolveButton.text(resolveButton.data("commentActionText"));
     }
   }
 

app/controllers/api/note_subscriptions_controller.rb

@@ -0,0 +1,22 @@
+module Api
+  class NoteSubscriptionsController < ApiController
+    before_action :check_api_writable
+    before_action :authorize
+
+    authorize_resource
+
+    def create
+      note_id = params[:note_id].to_i
+      note = Note.find(note_id)
+      note.subscribers << current_user
+    rescue ActiveRecord::RecordNotUnique
+      head :conflict
+    end
+
+    def destroy
+      note_id = params[:note_id].to_i
+      count = NoteSubscription.where(:user => current_user, :note => note_id).delete_all
+      head :not_found if count.zero?
+    end
+  end
+end

app/views/notes/show.html.erb

@@ -28,6 +28,32 @@
     <p class='alert alert-warning'><%= t ".anonymous_warning" %></p>
   <% end -%>
 
+  <div class="row">
+    <div class="col">
+      <h4><%= t(".discussion") %></h4>
+    </div>
+
+    <% if current_user %>
+      <div class="col-auto">
+        <% if @note.subscribers.exists?(current_user.id) %>
+          <%= tag.button t(".unsubscribe"),
+                         :type => "button",
+                         :class => "btn btn-sm btn-primary",
+                         :name => "unsubscribe",
+                         :data => { :method => "DELETE",
+                                    :url => api_note_subscription_path(@note) } %>
+        <% else %>
+          <%= tag.button t(".subscribe"),
+                         :type => "button",
+                         :class => "btn btn-sm btn-primary",
+                         :name => "subscribe",
+                         :data => { :method => "POST",
+                                    :url => api_note_subscription_path(@note) } %>
+        <% end %>
+      </div>
+    <% end %>
+  </div>
+
   <% if @note_comments.length > 1 %>
     <div class='note-comments'>
       <ul class="list-unstyled">

config/locales/en.yml

@@ -2984,6 +2984,9 @@ en:
       report: report this note
       coordinates_html: "%{latitude}, %{longitude}"
       anonymous_warning: This note includes comments from anonymous users which should be independently verified.
+      discussion: Discussion
+      subscribe: Subscribe
+      unsubscribe: Unsubscribe
       hide: Hide
       resolve: Resolve
       reactivate: Reactivate

config/routes.rb

@@ -108,6 +108,8 @@
         post "close"
         post "reopen"
       end
+
+      resource :subscription, :only => [:create, :destroy], :controller => "note_subscriptions"
     end
 
     resources :user_blocks, :only => :show, :id => /\d+/, :controller => "user_blocks"

test/controllers/api/note_subscriptions_controller_test.rb

@@ -0,0 +1,132 @@
+require "test_helper"
+
+module Api
+  class NoteSubscriptionsControllerTest < ActionDispatch::IntegrationTest
+    def test_routes
+      assert_routing(
+        { :path => "/api/0.6/notes/1/subscription", :method => :post },
+        { :controller => "api/note_subscriptions", :action => "create", :note_id => "1" }
+      )
+      assert_routing(
+        { :path => "/api/0.6/notes/1/subscription", :method => :delete },
+        { :controller => "api/note_subscriptions", :action => "destroy", :note_id => "1" }
+      )
+    end
+
+    def test_create
+      user = create(:user)
+      auth_header = bearer_authorization_header user
+      note = create(:note_with_comments)
+      assert_empty note.subscribers
+
+      assert_difference "NoteSubscription.count", 1 do
+        assert_difference "note.subscribers.count", 1 do
+          post api_note_subscription_path(note), :headers => auth_header
+          assert_response :success
+        end
+      end
+      assert_equal user, note.subscribers.last
+    end
+
+    def test_create_fail_anonymous
+      note = create(:note_with_comments)
+
+      assert_no_difference "NoteSubscription.count" do
+        assert_no_difference "note.subscribers.count" do
+          post api_note_subscription_path(note)
+          assert_response :unauthorized
+        end
+      end
+    end
+
+    def test_create_fail_no_scope
+      user = create(:user)
+      auth_header = bearer_authorization_header user, :scopes => %w[read_prefs]
+      note = create(:note_with_comments)
+
+      assert_no_difference "NoteSubscription.count" do
+        assert_no_difference "note.subscribers.count" do
+          post api_note_subscription_path(note), :headers => auth_header
+          assert_response :forbidden
+        end
+      end
+    end
+
+    def test_create_fail_note_not_found
+      user = create(:user)
+      auth_header = bearer_authorization_header user
+
+      assert_no_difference "NoteSubscription.count" do
+        post api_note_subscription_path(999111), :headers => auth_header
+        assert_response :not_found
+      end
+    end
+
+    def test_create_fail_already_subscribed
+      user = create(:user)
+      auth_header = bearer_authorization_header user
+      note = create(:note_with_comments)
+      create(:note_subscription, :user => user, :note => note)
+
+      assert_no_difference "NoteSubscription.count" do
+        assert_no_difference "note.subscribers.count" do
+          post api_note_subscription_path(note), :headers => auth_header
+          assert_response :conflict
+        end
+      end
+    end
+
+    def test_destroy
+      user = create(:user)
+      auth_header = bearer_authorization_header user
+      note = create(:note_with_comments)
+      create(:note_subscription, :user => user, :note => note)
+      assert_equal [user], note.subscribers
+
+      assert_difference "NoteSubscription.count", -1 do
+        assert_difference "note.subscribers.count", -1 do
+          delete api_note_subscription_path(note), :headers => auth_header
+          assert_response :success
+        end
+      end
+    end
+
+    def test_destroy_fail_anonymous
+      note = create(:note_with_comments)
+
+      delete api_note_subscription_path(note)
+      assert_response :unauthorized
+    end
+
+    def test_destroy_fail_no_scope
+      user = create(:user)
+      auth_header = bearer_authorization_header user, :scopes => %w[read_prefs]
+      note = create(:note_with_comments)
+      create(:note_subscription, :user => user, :note => note)
+
+      assert_no_difference "NoteSubscription.count" do
+        assert_no_difference "note.subscribers.count" do
+          delete api_note_subscription_path(note), :headers => auth_header
+          assert_response :forbidden
+        end
+      end
+    end
+
+    def test_destroy_fail_note_not_found
+      user = create(:user)
+      auth_header = bearer_authorization_header user
+
+      delete api_note_subscription_path(999111), :headers => auth_header
+      assert_response :not_found
+    end
+
+    def test_destroy_fail_not_subscribed
+      user = create(:user)
+      auth_header = bearer_authorization_header user
+      note = create(:note_with_comments)
+
+      delete api_note_subscription_path(note), :headers => auth_header
+      assert_response :not_found
+    end
+  end
+end

test/system/note_comments_test.rb

@@ -22,7 +22,7 @@ class NoteCommentsTest < ApplicationSystemTestCase
     end
   end
 
-  def test_add_comment
+  test "can add comment" do
     note = create(:note_with_comments)
     user = create(:user)
     sign_in_as(user)
@@ -125,4 +125,49 @@ def test_add_comment
       assert_button "Reactivate", :disabled => false
     end
   end
+
+  test "no subscribe button when not logged in" do
+    note = create(:note_with_comments)
+    visit note_path(note)
+
+    within_sidebar do
+      assert_no_button "Subscribe"
+      assert_no_button "Unsubscribe"
+    end
+  end
+
+  test "can subscribe" do
+    note = create(:note_with_comments)
+    user = create(:user)
+    sign_in_as(user)
+    visit note_path(note)
+
+    within_sidebar do
+      assert_button "Subscribe"
+      assert_no_button "Unsubscribe"
+
+      click_on "Subscribe"
+
+      assert_no_button "Subscribe"
+      assert_button "Unsubscribe"
+    end
+  end
+
+  test "can unsubscribe" do
+    note = create(:note_with_comments)
+    user = create(:user)
+    create(:note_subscription, :note => note, :user => user)
+    sign_in_as(user)
+    visit note_path(note)
+
+    within_sidebar do
+      assert_no_button "Subscribe"
+      assert_button "Unsubscribe"
+
+      click_on "Unsubscribe"
+
+      assert_button "Subscribe"
+      assert_no_button "Unsubscribe"
+    end
+  end
 end