From a9af6e7329dbcf2a050789b546b2345b2bafab5c Mon Sep 17 00:00:00 2001 From: Katsuya Hidaka Date: Sun, 8 Sep 2024 23:57:25 +0900 Subject: [PATCH 3/7] Add the ability to quote partial text --- app/assets/javascripts/application.js | 60 ++++++++++++++++++++- app/controllers/journals_controller.rb | 2 +- app/controllers/messages_controller.rb | 4 +- app/helpers/journals_helper.rb | 3 +- app/views/issues/show.html.erb | 7 ++- app/views/messages/show.html.erb | 6 +-- test/functional/journals_controller_test.rb | 24 +++++++++ test/functional/messages_controller_test.rb | 29 ++++++++++ test/system/issues_reply_test.rb | 59 ++++++++++++++++++++ test/system/messages_test.rb | 48 +++++++++++++++++ 10 files changed, 232 insertions(+), 10 deletions(-) diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index da0114dce..a155b7223 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -1262,13 +1262,69 @@ function inlineAutoComplete(element) { tribute.attach(element); } -function quoteReply(path) { +function quoteReply(path, selectorForContentElement) { + const contentElement = $(selectorForContentElement).get(0); + const quote = QuoteExtractor.extract(contentElement); + $.ajax({ url: path, - type: 'post' + type: 'post', + data: { quote: quote } }); } +class QuoteExtractor { + static extract(targetElement) { + return new QuoteExtractor(targetElement).extract(); + } + + constructor(targetElement) { + this.targetElement = targetElement; + this.selection = window.getSelection(); + } + + extract() { + const range = this.selectedRange; + + if (!range) { + return null; + } + + if (!this.targetElement.contains(range.startContainer)) { + range.setStartBefore(this.targetElement); + } + if (!this.targetElement.contains(range.endContainer)) { + range.setEndAfter(this.targetElement); + } + + return this.formatRange(range); + } + + formatRange(range) { + return range.toString().trim(); + } + + get selectedRange() { + if (!this.isSelected) { + return null; + } + + // Retrive the first range that intersects with the target element. + // NOTE: Firefox allows to select multiple ranges in the document. + for (let i = 0; i < this.selection.rangeCount; i++) { + let range = this.selection.getRangeAt(i); + if (range.intersectsNode(this.targetElement)) { + return range; + } + } + return null; + } + + get isSelected() { + return this.selection.containsNode(this.targetElement, true); + } +} + $(document).ready(setupAjaxIndicator); $(document).ready(hideOnLoad); $(document).ready(addFormObserversForDoubleSubmit); diff --git a/app/controllers/journals_controller.rb b/app/controllers/journals_controller.rb index cbf545187..55f68503d 100644 --- a/app/controllers/journals_controller.rb +++ b/app/controllers/journals_controller.rb @@ -75,7 +75,7 @@ class JournalsController < ApplicationController @content = "#{ll(Setting.default_language, :text_user_wrote, user)}\n> " end # Replaces pre blocks with [...] - text = text.to_s.strip.gsub(%r{
(.*?)
}m, '[...]') + text = params[:quote].presence || text.to_s.strip.gsub(%r{
(.*?)
}m, '[...]') @content << text.gsub(/(\r?\n|\r\n?)/, "\n> ") + "\n\n" rescue ActiveRecord::RecordNotFound render_404 diff --git a/app/controllers/messages_controller.rb b/app/controllers/messages_controller.rb index 5050d94a7..55caa0f43 100644 --- a/app/controllers/messages_controller.rb +++ b/app/controllers/messages_controller.rb @@ -124,7 +124,9 @@ class MessagesController < ApplicationController else @content = "#{ll(Setting.default_language, :text_user_wrote_in, {:value => @message.author, :link => "message##{@message.id}"})}\n> " end - @content << @message.content.to_s.strip.gsub(%r{
(.*?)
}m, '[...]').gsub(/(\r?\n|\r\n?)/, "\n> ") + "\n\n" + + quote_text = params[:quote].presence || @message.content.to_s.strip.gsub(%r{
(.*?)
}m, '[...]') + @content << quote_text.gsub(/(\r?\n|\r\n?)/, "\n> ") + "\n\n" respond_to do |format| format.html { render_404 } diff --git a/app/helpers/journals_helper.rb b/app/helpers/journals_helper.rb index f27799c7c..983644b63 100644 --- a/app/helpers/journals_helper.rb +++ b/app/helpers/journals_helper.rb @@ -40,8 +40,9 @@ module JournalsHelper if journal.notes.present? if options[:reply_links] + url = quoted_issue_path(issue, :journal_id => journal, :journal_indice => indice) links << link_to_function(icon_with_label('comment', l(:button_quote)), - "quoteReply('#{j quoted_issue_path(issue, :journal_id => journal, :journal_indice => indice)}')", + "quoteReply('#{j url}', '#journal-#{j journal.id}-notes')", :title => l(:button_quote), :class => 'icon-only icon-comment' ) diff --git a/app/views/issues/show.html.erb b/app/views/issues/show.html.erb index a3efa28b9..2a8f1a792 100644 --- a/app/views/issues/show.html.erb +++ b/app/views/issues/show.html.erb @@ -84,11 +84,14 @@ end %>
- <%= link_to_function icon_with_label('comment', l(:button_quote)), "quoteReply('#{j quoted_issue_path(@issue)}')",:class => 'icon icon-comment ' if @issue.notes_addable? %> + <%= link_to_function(icon_with_label('comment', l(:button_quote)), + "quoteReply('#{j quoted_issue_path(@issue)}', '#issue_description_wiki')", + :class => 'icon icon-comment ' + ) if @issue.notes_addable? %>

