From 6cb7238cf383aaf1fb0d7003b5b943e05f0c2dee Mon Sep 17 00:00:00 2001 From: Enric Tobella Date: Wed, 17 Dec 2025 09:55:37 +0100 Subject: [PATCH 1/5] [FIX] document_page_approval: Fix tests to use real html --- .../tests/test_document_page_approval.py | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/document_page_approval/tests/test_document_page_approval.py b/document_page_approval/tests/test_document_page_approval.py index e9154ade084..f245788419d 100644 --- a/document_page_approval/tests/test_document_page_approval.py +++ b/document_page_approval/tests/test_document_page_approval.py @@ -38,7 +38,7 @@ def setUp(self): { "name": "This page requires approval", "parent_id": self.category2.id, - "content": "This content will require approval", + "content": "

This content will require approval

", } ) @@ -73,22 +73,22 @@ def test_change_request_approve(self): self.assertEqual(chreq.state, "approved") self.assertEqual(chreq.content, page.content) - # new changes should create change requests - page.write({"content": "New content"}) - # Needed to compute calculated fields - page.invalidate_model() - self.assertNotEqual(page.content, "New content") + # Create new change request + page.write({"content": "

New content

"}) + page.invalidate_model() # Recompute fields chreq = self.history_obj.search( - [("page_id", "=", page.id), ("state", "!=", "approved")] - )[0] - chreq.action_approve() - self.assertEqual(page.content, "New content") + [("page_id", "=", page.id), ("state", "!=", "approved")], limit=1 + ) + + # Approve new changes + chreq.with_user(self.user2).action_approve() + self.assertEqual(page.content, "

New content

") def test_change_request_auto_approve(self): page = self.page1 self.assertFalse(page.is_approval_required) - page.write({"content": "New content"}) - self.assertEqual(page.content, "New content") + page.write({"content": "

New content

"}) + self.assertEqual(page.content, "

New content

") def test_change_request_from_scratch(self): page = self.page2 @@ -103,7 +103,7 @@ def test_change_request_from_scratch(self): { "page_id": page.id, "summary": "Changed something", - "content": "New content", + "content": "

New content

", } ) From 8cc1a353f7ecff5a687457d696048f1e20b595b6 Mon Sep 17 00:00:00 2001 From: Enric Tobella Date: Wed, 17 Dec 2025 09:55:54 +0100 Subject: [PATCH 2/5] [FIX] document_page_reference: Fix tests to use HTML --- document_page_reference/tests/test_document_reference.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/document_page_reference/tests/test_document_reference.py b/document_page_reference/tests/test_document_reference.py index 1a2ba268021..4f16744490d 100644 --- a/document_page_reference/tests/test_document_reference.py +++ b/document_page_reference/tests/test_document_reference.py @@ -51,8 +51,8 @@ def test_auto_reference(self): new_page_duplicated_name = self.page_obj.create( { "name": "test page with no reference", - "content": "this should have an empty reference " - "because reference must be unique", + "content": "

this should have an empty reference " + "because reference must be unique

", } ) self.assertFalse(new_page_duplicated_name.reference) From c08619e3ed3a11ed93e68233c4f94f51e4ae8cc0 Mon Sep 17 00:00:00 2001 From: Enric Tobella Date: Mon, 15 Dec 2025 09:59:54 +0100 Subject: [PATCH 3/5] [IMP] document_page: Use Odoo HTML Diff comparison method --- document_page/README.rst | 4 ++ document_page/__manifest__.py | 5 +- document_page/models/document_page_history.py | 29 +++-------- document_page/readme/CONTRIBUTORS.rst | 4 ++ document_page/static/description/index.html | 4 ++ .../static/src/scss/document_page.scss | 52 ++++++++++++------- .../src/scss/document_page_variables.scss | 5 ++ document_page/tests/test_document_page.py | 14 +++-- .../tests/test_document_page_history.py | 15 +++--- .../tests/test_document_page_show_diff.py | 11 ++-- document_page/views/document_page_history.xml | 1 + .../wizard/document_page_show_diff.py | 2 +- 12 files changed, 88 insertions(+), 58 deletions(-) create mode 100644 document_page/static/src/scss/document_page_variables.scss diff --git a/document_page/README.rst b/document_page/README.rst index f958b49472f..8ff1a6faa01 100644 --- a/document_page/README.rst +++ b/document_page/README.rst @@ -94,6 +94,10 @@ Trobz * Ángel García de la Chica Herrera +* `Dixmit `__: + + * Enric Tobella + Other credits ~~~~~~~~~~~~~ diff --git a/document_page/__manifest__.py b/document_page/__manifest__.py index 77c1698db6f..5d2e99ae00f 100644 --- a/document_page/__manifest__.py +++ b/document_page/__manifest__.py @@ -17,7 +17,7 @@ ], "website": "https://github.com/OCA/knowledge", "license": "AGPL-3", - "depends": ["mail", "document_knowledge"], + "depends": ["mail", "document_knowledge", "web_editor"], "data": [ "security/document_page_security.xml", "security/ir.model.access.csv", @@ -30,6 +30,9 @@ ], "demo": ["demo/document_page.xml"], "assets": { + "web._assets_primary_variables": [ + "document_page/static/src/**/document_page_variables.scss", + ], "web.assets_backend": [ "document_page/static/src/scss/document_page.scss", ], diff --git a/document_page/models/document_page_history.py b/document_page/models/document_page_history.py index 7b200e6abbb..3bfd369d64b 100644 --- a/document_page/models/document_page_history.py +++ b/document_page/models/document_page_history.py @@ -1,9 +1,12 @@ # Copyright (C) 2004-2010 Tiny SPRL (). # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). -import difflib -from odoo import _, api, fields, models +from odoo import fields, models + +from odoo.addons.web_editor.models.diff_utils import ( + generate_comparison, +) class DocumentPageHistory(models.Model): @@ -17,7 +20,7 @@ class DocumentPageHistory(models.Model): name = fields.Char(index=True) summary = fields.Char(index=True) content = fields.Html(sanitize=False) - diff = fields.Html(compute="_compute_diff") + diff = fields.Html(compute="_compute_diff", sanitize_tags=False) company_id = fields.Many2one( "res.company", @@ -43,28 +46,10 @@ def _compute_diff(self): ) rec.diff = self._get_diff(prev.id, rec.id) - @api.model def _get_diff(self, v1, v2): - """Return the difference between two version of document version.""" text1 = v1 and self.browse(v1).content or "" text2 = v2 and self.browse(v2).content or "" - # Include line breaks to make it more readable - # TODO: consider using a beautify library directly on the content - text1 = text1.replace("

", "

\r\n

") - text2 = text2.replace("

", "

\r\n

") - line1 = text1.splitlines(True) - line2 = text2.splitlines(True) - if line1 == line2: - return _("There are no changes in revisions.") - else: - diff = difflib.HtmlDiff() - return diff.make_table( - line1, - line2, - f"Revision-{v1}", - f"Revision-{v2}", - context=True, - ) + return generate_comparison(text1, text2) def name_get(self): return [(rec.id, "%s #%i" % (rec.page_id.name, rec.id)) for rec in self] diff --git a/document_page/readme/CONTRIBUTORS.rst b/document_page/readme/CONTRIBUTORS.rst index a45e3a717c2..0aa38704d31 100644 --- a/document_page/readme/CONTRIBUTORS.rst +++ b/document_page/readme/CONTRIBUTORS.rst @@ -16,3 +16,7 @@ Trobz * `Sygel `_: * Ángel García de la Chica Herrera + +* `Dixmit `__: + + * Enric Tobella diff --git a/document_page/static/description/index.html b/document_page/static/description/index.html index 634c1b38de0..8279f41202d 100644 --- a/document_page/static/description/index.html +++ b/document_page/static/description/index.html @@ -442,6 +442,10 @@

Contributors

  • Ángel García de la Chica Herrera
    • +
  • Dixmit:
      +
    • Enric Tobella
      • +
    +
    • diff --git a/document_page/static/src/scss/document_page.scss b/document_page/static/src/scss/document_page.scss index 82cfbaaccc8..064d415a9dd 100644 --- a/document_page/static/src/scss/document_page.scss +++ b/document_page/static/src/scss/document_page.scss @@ -1,28 +1,40 @@ -table.diff { - font-family: Courier; - border: medium; +.o_document_page_diff { + table.diff { + font-family: Courier; + border: medium; - .diff_header { - background-color: #e0e0e0; - } + .diff_header { + background-color: $o_document_page_diff_header_background; + } - td.diff_header { - text-align: right; - } + td.diff_header { + text-align: right; + } - .diff_next { - background-color: #c0c0c0; - } + .diff_next { + background-color: $o_document_page_diff_next_background; + } - .diff_add { - background-color: #aaffaa; - } + .diff_add { + background-color: $o_document_page_diff_add_background; + } - .diff_chg { - background-color: #ffff77; - } + .diff_chg { + background-color: $o_document_page_diff_change_background; + } - .diff_sub { - background-color: #ffaaaa; + .diff_sub { + background-color: $o_document_page_diff_subtract_background; + } + } + removed { + display: inline; + text-decoration: line-through; + opacity: 0.5; + background-color: $o_document_page_diff_subtract_background; + } + added { + display: inline; + background-color: $o_document_page_diff_add_background; } } diff --git a/document_page/static/src/scss/document_page_variables.scss b/document_page/static/src/scss/document_page_variables.scss new file mode 100644 index 00000000000..0c4d02fedeb --- /dev/null +++ b/document_page/static/src/scss/document_page_variables.scss @@ -0,0 +1,5 @@ +$o_document_page_diff_header_background: #e0e0e0; +$o_document_page_diff_next_background: #c0c0c0; +$o_document_page_diff_add_background: #aaffaa; +$o_document_page_diff_change_background: #ffff77; +$o_document_page_diff_subtract_background: #ffaaaa; diff --git a/document_page/tests/test_document_page.py b/document_page/tests/test_document_page.py index 22b03c2ae82..4ada5d71df4 100644 --- a/document_page/tests/test_document_page.py +++ b/document_page/tests/test_document_page.py @@ -31,12 +31,16 @@ def test_category_template(self): self.assertEqual(page.content, self.category1.template) def test_page_history_diff(self): - page = self.page_obj.create({"name": "Test Page 3", "content": "Test content"}) - page.content = "New content" + page = self.page_obj.create( + {"name": "Test Page 3", "content": "
    Test content
    "} + ) + page.content = "
    New content
    " self.assertIsNotNone(page.history_ids[0].diff) def test_page_link(self): - page = self.page_obj.create({"name": "Test Page 3", "content": "Test content"}) + page = self.page_obj.create( + {"name": "Test Page 3", "content": "
    Test content
    "} + ) self.assertEqual( page.backend_url, f"/web#id={page.id}&model=document.page&view_type=form", @@ -51,7 +55,9 @@ def test_page_link(self): ) def test_page_copy(self): - page = self.page_obj.create({"name": "Test Page 3", "content": "Test content"}) + page = self.page_obj.create( + {"name": "Test Page 3", "content": "
    Test content
    "} + ) page_copy = page.copy() self.assertEqual(page_copy.name, page.name + " (copy)") self.assertEqual(page_copy.content, page.content) diff --git a/document_page/tests/test_document_page_history.py b/document_page/tests/test_document_page_history.py index 856be77f783..dd09334191f 100644 --- a/document_page/tests/test_document_page_history.py +++ b/document_page/tests/test_document_page_history.py @@ -6,14 +6,15 @@ class TestDocumentPageHistory(common.TransactionCase): def test_page_history_demo_page1(self): """Test page history demo page1.""" - page = self.env.ref("document_page.demo_page1") - page.content = "Test content updated" + page = self.env["document.page"].create( + { + "name": "Test Page", + "content": "
    Initial content
    ", + } + ) + page.content = "
    Test content updated
    " history_document = self.env["document.page.history"] history_pages = history_document.search([("page_id", "=", page.id)]) active_ids = [i.id for i in history_pages] - result = history_document._get_diff(active_ids[0], active_ids[0]) - self.assertEqual(result, "There are no changes in revisions.") - - result = history_document._get_diff(active_ids[0], active_ids[1]) - self.assertNotEqual(result, "There are no changes in revisions.") + self.assertEqual(result, page.content) diff --git a/document_page/tests/test_document_page_show_diff.py b/document_page/tests/test_document_page_show_diff.py index 811c35e7cde..54f564ecc73 100644 --- a/document_page/tests/test_document_page_show_diff.py +++ b/document_page/tests/test_document_page_show_diff.py @@ -8,7 +8,12 @@ class TestDocumentPageShowDiff(common.TransactionCase): def test_show_demo_page1_diff(self): """Show test page history difference.""" - page = self.env.ref("document_page.demo_page1") + page = self.env["document.page"].create( + { + "name": "Test Page", + "content": "
    Initial content
    ", + } + ) show_diff_object = self.env["wizard.document.page.history.show_diff"] @@ -21,8 +26,8 @@ def test_show_demo_page1_diff(self): )._get_diff() ) - page.write({"content": "Text content updated"}) - page.write({"content": "Text updated"}) + page.write({"content": "
    Text content updated
    "}) + page.write({"content": "
    Text updated
    "}) history_pages = history_document.search([("page_id", "=", page.id)]) diff --git a/document_page/views/document_page_history.xml b/document_page/views/document_page_history.xml index 00107d75081..a01acfba7cf 100644 --- a/document_page/views/document_page_history.xml +++ b/document_page/views/document_page_history.xml @@ -70,6 +70,7 @@ diff --git a/document_page/wizard/document_page_show_diff.py b/document_page/wizard/document_page_show_diff.py index 1557ff7b8d3..26f025421dc 100644 --- a/document_page/wizard/document_page_show_diff.py +++ b/document_page/wizard/document_page_show_diff.py @@ -27,4 +27,4 @@ def _get_diff(self): raise UserError(_("Select one or maximum two history revisions!")) return diff - diff = fields.Html(readonly=True, default=_get_diff) + diff = fields.Html(readonly=True, default=_get_diff, sanitize_tags=False) From 9fb2829fcda0acb375e4b8bfdcfd9ee09ed64b4f Mon Sep 17 00:00:00 2001 From: Enric Tobella Date: Wed, 17 Dec 2025 18:59:18 +0100 Subject: [PATCH 4/5] [IMP] document_page: Include diff_utils from odoo --- document_page/models/diff_utils.py | 242 ++++++++++++++++++ document_page/models/document_page_history.py | 2 +- 2 files changed, 243 insertions(+), 1 deletion(-) create mode 100644 document_page/models/diff_utils.py diff --git a/document_page/models/diff_utils.py b/document_page/models/diff_utils.py new file mode 100644 index 00000000000..8cb7772a1e1 --- /dev/null +++ b/document_page/models/diff_utils.py @@ -0,0 +1,242 @@ +# This code has been ported from Odoo 18.0 web_editor module. +# License of this part should remain LGPL-3.0 or later following Odoo's licensing. +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +import re +from difflib import SequenceMatcher + +# ------------------------------------------------------------ +# Patch and comparison functions +# ------------------------------------------------------------ + + +OPERATION_SEPARATOR = "\n" +LINE_SEPARATOR = "<" + +PATCH_OPERATION_LINE_AT = "@" +PATCH_OPERATION_CONTENT = ":" + +PATCH_OPERATION_ADD = "+" +PATCH_OPERATION_REMOVE = "-" +PATCH_OPERATION_REPLACE = "R" + +PATCH_OPERATIONS = dict( + insert=PATCH_OPERATION_ADD, + delete=PATCH_OPERATION_REMOVE, + replace=PATCH_OPERATION_REPLACE, +) + +HTML_ATTRIBUTES_TO_REMOVE = [ + "data-last-history-steps", +] + + +HTML_TAG_ISOLATION_REGEX = r"^([^>]*>)(.*)$" +ADDITION_COMPARISON_REGEX = r"\1\2" +ADDITION_1ST_REPLACE_COMPARISON_REGEX = r"added>\2" +DELETION_COMPARISON_REGEX = r"\1\2" +EMPTY_OPERATION_TAG = r"<(added|removed)><\/(added|removed)>" +SAME_TAG_REPLACE_FIXER = r"<\/added><(?:[^\/>]|(?:><))+>" +UNNECESSARY_REPLACE_FIXER = ( + r"([^<](?!<\/added>)*)<\/added>" + r"([^<](?!<\/removed>)*)<\/removed>" +) + + +def generate_comparison(new_content, old_content): # noqa: C901 + """Compare a content to an older content + and generate a comparison html between both content. + + :param string new_content: the current content + :param string old_content: the old content + + :return: string: the comparison content + """ + new_content = _remove_html_attribute(new_content, HTML_ATTRIBUTES_TO_REMOVE) + old_content = _remove_html_attribute(old_content, HTML_ATTRIBUTES_TO_REMOVE) + + if new_content == old_content: + return new_content + + patch = generate_patch(new_content, old_content) + comparison = new_content.split(LINE_SEPARATOR) + patch_operations = patch.split(OPERATION_SEPARATOR) + # We need to apply operation from last to the first + # to preserve the indexes integrity. + patch_operations.reverse() + + for operation in patch_operations: + metadata, *patch_content_line = operation.split(LINE_SEPARATOR) + + metadata_split = metadata.split(PATCH_OPERATION_LINE_AT) + operation_type = metadata_split[0] + lines_index_range = metadata_split[1] if len(metadata_split) > 1 else "" + lines_index_range = lines_index_range.split(PATCH_OPERATION_CONTENT)[0] + indexes = lines_index_range.split(",") + start_index = int(indexes[0]) + end_index = int(indexes[1]) if len(indexes) > 1 else start_index + + # If the operation is a replace, we need to flag the changes that + # will generate ghost opening tags if we don't ignore + # them. + # this can append when: + # * A change concerning only html parameters. + #

    a

    =>

    a

    + # * An addition in a previously empty element opening tag + #

    =>

    a

    + if operation_type == PATCH_OPERATION_REPLACE: + for i, line in enumerate(patch_content_line): + current_index = start_index + i + if current_index > end_index: + break + + current_line = comparison[current_index] + current_line_tag = current_line.split(">")[0] + line_tag = line.split(">")[0] + if current_line[-1] == ">" and ( + current_line_tag == line_tag + or current_line_tag.split(" ")[0] == line_tag.split(" ")[0] + ): + comparison[start_index + i] = "delete_me>" + + # We need to insert lines from last to the first + # to preserve the indexes integrity. + patch_content_line.reverse() + + for index in range(end_index, start_index - 1, -1): + if operation_type in [ + PATCH_OPERATION_REMOVE, + PATCH_OPERATION_REPLACE, + ]: + deletion_flagged_comparison = re.sub( + HTML_TAG_ISOLATION_REGEX, + DELETION_COMPARISON_REGEX, + comparison[index], + ) + # Only use this line if it doesn't generate an empty + # tag + if not re.search(EMPTY_OPERATION_TAG, deletion_flagged_comparison): + comparison[index] = deletion_flagged_comparison + + if operation_type == PATCH_OPERATION_ADD: + for line in patch_content_line: + addition_flagged_line = re.sub( + HTML_TAG_ISOLATION_REGEX, ADDITION_COMPARISON_REGEX, line + ) + + if not re.search(EMPTY_OPERATION_TAG, addition_flagged_line): + comparison.insert(start_index + 1, addition_flagged_line) + else: + comparison.insert(start_index + 1, line) + + if operation_type == PATCH_OPERATION_REPLACE: + for _i, line in enumerate(patch_content_line): + addition_flagged_line = re.sub( + HTML_TAG_ISOLATION_REGEX, ADDITION_COMPARISON_REGEX, line + ) + if not re.search(EMPTY_OPERATION_TAG, addition_flagged_line): + comparison.insert(start_index, addition_flagged_line) + elif line.split(">")[0] != comparison[start_index].split(">")[0]: + comparison.insert(start_index, line) + + final_comparison = LINE_SEPARATOR.join(comparison) + # We can remove all the opening tags which are located between the end of an + # added tag and the start of a removed tag, because this should never happen + # as the added and removed tags should always be near each other. + # This can happen when the new container tag had a parameter change. + final_comparison = re.sub( + SAME_TAG_REPLACE_FIXER, "    ", final_comparison + ) + + # Remove al the tags + final_comparison = final_comparison.replace(r"", "") + + # This fix the issue of unnecessary replace tags. + # ex: abcabc -> abc + # This can occur when the new content is the same as the old content and + # their container tags are the same but the tags parameters are different + for match in re.finditer(UNNECESSARY_REPLACE_FIXER, final_comparison): + if match.group(1) == match.group(2): + final_comparison = final_comparison.replace(match.group(0), match.group(1)) + + return final_comparison + + +def _format_line_index(start, end): + """Format the line index to be used in a patch operation. + + :param start: the start index + :param end: the end index + :return: string + """ + length = end - start + if not length: + start -= 1 + if length <= 1: + return f"{PATCH_OPERATION_LINE_AT}{start}" + return f"{PATCH_OPERATION_LINE_AT}{start},{start + length - 1}" + + +def _patch_generator(new_content, old_content): + """Generate a patch (multiple operations) between two contents. + Each operation is a string with the following format: + @[,][:*] + patch format example: + +@4:

    ab

    cd

    + +@4,15:

    ef

    gh

    + -@32 + -@125,129 + R@523:sdf + + :param string new_content: the new content + :param string old_content: the old content + + :return: string: the patch containing all the operations to reverse + the new content to the old content + """ + # remove break line in contents to ensure they don't interfere with + # operations + new_content = new_content.replace("\n", "") + old_content = old_content.replace("\n", "") + + new_content_lines = new_content.split(LINE_SEPARATOR) + old_content_lines = old_content.split(LINE_SEPARATOR) + + for group in SequenceMatcher( + None, new_content_lines, old_content_lines, False + ).get_grouped_opcodes(0): + patch_content_line = [] + first, last = group[0], group[-1] + patch_operation = _format_line_index(first[1], last[2]) + + if any(tag in {"replace", "delete"} for tag, _, _, _, _ in group): + for tag, _, _, _, _ in group: + if tag not in {"insert", "equal", "replace"}: + patch_operation = PATCH_OPERATIONS[tag] + patch_operation + + if any(tag in {"replace", "insert"} for tag, _, _, _, _ in group): + for tag, _, _, j1, j2 in group: + if tag not in {"delete", "equal"}: + patch_operation = PATCH_OPERATIONS[tag] + patch_operation + for line in old_content_lines[j1:j2]: + patch_content_line.append(line) + + if patch_content_line: + patch_content = LINE_SEPARATOR + LINE_SEPARATOR.join(patch_content_line) + yield str(patch_operation) + PATCH_OPERATION_CONTENT + patch_content + else: + yield str(patch_operation) + + +def generate_patch(new_content, old_content): + new_content = _remove_html_attribute(new_content, HTML_ATTRIBUTES_TO_REMOVE) + old_content = _remove_html_attribute(old_content, HTML_ATTRIBUTES_TO_REMOVE) + + return OPERATION_SEPARATOR.join(list(_patch_generator(new_content, old_content))) + + +def _remove_html_attribute(html_content, attributes_to_remove): + for attribute in attributes_to_remove: + html_content = re.sub(rf' {attribute}="[^"]*"', "", html_content) + + return html_content diff --git a/document_page/models/document_page_history.py b/document_page/models/document_page_history.py index 3bfd369d64b..45d8b077a2a 100644 --- a/document_page/models/document_page_history.py +++ b/document_page/models/document_page_history.py @@ -4,7 +4,7 @@ from odoo import fields, models -from odoo.addons.web_editor.models.diff_utils import ( +from .diff_utils import ( generate_comparison, ) From a3fcb764be33f5c9bb5cf10947187d12736458bb Mon Sep 17 00:00:00 2001 From: Enric Tobella Date: Fri, 19 Dec 2025 13:05:33 +0100 Subject: [PATCH 5/5] [FIX] document_page_approval: Add email on test user --- document_page_approval/tests/test_document_page_approval.py | 1 + 1 file changed, 1 insertion(+) diff --git a/document_page_approval/tests/test_document_page_approval.py b/document_page_approval/tests/test_document_page_approval.py index f245788419d..30b51609601 100644 --- a/document_page_approval/tests/test_document_page_approval.py +++ b/document_page_approval/tests/test_document_page_approval.py @@ -25,6 +25,7 @@ def setUp(self): ], } ) + self.user2.partner_id.write({"email": "test@test.com"}) # demo_approval self.category2 = self.page_obj.create( {