Merge remote-tracking branch 'upstream/pull/4523'

diff --git a/app/controllers/diary_entries_controller.rb b/app/controllers/diary_entries_controller.rb
index 1f9574b9cbab8313e7cc0e9b7540c85ae5d9f3fe..1a3d648f460a4f089ca362ac00c9ac88efc7915e 100644 (file)
--- a/app/controllers/diary_entries_controller.rb
+++ b/app/controllers/diary_entries_controller.rb
@@ -157,21 +157,25 @@ class DiaryEntriesController < ApplicationController
   end
 
   def subscribe
-    diary_entry = DiaryEntry.find(params[:id])
+    @diary_entry = DiaryEntry.find(params[:id])
 
-    diary_entry.subscriptions.create(:user => current_user) unless diary_entry.subscribers.exists?(current_user.id)
+    if request.post?
+      @diary_entry.subscriptions.create(:user => current_user) unless @diary_entry.subscribers.exists?(current_user.id)
 
-    redirect_to diary_entry_path(diary_entry.user, diary_entry)
+      redirect_to diary_entry_path(@diary_entry.user, @diary_entry)
+    end
   rescue ActiveRecord::RecordNotFound
     render :action => "no_such_entry", :status => :not_found
   end
 
   def unsubscribe
-    diary_entry = DiaryEntry.find(params[:id])
+    @diary_entry = DiaryEntry.find(params[:id])
 
-    diary_entry.subscriptions.where(:user => current_user).delete_all if diary_entry.subscribers.exists?(current_user.id)
+    if request.post?
+      @diary_entry.subscriptions.where(:user => current_user).delete_all if @diary_entry.subscribers.exists?(current_user.id)
 
-    redirect_to diary_entry_path(diary_entry.user, diary_entry)
+      redirect_to diary_entry_path(@diary_entry.user, @diary_entry)
+    end
   rescue ActiveRecord::RecordNotFound
     render :action => "no_such_entry", :status => :not_found
   end

diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb
index d1ad60b2cad9c1d9aa2aa6ce150bd77999edc3ce..0894b972d7e0735d2e146054636d93a923f0b7ef 100644 (file)
--- a/app/mailers/user_mailer.rb
+++ b/app/mailers/user_mailer.rb
@@ -97,6 +97,7 @@ class UserMailer < ApplicationMailer
       @readurl = diary_entry_url(comment.diary_entry.user, comment.diary_entry, :anchor => "comment#{comment.id}")
       @commenturl = diary_entry_url(comment.diary_entry.user, comment.diary_entry, :anchor => "newcomment")
       @replyurl = new_message_url(comment.user, :message => { :title => "Re: #{comment.diary_entry.title}" })
+      @unsubscribeurl = diary_entry_unsubscribe_url(comment.diary_entry.user, comment.diary_entry)
       @author = @from_user
 
       attach_user_avatar(comment.user)

diff --git a/app/views/diary_entries/_diary_entry.html.erb b/app/views/diary_entries/_diary_entry.html.erb
index f69c7e552c3bf80f0adff79febae210093eb2a47..8bd4dc57fd652b78c6dc2a9d70abd332f1f1e537 100644 (file)
--- a/app/views/diary_entries/_diary_entry.html.erb
+++ b/app/views/diary_entries/_diary_entry.html.erb
@@ -1,26 +1,5 @@
 <article class='diary_post border-top border-grey py-3<%= " text-muted px-3 bg-danger bg-opacity-10" unless diary_entry.visible %> user_<%= diary_entry.user.id %>'>
-  <div class='mb-3'>
-    <% if @user %>
-      <h2><%= link_to diary_entry.title, diary_entry_path(diary_entry.user, diary_entry) %></h2>
-    <% else %>
-      <div class="row">
-        <div class="col-auto">
-          <%= user_thumbnail diary_entry.user %>
-        </div>
-        <div class="col">
-          <h2><%= link_to diary_entry.title, diary_entry_path(diary_entry.user, diary_entry) %></h2>
-        </div>
-      </div>
-    <% end %>
-
-    <small class='text-muted'>
-      <%= t(".posted_by_html", :link_user => (link_to diary_entry.user.display_name, user_path(diary_entry.user)), :created => l(diary_entry.created_at, :format => :blog), :language_link => (link_to diary_entry.language.name, :controller => "diary_entries", :action => "index", :display_name => nil, :language => diary_entry.language_code)) %>
-      <% if (l(diary_entry.updated_at, :format => :blog) != l(diary_entry.created_at, :format => :blog)) %>
-        <%= t(".updated_at_html", :updated => l(diary_entry.updated_at, :format => :blog)) %>
-      <% end %>
-    </small>
-
-  </div>
+  <%= render :partial => "diary_entry_heading", :object => diary_entry, :as => "diary_entry" %>
 
   <div class="richtext text-break" xml:lang="<%= diary_entry.language_code %>" lang="<%= diary_entry.language_code %>">
     <%= diary_entry.body.to_html %>

