From 4e8bbd6c93daf3bc9458382050861e2249cbaf6b Mon Sep 17 00:00:00 2001 From: Aradhya Date: Fri, 2 Dec 2022 12:47:17 +0530 Subject: [PATCH 01/13] refactor: allowing unlocking of doc when job id is not set --- .../doctype/submission_queue/submission_queue.js | 2 +- .../doctype/submission_queue/submission_queue.py | 14 +++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/frappe/core/doctype/submission_queue/submission_queue.js b/frappe/core/doctype/submission_queue/submission_queue.js index 93d6b981dc..414c8c9ee0 100644 --- a/frappe/core/doctype/submission_queue/submission_queue.js +++ b/frappe/core/doctype/submission_queue/submission_queue.js @@ -3,7 +3,7 @@ frappe.ui.form.on("Submission Queue", { refresh: function (frm) { - if (frm.doc.status === "Queued" && frm.doc.job_id) { + if (frm.doc.status === "Queued") { frm.add_custom_button(__("Unlock Reference Document"), () => { frappe.confirm(__("Are you sure you want to go ahead with this action?"), () => { frm.call("unlock_doc"); diff --git a/frappe/core/doctype/submission_queue/submission_queue.py b/frappe/core/doctype/submission_queue/submission_queue.py index 2bb4200a87..caa0352c97 100644 --- a/frappe/core/doctype/submission_queue/submission_queue.py +++ b/frappe/core/doctype/submission_queue/submission_queue.py @@ -69,6 +69,10 @@ class SubmissionQueue(Document): def background_submission(self, to_be_queued_doc: Document, action_for_queuing: str): # Set the job id for that submission doctype + submission_doc = frappe.get_doc(self.doctype, self.name) + if submission_doc.state == "Failed": + # If the document has already been unlocked by _unlock_reference_doc_unlock_reference_doc + return self.update_job_id(get_current_job().id) _action = action_for_queuing.lower() if _action == "update": @@ -129,9 +133,10 @@ class SubmissionQueue(Document): enqueue_create_notification([notify_to], notification_doc) def _unlock_reference_doc(self): - """ - Only execute if self.job_id is defined. - """ + if not self.job_id: + self.queued_doc.unlock() + frappe.db.set_value(self.doctype, self.name, {"status": "Failed"}) + try: job = Job.fetch(self.job_id, connection=get_redis_conn()) status = job.get_status(refresh=True) @@ -156,8 +161,7 @@ class SubmissionQueue(Document): # NOTE: this can lead to some weird unlocking/locking behaviours. # for example: hitting unlock on a submission could lead to unlocking of another submission # of the same reference document. - - if self.status != "Queued" and not self.job_id: + if self.status != "Queued": return self._unlock_reference_doc() From 1b46b0e34768540de855457e9f42331170c755c4 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Fri, 2 Dec 2022 13:54:25 +0530 Subject: [PATCH 02/13] fix: fixed status fetch and refactored message --- .../submission_queue/submission_queue.py | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/frappe/core/doctype/submission_queue/submission_queue.py b/frappe/core/doctype/submission_queue/submission_queue.py index caa0352c97..3e30ef1ef0 100644 --- a/frappe/core/doctype/submission_queue/submission_queue.py +++ b/frappe/core/doctype/submission_queue/submission_queue.py @@ -68,11 +68,10 @@ class SubmissionQueue(Document): ) def background_submission(self, to_be_queued_doc: Document, action_for_queuing: str): - # Set the job id for that submission doctype - submission_doc = frappe.get_doc(self.doctype, self.name) - if submission_doc.state == "Failed": - # If the document has already been unlocked by _unlock_reference_doc_unlock_reference_doc + if self.status == "Failed": + # If the document has already been unlocked by _unlock_reference_doc return + # Set the job id for that submission doctyp self.update_job_id(get_current_job().id) _action = action_for_queuing.lower() if _action == "update": @@ -97,17 +96,20 @@ class SubmissionQueue(Document): self.notify(values["status"], action_for_queuing) def notify(self, submission_status: str, action: str): + message = _("Action {0} run on {1} {2} ") if submission_status == "Failed": doctype = self.doctype docname = self.name - message = _("Submission of {0} {1} with action {2} failed") + message += "failed" else: doctype = self.ref_doctype docname = self.ref_docname - message = _("Submission of {0} {1} with action {2} completed successfully") + message += "finished" message = message.format( - frappe.bold(str(self.ref_doctype)), frappe.bold(self.ref_docname), frappe.bold(action) + frappe.bold(action), + frappe.bold(str(self.ref_doctype)), + frappe.bold(self.ref_docname), ) time_diff = time_diff_in_seconds(now(), self.created_at) if cint(time_diff) <= 60: @@ -133,10 +135,6 @@ class SubmissionQueue(Document): enqueue_create_notification([notify_to], notification_doc) def _unlock_reference_doc(self): - if not self.job_id: - self.queued_doc.unlock() - frappe.db.set_value(self.doctype, self.name, {"status": "Failed"}) - try: job = Job.fetch(self.job_id, connection=get_redis_conn()) status = job.get_status(refresh=True) @@ -169,7 +167,7 @@ class SubmissionQueue(Document): def queue_submission(doc: Document, action: str, alert: bool = True): queue = frappe.new_doc("Submission Queue") - queue.state = "Queued" + queue.status = "Queued" queue.ref_doctype = doc.doctype queue.ref_docname = doc.name queue.insert(doc, action) From e00d89f430206d338f770c464cd30ff0ab5f4d52 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Fri, 2 Dec 2022 15:06:58 +0530 Subject: [PATCH 03/13] feat: Added queue_submission to workflows --- frappe/model/workflow.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/frappe/model/workflow.py b/frappe/model/workflow.py index 8338157996..e7835fba8d 100644 --- a/frappe/model/workflow.py +++ b/frappe/model/workflow.py @@ -101,6 +101,9 @@ def is_transition_condition_satisfied(transition, doc) -> bool: @frappe.whitelist() def apply_workflow(doc, action): """Allow workflow action on the current doc""" + from frappe.core.doctype.submission_queue.submission_queue import queue_submission + from frappe.utils.scheduler import is_scheduler_inactive + doc = frappe.get_doc(frappe.parse_json(doc)) workflow = get_workflow(doc.doctype) transitions = get_transitions(doc, workflow) @@ -132,7 +135,10 @@ def apply_workflow(doc, action): if doc.docstatus.is_draft() and new_docstatus == DocStatus.draft(): doc.save() elif doc.docstatus.is_draft() and new_docstatus == DocStatus.submitted(): - doc.submit() + if doc.meta.queue_in_background and not is_scheduler_inactive(): + queue_submission(doc, action="submit") + else: + doc.submit() elif doc.docstatus.is_submitted() and new_docstatus == DocStatus.submitted(): doc.save() elif doc.docstatus.is_submitted() and new_docstatus == DocStatus.cancelled(): From cffcb0fa176eefbfb4bcc754b0621043255646f3 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Sat, 3 Dec 2022 23:40:57 +0530 Subject: [PATCH 04/13] refactor: failed attempts banner --- .../submission_queue/submission_queue.py | 28 ++++++++++++++++--- frappe/public/js/frappe/form/form.js | 10 +++++-- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/frappe/core/doctype/submission_queue/submission_queue.py b/frappe/core/doctype/submission_queue/submission_queue.py index 3e30ef1ef0..181f5f21cb 100644 --- a/frappe/core/doctype/submission_queue/submission_queue.py +++ b/frappe/core/doctype/submission_queue/submission_queue.py @@ -182,14 +182,34 @@ def queue_submission(doc: Document, action: str, alert: bool = True): ) +def format_tb(traceback: str): + traceback = traceback.strip().split("\n")[-1] + if len(traceback.split()) > 6: + return " ".join(traceback.split()[0:6]) + "..." + return traceback + + @frappe.whitelist() def get_latest_submissions(doctype, docname): # NOTE: not used creation as orderby intentianlly as we have used update_modified=False everywhere # hence assuming modified will be equal to creation for submission queue documents dt = "Submission Queue" + out = {} + filters = {"ref_doctype": doctype, "ref_docname": docname} - return { - "latest_submission": frappe.db.get_value(dt, filters), - "latest_failed_submission": frappe.db.get_value(dt, filters | {"status": "Failed"}), - } + failed_submission = frappe.db.get_value( + dt, filters=filters | {"status": "Failed"}, fieldname=["name", "exception"] + ) + latest_submission = frappe.db.get_value(dt, filters=filters, fieldname=["name", "status"]) + + if failed_submission: + out["latest_failed_submission"], out["latest_failed_submission_exc_info"] = ( + failed_submission[0], + format_tb(failed_submission[1]), + ) + + if latest_submission: + out["latest_submission"], out["latest_submission_status"] = latest_submission + + return out diff --git a/frappe/public/js/frappe/form/form.js b/frappe/public/js/frappe/form/form.js index 75a1def1dc..0f9ff22dee 100644 --- a/frappe/public/js/frappe/form/form.js +++ b/frappe/public/js/frappe/form/form.js @@ -2081,18 +2081,22 @@ frappe.ui.form.Form = class FrappeForm { col_width = 3; failed_link = ``; } else { - submission_label = __("Previous Falied Submission"); + if (r.message.latest_failed_submission_exc_info) { + submission_label = r.message.latest_failed_submission_exc_info; + } else { + submission_label = "Errored"; + } } } let html = `
- ${__("Submission Status:")} + ${__(`Submission Status: ${r.message.latest_submission_status}`)}
${submission_label} From 12f0be1906d2cd6d84c9d3759a1909cbed65c178 Mon Sep 17 00:00:00 2001 From: phot0n Date: Tue, 3 Jan 2023 00:41:37 +0530 Subject: [PATCH 05/13] refactor(minor): better banner and removed unnecessary complexity for unlocking ref document --- .../submission_queue/submission_queue.json | 5 +- .../submission_queue/submission_queue.py | 79 +++++++------------ frappe/public/js/frappe/form/form.js | 69 ++++++---------- frappe/public/scss/common/css_variables.scss | 2 +- 4 files changed, 57 insertions(+), 98 deletions(-) diff --git a/frappe/core/doctype/submission_queue/submission_queue.json b/frappe/core/doctype/submission_queue/submission_queue.json index d1f66ffa13..ce28007e23 100644 --- a/frappe/core/doctype/submission_queue/submission_queue.json +++ b/frappe/core/doctype/submission_queue/submission_queue.json @@ -20,8 +20,9 @@ "fields": [ { "fieldname": "job_id", - "fieldtype": "Data", + "fieldtype": "Link", "label": "Job Id", + "options": "RQ Job", "read_only": 1 }, { @@ -87,7 +88,7 @@ ], "index_web_pages_for_search": 1, "links": [], - "modified": "2022-11-12 16:48:37.797232", + "modified": "2023-01-02 23:53:55.010001", "modified_by": "Administrator", "module": "Core", "name": "Submission Queue", diff --git a/frappe/core/doctype/submission_queue/submission_queue.py b/frappe/core/doctype/submission_queue/submission_queue.py index 181f5f21cb..b1e20516a8 100644 --- a/frappe/core/doctype/submission_queue/submission_queue.py +++ b/frappe/core/doctype/submission_queue/submission_queue.py @@ -13,7 +13,6 @@ from frappe.desk.doctype.notification_log.notification_log import enqueue_create from frappe.model.document import Document from frappe.monitor import add_data_to_monitor from frappe.utils import now, time_diff_in_seconds -from frappe.utils.background_jobs import get_redis_conn from frappe.utils.data import cint @@ -39,6 +38,7 @@ class SubmissionQueue(Document): frappe.db.delete(table, filters=(table.modified < (Now() - Interval(days=days)))) def insert(self, to_be_queued_doc: Document, action: str): + self.status = "Queued" self.to_be_queued_doc = to_be_queued_doc self.action_for_queuing = action super().insert(ignore_permissions=True) @@ -68,11 +68,9 @@ class SubmissionQueue(Document): ) def background_submission(self, to_be_queued_doc: Document, action_for_queuing: str): - if self.status == "Failed": - # If the document has already been unlocked by _unlock_reference_doc - return # Set the job id for that submission doctyp self.update_job_id(get_current_job().id) + _action = action_for_queuing.lower() if _action == "update": _action = "submit" @@ -96,21 +94,21 @@ class SubmissionQueue(Document): self.notify(values["status"], action_for_queuing) def notify(self, submission_status: str, action: str): - message = _("Action {0} run on {1} {2} ") if submission_status == "Failed": doctype = self.doctype docname = self.name - message += "failed" + message = _("Submission of {0} {1} with action {2} failed") else: doctype = self.ref_doctype docname = self.ref_docname - message += "finished" + message = _("Submission of {0} {1} with action {2} completed successfully") message = message.format( - frappe.bold(action), frappe.bold(str(self.ref_doctype)), - frappe.bold(self.ref_docname), + frappe.bold(str(self.ref_docname)), + frappe.bold(action), ) + time_diff = time_diff_in_seconds(now(), self.created_at) if cint(time_diff) <= 60: frappe.publish_realtime( @@ -134,40 +132,21 @@ class SubmissionQueue(Document): notify_to = frappe.db.get_value("User", self.enqueued_by, fieldname="email") enqueue_create_notification([notify_to], notification_doc) - def _unlock_reference_doc(self): - try: - job = Job.fetch(self.job_id, connection=get_redis_conn()) - status = job.get_status(refresh=True) - exc = job.exc_info - except NoSuchJobError: - exc = None - status = "failed" - - if status in ("queued", "started"): - frappe.msgprint(_("Document in queue for execution!")) - return - - self.queued_doc.unlock() - values = ( - {"status": "Finished"} if status == "finished" else {"status": "Failed", "exception": exc} - ) - frappe.db.set_value(self.doctype, self.name, values, update_modified=False) - frappe.msgprint(_("Document Unlocked")) - @frappe.whitelist() def unlock_doc(self): # NOTE: this can lead to some weird unlocking/locking behaviours. # for example: hitting unlock on a submission could lead to unlocking of another submission # of the same reference document. + if self.status != "Queued": return - self._unlock_reference_doc() + self.queued_doc.unlock() + frappe.msgprint(_("Document Unlocked")) def queue_submission(doc: Document, action: str, alert: bool = True): queue = frappe.new_doc("Submission Queue") - queue.status = "Queued" queue.ref_doctype = doc.doctype queue.ref_docname = doc.name queue.insert(doc, action) @@ -182,34 +161,30 @@ def queue_submission(doc: Document, action: str, alert: bool = True): ) -def format_tb(traceback: str): - traceback = traceback.strip().split("\n")[-1] - if len(traceback.split()) > 6: - return " ".join(traceback.split()[0:6]) + "..." - return traceback - - @frappe.whitelist() def get_latest_submissions(doctype, docname): # NOTE: not used creation as orderby intentianlly as we have used update_modified=False everywhere # hence assuming modified will be equal to creation for submission queue documents - dt = "Submission Queue" - out = {} - - filters = {"ref_doctype": doctype, "ref_docname": docname} - failed_submission = frappe.db.get_value( - dt, filters=filters | {"status": "Failed"}, fieldname=["name", "exception"] + latest_submission = frappe.db.get_value( + "Submission Queue", + filters={"ref_doctype": doctype, "ref_docname": docname}, + fieldname=["name", "exception", "status"], ) - latest_submission = frappe.db.get_value(dt, filters=filters, fieldname=["name", "status"]) - - if failed_submission: - out["latest_failed_submission"], out["latest_failed_submission_exc_info"] = ( - failed_submission[0], - format_tb(failed_submission[1]), - ) + out = None if latest_submission: - out["latest_submission"], out["latest_submission_status"] = latest_submission + out = { + "latest_submission": latest_submission[0], + "exc": format_tb(latest_submission[1]), + "status": latest_submission[2], + } return out + + +def format_tb(traceback: str | None = None): + if not traceback: + return + + return traceback.strip().split("\n")[-1] diff --git a/frappe/public/js/frappe/form/form.js b/frappe/public/js/frappe/form/form.js index 0f9ff22dee..25b53638fe 100644 --- a/frappe/public/js/frappe/form/form.js +++ b/frappe/public/js/frappe/form/form.js @@ -2051,16 +2051,12 @@ frappe.ui.form.Form = class FrappeForm { this.doc.docstatus === 0 ) ) { - if (wrapper.length) { - wrapper.hide(); - wrapper.html(""); - } - + wrapper.length && wrapper.remove(); return; } if (!wrapper.length) { - wrapper = $('
'); + wrapper = $('
'); this.layout.wrapper.prepend(wrapper); } @@ -2070,53 +2066,40 @@ frappe.ui.form.Form = class FrappeForm { args: { doctype: this.doctype, docname: this.docname }, }) .then((r) => { - if (r.message.latest_submission) { + if (r.message?.latest_submission) { // if we are here that means some submission(s) were queued and are in queued/failed state - let col_width = 4; - let failed_link = ""; let submission_label = __("Previous Submission"); + let secondary = ""; + let div_class = "col-md-12"; - if (r.message.latest_failed_submission) { - if (r.message.latest_failed_submission !== r.message.latest_submission) { - col_width = 3; - failed_link = ``; - } else { - if (r.message.latest_failed_submission_exc_info) { - submission_label = r.message.latest_failed_submission_exc_info; - } else { - submission_label = "Errored"; - } - } + if (r.message.exc) { + secondary = `: ${r.message.exc}`; + } else { + div_class = "col-md-6"; + secondary = ` +
+
+ ${__( + "All Submissions" + )} + `; } let html = ` -
-
- ${__(`Submission Status: ${r.message.latest_submission_status}`)} + - - ${failed_link} - -
- `; + `; - wrapper.show(); + wrapper.removeClass("red").removeClass("yellow"); + wrapper.addClass(r.message.status == "Failed" ? "red" : "yellow"); wrapper.html(html); } else { - wrapper.hide(); - wrapper.html(""); + wrapper.remove(); } }); } diff --git a/frappe/public/scss/common/css_variables.scss b/frappe/public/scss/common/css_variables.scss index 1914e7479b..cfbcc001b6 100644 --- a/frappe/public/scss/common/css_variables.scss +++ b/frappe/public/scss/common/css_variables.scss @@ -163,7 +163,7 @@ $input-height: 28px !default; --bg-green: var(--dark-green-50); --bg-yellow: var(--yellow-50); --bg-orange: var(--orange-50); - --bg-red: var(--red-50); + --bg-red: var(--red-100); --bg-gray: var(--gray-200); --bg-light-gray: var(--gray-100); --bg-dark-gray: var(--gray-900); From f6489a6de861317f6ae4ac154814317eb528de14 Mon Sep 17 00:00:00 2001 From: phot0n Date: Tue, 3 Jan 2023 21:06:45 +0530 Subject: [PATCH 06/13] fix: allow submission queue doc reads from users if theyre owners * only show unlock doc button to system managers --- frappe/core/doctype/submission_queue/submission_queue.js | 2 +- frappe/core/doctype/submission_queue/submission_queue.json | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/submission_queue/submission_queue.js b/frappe/core/doctype/submission_queue/submission_queue.js index 414c8c9ee0..fc1e83ac49 100644 --- a/frappe/core/doctype/submission_queue/submission_queue.js +++ b/frappe/core/doctype/submission_queue/submission_queue.js @@ -3,7 +3,7 @@ frappe.ui.form.on("Submission Queue", { refresh: function (frm) { - if (frm.doc.status === "Queued") { + if (frm.doc.status === "Queued" && frappe.boot.user.roles.includes("System Manager")) { frm.add_custom_button(__("Unlock Reference Document"), () => { frappe.confirm(__("Are you sure you want to go ahead with this action?"), () => { frm.call("unlock_doc"); diff --git a/frappe/core/doctype/submission_queue/submission_queue.json b/frappe/core/doctype/submission_queue/submission_queue.json index ce28007e23..4058276319 100644 --- a/frappe/core/doctype/submission_queue/submission_queue.json +++ b/frappe/core/doctype/submission_queue/submission_queue.json @@ -88,7 +88,7 @@ ], "index_web_pages_for_search": 1, "links": [], - "modified": "2023-01-02 23:53:55.010001", + "modified": "2023-01-03 20:54:40.904584", "modified_by": "Administrator", "module": "Core", "name": "Submission Queue", @@ -103,6 +103,11 @@ "report": 1, "role": "System Manager", "share": 1 + }, + { + "if_owner": 1, + "read": 1, + "role": "All" } ], "sort_field": "modified", From 64cb507fae19b2fe367a4b19d0987e323a9e8652 Mon Sep 17 00:00:00 2001 From: phot0n Date: Wed, 4 Jan 2023 14:18:00 +0530 Subject: [PATCH 07/13] chore: verbose confimation dialog message --- .../doctype/submission_queue/submission_queue.js | 12 +++++++++--- .../doctype/submission_queue/submission_queue.py | 4 +--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/frappe/core/doctype/submission_queue/submission_queue.js b/frappe/core/doctype/submission_queue/submission_queue.js index fc1e83ac49..6e64be780a 100644 --- a/frappe/core/doctype/submission_queue/submission_queue.js +++ b/frappe/core/doctype/submission_queue/submission_queue.js @@ -5,9 +5,15 @@ frappe.ui.form.on("Submission Queue", { refresh: function (frm) { if (frm.doc.status === "Queued" && frappe.boot.user.roles.includes("System Manager")) { frm.add_custom_button(__("Unlock Reference Document"), () => { - frappe.confirm(__("Are you sure you want to go ahead with this action?"), () => { - frm.call("unlock_doc"); - }); + frappe.confirm( + ` + Are you sure you want to go ahead with this action? + Doing this could unlock other submissions of this document which are in queue (if present) + and could lead to non-ideal conditions.`, + () => { + frm.call("unlock_doc"); + } + ); }); } }, diff --git a/frappe/core/doctype/submission_queue/submission_queue.py b/frappe/core/doctype/submission_queue/submission_queue.py index b1e20516a8..83b3154780 100644 --- a/frappe/core/doctype/submission_queue/submission_queue.py +++ b/frappe/core/doctype/submission_queue/submission_queue.py @@ -4,8 +4,6 @@ from urllib.parse import quote from rq import get_current_job -from rq.exceptions import NoSuchJobError -from rq.job import Job import frappe from frappe import _ @@ -68,7 +66,7 @@ class SubmissionQueue(Document): ) def background_submission(self, to_be_queued_doc: Document, action_for_queuing: str): - # Set the job id for that submission doctyp + # Set the job id for that submission doctype self.update_job_id(get_current_job().id) _action = action_for_queuing.lower() From 08c8ab02291cc8246cda21029aa50bc0629dde01 Mon Sep 17 00:00:00 2001 From: phot0n Date: Wed, 4 Jan 2023 17:00:27 +0530 Subject: [PATCH 08/13] chore: better notification message Co-authored-by: Aradhya-Tripathi --- frappe/core/doctype/doctype/doctype.json | 5 +++-- .../doctype/submission_queue/submission_queue.py | 16 +++++++++------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.json b/frappe/core/doctype/doctype/doctype.json index 14ef2fd8fb..671a6e86e6 100644 --- a/frappe/core/doctype/doctype/doctype.json +++ b/frappe/core/doctype/doctype/doctype.json @@ -604,6 +604,7 @@ { "default": "0", "depends_on": "eval: doc.is_submittable", + "description": "Enabling this will submit documents in background", "fieldname": "queue_in_background", "fieldtype": "Check", "label": "Queue in Background" @@ -707,7 +708,7 @@ "link_fieldname": "reference_doctype" } ], - "modified": "2022-12-14 09:47:27.315351", + "modified": "2023-01-04 17:23:09.206018", "modified_by": "Administrator", "module": "Core", "name": "DocType", @@ -744,4 +745,4 @@ "states": [], "track_changes": 1, "translated_doctype": 1 -} +} \ No newline at end of file diff --git a/frappe/core/doctype/submission_queue/submission_queue.py b/frappe/core/doctype/submission_queue/submission_queue.py index 83b3154780..0400fbef67 100644 --- a/frappe/core/doctype/submission_queue/submission_queue.py +++ b/frappe/core/doctype/submission_queue/submission_queue.py @@ -95,16 +95,16 @@ class SubmissionQueue(Document): if submission_status == "Failed": doctype = self.doctype docname = self.name - message = _("Submission of {0} {1} with action {2} failed") + message = _("Action {0} failed on {1} {2}. View it {3}") else: doctype = self.ref_doctype docname = self.ref_docname - message = _("Submission of {0} {1} with action {2} completed successfully") + message = _("Action {0} completed successfully on {1} {2}. View it {3}") - message = message.format( + message_replacements = ( + frappe.bold(action), frappe.bold(str(self.ref_doctype)), frappe.bold(str(self.ref_docname)), - frappe.bold(action), ) time_diff = time_diff_in_seconds(now(), self.created_at) @@ -112,8 +112,10 @@ class SubmissionQueue(Document): frappe.publish_realtime( "msgprint", { - "message": message - + f". View it here", + "message": message.format( + *message_replacements, + f"here", + ), "alert": True, "indicator": "red" if submission_status == "Failed" else "green", }, @@ -124,7 +126,7 @@ class SubmissionQueue(Document): "type": "Alert", "document_type": doctype, "document_name": docname, - "subject": message, + "subject": message.format(*message_replacements, "here"), } notify_to = frappe.db.get_value("User", self.enqueued_by, fieldname="email") From bb8b0d415ec9881f8dfd4afa62ebc51f159ba538 Mon Sep 17 00:00:00 2001 From: phot0n Date: Thu, 5 Jan 2023 16:22:52 +0530 Subject: [PATCH 09/13] fix: workflow mechanics for submission queue (start) --- .../submission_queue/submission_queue.py | 15 ++++++---- frappe/model/workflow.py | 15 ++++++++-- .../workflow_action/workflow_action.py | 30 ++++++++++++++----- 3 files changed, 46 insertions(+), 14 deletions(-) diff --git a/frappe/core/doctype/submission_queue/submission_queue.py b/frappe/core/doctype/submission_queue/submission_queue.py index 0400fbef67..d64ecd7d93 100644 --- a/frappe/core/doctype/submission_queue/submission_queue.py +++ b/frappe/core/doctype/submission_queue/submission_queue.py @@ -1,6 +1,7 @@ # Copyright (c) 2022, Frappe Technologies and contributors # For license information, please see license.txt +from typing import Callable from urllib.parse import quote from rq import get_current_job @@ -35,10 +36,11 @@ class SubmissionQueue(Document): table = frappe.qb.DocType("Submission Queue") frappe.db.delete(table, filters=(table.modified < (Now() - Interval(days=days)))) - def insert(self, to_be_queued_doc: Document, action: str): + def insert(self, to_be_queued_doc: Document, action: str, **job_kwargs): self.status = "Queued" self.to_be_queued_doc = to_be_queued_doc self.action_for_queuing = action + self.kwargs = job_kwargs super().insert(ignore_permissions=True) def lock(self): @@ -57,12 +59,15 @@ class SubmissionQueue(Document): frappe.db.commit() def after_insert(self): + self.kwargs.pop("enqueue_after_commit", None) + self.queue_action( "background_submission", to_be_queued_doc=self.queued_doc, action_for_queuing=self.action_for_queuing, - timeout=600, + timeout=self.kwargs.pop("timeout", 600), enqueue_after_commit=True, + **self.kwargs, ) def background_submission(self, to_be_queued_doc: Document, action_for_queuing: str): @@ -99,7 +104,7 @@ class SubmissionQueue(Document): else: doctype = self.ref_doctype docname = self.ref_docname - message = _("Action {0} completed successfully on {1} {2}. View it {3}") + message = _("Action {0} completed on {1} {2}. View it {3}") message_replacements = ( frappe.bold(action), @@ -145,11 +150,11 @@ class SubmissionQueue(Document): frappe.msgprint(_("Document Unlocked")) -def queue_submission(doc: Document, action: str, alert: bool = True): +def queue_submission(doc: Document, action: str, alert: bool = True, **job_kwargs): queue = frappe.new_doc("Submission Queue") queue.ref_doctype = doc.doctype queue.ref_docname = doc.name - queue.insert(doc, action) + queue.insert(doc, action, **job_kwargs) if alert: frappe.msgprint( diff --git a/frappe/model/workflow.py b/frappe/model/workflow.py index e7835fba8d..a2155e43f2 100644 --- a/frappe/model/workflow.py +++ b/frappe/model/workflow.py @@ -136,7 +136,15 @@ def apply_workflow(doc, action): doc.save() elif doc.docstatus.is_draft() and new_docstatus == DocStatus.submitted(): if doc.meta.queue_in_background and not is_scheduler_inactive(): - queue_submission(doc, action="submit") + queue_submission( + doc, + action="submit", + on_success=lambda job, connection, result, *args, **kwargs: ( + doc.add_comment("Workflow", _(next_state.state)), + frappe.db.commit(), + ), + ) + return else: doc.submit() elif doc.docstatus.is_submitted() and new_docstatus == DocStatus.submitted(): @@ -257,8 +265,11 @@ def bulk_workflow_approval(docnames, doctype, action): message_dict = {} try: show_progress(docnames, _("Applying: {0}").format(action), idx, docname) - apply_workflow(frappe.get_doc(doctype, docname), action) + d = apply_workflow(frappe.get_doc(doctype, docname), action) frappe.db.commit() + if not d: + # for background submission + continue except Exception as e: if not frappe.message_log: # Exception is raised manually and not from msgprint or throw diff --git a/frappe/workflow/doctype/workflow_action/workflow_action.py b/frappe/workflow/doctype/workflow_action/workflow_action.py index 545ad6ec77..cc3a6c25dd 100644 --- a/frappe/workflow/doctype/workflow_action/workflow_action.py +++ b/frappe/workflow/doctype/workflow_action/workflow_action.py @@ -140,20 +140,36 @@ def confirm_action(doctype, docname, user, action): doc = frappe.get_doc(doctype, docname) newdoc = apply_workflow(doc, action) frappe.db.commit() - return_success_page(newdoc) + + return_response_page(doc, not newdoc) # reset session user if logged_in_user == "Guest": frappe.set_user(logged_in_user) -def return_success_page(doc): +def return_response_page(doc, is_background=False): + if is_background: + title = _("Pending") + message = ( + _("{0}: {1} is added in queue to set to next state").format( + doc.get("doctype"), frappe.bold(doc.get("name")) + ), + ) + color = "orange" + else: + title = _("Success") + message = ( + _("{0}: {1} is set to state {2}").format( + doc.get("doctype"), frappe.bold(doc.get("name")), frappe.bold(get_doc_workflow_state(doc)) + ), + ) + color = "green" + frappe.respond_as_web_page( - _("Success"), - _("{0}: {1} is set to state {2}").format( - doc.get("doctype"), frappe.bold(doc.get("name")), frappe.bold(get_doc_workflow_state(doc)) - ), - indicator_color="green", + title, + message, + indicator_color=color, ) From af093dd5987adec83f02b5ff6579fd3a552952be Mon Sep 17 00:00:00 2001 From: phot0n Date: Mon, 23 Jan 2023 12:46:21 +0530 Subject: [PATCH 10/13] fix: traceback with context for submission queue --- .../doctype/submission_queue/submission_queue.json | 12 +++--------- .../doctype/submission_queue/submission_queue.py | 2 +- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/frappe/core/doctype/submission_queue/submission_queue.json b/frappe/core/doctype/submission_queue/submission_queue.json index 4058276319..590346a53c 100644 --- a/frappe/core/doctype/submission_queue/submission_queue.json +++ b/frappe/core/doctype/submission_queue/submission_queue.json @@ -20,9 +20,8 @@ "fields": [ { "fieldname": "job_id", - "fieldtype": "Link", + "fieldtype": "Data", "label": "Job Id", - "options": "RQ Job", "read_only": 1 }, { @@ -81,14 +80,14 @@ }, { "fieldname": "exception", - "fieldtype": "Text", + "fieldtype": "Long Text", "label": "Exception", "read_only": 1 } ], "index_web_pages_for_search": 1, "links": [], - "modified": "2023-01-03 20:54:40.904584", + "modified": "2023-01-23 12:45:53.997708", "modified_by": "Administrator", "module": "Core", "name": "Submission Queue", @@ -103,11 +102,6 @@ "report": 1, "role": "System Manager", "share": 1 - }, - { - "if_owner": 1, - "read": 1, - "role": "All" } ], "sort_field": "modified", diff --git a/frappe/core/doctype/submission_queue/submission_queue.py b/frappe/core/doctype/submission_queue/submission_queue.py index d64ecd7d93..07e3a64055 100644 --- a/frappe/core/doctype/submission_queue/submission_queue.py +++ b/frappe/core/doctype/submission_queue/submission_queue.py @@ -89,7 +89,7 @@ class SubmissionQueue(Document): ) values = {"status": "Finished"} except Exception: - values = {"status": "Failed", "exception": frappe.get_traceback()} + values = {"status": "Failed", "exception": frappe.get_traceback(with_context=True)} frappe.db.rollback() values["ended_at"] = now() From 99fbe969e89bfd14486fbc82876b33e247492ef2 Mon Sep 17 00:00:00 2001 From: phot0n Date: Mon, 23 Jan 2023 12:48:04 +0530 Subject: [PATCH 11/13] Revert "fix: workflow mechanics for submission queue (start)" This reverts commit bb8b0d415ec9881f8dfd4afa62ebc51f159ba538. --- .../submission_queue/submission_queue.py | 15 ++++------ frappe/model/workflow.py | 15 ++-------- .../workflow_action/workflow_action.py | 30 +++++-------------- 3 files changed, 14 insertions(+), 46 deletions(-) diff --git a/frappe/core/doctype/submission_queue/submission_queue.py b/frappe/core/doctype/submission_queue/submission_queue.py index 07e3a64055..be0c20fc32 100644 --- a/frappe/core/doctype/submission_queue/submission_queue.py +++ b/frappe/core/doctype/submission_queue/submission_queue.py @@ -1,7 +1,6 @@ # Copyright (c) 2022, Frappe Technologies and contributors # For license information, please see license.txt -from typing import Callable from urllib.parse import quote from rq import get_current_job @@ -36,11 +35,10 @@ class SubmissionQueue(Document): table = frappe.qb.DocType("Submission Queue") frappe.db.delete(table, filters=(table.modified < (Now() - Interval(days=days)))) - def insert(self, to_be_queued_doc: Document, action: str, **job_kwargs): + def insert(self, to_be_queued_doc: Document, action: str): self.status = "Queued" self.to_be_queued_doc = to_be_queued_doc self.action_for_queuing = action - self.kwargs = job_kwargs super().insert(ignore_permissions=True) def lock(self): @@ -59,15 +57,12 @@ class SubmissionQueue(Document): frappe.db.commit() def after_insert(self): - self.kwargs.pop("enqueue_after_commit", None) - self.queue_action( "background_submission", to_be_queued_doc=self.queued_doc, action_for_queuing=self.action_for_queuing, - timeout=self.kwargs.pop("timeout", 600), + timeout=600, enqueue_after_commit=True, - **self.kwargs, ) def background_submission(self, to_be_queued_doc: Document, action_for_queuing: str): @@ -104,7 +99,7 @@ class SubmissionQueue(Document): else: doctype = self.ref_doctype docname = self.ref_docname - message = _("Action {0} completed on {1} {2}. View it {3}") + message = _("Action {0} completed successfully on {1} {2}. View it {3}") message_replacements = ( frappe.bold(action), @@ -150,11 +145,11 @@ class SubmissionQueue(Document): frappe.msgprint(_("Document Unlocked")) -def queue_submission(doc: Document, action: str, alert: bool = True, **job_kwargs): +def queue_submission(doc: Document, action: str, alert: bool = True): queue = frappe.new_doc("Submission Queue") queue.ref_doctype = doc.doctype queue.ref_docname = doc.name - queue.insert(doc, action, **job_kwargs) + queue.insert(doc, action) if alert: frappe.msgprint( diff --git a/frappe/model/workflow.py b/frappe/model/workflow.py index a2155e43f2..e7835fba8d 100644 --- a/frappe/model/workflow.py +++ b/frappe/model/workflow.py @@ -136,15 +136,7 @@ def apply_workflow(doc, action): doc.save() elif doc.docstatus.is_draft() and new_docstatus == DocStatus.submitted(): if doc.meta.queue_in_background and not is_scheduler_inactive(): - queue_submission( - doc, - action="submit", - on_success=lambda job, connection, result, *args, **kwargs: ( - doc.add_comment("Workflow", _(next_state.state)), - frappe.db.commit(), - ), - ) - return + queue_submission(doc, action="submit") else: doc.submit() elif doc.docstatus.is_submitted() and new_docstatus == DocStatus.submitted(): @@ -265,11 +257,8 @@ def bulk_workflow_approval(docnames, doctype, action): message_dict = {} try: show_progress(docnames, _("Applying: {0}").format(action), idx, docname) - d = apply_workflow(frappe.get_doc(doctype, docname), action) + apply_workflow(frappe.get_doc(doctype, docname), action) frappe.db.commit() - if not d: - # for background submission - continue except Exception as e: if not frappe.message_log: # Exception is raised manually and not from msgprint or throw diff --git a/frappe/workflow/doctype/workflow_action/workflow_action.py b/frappe/workflow/doctype/workflow_action/workflow_action.py index cc3a6c25dd..545ad6ec77 100644 --- a/frappe/workflow/doctype/workflow_action/workflow_action.py +++ b/frappe/workflow/doctype/workflow_action/workflow_action.py @@ -140,36 +140,20 @@ def confirm_action(doctype, docname, user, action): doc = frappe.get_doc(doctype, docname) newdoc = apply_workflow(doc, action) frappe.db.commit() - - return_response_page(doc, not newdoc) + return_success_page(newdoc) # reset session user if logged_in_user == "Guest": frappe.set_user(logged_in_user) -def return_response_page(doc, is_background=False): - if is_background: - title = _("Pending") - message = ( - _("{0}: {1} is added in queue to set to next state").format( - doc.get("doctype"), frappe.bold(doc.get("name")) - ), - ) - color = "orange" - else: - title = _("Success") - message = ( - _("{0}: {1} is set to state {2}").format( - doc.get("doctype"), frappe.bold(doc.get("name")), frappe.bold(get_doc_workflow_state(doc)) - ), - ) - color = "green" - +def return_success_page(doc): frappe.respond_as_web_page( - title, - message, - indicator_color=color, + _("Success"), + _("{0}: {1} is set to state {2}").format( + doc.get("doctype"), frappe.bold(doc.get("name")), frappe.bold(get_doc_workflow_state(doc)) + ), + indicator_color="green", ) From 230d36e7899e1a1307a59abe5e8ee732102ec88e Mon Sep 17 00:00:00 2001 From: phot0n Date: Mon, 23 Jan 2023 12:49:36 +0530 Subject: [PATCH 12/13] chore: revert things related to workflow --- frappe/model/workflow.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/frappe/model/workflow.py b/frappe/model/workflow.py index e7835fba8d..8338157996 100644 --- a/frappe/model/workflow.py +++ b/frappe/model/workflow.py @@ -101,9 +101,6 @@ def is_transition_condition_satisfied(transition, doc) -> bool: @frappe.whitelist() def apply_workflow(doc, action): """Allow workflow action on the current doc""" - from frappe.core.doctype.submission_queue.submission_queue import queue_submission - from frappe.utils.scheduler import is_scheduler_inactive - doc = frappe.get_doc(frappe.parse_json(doc)) workflow = get_workflow(doc.doctype) transitions = get_transitions(doc, workflow) @@ -135,10 +132,7 @@ def apply_workflow(doc, action): if doc.docstatus.is_draft() and new_docstatus == DocStatus.draft(): doc.save() elif doc.docstatus.is_draft() and new_docstatus == DocStatus.submitted(): - if doc.meta.queue_in_background and not is_scheduler_inactive(): - queue_submission(doc, action="submit") - else: - doc.submit() + doc.submit() elif doc.docstatus.is_submitted() and new_docstatus == DocStatus.submitted(): doc.save() elif doc.docstatus.is_submitted() and new_docstatus == DocStatus.cancelled(): From 4144c45b2a12b7b50be214d378c01e3f271cdfba Mon Sep 17 00:00:00 2001 From: phot0n Date: Mon, 23 Jan 2023 13:13:27 +0530 Subject: [PATCH 13/13] fix: make job_id link field and allow all (if owner) to read their submission queues --- .../core/doctype/submission_queue/submission_queue.json | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/frappe/core/doctype/submission_queue/submission_queue.json b/frappe/core/doctype/submission_queue/submission_queue.json index 590346a53c..04668e1c76 100644 --- a/frappe/core/doctype/submission_queue/submission_queue.json +++ b/frappe/core/doctype/submission_queue/submission_queue.json @@ -20,8 +20,9 @@ "fields": [ { "fieldname": "job_id", - "fieldtype": "Data", + "fieldtype": "Link", "label": "Job Id", + "options": "RQ Job", "read_only": 1 }, { @@ -102,6 +103,11 @@ "report": 1, "role": "System Manager", "share": 1 + }, + { + "if_owner": 1, + "read": 1, + "role": "All" } ], "sort_field": "modified",