From: Mikel Maron <mikel_maron@yahoo.com>
Date: Wed, 12 Oct 2016 14:52:05 +0000 (-0400)
Subject: merge upstream/master and resolve conflicts from https://github.com/openstreetmap... 
X-Git-Tag: live~3711^2
X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/be40536440b8677323bcf30bc2b869e36f907dd9?hp=-c

merge upstream/master and resolve conflicts from https://github.com/openstreetmap/openstreetmap-website/pull/1318
---

be40536440b8677323bcf30bc2b869e36f907dd9
diff --combined app/controllers/diary_entry_controller.rb
index fdc0e9d24,384362364..bf9f2a9a2
--- a/app/controllers/diary_entry_controller.rb
+++ b/app/controllers/diary_entry_controller.rb
@@@ -3,10 -3,10 +3,10 @@@ class DiaryEntryController < Applicatio
  
    before_action :authorize_web
    before_action :set_locale
 -  before_action :require_user, :only => [:new, :edit, :comment, :hide, :hidecomment]
 +  before_action :require_user, :only => [:new, :edit, :comment, :hide, :hidecomment, :subscribe, :unsubscribe]
    before_action :lookup_this_user, :only => [:view, :comments]
    before_action :check_database_readable
 -  before_action :check_database_writable, :only => [:new, :edit]
 +  before_action :check_database_writable, :only => [:new, :edit, :comment, :hide, :hidecomment, :subscribe, :unsubscribe]
    before_action :require_administrator, :only => [:hide, :hidecomment]
  
    def new
@@@ -24,11 -24,7 +24,11 @@@
          else
            @user.preferences.create(:k => "diary.default_language", :v => @diary_entry.language_code)
          end
 +
 +        # Subscribe user to diary comments
 +        @diary_entry.subscriptions.create(:user => @user)
 +
-         redirect_to :controller => "diary_entry", :action => "list", :display_name => @user.display_name
+         redirect_to :action => "list", :display_name => @user.display_name
        else
          render :action => "edit"
        end
@@@ -46,9 -42,9 +46,9 @@@
      @diary_entry = DiaryEntry.find(params[:id])
  
      if @user != @diary_entry.user
-       redirect_to :controller => "diary_entry", :action => "view", :id => params[:id]
+       redirect_to :action => "view", :id => params[:id]
      elsif params[:diary_entry] && @diary_entry.update_attributes(entry_params)
-       redirect_to :controller => "diary_entry", :action => "view", :id => params[:id]
+       redirect_to :action => "view", :id => params[:id]
      end
  
      set_map_location
@@@ -61,44 -57,17 +61,44 @@@
      @diary_comment = @entry.comments.build(comment_params)
      @diary_comment.user = @user
      if @diary_comment.save
 -      if @diary_comment.user != @entry.user
 -        Notifier.diary_comment_notification(@diary_comment).deliver_now
 +
 +      # Notify current subscribers of the new comment
 +      @entry.subscribers.visible.each do |user|
 +        if @user != user
 +          Notifier.diary_comment_notification(@diary_comment, user).deliver_now
 +        end
        end
  
 +      # Add the commenter to the subscribers if necessary
 +      @entry.subscriptions.create(:user => @user) unless @entry.subscribers.exists?(@user.id)
 +
-       redirect_to :controller => "diary_entry", :action => "view", :display_name => @entry.user.display_name, :id => @entry.id
+       redirect_to :action => "view", :display_name => @entry.user.display_name, :id => @entry.id
      else
        render :action => "view"
      end
    rescue ActiveRecord::RecordNotFound
      render :action => "no_such_entry", :status => :not_found
    end
 +
 +  def subscribe
 +    diary_entry = DiaryEntry.find(params[:id])
 +
 +    diary_entry.subscriptions.create(:user => @user) unless diary_entry.subscribers.exists?(@user.id)
 +
-     redirect_to :controller => "diary_entry", :action => "view", :display_name => diary_entry.user.display_name, :id => diary_entry.id
++    redirect_to :action => "view", :display_name => diary_entry.user.display_name, :id => diary_entry.id
 +  rescue ActiveRecord::RecordNotFound
 +    render :action => "no_such_entry", :status => :not_found
 +  end
 +
 +  def unsubscribe
 +    diary_entry = DiaryEntry.find(params[:id])
 +
 +    diary_entry.subscriptions.where(:user => @user).delete_all if diary_entry.subscribers.exists?(@user.id)
 +
-     redirect_to :controller => "diary_entry", :action => "view", :display_name => diary_entry.user.display_name, :id => diary_entry.id
++    redirect_to :action => "view", :display_name => diary_entry.user.display_name, :id => diary_entry.id
 +  rescue ActiveRecord::RecordNotFound
 +    render :action => "no_such_entry", :status => :not_found
 +  end
  
    def list
      if params[:display_name]
@@@ -232,7 -201,7 +232,7 @@@
    def require_administrator
      unless @user.administrator?
        flash[:error] = t("user.filter.not_an_administrator")