diff --git a/app/views/diary_entries/_diary_entry_heading.html.erb b/app/views/diary_entries/_diary_entry_heading.html.erb
new file mode 100644 (file)
index 0000000..ef924ff
--- /dev/null
+++ b/app/views/diary_entries/_diary_entry_heading.html.erb
@@ -0,0 +1,21 @@
+<div class='mb-3'>
+  <% if @user %>
+    <h2><%= link_to diary_entry.title, diary_entry_path(diary_entry.user, diary_entry) %></h2>
+  <% else %>
+    <div class="row">
+      <div class="col-auto">
+        <%= user_thumbnail diary_entry.user %>
+      </div>
+      <div class="col">
+        <h2><%= link_to diary_entry.title, diary_entry_path(diary_entry.user, diary_entry) %></h2>
+      </div>
+    </div>
+  <% end %>
+
+  <small class='text-muted'>
+    <%= t("diary_entries.diary_entry.posted_by_html", :link_user => (link_to diary_entry.user.display_name, user_path(diary_entry.user)), :created => l(diary_entry.created_at, :format => :blog), :language_link => (link_to diary_entry.language.name, :controller => "diary_entries", :action => "index", :display_name => nil, :language => diary_entry.language_code)) %>
+    <% if (l(diary_entry.updated_at, :format => :blog) != l(diary_entry.created_at, :format => :blog)) %>
+      <%= t("diary_entries.diary_entry.updated_at_html", :updated => l(diary_entry.updated_at, :format => :blog)) %>
+    <% end %>
+  </small>
+</div>

diff --git a/app/views/diary_entries/subscribe.html.erb b/app/views/diary_entries/subscribe.html.erb
new file mode 100644 (file)
index 0000000..e18be2b
--- /dev/null
+++ b/app/views/diary_entries/subscribe.html.erb
@@ -0,0 +1,12 @@
+<% content_for :heading do %>
+  <h1><%= t ".heading", :user => @diary_entry.user.display_name %></h1>
+<% end %>
+
+<%= render :partial => "diary_entry_heading", :object => @diary_entry, :as => "diary_entry" %>
+
+<%= bootstrap_form_tag do |f| %>
+  <% if params[:referer] -%>
+  <%= f.hidden_field :referer, :value => params[:referer] %>
+  <% end -%>
+  <%= f.primary t(".button") %>
+<% end %>

diff --git a/app/views/diary_entries/unsubscribe.html.erb b/app/views/diary_entries/unsubscribe.html.erb
new file mode 100644 (file)
index 0000000..e18be2b
--- /dev/null
+++ b/app/views/diary_entries/unsubscribe.html.erb
@@ -0,0 +1,12 @@
+<% content_for :heading do %>
+  <h1><%= t ".heading", :user => @diary_entry.user.display_name %></h1>
+<% end %>
+
+<%= render :partial => "diary_entry_heading", :object => @diary_entry, :as => "diary_entry" %>
+
+<%= bootstrap_form_tag do |f| %>
+  <% if params[:referer] -%>
+  <%= f.hidden_field :referer, :value => params[:referer] %>
+  <% end -%>
+  <%= f.primary t(".button") %>
+<% end %>

diff --git a/app/views/user_mailer/diary_comment_notification.html.erb b/app/views/user_mailer/diary_comment_notification.html.erb
index 7f9368fe33b038bb25c2e185c7f07d2b8fb917ef..dab4510edb9467538558ce6dec62da05b1c7b4a6 100644 (file)
--- a/app/views/user_mailer/diary_comment_notification.html.erb
+++ b/app/views/user_mailer/diary_comment_notification.html.erb
@@ -15,4 +15,7 @@
            :commenturl => link_to(@commenturl, @commenturl) + tag.br,
            :replyurl => link_to(@replyurl, @replyurl) %>
   </p>
