Merge pull request #16152 from nsalguero/dev

[55263] Fix layout issues when diffing wiki pages

app/controllers/wiki_controller.rb

@@ -261,10 +261,10 @@ def history
 
   def diff
     if (@diff = @page.diff(params[:version_to], params[:version_from]))
-      @html_diff = HTMLDiff::DiffBuilder.new(
-        helpers.format_text(@diff.content_from.data.text, disable_macro_expansion: true),
-        helpers.format_text(@diff.content_to.data.text, disable_macro_expansion: true)
-      ).build
+      @html_diff = OpenProject::HtmlDiff.from_markdown(
+        @diff.content_from.data.text,
+        @diff.content_to.data.text
+      )
     else
       render_404
     end

app/helpers/wiki_helper.rb

@@ -58,10 +58,6 @@ def breadcrumb_for_page(page, action = nil)
     breadcrumb_paths(*paths)
   end
 
-  def nl2br(content)
-    content.gsub(/(?:\n\r?|\r\n?)/, "<br />").html_safe
-  end
-
   private
 
   def wiki_page_options_for_select_of_level(pages,

app/views/wiki/diff.html.erb

@@ -44,5 +44,5 @@ See COPYRIGHT and LICENSE files for more details.
   <em>(<%= @diff.content_to.user ? link_to_user(@diff.content_to.user) : t(:label_user_anonymous) %>, <%= format_time(@diff.content_to.created_at) %>)</em>
 </p>
 <div class="text-diff">
-  <%= nl2br @html_diff %>
+  <%= @html_diff.html_safe %>
 </div>

lib/open_project/html_diff.rb

@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2024 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# 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.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+module OpenProject
+  module HtmlDiff
+    extend OpenProject::TextFormatting
+
+    module_function
+
+    def from_markdown(markdown_from, markdown_to)
+      ::HTMLDiff::DiffBuilder.new(
+        format_text(markdown_from, disable_macro_expansion: true),
+        format_text(markdown_to, disable_macro_expansion: true)
+      ).build
+       .gsub(/<ins class="diff(?:ins|mod)">(\n\r?)<\/ins>/, '\1')
+       .gsub(/<del class="diff(?:del|mod)">(\n\r?)<\/del>/, '\1')
+       .gsub(/^<figure class="op-uc-figure">(<figure class="table op-uc-figure_align-center op-uc-figure">)/, '\1')
+    end
+  end
+end

spec/lib/open_project/html_diff_spec.rb

@@ -0,0 +1,110 @@
+# frozen_string_literal: true
+
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2024 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# 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.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+require "spec_helper"
+
+RSpec.describe OpenProject::HtmlDiff do
+  describe ".from_markdown" do
+    it "hightlights additions with <ins> tags" do
+      from = ""
+      to = "Hello, world!"
+      expect(described_class.from_markdown(from, to))
+        .to eq(<<~HTML.strip)
+          <p class="op-uc-p"><ins class="diffins">Hello, world!</ins></p>
+        HTML
+    end
+
+    it "hightlights removals with <del> tags" do
+      from = "Hello, world!"
+      to = ""
+      expect(described_class.from_markdown(from, to))
+        .to eq(<<~HTML.strip)
+          <p class="op-uc-p"><del class="diffdel">Hello, world!</del></p>
+        HTML
+    end
+
+    it "hightlights modifications with both <ins> and <del> tags" do
+      from = "Hello, world!"
+      to = "Hello, OpenProject!"
+      expect(described_class.from_markdown(from, to))
+        .to eq(<<~HTML.strip)
+          <p class="op-uc-p">Hello, <del class="diffmod">world!</del><ins class="diffmod">OpenProject!</ins></p>
+        HTML
+    end
+
+    context "with a list" do
+      it "removes extra newlines from the diff" do # rubocop:disable RSpec/ExampleLength
+        from = <<~MARKDOWN
+          Deletion:
+
+          *   Item 1
+
+          *   Item 2
+
+          Insertion:
+
+          *   Item A
+        MARKDOWN
+        to = <<~MARKDOWN
+          Deletion:
+
+          *   Item 1
+
+          Insertion:
+
+          *   Item A
+
+          *   Item B
+        MARKDOWN
+        expect(described_class.from_markdown(from, to))
+          .to eq(<<~HTML.strip)
+            <p class="op-uc-p">Deletion:</p>
+            <ul class="op-uc-list">
+            <li class="op-uc-list--item">
+            <p class="op-uc-p">Item 1</p>
+            </li>
+            <li class="op-uc-list--item">
+            <p class="op-uc-p"><del class="diffdel">Item 2</del></p>
+            </li>
+            </ul>
+            <p class="op-uc-p">Insertion:</p>
+            <ul class="op-uc-list">
+            <li class="op-uc-list--item">
+            <p class="op-uc-p">Item A</p>
+            </li>
+            <li class="op-uc-list--item">
+            <p class="op-uc-p"><ins class="diffins">Item B</ins></p>
+            </li>
+            </ul>
+          HTML
+      end
+    end
+  end
+end

spec/models/journable/with_historic_attributes_spec.rb

@@ -149,7 +149,7 @@
     end
 
     context "with active record relation of work packages" do
-      let(:work_packages) { WorkPackage.all }
+      let(:work_packages) { WorkPackage.order(subject: :asc).all }
 
       it "provides access to the work-package attributes" do
         expect(subject.map(&:subject)).to eq ["The current work package 1", "The current work package 2"]
@@ -246,7 +246,7 @@
     end
 
     context "with active record relation of work packages" do
-      let(:work_packages) { WorkPackage.all }
+      let(:work_packages) { WorkPackage.order(subject: :asc).all }
 
       it "provides access to the work-package attributes at timestamps" do
         expect(subject.first.attributes_by_timestamp["2022-01-01T00:00:00Z"].subject).to eq "The original work package 1"