<%=l(:field_description)%>

-
+
<%= textilizable @issue, :description, :attachments => @issue.attachments %>
diff --git a/app/views/messages/show.html.erb b/app/views/messages/show.html.erb index 6228a366b..50e2f65e7 100644 --- a/app/views/messages/show.html.erb +++ b/app/views/messages/show.html.erb @@ -4,7 +4,7 @@ <%= watcher_link(@topic, User.current) %> <%= link_to_function( icon_with_label('comment', l(:button_quote)), - "quoteReply('#{j url_for(:action => 'quote', :id => @topic, :format => 'js')}')", + "quoteReply('#{j url_for(:action => 'quote', :id => @topic, :format => 'js')}', '#message_topic_wiki')", :class => 'icon icon-comment') if !@topic.locked? && authorize_for('messages', 'reply') %> <%= link_to( icon_with_label('edit', l(:button_edit)), @@ -24,7 +24,7 @@

<%= authoring @topic.created_on, @topic.author %>

-
+
<%= textilizable(@topic, :content) %>
<%= link_to_attachments @topic, :author => false, :thumbnails => true %> @@ -42,7 +42,7 @@
<%= link_to_function( icon_with_label('comment', l(:button_quote), icon_only: true), - "quoteReply('#{j url_for(:action => 'quote', :id => message, :format => 'js')}')", + "quoteReply('#{j url_for(:action => 'quote', :id => message, :format => 'js')}', '#message-#{j message.id} .wiki')", :title => l(:button_quote), :class => 'icon icon-comment' ) if !@topic.locked? && authorize_for('messages', 'reply') %> diff --git a/test/functional/journals_controller_test.rb b/test/functional/journals_controller_test.rb index 5fde7362b..26419e33b 100644 --- a/test/functional/journals_controller_test.rb +++ b/test/functional/journals_controller_test.rb @@ -226,6 +226,30 @@ class JournalsControllerTest < Redmine::ControllerTest assert_response :not_found end + def test_reply_to_issue_with_partial_quote + @request.session[:user_id] = 2 + + params = { id: 6, quote: 'a private subproject of cookbook' } + post :new, params: params, xhr: true + + assert_response :success + assert_equal 'text/javascript', response.media_type + assert_include 'John Smith wrote:', response.body + assert_include '> a private subproject of cookbook', response.body + end + + def test_reply_to_note_with_partial_quote + @request.session[:user_id] = 2 + + params = { id: 6, journal_id: 4, journal_indice: 1, quote: 'a private version' } + post :new, params: params, xhr: true + + assert_response :success + assert_equal 'text/javascript', response.media_type + assert_include 'Redmine Admin wrote in #note-1:', response.body + assert_include '> a private version', response.body + end + def test_edit_xhr @request.session[:user_id] = 1 get(:edit, :params => {:id => 2}, :xhr => true) diff --git a/test/functional/messages_controller_test.rb b/test/functional/messages_controller_test.rb index 460651593..c0723643b 100644 --- a/test/functional/messages_controller_test.rb +++ b/test/functional/messages_controller_test.rb @@ -322,6 +322,35 @@ class MessagesControllerTest < Redmine::ControllerTest assert_include '> An other reply', response.body end + def test_quote_with_partial_quote_if_message_is_root + @request.session[:user_id] = 2 + + params = { board_id: 1, id: 1, + quote: "the very first post\nin the forum" } + post :quote, params: params, xhr: true + + assert_response :success + assert_equal 'text/javascript', response.media_type + + assert_include 'RE: First post', response.body + assert_include "Redmine Admin wrote:", response.body + assert_include '> the very first post\n> in the forum', response.body + end + + def test_quote_with_partial_quote_if_message_is_not_root + @request.session[:user_id] = 2 + + params = { board_id: 1, id: 3, quote: 'other reply' } + post :quote, params: params, xhr: true + + assert_response :success + assert_equal 'text/javascript', response.media_type + + assert_include 'RE: First post', response.body + assert_include 'John Smith wrote in message#3:', response.body + assert_include '> other reply', response.body + end + def test_quote_as_html_should_respond_with_404 @request.session[:user_id] = 2 post( diff --git a/test/system/issues_reply_test.rb b/test/system/issues_reply_test.rb index e2dc18804..91ac0937d 100644 --- a/test/system/issues_reply_test.rb +++ b/test/system/issues_reply_test.rb @@ -37,6 +37,15 @@ class IssuesReplyTest < ApplicationSystemTestCase click_link 'Quote' end + # Select the other than the issue description element. + page.execute_script <<-JS + const range = document.createRange(); + // Select "Description" text. + range.selectNodeContents(document.querySelector('.description > p')) + + window.getSelection().addRange(range); + JS + assert_field 'issue_notes', with: <<~TEXT John Smith wrote: > Unable to print recipes @@ -57,4 +66,54 @@ class IssuesReplyTest < ApplicationSystemTestCase TEXT assert_selector :css, '#issue_notes:focus' end + + def test_reply_to_issue_with_partial_quote + assert_text 'Unable to print recipes' + + # Select only the "print" text from the text "Unable to print recipes" in the description. + page.execute_script <<-JS + const range = document.createRange(); + const wiki = document.querySelector('#issue_description_wiki > p').childNodes[0]; + range.setStart(wiki, 10); + range.setEnd(wiki, 15); + + window.getSelection().addRange(range); + JS + + within '.issue.details' do + click_link 'Quote' + end + + assert_field 'issue_notes', with: <<~TEXT + John Smith wrote: + > print + + TEXT + assert_selector :css, '#issue_notes:focus' + end + + def test_reply_to_note_with_partial_quote + assert_text 'Journal notes' + + # Select the entire details of the note#1 and the part of the note#1's text. + page.execute_script <<-JS + const range = document.createRange(); + range.setStartBefore(document.querySelector('#change-1 .details')); + // Select only the text "Journal" from the text "Journal notes" in the note-1. + range.setEnd(document.querySelector('#change-1 .wiki > p').childNodes[0], 7); + + window.getSelection().addRange(range); + JS + + within '#change-1' do + click_link 'Quote' + end + + assert_field 'issue_notes', with: <<~TEXT + Redmine Admin wrote in #note-1: + > Journal + + TEXT + assert_selector :css, '#issue_notes:focus' + end end diff --git a/test/system/messages_test.rb b/test/system/messages_test.rb index 777781991..5ca53adf7 100644 --- a/test/system/messages_test.rb +++ b/test/system/messages_test.rb @@ -54,4 +54,52 @@ class MessagesTest < ApplicationSystemTestCase TEXT end + + def test_reply_to_topic_message_with_partial_quote + assert_text /This is the very first post/ + + # Select the part of the topic message through the entire text of the attachment below it. + page.execute_script <<-'JS' + const range = document.createRange(); + const message = document.querySelector('#message_topic_wiki'); + // Select only the text "in the forum" from the text "This is the very first post\nin the forum". + range.setStartBefore(message.querySelector('p').childNodes[2]); + range.setEndAfter(message.parentNode.querySelector('.attachments')); + + window.getSelection().addRange(range); + JS + + within '#content > .contextual' do + click_link 'Quote' + end + + assert_field 'message_content', with: <<~TEXT + Redmine Admin wrote: + > in the forum + + TEXT + end + + def test_reply_to_message_with_partial_quote + assert_text 'Reply to the first post' + + # Select the entire message, including the subject and headers of messages #2 and #3. + page.execute_script <<-JS + const range = document.createRange(); + range.setStartBefore(document.querySelector('#message-2')); + range.setEndAfter(document.querySelector('#message-3')); + + window.getSelection().addRange(range); + JS + + within '#message-2' do + click_link 'Quote' + end + + assert_field 'message_content', with: <<~TEXT + Redmine Admin wrote in message#2: + > Reply to the first post + + TEXT + end end -- 2.44.0