+  <p><%= t ".footer_unsubscribe_html",
+           :unsubscribeurl => link_to(@unsubscribeurl, @unsubscribeurl) %>
+  </p>
 <% end %>

diff --git a/app/views/user_mailer/diary_comment_notification.text.erb b/app/views/user_mailer/diary_comment_notification.text.erb
index cbf9ddaa056d271432de8e8fc38a9cae3c942d88..13aa460f1f8fec9a01682541f6cf2a239465026d 100644 (file)
--- a/app/views/user_mailer/diary_comment_notification.text.erb
+++ b/app/views/user_mailer/diary_comment_notification.text.erb
@@ -7,3 +7,5 @@
 ==
 
 <%= t '.footer', :readurl => @readurl, :commenturl => @commenturl, :replyurl => @replyurl %>
+
+<%= t '.footer_unsubscribe', :unsubscribeurl => @unsubscribeurl %>

diff --git a/config/locales/en.yml b/config/locales/en.yml
index e6fde54c2625674ef5f08254454ad1e0974f73e3..775030638a64413d56313ac1b8d4c750b5f06db9 100644 (file)
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -576,6 +576,12 @@ en:
       comment: Comment
       newer_comments: "Newer Comments"
       older_comments: "Older Comments"
+    subscribe:
+      heading: Subscribe to the following diary entry discussion?
+      button: Subscribe to discussion
+    unsubscribe:
+      heading: Unsubscribe from the following diary entry discussion?
+      button: Unsubscribe from discussion
   doorkeeper:
     errors:
       messages:
@@ -1571,6 +1577,8 @@ en:
       header_html: "%{from_user} has commented on the OpenStreetMap diary entry with the subject %{subject}:"
       footer: "You can also read the comment at %{readurl} and you can comment at %{commenturl} or send a message to the author at %{replyurl}"
       footer_html: "You can also read the comment at %{readurl} and you can comment at %{commenturl} or send a message to the author at %{replyurl}"
+      footer_unsubscribe: "You can unsubscribe from the discussion at %{unsubscribeurl}"
+      footer_unsubscribe_html: "You can unsubscribe from the discussion at %{unsubscribeurl}"
     message_notification:
       subject: "[OpenStreetMap] %{message_title}"
       hi: "Hi %{to_user},"

diff --git a/config/routes.rb b/config/routes.rb
index 09afe8fd928926139b8b80c93f26bd531c535522..da27f07543b23dc96e3c7a812ff60b049d0b0018 100644 (file)
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -244,8 +244,8 @@ OpenStreetMap::Application.routes.draw do
   post "/user/:display_name/diary/:id/unhide" => "diary_entries#unhide", :id => /\d+/, :as => :unhide_diary_entry
   post "/user/:display_name/diary/:id/hidecomment/:comment" => "diary_entries#hidecomment", :id => /\d+/, :comment => /\d+/, :as => :hide_diary_comment
   post "/user/:display_name/diary/:id/unhidecomment/:comment" => "diary_entries#unhidecomment", :id => /\d+/, :comment => /\d+/, :as => :unhide_diary_comment
-  post "/user/:display_name/diary/:id/subscribe" => "diary_entries#subscribe", :as => :diary_entry_subscribe, :id => /\d+/
-  post "/user/:display_name/diary/:id/unsubscribe" => "diary_entries#unsubscribe", :as => :diary_entry_unsubscribe, :id => /\d+/
+  match "/user/:display_name/diary/:id/subscribe" => "diary_entries#subscribe", :via => [:get, :post], :as => :diary_entry_subscribe, :id => /\d+/
+  match "/user/:display_name/diary/:id/unsubscribe" => "diary_entries#unsubscribe", :via => [:get, :post], :as => :diary_entry_unsubscribe, :id => /\d+/
 
   # user pages
   resources :users, :path => "user", :param => :display_name, :only => [:show, :destroy]

diff --git a/test/controllers/diary_entries_controller_test.rb b/test/controllers/diary_entries_controller_test.rb
index b83e9b7a0e07e96e287514010daf76e930f891d1..e7d812b3e7fa8fabae7d9fe8151834eb601caeb3 100644 (file)
--- a/test/controllers/diary_entries_controller_test.rb
+++ b/test/controllers/diary_entries_controller_test.rb
@@ -98,10 +98,18 @@ class DiaryEntriesControllerTest < ActionDispatch::IntegrationTest
       { :path => "/user/username/diary/1/unhidecomment/2", :method => :post },
       { :controller => "diary_entries", :action => "unhidecomment", :display_name => "username", :id => "1", :comment => "2" }
     )
