diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb
index 6638016d72..7ed6470b84 100644
--- a/app/abilities/ability.rb
+++ b/app/abilities/ability.rb
@@ -42,7 +42,7 @@ def initialize(user)
can :update, DiaryEntry, :user => user
can [:create], DiaryComment
can [:make_friend, :remove_friend], Friendship
- can [:read, :create, :reply, :inbox, :outbox, :muted, :mark, :unmute, :destroy], Message
+ can [:read, :create, :mark, :unmute, :destroy], Message
can [:close, :reopen], Note
can [:read, :update], :preference
can :update, :profile
diff --git a/app/controllers/messages/replies_controller.rb b/app/controllers/messages/replies_controller.rb
new file mode 100644
index 0000000000..b1e7b8d192
--- /dev/null
+++ b/app/controllers/messages/replies_controller.rb
@@ -0,0 +1,50 @@
+module Messages
+ class RepliesController < ApplicationController
+ layout "site"
+
+ before_action :authorize_web
+ before_action :set_locale
+
+ authorize_resource :class => Message
+
+ before_action :check_database_readable
+ before_action :check_database_writable
+
+ allow_thirdparty_images
+
+ # Allow the user to reply to another message.
+ def new
+ message = Message.find(params[:message_id])
+
+ if message.recipient == current_user
+ message.update(:message_read => true)
+
+ @message = Message.new(
+ :recipient => message.sender,
+ :title => "Re: #{message.title.sub(/^Re:\s*/, '')}",
+ :body => "On #{message.sent_on} #{message.sender.display_name} wrote:\n\n#{message.body.gsub(/^/, '> ')}"
+ )
+
+ @title = @message.title
+
+ render "messages/new"
+ elsif message.sender == current_user
+ @message = Message.new(
+ :recipient => message.recipient,
+ :title => "Re: #{message.title.sub(/^Re:\s*/, '')}",
+ :body => "On #{message.sent_on} #{message.sender.display_name} wrote:\n\n#{message.body.gsub(/^/, '> ')}"
+ )
+
+ @title = @message.title
+
+ render "messages/new"
+ else
+ flash[:notice] = t ".wrong_user", :user => current_user.display_name
+ redirect_to login_path(:referer => request.fullpath)
+ end
+ rescue ActiveRecord::RecordNotFound
+ @title = t "messages.no_such_message.title"
+ render "messages/no_such_message", :status => :not_found
+ end
+ end
+end
diff --git a/app/controllers/messages_controller.rb b/app/controllers/messages_controller.rb
index 7162b900a0..26e8a5e09e 100644
--- a/app/controllers/messages_controller.rb
+++ b/app/controllers/messages_controller.rb
@@ -10,7 +10,7 @@ class MessagesController < ApplicationController
before_action :lookup_user, :only => [:new, :create]
before_action :check_database_readable
- before_action :check_database_writable, :only => [:new, :create, :reply, :mark, :destroy]
+ before_action :check_database_writable, :only => [:new, :create, :mark, :destroy]
allow_thirdparty_images :only => [:new, :create, :show]
@@ -73,41 +73,6 @@ def destroy
render :action => "no_such_message", :status => :not_found
end
- # Allow the user to reply to another message.
- def reply
- message = Message.find(params[:message_id])
-
- if message.recipient == current_user
- message.update(:message_read => true)
-
- @message = Message.new(
- :recipient => message.sender,
- :title => "Re: #{message.title.sub(/^Re:\s*/, '')}",
- :body => "On #{message.sent_on} #{message.sender.display_name} wrote:\n\n#{message.body.gsub(/^/, '> ')}"
- )
-
- @title = @message.title
-
- render :action => "new"
- elsif message.sender == current_user
- @message = Message.new(
- :recipient => message.recipient,
- :title => "Re: #{message.title.sub(/^Re:\s*/, '')}",
- :body => "On #{message.sent_on} #{message.sender.display_name} wrote:\n\n#{message.body.gsub(/^/, '> ')}"
- )
-
- @title = @message.title
-
- render :action => "new"
- else
- flash[:notice] = t ".wrong_user", :user => current_user.display_name
- redirect_to login_path(:referer => request.fullpath)
- end
- rescue ActiveRecord::RecordNotFound
- @title = t "messages.no_such_message.title"
- render :action => "no_such_message", :status => :not_found
- end
-
# Set the message as being read or unread.
def mark
@message = current_user.messages.unscope(:where => :muted).find(params[:message_id])
diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb
index dee3dafbed..1dd13fb2d2 100644
--- a/app/mailers/user_mailer.rb
+++ b/app/mailers/user_mailer.rb
@@ -78,7 +78,7 @@ def message_notification(message)
@text = message.body
@title = message.title
@readurl = message_url(message)
- @replyurl = message_reply_url(message)
+ @replyurl = new_message_reply_url(message)
@author = @from_user
attach_user_avatar(message.sender)
diff --git a/app/views/messages/show.html.erb b/app/views/messages/show.html.erb
index 99d6d04355..d3147c9699 100644
--- a/app/views/messages/show.html.erb
+++ b/app/views/messages/show.html.erb
@@ -18,7 +18,7 @@
<%= @message.body.to_html %>
- <%= link_to t(".reply_button"), message_reply_path(@message), :class => "btn btn-primary" %>
+ <%= link_to t(".reply_button"), new_message_reply_path(@message), :class => "btn btn-primary" %>
<% if current_user == @message.recipient %>
<%= link_to t(".unread_button"), message_mark_path(@message, :mark => "unread"), :method => "post", :class => "btn btn-primary" %>
<%= link_to t(".destroy_button"), message_path(@message), :method => "delete", :class => "btn btn-danger" %>
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 4f21742065..2a34c0a599 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -1752,8 +1752,6 @@ en:
title: "No such message"
heading: "No such message"
body: "Sorry there is no message with that id."
- reply:
- wrong_user: "You are logged in as '%{user}' but the message you have asked to reply to was not sent to that user. Please log in as the correct user in order to reply."
show:
title: "Read message"
reply_button: "Reply"
@@ -1813,6 +1811,9 @@ en:
people_mapping_nearby: "people mapping nearby"
message:
destroy_button: "Delete"
+ replies:
+ new:
+ wrong_user: "You are logged in as '%{user}' but the message you have asked to reply to was not sent to that user. Please log in as the correct user in order to reply."
passwords:
new:
title: "Lost password"
diff --git a/config/routes.rb b/config/routes.rb
index 4ab8f307b0..d213030c67 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -315,7 +315,7 @@
post :mark
patch :unmute
- match :reply, :via => [:get, :post]
+ resource :reply, :module => :messages, :path_names => { :new => "new" }, :only => :new
end
namespace :messages, :path => "/messages" do
resource :inbox, :only => :show
@@ -326,6 +326,7 @@
get "/user/:display_name/outbox", :to => redirect(:path => "/messages/outbox")
get "/message/new/:display_name", :to => redirect(:path => "/messages/new/%{display_name}")
get "/message/read/:message_id", :to => redirect(:path => "/messages/%{message_id}")
+ get "/messages/:message_id/reply", :to => redirect(:path => "/messages/%{message_id}/reply/new")
# muting users
scope "/user/:display_name" do
diff --git a/test/controllers/messages/replies_controller_test.rb b/test/controllers/messages/replies_controller_test.rb
new file mode 100644
index 0000000000..a839a44582
--- /dev/null
+++ b/test/controllers/messages/replies_controller_test.rb
@@ -0,0 +1,69 @@
+require "test_helper"
+
+module Messages
+ class RepliesControllerTest < ActionDispatch::IntegrationTest
+ ##
+ # test all routes which lead to this controller
+ def test_routes
+ assert_routing(
+ { :path => "/messages/1/reply/new", :method => :get },
+ { :controller => "messages/replies", :action => "new", :message_id => "1" }
+ )
+ end
+
+ def test_new
+ user = create(:user)
+ recipient_user = create(:user)
+ other_user = create(:user)
+ message = create(:message, :unread, :sender => user, :recipient => recipient_user)
+
+ # Check that the message reply page requires us to login
+ get new_message_reply_path(message)
+ assert_redirected_to login_path(:referer => new_message_reply_path(message))
+
+ # Login as the wrong user
+ session_for(other_user)
+
+ # Check that we can't reply to somebody else's message
+ get new_message_reply_path(message)
+ assert_redirected_to login_path(:referer => new_message_reply_path(message))
+ assert_equal "You are logged in as '#{other_user.display_name}' but the message you have asked to reply to was not sent to that user. Please log in as the correct user in order to reply.", flash[:notice]
+
+ # Login as the right user
+ session_for(recipient_user)
+
+ # Check that the message reply page loads
+ get new_message_reply_path(message)
+ assert_response :success
+ assert_template "new"
+ assert_select "title", "Re: #{message.title} | OpenStreetMap"
+ assert_select "form[action='/messages']", :count => 1 do
+ assert_select "input[type='hidden'][name='display_name'][value='#{user.display_name}']"
+ assert_select "input#message_title[value='Re: #{message.title}']", :count => 1
+ assert_select "textarea#message_body", :count => 1
+ assert_select "input[type='submit'][value='Send']", :count => 1
+ end
+ assert Message.find(message.id).message_read
+
+ # Login as the sending user
+ session_for(user)
+
+ # Check that the message reply page loads
+ get new_message_reply_path(message)
+ assert_response :success
+ assert_template "new"
+ assert_select "title", "Re: #{message.title} | OpenStreetMap"
+ assert_select "form[action='/messages']", :count => 1 do
+ assert_select "input[type='hidden'][name='display_name'][value='#{recipient_user.display_name}']"
+ assert_select "input#message_title[value='Re: #{message.title}']", :count => 1
+ assert_select "textarea#message_body", :count => 1
+ assert_select "input[type='submit'][value='Send']", :count => 1
+ end
+
+ # Asking to reply to a message with a bogus ID should fail
+ get new_message_reply_path(99999)
+ assert_response :not_found
+ assert_template "no_such_message"
+ end
+ end
+end
diff --git a/test/controllers/messages_controller_test.rb b/test/controllers/messages_controller_test.rb
index f72e695936..9249908929 100644
--- a/test/controllers/messages_controller_test.rb
+++ b/test/controllers/messages_controller_test.rb
@@ -20,14 +20,6 @@ def test_routes
{ :path => "/messages/1/mark", :method => :post },
{ :controller => "messages", :action => "mark", :message_id => "1" }
)
- assert_routing(
- { :path => "/messages/1/reply", :method => :get },
- { :controller => "messages", :action => "reply", :message_id => "1" }
- )
- assert_routing(
- { :path => "/messages/1/reply", :method => :post },
- { :controller => "messages", :action => "reply", :message_id => "1" }
- )
assert_routing(
{ :path => "/messages/1", :method => :delete },
{ :controller => "messages", :action => "destroy", :id => "1" }
@@ -219,63 +211,6 @@ def test_new_limit
end
end
- ##
- # test the reply action
- def test_reply
- user = create(:user)
- recipient_user = create(:user)
- other_user = create(:user)
- message = create(:message, :unread, :sender => user, :recipient => recipient_user)
-
- # Check that the message reply page requires us to login
- get message_reply_path(message)
- assert_redirected_to login_path(:referer => message_reply_path(message))
-
- # Login as the wrong user
- session_for(other_user)
-
- # Check that we can't reply to somebody else's message
- get message_reply_path(message)
- assert_redirected_to login_path(:referer => message_reply_path(message))
- assert_equal "You are logged in as '#{other_user.display_name}' but the message you have asked to reply to was not sent to that user. Please log in as the correct user in order to reply.", flash[:notice]
-
- # Login as the right user
- session_for(recipient_user)
-
- # Check that the message reply page loads
- get message_reply_path(message)
- assert_response :success
- assert_template "new"
- assert_select "title", "Re: #{message.title} | OpenStreetMap"
- assert_select "form[action='/messages']", :count => 1 do
- assert_select "input[type='hidden'][name='display_name'][value='#{user.display_name}']"
- assert_select "input#message_title[value='Re: #{message.title}']", :count => 1
- assert_select "textarea#message_body", :count => 1
- assert_select "input[type='submit'][value='Send']", :count => 1
- end
- assert Message.find(message.id).message_read
-
- # Login as the sending user
- session_for(user)
-
- # Check that the message reply page loads
- get message_reply_path(message)
- assert_response :success
- assert_template "new"
- assert_select "title", "Re: #{message.title} | OpenStreetMap"
- assert_select "form[action='/messages']", :count => 1 do
- assert_select "input[type='hidden'][name='display_name'][value='#{recipient_user.display_name}']"
- assert_select "input#message_title[value='Re: #{message.title}']", :count => 1
- assert_select "textarea#message_body", :count => 1
- assert_select "input[type='submit'][value='Send']", :count => 1
- end
-
- # Asking to reply to a message with a bogus ID should fail
- get message_reply_path(99999)
- assert_response :not_found
- assert_template "no_such_message"
- end
-
##
# test the show action
def test_show