From cc3bccb9b3adc5f057d902a07282ed77e96ce5c2 Mon Sep 17 00:00:00 2001 From: Anton Khorev Date: Thu, 23 Jan 2025 03:41:19 +0300 Subject: [PATCH] Create message read_mark resource --- app/abilities/ability.rb | 2 +- .../messages/read_marks_controller.rb | 39 ++++++ app/controllers/messages_controller.rb | 26 +--- .../messages/mailboxes/_message.html.erb | 4 +- app/views/messages/show.html.erb | 2 +- config/locales/en.yml | 8 +- config/routes.rb | 6 +- .../messages/read_marks_controller_test.rb | 126 ++++++++++++++++++ test/controllers/messages_controller_test.rb | 82 ------------ 9 files changed, 179 insertions(+), 116 deletions(-) create mode 100644 app/controllers/messages/read_marks_controller.rb create mode 100644 test/controllers/messages/read_marks_controller_test.rb diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb index d4172bfd1..262e92c26 100644 --- a/app/abilities/ability.rb +++ b/app/abilities/ability.rb @@ -43,7 +43,7 @@ class Ability can :update, DiaryEntry, :user => user can [:create], DiaryComment can [:show, :create, :destroy], Follow - can [:read, :create, :mark, :unmute, :destroy], Message + can [:read, :create, :unmute, :destroy], Message can [:close, :reopen], Note can [:read, :update], :preference can :update, :profile diff --git a/app/controllers/messages/read_marks_controller.rb b/app/controllers/messages/read_marks_controller.rb new file mode 100644 index 000000000..2db0b89e3 --- /dev/null +++ b/app/controllers/messages/read_marks_controller.rb @@ -0,0 +1,39 @@ +module Messages + class ReadMarksController < ApplicationController + layout "site" + + before_action :authorize_web + before_action :set_locale + + authorize_resource :message + + before_action :check_database_readable + before_action :check_database_writable + + def create + mark true + end + + def destroy + mark false + end + + private + + def mark(message_read) + @message = current_user.messages.unscope(:where => :muted).find(params[:message_id]) + @message.message_read = message_read + if @message.save + flash[:notice] = t ".notice" + if @message.muted? + redirect_to messages_muted_inbox_path, :status => :see_other + else + redirect_to messages_inbox_path, :status => :see_other + end + end + rescue ActiveRecord::RecordNotFound + @title = t "messages.no_such_message.title" + render :template => "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 cc5f6c56d..5a63dfd6e 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, :mark, :destroy] + before_action :check_database_writable, :only => [:new, :create, :destroy] allow_thirdparty_images :only => [:new, :create, :show] @@ -73,30 +73,6 @@ class MessagesController < ApplicationController 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]) - if params[:mark] == "unread" - message_read = false - notice = t ".as_unread" - else - message_read = true - notice = t ".as_read" - end - @message.message_read = message_read - if @message.save - flash[:notice] = notice - if @message.muted? - redirect_to messages_muted_inbox_path, :status => :see_other - else - redirect_to messages_inbox_path, :status => :see_other - end - end - rescue ActiveRecord::RecordNotFound - @title = t "messages.no_such_message.title" - render :action => "no_such_message", :status => :not_found - end - # Moves message into Inbox by unsetting the muted-flag def unmute message = current_user.muted_messages.find(params[:message_id]) diff --git a/app/views/messages/mailboxes/_message.html.erb b/app/views/messages/mailboxes/_message.html.erb index 7213cf80d..22ef4d7c2 100644 --- a/app/views/messages/mailboxes/_message.html.erb +++ b/app/views/messages/mailboxes/_message.html.erb @@ -4,8 +4,8 @@ <%= l message.sent_on, :format => :friendly %>
- <%= button_to t(".unread_button"), message_mark_path(message, :mark => "unread"), :class => "btn btn-sm btn-primary", :form => { :data => { :turbo => true }, :class => "inbox-mark-unread", :hidden => !message.message_read? } %> - <%= button_to t(".read_button"), message_mark_path(message, :mark => "read"), :class => "btn btn-sm btn-primary", :form => { :data => { :turbo => true }, :class => "inbox-mark-read", :hidden => message.message_read? } %> + <%= button_to t(".unread_button"), message_read_mark_path(message), :method => :delete, :class => "btn btn-sm btn-primary", :form => { :data => { :turbo => true }, :class => "inbox-mark-unread", :hidden => !message.message_read? } %> + <%= button_to t(".read_button"), message_read_mark_path(message), :method => :post, :class => "btn btn-sm btn-primary", :form => { :data => { :turbo => true }, :class => "inbox-mark-read", :hidden => message.message_read? } %> <%= button_to t(".destroy_button"), message_path(message, :referer => request.fullpath), :method => :delete, :class => "btn btn-sm btn-danger", :form => { :data => { :turbo => true }, :class => "destroy-message" } %> <% if message.muted? %> <%= button_to t(".unmute_button"), message_unmute_path(message), :method => :patch, :class => "btn btn-sm btn-secondary", :form => { :data => { :turbo => true } } %> diff --git a/app/views/messages/show.html.erb b/app/views/messages/show.html.erb index d3147c969..ff403b099 100644 --- a/app/views/messages/show.html.erb +++ b/app/views/messages/show.html.erb @@ -20,7 +20,7 @@
<%= 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(".unread_button"), message_read_mark_path(@message), :method => "delete", :class => "btn btn-primary" %> <%= link_to t(".destroy_button"), message_path(@message), :method => "delete", :class => "btn btn-danger" %> <% else %> <%= 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 a7125a3a7..d1df36f83 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1782,14 +1782,16 @@ en: destroy_button: "Delete" back: "Back" wrong_user: "You are logged in as '%{user}' but the message you have asked to read was not sent by or to that user. Please log in as the correct user in order to read it." - mark: - as_read: "Message marked as read" - as_unread: "Message marked as unread" unmute: notice: "Message has been moved to Inbox" error: "The message could not be moved to the Inbox." destroy: destroyed: "Message deleted" + read_marks: + create: + notice: "Message marked as read" + destroy: + notice: "Message marked as unread" mailboxes: heading: my_inbox: "My Inbox" diff --git a/config/routes.rb b/config/routes.rb index d8e83b97b..453c625ad 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -322,10 +322,12 @@ OpenStreetMap::Application.routes.draw do # messages resources :messages, :path_names => { :new => "new/:display_name" }, :id => /\d+/, :only => [:new, :create, :show, :destroy] do - post :mark patch :unmute - resource :reply, :module => :messages, :path_names => { :new => "new" }, :only => :new + scope :module => :messages do + resource :reply, :path_names => { :new => "new" }, :only => :new + resource :read_mark, :only => [:create, :destroy] + end end namespace :messages, :path => "/messages" do resource :inbox, :only => :show diff --git a/test/controllers/messages/read_marks_controller_test.rb b/test/controllers/messages/read_marks_controller_test.rb new file mode 100644 index 000000000..e02e46b72 --- /dev/null +++ b/test/controllers/messages/read_marks_controller_test.rb @@ -0,0 +1,126 @@ +require "test_helper" + +module Messages + class ReadMarksControllerTest < ActionDispatch::IntegrationTest + ## + # test all routes which lead to this controller + def test_routes + assert_routing( + { :path => "/messages/1/read_mark", :method => :post }, + { :controller => "messages/read_marks", :action => "create", :message_id => "1" } + ) + assert_routing( + { :path => "/messages/1/read_mark", :method => :delete }, + { :controller => "messages/read_marks", :action => "destroy", :message_id => "1" } + ) + end + + def test_create_when_not_logged_in + message = create(:message, :unread) + + post message_read_mark_path(message) + assert_response :forbidden + end + + def test_create_as_other_user + message = create(:message, :unread) + session_for(create(:user)) + + post message_read_mark_path(message) + assert_response :not_found + assert_template "no_such_message" + end + + def test_create_as_sender + message = create(:message, :unread) + session_for(message.sender) + + post message_read_mark_path(message) + assert_response :not_found + assert_template "no_such_message" + end + + def test_create_as_recipient + message = create(:message, :unread) + session_for(message.recipient) + + post message_read_mark_path(message) + assert_redirected_to messages_inbox_path + assert message.reload.message_read + end + + def test_create_on_missing_message + session_for(create(:user)) + + post message_read_mark_path(99999) + assert_response :not_found + assert_template "no_such_message" + end + + def test_create_on_message_from_muted_user + sender_user = create(:user) + recipient_user = create(:user) + create(:user_mute, :owner => recipient_user, :subject => sender_user) + message = create(:message, :unread, :sender => sender_user, :recipient => recipient_user) + session_for(recipient_user) + + post message_read_mark_path(message) + assert_redirected_to messages_muted_inbox_path + assert message.reload.message_read + end + + def test_destroy_when_not_logged_in + message = create(:message, :read) + + delete message_read_mark_path(message) + assert_response :forbidden + end + + def test_destroy_as_other_user + message = create(:message, :read) + session_for(create(:user)) + + delete message_read_mark_path(message) + assert_response :not_found + assert_template "no_such_message" + end + + def test_destroy_as_sender + message = create(:message, :read) + session_for(message.sender) + + delete message_read_mark_path(message) + assert_response :not_found + assert_template "no_such_message" + end + + def test_destroy_as_recipient + message = create(:message, :read) + session_for(message.recipient) + + delete message_read_mark_path(message) + assert_redirected_to messages_inbox_path + assert_not message.reload.message_read + end + + def test_destroy_on_missing_message + session_for(create(:user)) + + delete message_read_mark_path(99999) + assert_response :not_found + assert_template "no_such_message" + end + + def test_destroy_on_message_from_muted_user + sender_user = create(:user) + recipient_user = create(:user) + create(:user_mute, :owner => recipient_user, :subject => sender_user) + message = create(:message, :read, :sender => sender_user, :recipient => recipient_user) + session_for(recipient_user) + + delete message_read_mark_path(message, :mark => "unread") + assert_redirected_to messages_muted_inbox_path + assert_not message.reload.message_read + end + end +end diff --git a/test/controllers/messages_controller_test.rb b/test/controllers/messages_controller_test.rb index b2bb71b1c..bdf88600c 100644 --- a/test/controllers/messages_controller_test.rb +++ b/test/controllers/messages_controller_test.rb @@ -16,10 +16,6 @@ class MessagesControllerTest < ActionDispatch::IntegrationTest { :path => "/messages/1", :method => :get }, { :controller => "messages", :action => "show", :id => "1" } ) - assert_routing( - { :path => "/messages/1/mark", :method => :post }, - { :controller => "messages", :action => "mark", :message_id => "1" } - ) assert_routing( { :path => "/messages/1", :method => :delete }, { :controller => "messages", :action => "destroy", :id => "1" } @@ -259,84 +255,6 @@ class MessagesControllerTest < ActionDispatch::IntegrationTest assert_template "no_such_message" end - ## - # test the mark action - def test_mark - sender_user = create(:user) - recipient_user = create(:user) - other_user = create(:user) - message = create(:message, :unread, :sender => sender_user, :recipient => recipient_user) - - # Check that the marking a message requires us to login - post message_mark_path(message) - assert_response :forbidden - - # Login as a user with no messages - session_for(other_user) - - # Check that marking a message we didn't send or receive fails - post message_mark_path(message) - assert_response :not_found - assert_template "no_such_message" - - # Login as the message sender_user - session_for(sender_user) - - # Check that marking a message we sent fails - post message_mark_path(message) - assert_response :not_found - assert_template "no_such_message" - - # Login as the message recipient_user - session_for(recipient_user) - - # Check that the marking a message read works - post message_mark_path(message, :mark => "read") - assert_redirected_to messages_inbox_path - assert Message.find(message.id).message_read - - # Check that the marking a message unread works - post message_mark_path(message, :mark => "unread") - assert_redirected_to messages_inbox_path - assert_not Message.find(message.id).message_read - - # Check that the marking a message read works and redirects to inbox from the message page - post message_mark_path(message, :mark => "read"), :headers => { :referer => message_path(message) } - assert_redirected_to messages_inbox_path - assert Message.find(message.id).message_read - - # Check that the marking a message unread works and redirects to inbox from the message page - post message_mark_path(message, :mark => "unread"), :headers => { :referer => message_path(message) } - assert_redirected_to messages_inbox_path - assert_not Message.find(message.id).message_read - - # Asking to mark a message with a bogus ID should fail - post message_mark_path(99999) - assert_response :not_found - assert_template "no_such_message" - end - - ## - # test the mark action for messages from muted users - def test_mark_muted - sender_user = create(:user) - recipient_user = create(:user) - create(:user_mute, :owner => recipient_user, :subject => sender_user) - message = create(:message, :unread, :sender => sender_user, :recipient => recipient_user) - - session_for(recipient_user) - - # Check that the marking a message read works - post message_mark_path(message, :mark => "read") - assert_redirected_to messages_muted_inbox_path - assert Message.find(message.id).message_read - - # Check that the marking a message unread works - post message_mark_path(message, :mark => "unread") - assert_redirected_to messages_muted_inbox_path - assert_not Message.find(message.id).message_read - end - ## # test the destroy action def test_destroy -- 2.39.5