+    assert_routing(
+      { :path => "/user/username/diary/1/subscribe", :method => :get },
+      { :controller => "diary_entries", :action => "subscribe", :display_name => "username", :id => "1" }
+    )
     assert_routing(
       { :path => "/user/username/diary/1/subscribe", :method => :post },
       { :controller => "diary_entries", :action => "subscribe", :display_name => "username", :id => "1" }
     )
+    assert_routing(
+      { :path => "/user/username/diary/1/unsubscribe", :method => :get },
+      { :controller => "diary_entries", :action => "unsubscribe", :display_name => "username", :id => "1" }
+    )
     assert_routing(
       { :path => "/user/username/diary/1/unsubscribe", :method => :post },
       { :controller => "diary_entries", :action => "unsubscribe", :display_name => "username", :id => "1" }
@@ -916,6 +924,25 @@ class DiaryEntriesControllerTest < ActionDispatch::IntegrationTest
     assert_response :not_found
   end
 
+  def test_subscribe_page
+    user = create(:user)
+    other_user = create(:user)
+    diary_entry = create(:diary_entry, :user => user)
+    path = diary_entry_subscribe_path(:id => diary_entry, :display_name => user.display_name)
+
+    get path
+    assert_response :redirect
+    assert_redirected_to login_path(:referer => path)
+
+    session_for(other_user)
+    get path
+    assert_response :success
+    assert_dom ".content-body" do
+      assert_dom "a[href='#{diary_entry_path(:id => diary_entry, :display_name => user.display_name)}']", :text => diary_entry.title
+      assert_dom "a[href='#{user_path(user)}']", :text => user.display_name
+    end
+  end
+
   def test_subscribe_success
     user = create(:user)
     other_user = create(:user)
@@ -953,6 +980,25 @@ class DiaryEntriesControllerTest < ActionDispatch::IntegrationTest
     end
   end
 
+  def test_unsubscribe_page
+    user = create(:user)
+    other_user = create(:user)
+    diary_entry = create(:diary_entry, :user => user)
+    path = diary_entry_unsubscribe_path(:id => diary_entry, :display_name => user.display_name)
+
+    get path
+    assert_response :redirect
+    assert_redirected_to login_path(:referer => path)
+
+    session_for(other_user)
+    get path
+    assert_response :success
+    assert_dom ".content-body" do
+      assert_dom "a[href='#{diary_entry_path(:id => diary_entry, :display_name => user.display_name)}']", :text => diary_entry.title
+      assert_dom "a[href='#{user_path(user)}']", :text => user.display_name
+    end
+  end
+
   def test_unsubscribe_success
     user = create(:user)
     other_user = create(:user)

diff --git a/test/mailers/user_mailer_test.rb b/test/mailers/user_mailer_test.rb
index 751adcd82dc361583bf4e8ea7ca4ad8a8d3ac8f9..998e97330059c4ab4cd41a019851131cda35893d 100644 (file)
--- a/test/mailers/user_mailer_test.rb
+++ b/test/mailers/user_mailer_test.rb
@@ -53,4 +53,19 @@ class UserMailerTest < ActionMailer::TestCase
     assert_match("Jack & Jill <br>", email.text_part.body.to_s)
     assert_match("Jack &amp; Jill &lt;br&gt;", email.html_part.body.to_s)
   end
+
+  def test_diary_comment_notification
+    create(:language, :code => "en")
+    user = create(:user)
+    other_user = create(:user)
+    diary_entry = create(:diary_entry, :user => user)
+    diary_comment = create(:diary_comment, :diary_entry => diary_entry)
+    email = UserMailer.diary_comment_notification(diary_comment, other_user)
+    body = Rails::Dom::Testing.html_document_fragment.parse(email.html_part.body)
+
+    url = Rails.application.routes.url_helpers.diary_entry_url(user, diary_entry, :host => Settings.server_url, :protocol => Settings.server_protocol)
+    unsubscribe_url = Rails.application.routes.url_helpers.diary_entry_unsubscribe_url(user, diary_entry, :host => Settings.server_url, :protocol => Settings.server_protocol)
+    assert_select body, "a[href^='#{url}']"
+    assert_select body, "a[href='#{unsubscribe_url}']", :count => 1
+  end
 end