From ab3f706948fc4052b86c9044d22c392d7708839b Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 12 Oct 2022 18:03:57 +0530 Subject: [PATCH] Revert "feat: consistent, translatable timeline messages (#17526)" This reverts commit 112c492f3ecdb45b0b0123cfe6bd844d9b56a28f. --- cypress/integration/timeline.js | 4 +- frappe/patches.txt | 1 - .../v14_0/update_attachment_comment.py | 33 ---- .../js/frappe/form/footer/form_timeline.js | 152 ++++++++---------- .../version_timeline_content_builder.js | 137 +++++----------- frappe/translations/de.csv | 4 - frappe/utils/file_manager.py | 17 +- 7 files changed, 117 insertions(+), 231 deletions(-) delete mode 100644 frappe/patches/v14_0/update_attachment_comment.py diff --git a/cypress/integration/timeline.js b/cypress/integration/timeline.js index c6076088fb..7835819334 100644 --- a/cypress/integration/timeline.js +++ b/cypress/integration/timeline.js @@ -72,14 +72,14 @@ context("Timeline", () => { cy.click_listview_row_item(0); //To check if the submission of the documemt is visible in the timeline content - cy.get(".timeline-content").should("contain", "You submitted this document"); + cy.get(".timeline-content").should("contain", "Frappe submitted this document"); cy.get('[id="page-Custom Submittable DocType"] .page-actions') .findByRole("button", { name: "Cancel" }) .click(); cy.get_open_dialog().findByRole("button", { name: "Yes" }).click(); //To check if the cancellation of the documemt is visible in the timeline content - cy.get(".timeline-content").should("contain", "You cancelled this document"); + cy.get(".timeline-content").should("contain", "Frappe cancelled this document"); //Deleting the document cy.visit("/app/custom-submittable-doctype"); diff --git a/frappe/patches.txt b/frappe/patches.txt index 278a351093..24b07012da 100644 --- a/frappe/patches.txt +++ b/frappe/patches.txt @@ -214,4 +214,3 @@ execute:frappe.delete_doc('Page', 'background_jobs', ignore_missing=True, force= frappe.patches.v14_0.drop_unused_indexes frappe.patches.v15_0.drop_modified_index frappe.patches.v14_0.add_manage_subscriptions_in_navbar_settings -frappe.patches.v14_0.update_attachment_comment diff --git a/frappe/patches/v14_0/update_attachment_comment.py b/frappe/patches/v14_0/update_attachment_comment.py deleted file mode 100644 index 042579d86d..0000000000 --- a/frappe/patches/v14_0/update_attachment_comment.py +++ /dev/null @@ -1,33 +0,0 @@ -import frappe - - -def execute(): - frappe.db.auto_commit_on_many_writes = 1 - - # Strip everything except link to attachment and icon from comments of type "Attached" - for name, content in frappe.get_all( - "Comment", filters={"comment_type": "Attachment"}, fields=["name", "content"], as_list=True - ): - if not content: - continue - - start = content.find("") - end = content.find("") if end == -1 else end - if end != -1: - content = content[: end + 4] - - frappe.db.set_value("Comment", name, "content", content, update_modified=False) - - # Strip "Removed " from comments of type "Attachment Removed" - for name, content in frappe.get_all( - "Comment", - filters={"comment_type": "Attachment Removed"}, - fields=["name", "content"], - as_list=True, - ): - if content and content.startswith("Removed "): - frappe.db.set_value("Comment", name, "content", content[8:], update_modified=False) diff --git a/frappe/public/js/frappe/form/footer/form_timeline.js b/frappe/public/js/frappe/form/footer/form_timeline.js index d70da5f030..fe4b096166 100644 --- a/frappe/public/js/frappe/form/footer/form_timeline.js +++ b/frappe/public/js/frappe/form/footer/form_timeline.js @@ -1,11 +1,7 @@ // Copyright (c) 2020, Frappe Technologies Pvt. Ltd. and Contributors // MIT License. See license.txt import BaseTimeline from "./base_timeline"; -import { - get_version_timeline_content, - get_user_link, - get_user_message, -} from "./version_timeline_content_builder"; +import { get_version_timeline_content } from "./version_timeline_content_builder"; class FormTimeline extends BaseTimeline { make() { @@ -110,50 +106,54 @@ class FormTimeline extends BaseTimeline { render_timeline_items() { super.render_timeline_items(); - this.add_web_page_view_count(); + this.set_document_info(); frappe.utils.bind_actions_with_object(this.timeline_items_wrapper, this); } - add_web_page_view_count() { + set_document_info() { + // TODO: handle creation via automation + const creation = comment_when(this.frm.doc.creation); + let creation_message = frappe.utils.is_current_user(this.frm.doc.owner) + ? __("You created this {0}", [creation], "Form timeline") + : __( + "{0} created this {1}", + [this.get_user_link(this.frm.doc.owner), creation], + "Form timeline" + ); + + const modified = comment_when(this.frm.doc.modified); + let modified_message = frappe.utils.is_current_user(this.frm.doc.modified_by) + ? __("You edited this {0}", [modified], "Form timeline") + : __( + "{0} edited this {1}", + [this.get_user_link(this.frm.doc.modified_by), modified], + "Form timeline" + ); + if (this.frm.doc.route && cint(frappe.boot.website_tracking_enabled)) { - frappe.utils.get_page_view_count(this.frm.doc.route).then((res) => { - this.add_timeline_item({ - content: __("{0} Web page views", [res.message], "Form timeline"), - hide_timestamp: true, - }); + let route = this.frm.doc.route; + frappe.utils.get_page_view_count(route).then((res) => { + let page_view_count_message = __("{0} Page views", [res.message], "Form timeline"); + this.add_timeline_item( + { + content: `${creation_message} • ${modified_message} • ${page_view_count_message}`, + hide_timestamp: true, + }, + true + ); }); + } else { + this.add_timeline_item( + { + content: `${creation_message} • ${modified_message}`, + hide_timestamp: true, + }, + true + ); } } - get_creation_message() { - const user_link = get_user_link(this.frm.doc.owner); - - return { - creation: this.frm.doc.creation, - content: get_user_message( - this.frm.doc.owner, - __("You created this", null, "Form timeline"), - __("{0} created this", [user_link], "Form timeline") - ), - }; - } - - get_modified_message() { - const user_link = get_user_link(this.frm.doc.modified_by); - - return { - creation: this.frm.doc.modified, - content: get_user_message( - this.frm.doc.modified_by, - __("You last edited this", null, "Form timeline"), - __("{0} last edited this", [user_link], "Form timeline") - ), - }; - } - prepare_timeline_contents() { - this.timeline_items.push(this.get_creation_message()); - this.timeline_items.push(this.get_modified_message()); this.timeline_items.push(...this.get_communication_timeline_contents()); this.timeline_items.push(...this.get_auto_messages_timeline_contents()); this.timeline_items.push(...this.get_comment_timeline_contents()); @@ -172,24 +172,29 @@ class FormTimeline extends BaseTimeline { } } + get_user_link(user) { + const user_display_text = (frappe.user_info(user).fullname || "").bold(); + return frappe.utils.get_form_link("User", user, true, user_display_text); + } + get_view_timeline_contents() { let view_timeline_contents = []; (this.doc_info.views || []).forEach((view) => { const view_time = comment_when(view.creation); - const user_link = get_user_link(view.owner); - const timeline_content = get_user_message( - view.owner, - __("You viewed this {0}", [view_time], "Form timeline"), - __("{0} viewed this {1}", [user_link, view_time], "Form timeline") - ); + let view_message = frappe.utils.is_current_user(view.owner) + ? __("You viewed this {0}", [view_time], "Form timeline") + : __( + "{0} viewed this {1}", + [this.get_user_link(view.owner), view_time], + "Form timeline" + ); view_timeline_contents.push({ creation: view.creation, - content: timeline_content, + content: view_message, hide_timestamp: true, }); }); - return view_timeline_contents; } @@ -332,7 +337,7 @@ class FormTimeline extends BaseTimeline { (this.doc_info.info_logs || []).forEach((info_log) => { info_timeline_contents.push({ creation: info_log.creation, - content: `${get_user_link(info_log.owner)} ${info_log.content}`, + content: `${this.get_user_link(info_log.owner)} ${info_log.content}`, }); }); return info_timeline_contents; @@ -340,76 +345,45 @@ class FormTimeline extends BaseTimeline { get_attachment_timeline_contents() { let attachment_timeline_contents = []; - (this.doc_info.attachment_logs || []).forEach((attachment_log) => { - const is_file_upload = attachment_log.comment_type == "Attachment"; - const user_link = get_user_link(attachment_log.owner); - const filename = attachment_log.content; - const timeline_content = is_file_upload - ? get_user_message( - attachment_log.owner, - __("You attached {0}", [filename], "Form timeline"), - __("{0} attached {1}", [user_link, filename], "Form timeline") - ) - : get_user_message( - attachment_log.owner, - __("You removed attachment {0}", [filename], "Form timeline"), - __("{0} removed attachment {1}", [user_link, filename], "Form timeline") - ); - + let is_file_upload = attachment_log.comment_type == "Attachment"; attachment_timeline_contents.push({ icon: is_file_upload ? "upload" : "delete", icon_size: "sm", creation: attachment_log.creation, - content: timeline_content, + content: `${this.get_user_link(attachment_log.owner)} ${attachment_log.content}`, }); }); - return attachment_timeline_contents; } get_milestone_timeline_contents() { let milestone_timeline_contents = []; - (this.doc_info.milestones || []).forEach((milestone_log) => { - const field = frappe.meta.get_label(this.frm.doctype, milestone_log.track_field); - const value = milestone_log.value.bold(); - const user_link = get_user_link(milestone_log.owner); - const timeline_content = get_user_message( - milestone_log.owner, - __("You changed {0} to {1}", [field, value], "Form timeline"), - __("{0} changed {1} to {2}", [user_link, field, value], "Form timeline") - ); - milestone_timeline_contents.push({ icon: "milestone", creation: milestone_log.creation, - content: timeline_content, + content: __("{0} changed {1} to {2}", [ + this.get_user_link(milestone_log.owner), + frappe.meta.get_label(this.frm.doctype, milestone_log.track_field), + milestone_log.value.bold(), + ]), }); }); - return milestone_timeline_contents; } get_like_timeline_contents() { let like_timeline_contents = []; - (this.doc_info.like_logs || []).forEach((like_log) => { - const timeline_content = get_user_message( - like_log.owner, - __("You Liked", null, "Form timeline"), - __("{0} Liked", [get_user_link(like_log.owner)], "Form timeline") - ); - like_timeline_contents.push({ icon: "heart", icon_size: "sm", creation: like_log.creation, - content: timeline_content, + content: __("{0} Liked", [this.get_user_link(like_log.owner)]), title: "Like", }); }); - return like_timeline_contents; } @@ -420,7 +394,7 @@ class FormTimeline extends BaseTimeline { icon: "branch", icon_size: "sm", creation: workflow_log.creation, - content: `${get_user_link(workflow_log.owner)} ${__(workflow_log.content)}`, + content: `${this.get_user_link(workflow_log.owner)} ${__(workflow_log.content)}`, title: "Workflow", }); }); diff --git a/frappe/public/js/frappe/form/footer/version_timeline_content_builder.js b/frappe/public/js/frappe/form/footer/version_timeline_content_builder.js index 1912b5928e..33c87e458a 100644 --- a/frappe/public/js/frappe/form/footer/version_timeline_content_builder.js +++ b/frappe/public/js/frappe/form/footer/version_timeline_content_builder.js @@ -30,55 +30,19 @@ function get_version_timeline_content(version_doc, frm) { if (p[0] === "docstatus") { if (p[2] === 1) { let message = updater_reference_link - ? get_user_message( - version_doc.owner, - __( - "You submitted this document {0}", - [updater_reference_link], - "Form timeline" - ), - __( - "{0} submitted this document {1}", - [get_user_link(version_doc.owner), updater_reference_link], - "Form timeline" - ) - ) - : get_user_message( - version_doc.owner, - __("You submitted this document", null, "Form timeline"), - __( - "{0} submitted this document", - [get_user_link(version_doc.owner)], - "Form timeline" - ) - ); - + ? __("{0} submitted this document {1}", [ + get_user_link(version_doc), + updater_reference_link, + ]) + : __("{0} submitted this document", [get_user_link(version_doc)]); out.push(get_version_comment(version_doc, message)); } else if (p[2] === 2) { let message = updater_reference_link - ? get_user_message( - version_doc.owner, - __( - "You cancelled this document {1}", - [updater_reference_link], - "Form timeline" - ), - __( - "{0} cancelled this document {1}", - [get_user_link(version_doc.owner), updater_reference_link], - "Form timeline" - ) - ) - : get_user_message( - version_doc.owner, - __("You cancelled this document", null, "Form timeline"), - __( - "{0} cancelled this document", - [get_user_link(version_doc.owner)], - "Form timeline" - ) - ); - + ? __("{0} cancelled this document {1}", [ + get_user_link(version_doc), + updater_reference_link, + ]) + : __("{0} cancelled this document", [get_user_link(version_doc)]); out.push(get_version_comment(version_doc, message)); } } else { @@ -103,28 +67,19 @@ function get_version_timeline_content(version_doc, frm) { return parts.length < 3; }); if (parts.length) { - let message = updater_reference_link - ? get_user_message( - version_doc.owner, - __("You changed the value of {0} {1}", [ - parts.join(", "), - updater_reference_link, - ]), - __("{0} changed the value of {1} {2}", [ - get_user_link(version_doc.owner), - parts.join(", "), - updater_reference_link, - ]) - ) - : get_user_message( - version_doc.owner, - __("You changed the value of {0}", [parts.join(", ")]), - __("{0} changed the value of {1}", [ - get_user_link(version_doc.owner), - parts.join(", "), - ]) - ); - + let message; + if (updater_reference_link) { + message = __("{0} changed value of {1} {2}", [ + get_user_link(version_doc), + parts.join(", "), + updater_reference_link, + ]); + } else { + message = __("{0} changed value of {1}", [ + get_user_link(version_doc), + parts.join(", "), + ]); + } out.push(get_version_comment(version_doc, message)); } } @@ -165,28 +120,19 @@ function get_version_timeline_content(version_doc, frm) { return parts.length < 3; }); if (parts.length) { - let message = updater_reference_link - ? get_user_message( - version_doc.owner, - __("You changed the values for {0} {1}", [ - parts.join(", "), - updater_reference_link, - ]), - __("{0} changed the values for {1} {2}", [ - get_user_link(version_doc.owner), - parts.join(", "), - updater_reference_link, - ]) - ) - : get_user_message( - version_doc.owner, - __("You changed the values for {0}", [parts.join(", ")]), - __("{0} changed the values for {1}", [ - get_user_link(version_doc.owner), - parts.join(", "), - ]) - ); - + let message; + if (updater_reference_link) { + message = __("{0} changed values for {1} {2}", [ + get_user_link(version_doc), + parts.join(", "), + updater_reference_link, + ]); + } else { + message = __("{0} changed values for {1}", [ + get_user_link(version_doc), + parts.join(", "), + ]); + } out.push(get_version_comment(version_doc, message)); } } @@ -222,7 +168,7 @@ function get_version_timeline_content(version_doc, frm) { } let version_comment = get_version_comment(version_doc, message); - let user_link = get_user_link(version_doc.owner); + let user_link = get_user_link(version_doc); out.push(`${user_link} ${version_comment}`); } } @@ -284,13 +230,10 @@ function format_content_for_timeline(content) { return content.bold(); } -function get_user_link(user) { +function get_user_link(doc) { + const user = doc.owner; const user_display_text = (frappe.user_info(user).fullname || "").bold(); return frappe.utils.get_form_link("User", user, true, user_display_text); } -function get_user_message(user, message_self, message_other) { - return frappe.utils.is_current_user(user) ? message_self : message_other; -} - -export { get_version_timeline_content, get_user_link, get_user_message }; +export { get_version_timeline_content }; diff --git a/frappe/translations/de.csv b/frappe/translations/de.csv index 3d481e740e..2df384bc43 100644 --- a/frappe/translations/de.csv +++ b/frappe/translations/de.csv @@ -4727,12 +4727,8 @@ Copy,Kopieren, Hide Saved,Gespeicherte ausblenden, Show Saved,Gespeicherte anzeigen, {0} created this {1},{0} erstellte dies {1}, -{0} created this,{0} erstellte dies, -You created this,Sie erstellten dies, {0} edited this {1},{0} bearbeitete dies {1}, -You edited this {0},Sie bearbeiteten dies {0}, {0} viewed this {1},{0} sah sich dies {1} an, -You viewed this {0},Sie sahen sich dies {0} an, Toggle Full Width,Toggle Volle Breite, Documentation,Dokumentation, About,Über, diff --git a/frappe/utils/file_manager.py b/frappe/utils/file_manager.py index ae53f8d9f7..875ca10c63 100644 --- a/frappe/utils/file_manager.py +++ b/frappe/utils/file_manager.py @@ -54,12 +54,19 @@ def upload(): comment = {} if dt and dn: - file_url = file_doc.file_url.replace("#", "%23") if file_doc.file_name else file_doc.file_url - icon = ' ' if file_doc.is_private else "" - file_name = file_doc.file_name or file_doc.file_url comment = frappe.get_doc(dt, dn).add_comment( "Attachment", - f"{file_name}{icon}", + _("added {0}").format( + "{file_name}{icon}".format( + **{ + "icon": ' ' if file_doc.is_private else "", + "file_url": file_doc.file_url.replace("#", "%23") + if file_doc.file_name + else file_doc.file_url, + "file_name": file_doc.file_name or file_doc.file_url, + } + ) + ), ) return { @@ -288,7 +295,7 @@ def remove_file( ignore_permissions = True if not file_name: file_name = frappe.db.get_value("File", fid, "file_name") - comment = doc.add_comment("Attachment Removed", file_name) + comment = doc.add_comment("Attachment Removed", _("Removed {0}").format(file_name)) frappe.delete_doc( "File", fid, ignore_permissions=ignore_permissions, delete_permanently=delete_permanently )