From 112c492f3ecdb45b0b0123cfe6bd844d9b56a28f Mon Sep 17 00:00:00 2001 From: Raffael Meyer <14891507+barredterra@users.noreply.github.com> Date: Thu, 6 Oct 2022 16:05:34 +0530 Subject: [PATCH] feat: consistent, translatable timeline messages (#17526) * feat: consistent, translatable timeline messages * fix: save only filename for attachment comments The rest of the comment will be added and translated ad-hoc by form_timeline.js * patch: clean up Comments of type "Attachment" * feat: further message types * style: format with prettier * test: cypress timeline Timeline now correctly displays "You ..." instead of "{User} ..." * feat: german transations for timeline comments * fix: enable auto_commit_on_many_writes * fix: don't update modified timestamp in patch --- 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, 231 insertions(+), 117 deletions(-) create mode 100644 frappe/patches/v14_0/update_attachment_comment.py diff --git a/cypress/integration/timeline.js b/cypress/integration/timeline.js index 7835819334..c6076088fb 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", "Frappe submitted this document"); + cy.get(".timeline-content").should("contain", "You 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", "Frappe cancelled this document"); + cy.get(".timeline-content").should("contain", "You cancelled this document"); //Deleting the document cy.visit("/app/custom-submittable-doctype"); diff --git a/frappe/patches.txt b/frappe/patches.txt index 24b07012da..278a351093 100644 --- a/frappe/patches.txt +++ b/frappe/patches.txt @@ -214,3 +214,4 @@ 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 new file mode 100644 index 0000000000..042579d86d --- /dev/null +++ b/frappe/patches/v14_0/update_attachment_comment.py @@ -0,0 +1,33 @@ +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 fe4b096166..d70da5f030 100644 --- a/frappe/public/js/frappe/form/footer/form_timeline.js +++ b/frappe/public/js/frappe/form/footer/form_timeline.js @@ -1,7 +1,11 @@ // Copyright (c) 2020, Frappe Technologies Pvt. Ltd. and Contributors // MIT License. See license.txt import BaseTimeline from "./base_timeline"; -import { get_version_timeline_content } from "./version_timeline_content_builder"; +import { + get_version_timeline_content, + get_user_link, + get_user_message, +} from "./version_timeline_content_builder"; class FormTimeline extends BaseTimeline { make() { @@ -106,54 +110,50 @@ class FormTimeline extends BaseTimeline { render_timeline_items() { super.render_timeline_items(); - this.set_document_info(); + this.add_web_page_view_count(); frappe.utils.bind_actions_with_object(this.timeline_items_wrapper, this); } - 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" - ); - + add_web_page_view_count() { if (this.frm.doc.route && cint(frappe.boot.website_tracking_enabled)) { - 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}`, + 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, - }, - 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,29 +172,24 @@ 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); - 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" - ); + 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") + ); view_timeline_contents.push({ creation: view.creation, - content: view_message, + content: timeline_content, hide_timestamp: true, }); }); + return view_timeline_contents; } @@ -337,7 +332,7 @@ class FormTimeline extends BaseTimeline { (this.doc_info.info_logs || []).forEach((info_log) => { info_timeline_contents.push({ creation: info_log.creation, - content: `${this.get_user_link(info_log.owner)} ${info_log.content}`, + content: `${get_user_link(info_log.owner)} ${info_log.content}`, }); }); return info_timeline_contents; @@ -345,45 +340,76 @@ class FormTimeline extends BaseTimeline { get_attachment_timeline_contents() { let attachment_timeline_contents = []; + (this.doc_info.attachment_logs || []).forEach((attachment_log) => { - let is_file_upload = attachment_log.comment_type == "Attachment"; + 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") + ); + attachment_timeline_contents.push({ icon: is_file_upload ? "upload" : "delete", icon_size: "sm", creation: attachment_log.creation, - content: `${this.get_user_link(attachment_log.owner)} ${attachment_log.content}`, + content: timeline_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: __("{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(), - ]), + content: timeline_content, }); }); + 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: __("{0} Liked", [this.get_user_link(like_log.owner)]), + content: timeline_content, title: "Like", }); }); + return like_timeline_contents; } @@ -394,7 +420,7 @@ class FormTimeline extends BaseTimeline { icon: "branch", icon_size: "sm", creation: workflow_log.creation, - content: `${this.get_user_link(workflow_log.owner)} ${__(workflow_log.content)}`, + content: `${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 33c87e458a..1912b5928e 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,19 +30,55 @@ function get_version_timeline_content(version_doc, frm) { if (p[0] === "docstatus") { if (p[2] === 1) { let message = updater_reference_link - ? __("{0} submitted this document {1}", [ - get_user_link(version_doc), - updater_reference_link, - ]) - : __("{0} submitted this document", [get_user_link(version_doc)]); + ? 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" + ) + ); + out.push(get_version_comment(version_doc, message)); } else if (p[2] === 2) { let message = updater_reference_link - ? __("{0} cancelled this document {1}", [ - get_user_link(version_doc), - updater_reference_link, - ]) - : __("{0} cancelled this document", [get_user_link(version_doc)]); + ? 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" + ) + ); + out.push(get_version_comment(version_doc, message)); } } else { @@ -67,19 +103,28 @@ function get_version_timeline_content(version_doc, frm) { return parts.length < 3; }); if (parts.length) { - 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(", "), - ]); - } + 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(", "), + ]) + ); + out.push(get_version_comment(version_doc, message)); } } @@ -120,19 +165,28 @@ function get_version_timeline_content(version_doc, frm) { return parts.length < 3; }); if (parts.length) { - 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(", "), - ]); - } + 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(", "), + ]) + ); + out.push(get_version_comment(version_doc, message)); } } @@ -168,7 +222,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); + let user_link = get_user_link(version_doc.owner); out.push(`${user_link} ${version_comment}`); } } @@ -230,10 +284,13 @@ function format_content_for_timeline(content) { return content.bold(); } -function get_user_link(doc) { - const user = doc.owner; +function 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); } -export { get_version_timeline_content }; +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 }; diff --git a/frappe/translations/de.csv b/frappe/translations/de.csv index 2df384bc43..3d481e740e 100644 --- a/frappe/translations/de.csv +++ b/frappe/translations/de.csv @@ -4727,8 +4727,12 @@ 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 875ca10c63..ae53f8d9f7 100644 --- a/frappe/utils/file_manager.py +++ b/frappe/utils/file_manager.py @@ -54,19 +54,12 @@ 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", - _("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, - } - ) - ), + f"{file_name}{icon}", ) return { @@ -295,7 +288,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", _("Removed {0}").format(file_name)) + comment = doc.add_comment("Attachment Removed", file_name) frappe.delete_doc( "File", fid, ignore_permissions=ignore_permissions, delete_permanently=delete_permanently )