-       redirect_to :controller => "diary_entry", :action => "view"
+       redirect_to :action => "view"
      end
    end
  
diff --combined config/locales/en.yml
index 23307bdc5,b0ae8548a..6403087ef
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@@ -1235,9 -1235,9 +1235,9 @@@ en
      partners_title: Partners
    notifier:
      diary_comment_notification:
 -      subject: "[OpenStreetMap] %{user} commented on your diary entry"
 +      subject: "[OpenStreetMap] %{user} commented on a diary entry"
        hi: "Hi %{to_user},"
 -      header: "%{from_user} has commented on your recent OpenStreetMap diary entry with the subject %{subject}:"
 +      header: "%{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 reply at %{replyurl}"
      message_notification:
        subject_header: "[OpenStreetMap] %{subject}"
@@@ -2191,7 -2191,7 +2191,7 @@@
      key:
        title: "Map Key"
        tooltip: "Map Key"
-       tooltip_disabled: "Map Key available only for Standard layer"
+       tooltip_disabled: "Map Key not available for this layer"
      map:
        zoom:
          in: Zoom In
diff --combined test/controllers/diary_entry_controller_test.rb
index 6ebf4ec09,813b50b2f..d926f2656
--- a/test/controllers/diary_entry_controller_test.rb
+++ b/test/controllers/diary_entry_controller_test.rb
@@@ -1,7 -1,7 +1,7 @@@
  require "test_helper"
  
  class DiaryEntryControllerTest < ActionController::TestCase
-   fixtures :users, :user_roles, :languages, :friends
+   fixtures :users, :user_roles, :languages
  
    include ActionView::Helpers::NumberHelper
  
@@@ -83,14 -83,6 +83,14 @@@
        { :path => "/user/username/diary/1/hidecomment/2", :method => :post },
        { :controller => "diary_entry", :action => "hidecomment", :display_name => "username", :id => "1", :comment => "2" }
      )
 +    assert_routing(
 +      { :path => "/user/username/diary/1/subscribe", :method => :post },
 +      { :controller => "diary_entry", :action => "subscribe", :display_name => "username", :id => "1" }
 +    )
 +    assert_routing(
 +      { :path => "/user/username/diary/1/unsubscribe", :method => :post },
 +      { :controller => "diary_entry", :action => "unsubscribe", :display_name => "username", :id => "1" }
 +    )
    end
  
    def test_new
@@@ -156,9 -148,6 +156,9 @@@
      assert_equal new_longitude.to_f, entry.longitude
      assert_equal new_language_code, entry.language_code
  
 +    # checks if user was subscribed
 +    assert_equal 1, entry.subscribers.length
 +
      assert_equal new_language_code, UserPreference.where(:user_id => users(:normal_user).id, :k => "diary.default_language").first.v
  
      new_language_code = "de"
@@@ -180,9 -169,6 +180,9 @@@
      assert_equal new_longitude.to_f, entry.longitude
      assert_equal new_language_code, entry.language_code
  
 +    # checks if user was subscribed
 +    assert_equal 1, entry.subscribers.length
 +
      assert_equal new_language_code, UserPreference.where(:user_id => users(:normal_user).id, :k => "diary.default_language").first.v
    end
  
@@@ -330,32 -316,26 +330,32 @@@
        assert_select "h2", :text => "No entry with the id: 9999", :count => 1
      end
  
 +    post :subscribe, { :id => entry.id, :display_name => entry.user.display_name }, { :user => users(:normal_user).id }
 +
      # Now try an invalid comment with an empty body
      assert_no_difference "ActionMailer::Base.deliveries.size" do
        assert_no_difference "DiaryComment.count" do
 -        post :comment, { :display_name => entry.user.display_name, :id => entry.id, :diary_comment => { :body => "" } }, { :user => users(:public_user).id }
 +        assert_no_difference "entry.subscribers.count" do
 +          post :comment, { :display_name => entry.user.display_name, :id => entry.id, :diary_comment => { :body => "" } }, { :user => users(:public_user).id }
 +        end
        end
      end
      assert_response :success
      assert_template :view
  
      # Now try again with the right id
 -    assert_difference "ActionMailer::Base.deliveries.size", 1 do
 +    assert_difference "ActionMailer::Base.deliveries.size", entry.subscribers.count do
        assert_difference "DiaryComment.count", 1 do
 -        post :comment, { :display_name => entry.user.display_name, :id => entry.id, :diary_comment => { :body => "New comment" } }, { :user => users(:public_user).id }
 +        assert_difference "entry.subscribers.count", 1 do
 +          post :comment, { :display_name => entry.user.display_name, :id => entry.id, :diary_comment => { :body => "New comment" } }, { :user => users(:public_user).id }
 +        end
        end
      end
      assert_response :redirect
      assert_redirected_to :action => :view, :display_name => entry.user.display_name, :id => entry.id
      email = ActionMailer::Base.deliveries.first
      assert_equal [users(:normal_user).email], email.to
 -    assert_equal "[OpenStreetMap] #{users(:public_user).display_name} commented on your diary entry", email.subject
 +    assert_equal "[OpenStreetMap] #{users(:public_user).display_name} commented on a diary entry", email.subject
      assert_match /New comment/, email.text_part.decoded
      assert_match /New comment/, email.html_part.decoded
      ActionMailer::Base.deliveries.clear
