From 3fbf2d557df992dd5bbff7ab7774a91b7fc64c08 Mon Sep 17 00:00:00 2001 From: MAEDA Go Date: Thu, 4 Jun 2026 18:25:26 +0900 Subject: [PATCH 2/2] Add REST API for forum messages --- app/controllers/messages_controller.rb | 209 ++++++++++++-- app/helpers/messages_helper.rb | 33 +++ app/views/messages/replies.api.rsb | 7 + app/views/messages/show.api.rsb | 3 + app/views/messages/topics.api.rsb | 7 + config/routes.rb | 8 + lib/redmine/preparation.rb | 8 +- test/integration/api_test/api_routing_test.rb | 12 + test/integration/api_test/messages_test.rb | 264 ++++++++++++++++++ 9 files changed, 520 insertions(+), 31 deletions(-) create mode 100644 app/views/messages/replies.api.rsb create mode 100644 app/views/messages/show.api.rsb create mode 100644 app/views/messages/topics.api.rsb create mode 100644 test/integration/api_test/messages_test.rb diff --git a/app/controllers/messages_controller.rb b/app/controllers/messages_controller.rb index 8b26bee73..c70cbf5d2 100644 --- a/app/controllers/messages_controller.rb +++ b/app/controllers/messages_controller.rb @@ -20,10 +20,12 @@ class MessagesController < ApplicationController menu_item :boards default_search_scope :messages - before_action :find_board, :only => [:new, :preview] + before_action :find_board, :only => [:index, :create, :new, :preview] + before_action :find_topic, :only => [:replies, :create_reply] before_action :find_attachments, :only => [:preview] - before_action :find_message, :except => [:new, :preview] - before_action :authorize, :except => [:preview, :edit, :destroy] + before_action :find_message, :only => [:show, :reply, :edit, :update, :destroy, :quote] + before_action :authorize, :except => [:preview, :edit, :update, :destroy] + accept_api_auth :index, :show, :create, :update, :destroy, :replies, :create_reply helper :boards helper :watchers @@ -33,28 +35,81 @@ class MessagesController < ApplicationController REPLIES_PER_PAGE = 25 unless const_defined?(:REPLIES_PER_PAGE) + # List topics of a board + def index + @offset, @limit = api_offset_and_limit + scope = @board.topics + @message_count = scope.count + @messages = scope. + reorder(:sticky => :desc, :id => :desc). + includes(:author, :parent, {:last_reply => :author}, {:board => :project}). + limit(@limit). + offset(@offset). + to_a + + respond_to do |format| + format.html {render_404} + format.api {render :action => 'topics'} + end + end + # Show a topic and its replies def show - page = params[:page] - # Find the page of the requested reply - if params[:r] && page.nil? - offset = @topic.children.where("#{Message.table_name}.id < ?", params[:r].to_i).count - page = 1 + offset / REPLIES_PER_PAGE - end + respond_to do |format| + format.html do + page = params[:page] + # Find the page of the requested reply + if params[:r] && page.nil? + offset = @topic.children.where("#{Message.table_name}.id < ?", params[:r].to_i).count + page = 1 + offset / REPLIES_PER_PAGE + end - @reply_count = @topic.children.count - @reply_pages = Paginator.new @reply_count, REPLIES_PER_PAGE, page - @replies = @topic.children. - includes(:author, :attachments, {:board => :project}). - reorder("#{Message.table_name}.created_on ASC, #{Message.table_name}.id ASC"). - limit(@reply_pages.per_page). - offset(@reply_pages.offset). - to_a + @reply_count = @topic.children.count + @reply_pages = Paginator.new @reply_count, REPLIES_PER_PAGE, page + @replies = @topic.children. + includes(:author, :attachments, {:board => :project}). + reorder("#{Message.table_name}.created_on ASC, #{Message.table_name}.id ASC"). + limit(@reply_pages.per_page). + offset(@reply_pages.offset). + to_a - Message.preload_reaction_details(@replies) + Message.preload_reaction_details(@replies) + + @reply = Message.new(:subject => "RE: #{@message.subject}") + render :action => "show", :layout => false if request.xhr? + end + format.api + end + end - @reply = Message.new(:subject => "RE: #{@message.subject}") - render :action => "show", :layout => false if request.xhr? + # Create a new topic through the REST API + def create + @message = Message.new + @message.author = User.current + @message.board = @board + @message.safe_attributes = params[:message] + # Ignore board_id overrides from the request body for this scoped API. + @message.board = @board + @message.save_attachments(params[:attachments] || (params[:message] && params[:message][:uploads])) + if @message.save + call_hook(:controller_messages_new_after_save, {:params => params, :message => @message}) + render_attachment_warning_if_needed(@message) + respond_to do |format| + format.html {render_404} + format.api do + render( + :action => 'show', + :status => :created, + :location => message_url(@message) + ) + end + end + else + respond_to do |format| + format.html {render_404} + format.api {render_validation_errors(@message)} + end + end end # Create a new topic @@ -74,6 +129,24 @@ class MessagesController < ApplicationController end end + # List replies of a topic + def replies + @offset, @limit = api_offset_and_limit + scope = @topic.children + @message_count = scope.count + @messages = scope. + includes(:author, :parent, {:last_reply => :author}, {:board => :project}). + reorder("#{Message.table_name}.created_on ASC, #{Message.table_name}.id ASC"). + limit(@limit). + offset(@offset). + to_a + + respond_to do |format| + format.html {render_404} + format.api + end + end + # Reply to a topic def reply @reply = Message.new @@ -90,6 +163,38 @@ class MessagesController < ApplicationController redirect_to board_message_path(@board, @topic, :r => @reply) end + # Reply to a topic through the REST API + def create_reply + @reply = Message.new(:subject => "RE: #{@topic.subject}") + @reply.author = User.current + @reply.board = @board + @reply.safe_attributes = params[:reply] + # Ignore board_id overrides from the request body for this scoped API. + @reply.board = @board + @reply.save_attachments(params[:attachments] || (params[:reply] && params[:reply][:uploads])) + @topic.children << @reply + if @reply.new_record? + respond_to do |format| + format.html {render_404} + format.api {render_validation_errors(@reply)} + end + else + call_hook(:controller_messages_reply_after_save, {:params => params, :message => @reply}) + render_attachment_warning_if_needed(@reply) + @message = @reply + respond_to do |format| + format.html {render_404} + format.api do + render( + :action => 'show', + :status => :created, + :location => message_url(@reply) + ) + end + end + end + end + # Edit a message def edit (render_403; return false) unless @message.editable_by?(User.current) @@ -105,16 +210,48 @@ class MessagesController < ApplicationController end end + def update + (render_403; return false) unless @message.editable_by?(User.current) + project = @message.project + @message.safe_attributes = params[:message] + unless project.boards.exists?(@message.board_id) + @message.errors.add(:board_id, :invalid) + respond_to do |format| + format.html {render_404} + format.api {render_validation_errors(@message)} + end + return + end + @message.save_attachments(params[:attachments] || (params[:message] && params[:message][:uploads])) + if @message.save + render_attachment_warning_if_needed(@message) + respond_to do |format| + format.html {render_404} + format.api {render_api_ok} + end + else + respond_to do |format| + format.html {render_404} + format.api {render_validation_errors(@message)} + end + end + end + # Delete a messages def destroy (render_403; return false) unless @message.destroyable_by?(User.current) r = @message.to_param @message.destroy - flash[:notice] = l(:notice_successful_delete) - if @message.parent - redirect_to board_message_path(@board, @message.parent, :r => r) - else - redirect_to project_board_path(@project, @board) + respond_to do |format| + format.html do + flash[:notice] = l(:notice_successful_delete) + if @message.parent + redirect_to board_message_path(@board, @message.parent, :r => r) + else + redirect_to project_board_path(@project, @board) + end + end + format.api {render_api_ok} end end @@ -144,14 +281,32 @@ class MessagesController < ApplicationController private def find_message - return unless find_board + if params[:board_id] + return unless find_board - @message = @board.messages.includes(:parent).find(params[:id]) + @message = @board.messages.includes(:parent).find(params[:id]) + else + @message = Message.visible.includes(:parent, :author, :attachments, {:board => :project}).find(params[:id]) + @board = @message.board + @project = @board.project + end @topic = @message.root rescue ActiveRecord::RecordNotFound render_404 end + def find_topic + @topic = Message.visible.includes(:author, :parent, {:board => :project}).find(params[:topic_id]) + if @topic.parent + render_404 + return false + end + @board = @topic.board + @project = @board.project + rescue ActiveRecord::RecordNotFound + render_404 + end + def find_board @board = Board.includes(:project).find(params[:board_id]) @project = @board.project diff --git a/app/helpers/messages_helper.rb b/app/helpers/messages_helper.rb index 92f788d0c..dd5f6ef8f 100644 --- a/app/helpers/messages_helper.rb +++ b/app/helpers/messages_helper.rb @@ -20,4 +20,37 @@ module MessagesHelper include Redmine::QuoteReply::Helper include ReactionsHelper + + def render_api_message(message, api) + api.id message.id + api.project(:id => message.project.id, :name => message.project.name) + api.board(:id => message.board_id, :name => message.board.name) + api.parent(:id => message.parent_id, :subject => message.parent.subject) unless message.parent.nil? + api.root(:id => message.root.id, :subject => message.root.subject) + api.author(:id => message.author_id, :name => message.author.name) unless message.author.nil? + api.subject message.subject + api.content message.content + api.replies_count message.replies_count + render_api_message_last_reply(message.last_reply, api) unless message.last_reply.nil? + api.locked message.locked? + api.sticky message.sticky? + api.created_on message.created_on + api.updated_on message.updated_on + + if include_in_api_response?('attachments') + api.array :attachments do + message.attachments.each do |attachment| + render_api_attachment(attachment, api) + end + end + end + end + + def render_api_message_last_reply(message, api) + api.last_reply(:id => message.id) do + api.author(:id => message.author_id, :name => message.author.name) unless message.author.nil? + api.subject message.subject + api.created_on message.created_on + end + end end diff --git a/app/views/messages/replies.api.rsb b/app/views/messages/replies.api.rsb new file mode 100644 index 000000000..0670cd1f9 --- /dev/null +++ b/app/views/messages/replies.api.rsb @@ -0,0 +1,7 @@ +api.array :replies, api_meta(:total_count => @message_count, :offset => @offset, :limit => @limit) do + @messages.each do |message| + api.reply do + render_api_message(message, api) + end + end +end diff --git a/app/views/messages/show.api.rsb b/app/views/messages/show.api.rsb new file mode 100644 index 000000000..f8993058b --- /dev/null +++ b/app/views/messages/show.api.rsb @@ -0,0 +1,3 @@ +api.message do + render_api_message(@message, api) +end diff --git a/app/views/messages/topics.api.rsb b/app/views/messages/topics.api.rsb new file mode 100644 index 000000000..44f9f3bde --- /dev/null +++ b/app/views/messages/topics.api.rsb @@ -0,0 +1,7 @@ +api.array :topics, api_meta(:total_count => @message_count, :offset => @offset, :limit => @limit) do + @messages.each do |message| + api.topic do + render_api_message(message, api) + end + end +end diff --git a/config/routes.rb b/config/routes.rb index 1cc23df27..b3c8e4283 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -43,6 +43,8 @@ Rails.application.routes.draw do match 'projects/:id/wiki/destroy', :to => 'wikis#destroy', :via => [:get, :post] + get 'boards/:board_id/topics.:format', :to => 'messages#index', :constraints => {:format => /xml|json/} + post 'boards/:board_id/topics.:format', :to => 'messages#create', :constraints => {:format => /xml|json/} match 'boards/:board_id/topics/new', :to => 'messages#new', :via => [:get, :post], :as => 'new_board_message' get 'boards/:board_id/topics/:id', :to => 'messages#show', :as => 'board_message' match 'boards/:board_id/topics/quote/:id', :to => 'messages#quote', :via => [:get, :post] @@ -52,6 +54,12 @@ Rails.application.routes.draw do post 'boards/:board_id/topics/:id/replies', :to => 'messages#reply' post 'boards/:board_id/topics/:id/edit', :to => 'messages#edit' post 'boards/:board_id/topics/:id/destroy', :to => 'messages#destroy' + get 'messages/:topic_id/replies.:format', :to => 'messages#replies', :constraints => {:format => /xml|json/} + post 'messages/:topic_id/replies.:format', :to => 'messages#create_reply', :constraints => {:format => /xml|json/} + get 'messages/:id.:format', :to => 'messages#show', :as => 'message', :constraints => {:format => /xml|json/} + put 'messages/:id.:format', :to => 'messages#update', :constraints => {:format => /xml|json/} + patch 'messages/:id.:format', :to => 'messages#update', :constraints => {:format => /xml|json/} + delete 'messages/:id.:format', :to => 'messages#destroy', :constraints => {:format => /xml|json/} get 'boards/:id.:format', :to => 'boards#show', :as => 'board', :constraints => {:format => /xml|json/} put 'boards/:id.:format', :to => 'boards#update', :constraints => {:format => /xml|json/} patch 'boards/:id.:format', :to => 'boards#update', :constraints => {:format => /xml|json/} diff --git a/lib/redmine/preparation.rb b/lib/redmine/preparation.rb index 0dd20f15b..b140f1333 100644 --- a/lib/redmine/preparation.rb +++ b/lib/redmine/preparation.rb @@ -144,10 +144,10 @@ module Redmine end map.project_module :boards do |map| - map.permission :view_messages, {:boards => [:index, :show], :messages => [:show]}, :read => true - map.permission :add_messages, {:messages => [:new, :reply, :quote], :attachments => :upload} - map.permission :edit_messages, {:messages => :edit, :attachments => :upload}, :require => :member - map.permission :edit_own_messages, {:messages => :edit, :attachments => :upload}, :require => :loggedin + map.permission :view_messages, {:boards => [:index, :show], :messages => [:index, :show, :replies]}, :read => true + map.permission :add_messages, {:messages => [:new, :create, :reply, :create_reply, :quote], :attachments => :upload} + map.permission :edit_messages, {:messages => [:edit, :update], :attachments => :upload}, :require => :member + map.permission :edit_own_messages, {:messages => [:edit, :update], :attachments => :upload}, :require => :loggedin map.permission :delete_messages, {:messages => :destroy}, :require => :member map.permission :delete_own_messages, {:messages => :destroy}, :require => :loggedin map.permission :view_message_watchers, {}, :read => true diff --git a/test/integration/api_test/api_routing_test.rb b/test/integration/api_test/api_routing_test.rb index c99b0f264..4cd1477f8 100644 --- a/test/integration/api_test/api_routing_test.rb +++ b/test/integration/api_test/api_routing_test.rb @@ -99,6 +99,18 @@ class Redmine::ApiTest::ApiRoutingTest < Redmine::ApiTest::Routing should_route 'DELETE /issues/12/watchers/3' => 'watchers#destroy', :object_type => 'issue', :object_id => '12', :user_id => '3' end + def test_messages + should_route 'GET /boards/1/topics' => 'messages#index', :board_id => '1' + should_route 'POST /boards/1/topics' => 'messages#create', :board_id => '1' + + should_route 'GET /messages/1/replies' => 'messages#replies', :topic_id => '1' + should_route 'POST /messages/1/replies' => 'messages#create_reply', :topic_id => '1' + + should_route 'GET /messages/2' => 'messages#show', :id => '2' + should_route 'PUT /messages/2' => 'messages#update', :id => '2' + should_route 'DELETE /messages/2' => 'messages#destroy', :id => '2' + end + def test_memberships should_route 'GET /projects/5234/memberships' => 'members#index', :project_id => '5234' should_route 'POST /projects/5234/memberships' => 'members#create', :project_id => '5234' diff --git a/test/integration/api_test/messages_test.rb b/test/integration/api_test/messages_test.rb new file mode 100644 index 000000000..7f15abd52 --- /dev/null +++ b/test/integration/api_test/messages_test.rb @@ -0,0 +1,264 @@ +# frozen_string_literal: true + +# Redmine - project management software +# Copyright (C) 2006- Jean-Philippe Lang +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +require_relative '../../test_helper' + +class Redmine::ApiTest::MessagesTest < Redmine::ApiTest::Base + test "GET /boards/:board_id/topics.xml should return the board topics" do + get '/boards/1/topics.xml', :headers => credentials('jsmith') + + assert_response :success + assert_equal 'application/xml', @response.media_type + assert_select 'topics[type=array] topic id', :text => '4' + assert_select 'topics[type=array] topic parent', 0 + end + + test "POST /boards/:board_id/topics.xml should create a topic" do + assert_difference 'Message.count' do + post( + '/boards/1/topics.xml', + :params => {:message => {:subject => 'API topic', :content => 'API topic body'}}, + :headers => credentials('jsmith')) + end + + message = Message.order(id: :desc).first + assert_response :created + assert_equal 'application/xml', @response.media_type + assert_nil message.parent + assert_equal 'API topic', message.subject + assert_equal 'API topic body', message.content + assert_equal Board.find(1), message.board + assert_select 'message id', :text => message.id.to_s + end + + test "POST /boards/:board_id/topics.xml should ignore board_id parameter" do + assert_difference 'Message.count' do + post( + '/boards/1/topics.xml', + :params => { + :message => { + :subject => 'API topic', + :content => 'API topic body', + :board_id => 2 + } + }, + :headers => credentials('jsmith')) + end + + assert_response :created + assert_equal Board.find(1), Message.order(id: :desc).first.board + end + + test "POST /boards/:board_id/topics.xml with attachment should create a topic with attachment" do + token = xml_upload('File content', credentials('jsmith')) + attachment = Attachment.find_by_token(token) + + assert_difference 'Message.count' do + post( + '/boards/1/topics.xml', + :params => { + :message => { + :subject => 'API topic with attachment', + :content => 'API topic body' + }, + :attachments => [ + { + :token => token, + :filename => 'message.txt', + :content_type => 'text/plain' + } + ] + }, + :headers => credentials('jsmith')) + end + + message = Message.order(id: :desc).first + assert_response :created + assert_equal attachment, message.attachments.first + + attachment.reload + assert_equal 'message.txt', attachment.filename + assert_equal 'text/plain', attachment.content_type + assert_equal 2, attachment.author_id + end + + test "POST /boards/:board_id/topics.xml with invalid parameters should return errors" do + assert_no_difference 'Message.count' do + post( + '/boards/1/topics.xml', + :params => {:message => {:subject => '', :content => 'API topic body'}}, + :headers => credentials('jsmith')) + end + + assert_response :unprocessable_content + assert_equal 'application/xml', @response.media_type + assert_select 'errors error', :text => 'Subject cannot be blank' + end + + test "GET /messages/:topic_id/replies.xml should return replies" do + get '/messages/1/replies.xml', :headers => credentials('jsmith') + + assert_response :success + assert_equal 'application/xml', @response.media_type + assert_select 'replies[type=array] reply id', :text => '2' + assert_select 'replies[type=array] reply parent[id="1"][subject="First post"]' + end + + test "POST /messages/:topic_id/replies.xml should create a reply" do + assert_difference 'Message.count' do + post( + '/messages/1/replies.xml', + :params => {:reply => {:content => 'API reply body'}}, + :headers => credentials('jsmith')) + end + + reply = Message.order(id: :desc).first + assert_response :created + assert_equal 'application/xml', @response.media_type + assert_equal Message.find(1), reply.parent + assert_equal 'RE: First post', reply.subject + assert_equal 'API reply body', reply.content + assert_select 'message id', :text => reply.id.to_s + end + + test "POST /messages/:topic_id/replies.xml should ignore board_id parameter" do + assert_difference 'Message.count' do + post( + '/messages/1/replies.xml', + :params => {:reply => {:content => 'API reply body', :board_id => 2}}, + :headers => credentials('jsmith')) + end + + reply = Message.order(id: :desc).first + assert_response :created + assert_equal Message.find(1), reply.parent + assert_equal Board.find(1), reply.board + end + + test "POST /messages/:topic_id/replies.xml to locked topic should return errors" do + Message.find(1).update!(:locked => true) + + assert_no_difference 'Message.count' do + post( + '/messages/1/replies.xml', + :params => {:reply => {:content => 'API reply body'}}, + :headers => credentials('jsmith')) + end + + assert_response :unprocessable_content + assert_equal 'application/xml', @response.media_type + assert_select 'errors error', :text => 'Topic is locked' + end + + test "POST /messages/:topic_id/replies without API format should not create a reply" do + assert_no_difference 'Message.count' do + post( + '/messages/1/replies', + :params => {:reply => {:content => 'HTML reply body'}}, + :headers => credentials('jsmith')) + end + + assert_response :not_found + end + + test "GET /messages/:id.xml should return a topic by message id" do + get '/messages/1.xml', :headers => credentials('jsmith') + + assert_response :success + assert_equal 'application/xml', @response.media_type + assert_select 'message' do + assert_select 'id', :text => '1' + assert_select 'project[id="1"][name="eCookbook"]' + assert_select 'board[id="1"][name="Help"]' + assert_select 'root[id="1"][subject="First post"]' + assert_select 'subject', :text => 'First post' + assert_select 'replies_count', :text => '2' + end + end + + test "GET /messages/:id.xml should return a reply by message id" do + get '/messages/2.xml', :headers => credentials('jsmith') + + assert_response :success + assert_equal 'application/xml', @response.media_type + assert_select 'message' do + assert_select 'id', :text => '2' + assert_select 'parent[id="1"][subject="First post"]' + assert_select 'root[id="1"][subject="First post"]' + assert_select 'subject', :text => 'First reply' + end + end + + test "GET /messages/:id.xml without permission should return 404" do + get '/messages/7.xml' + + assert_response :not_found + end + + test "PUT /messages/:id.xml should update a message" do + put( + '/messages/1.xml', + :params => {:message => {:subject => 'Updated topic', :content => 'Updated topic body'}}, + :headers => credentials('jsmith')) + + assert_response :no_content + assert_equal '', @response.body + assert_equal 'Updated topic', Message.find(1).subject + assert_equal 'Updated topic body', Message.find(1).content + end + + test "PUT /messages/:id.xml should allow moving a message to a board in the same project" do + put( + '/messages/1.xml', + :params => {:message => {:board_id => 2}}, + :headers => credentials('jsmith')) + + assert_response :no_content + assert_equal Board.find(2), Message.find(1).board + end + + test "PUT /messages/:id.xml should not move a message to a board in another project" do + put( + '/messages/1.xml', + :params => {:message => {:board_id => 3}}, + :headers => credentials('jsmith')) + + assert_response :unprocessable_content + assert_equal Board.find(1), Message.find(1).board + end + + test "DELETE /messages/:id.xml should destroy a message" do + assert_difference 'Message.count', -1 do + delete '/messages/2.xml', :headers => credentials('jsmith') + end + + assert_response :no_content + assert_equal '', @response.body + assert_nil Message.find_by_id(2) + end + + test "DELETE /messages/:id without API format should not destroy a message" do + assert_no_difference 'Message.count' do + delete '/messages/2', :headers => credentials('jsmith') + end + + assert_response :not_found + assert Message.exists?(2) + end +end -- 2.50.1