@@@ -378,7 -358,6 +378,7 @@@
    def test_comment_spammy
      # Find the entry to comment on
      entry = create(:diary_entry, :user_id => users(:normal_user).id)
 +    post :subscribe, { :id => entry.id, :display_name => entry.user.display_name }, { :user => users(:normal_user).id }
  
      # Generate some spammy content
      spammy_text = 1.upto(50).map { |n| "http://example.com/spam#{n}" }.join(" ")
@@@ -393,7 -372,7 +393,7 @@@
      assert_redirected_to :action => :view, :display_name => entry.user.display_name, :id => entry.id
      email = ActionMailer::Base.deliveries.first
      assert_equal [users(:normal_user).email], email.to
 -    assert_equal "[OpenStreetMap] #{users(:public_user).display_name} commented on your diary entry", email.subject
 +    assert_equal "[OpenStreetMap] #{users(:public_user).display_name} commented on a diary entry", email.subject
      assert_match %r{http://example.com/spam}, email.text_part.decoded
      assert_match %r{http://example.com/spam}, email.html_part.decoded
      ActionMailer::Base.deliveries.clear
@@@ -440,7 -419,8 +440,8 @@@
    end
  
    def test_list_friends
-     diary_entry = create(:diary_entry, :user_id => friends(:normal_user_with_second_user).friend_user_id)
+     friend = create(:friend, :user_id => users(:normal_user).id)
+     diary_entry = create(:diary_entry, :user_id => friend.friend_user_id)
      _other_entry = create(:diary_entry, :user_id => users(:second_public_user).id)
  
      # Try a list of diary entries for your friends when not logged in
@@@ -662,64 -642,6 +663,64 @@@
      assert_response :not_found
    end
  
 +  def test_subscribe_success
 +    diary_entry = create(:diary_entry, :user_id => users(:normal_user).id)
 +
 +    assert_difference "diary_entry.subscribers.count", 1 do
 +      post :subscribe, { :id => diary_entry.id, :display_name => diary_entry.user.display_name }, { :user => users(:public_user).id }
 +    end
 +    assert_response :redirect
 +  end
 +
 +  def test_subscribe_fail
 +    diary_entry = create(:diary_entry, :user_id => users(:normal_user).id)
 +
 +    # not signed in
 +    assert_no_difference "diary_entry.subscribers.count" do
 +      post :subscribe, :id => diary_entry.id, :display_name => diary_entry.user.display_name
 +    end
 +    assert_response :forbidden
 +
 +    # bad diary id
 +    post :subscribe, { :id => 999111, :display_name => "username" }, { :user => users(:public_user).id }
 +    assert_response :not_found
 +
 +    # trying to subscribe when already subscribed
 +    post :subscribe, { :id => diary_entry.id, :display_name => diary_entry.user.display_name }, { :user => users(:public_user).id }
 +    assert_no_difference "diary_entry.subscribers.count" do
 +      post :subscribe, { :id => diary_entry.id, :display_name => diary_entry.user.display_name }, { :user => users(:public_user).id }
 +    end
 +  end
 +
 +  def test_unsubscribe_success
 +    diary_entry = create(:diary_entry, :user_id => users(:normal_user).id)
 +
 +    post :subscribe, { :id => diary_entry.id, :display_name => diary_entry.user.display_name }, { :user => users(:public_user).id }
 +    assert_difference "diary_entry.subscribers.count", -1 do
 +      post :unsubscribe, { :id => diary_entry.id, :display_name => diary_entry.user.display_name }, { :user => users(:public_user).id }
 +    end
 +    assert_response :redirect
 +  end
 +
 +  def test_unsubscribe_fail
 +    diary_entry = create(:diary_entry, :user_id => users(:normal_user).id)
 +
 +    # not signed in
 +    assert_no_difference "diary_entry.subscribers.count" do
 +      post :unsubscribe, :id => diary_entry.id, :display_name => diary_entry.user.display_name
 +    end
 +    assert_response :forbidden
 +
 +    # bad diary id
 +    post :unsubscribe, { :id => 999111, :display_name => "username" }, { :user => users(:public_user).id }
 +    assert_response :not_found
 +
 +    # trying to unsubscribe when not subscribed
 +    assert_no_difference "diary_entry.subscribers.count" do
 +      post :unsubscribe, { :id => diary_entry.id, :display_name => diary_entry.user.display_name }, { :user => users(:public_user).id }
 +    end
 +  end
 +
    private
  
    def check_diary_list(*entries)