From 4e8bbd6c93daf3bc9458382050861e2249cbaf6b Mon Sep 17 00:00:00 2001 From: Aradhya Date: Fri, 2 Dec 2022 12:47:17 +0530 Subject: [PATCH 01/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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 dcbc9e54ca705fe7741c91965a3eb0e4e7e07e6b Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Fri, 20 Jan 2023 14:16:04 +0530 Subject: [PATCH 07/19] fix: Indicator dot colors in dark mode --- frappe/public/scss/common/css_variables.scss | 7 +- frappe/public/scss/common/indicator.scss | 141 +++---------------- frappe/public/scss/desk/dark.scss | 15 +- 3 files changed, 42 insertions(+), 121 deletions(-) diff --git a/frappe/public/scss/common/css_variables.scss b/frappe/public/scss/common/css_variables.scss index 1914e7479b..aa40c9a17f 100644 --- a/frappe/public/scss/common/css_variables.scss +++ b/frappe/public/scss/common/css_variables.scss @@ -165,8 +165,10 @@ $input-height: 28px !default; --bg-orange: var(--orange-50); --bg-red: var(--red-50); --bg-gray: var(--gray-200); + --bg-grey: var(--gray-200); --bg-light-gray: var(--gray-100); - --bg-dark-gray: var(--gray-900); + --bg-dark-gray: var(--gray-400); + --bg-darkgrey: var(--gray-400); --bg-purple: var(--purple-100); --bg-pink: var(--pink-50); --bg-cyan: var(--cyan-50); @@ -189,6 +191,9 @@ $input-height: 28px !default; --text-on-orange: var(--orange-600); --text-on-red: var(--red-600); --text-on-gray: var(--gray-600); + --text-on-grey: var(--gray-600); + --text-on-darkgrey: var(--gray-700); + --text-on-dark-gray: var(--gray-700); --text-on-light-gray: var(--gray-800); --text-on-purple: var(--purple-700); --text-on-pink: var(--pink-600); diff --git a/frappe/public/scss/common/indicator.scss b/frappe/public/scss/common/indicator.scss index b995cf856f..023e463aef 100644 --- a/frappe/public/scss/common/indicator.scss +++ b/frappe/public/scss/common/indicator.scss @@ -1,19 +1,3 @@ -@mixin indicator-pill-color($color) { - background: var(--bg-#{$color}); - color: var(--text-on-#{$color}); - &::before, - &::after { - background: var(--text-on-#{$color}); - } -} - -@mixin indicator-color($color) { - &::before, - &::after { - background: var(--text-on-#{$color}); - } -} - .indicator, .indicator-pill, .indicator-pill-right, @@ -67,111 +51,30 @@ margin: 0 0 0 4px; } -.indicator.green { - @include indicator-color('green'); +$indicator-colors: green, cyan, blue, orange, yellow, gray, grey, red, pink, darkgrey, purple, light-blue; +@each $color in $indicator-colors { + .indicator.#{"" + $color} { + &::before, + &::after { + background: var(--indicator-dot-#{$color}); + } + } + + .indicator-pill.#{"" + $color}, + .indicator-pill-right.#{"" + $color}, + .indicator-pill-round.#{"" + $color} { + background: var(--bg-#{$color}); + color: var(--text-on-#{$color}); + &::before, + &::after { + background: var(--text-on-#{$color}); + } + } + .indicator { + --indicator-dot-#{"" + $color}: var(--text-on-#{$color}); + } } -.indicator-pill.green, -.indicator-pill-right.green, -.indicator-pill-round.green { - @include indicator-pill-color('green'); -} - -.indicator.cyan { - @include indicator-color('cyan'); -} - -.indicator-pill.cyan, -.indicator-pill-right.cyan, -.indicator-pill-round.cyan { - @include indicator-pill-color('cyan'); -} - -.indicator.blue { - @include indicator-color('blue'); -} - -.indicator-pill.blue, -.indicator-pill-right.blue, -.indicator-pill-round.blue { - @include indicator-pill-color('blue'); -} - -.indicator.orange { - @include indicator-color('orange'); -} - -.indicator-pill.orange, -.indicator-pill-right.orange -.indicator-pill-round.orange { - @include indicator-pill-color('orange'); -} - -.indicator.yellow { - @include indicator-color('yellow'); -} - -.indicator-pill.yellow, -.indicator-pill-right.yellow -.indicator-pill-round.yellow { - @include indicator-pill-color('yellow'); -} - -.indicator.gray, -.indicator.grey { - @include indicator-color('gray'); -} - -.indicator-pill.gray, -.indicator-pill-right.gray, -.indicator-pill-round.gray, -.indicator-pill.grey, -.indicator-pill-right.grey, -.indicator-pill-round.grey { - @include indicator-pill-color('light-gray'); -} - -.indicator.red { - @include indicator-color('red'); -} - -.indicator-pill.red, -.indicator-pill-right.red, -.indicator-pill-round.red { - @include indicator-pill-color('red'); -} - -.indicator.pink { - @include indicator-color('pink'); -} - -.indicator-pill.pink, -.indicator-pill-right.pink, -.indicator-pill-round.pink { - @include indicator-pill-color('pink'); -} - -.indicator-pill.darkgrey, -.indicator-pill-right.darkgrey, -.indicator-pill-round.darkgrey { - @include indicator-pill-color('gray'); -} - -.indicator-pill.purple, -.indicator-pill-right.purple, -.indicator-pill-round.purple { - @include indicator-pill-color('purple'); -} - -.indicator.light-blue { - @include indicator-color('light-blue'); -} - -.indicator-pill.light-blue, -.indicator-pill-right.light-blue, -.indicator-pill-round.light-blue { - @include indicator-pill-color('light-blue'); -} .indicator.blink { animation: blink 1s linear infinite; diff --git a/frappe/public/scss/desk/dark.scss b/frappe/public/scss/desk/dark.scss index aad3e4b597..2d37fc3c4b 100644 --- a/frappe/public/scss/desk/dark.scss +++ b/frappe/public/scss/desk/dark.scss @@ -54,6 +54,9 @@ --bg-orange: var(--orange-500); --bg-red: var(--red-500); --bg-gray: var(--gray-600); + --bg-grey: var(--gray-600); + --bg-darkgrey: var(--gray-800); + --bg-dark-gray: var(--gray-800); --bg-light-gray: var(--gray-700); --bg-dark-gray: var(--gray-300); --bg-purple: var(--purple-600); @@ -65,7 +68,10 @@ --text-on-yellow: var(--yellow-50); --text-on-orange: var(--orange-100); --text-on-red: var(--red-50); - --text-on-gray: var(--gray-300); + --text-on-gray: var(--gray-200); + --text-on-grey: var(--gray-200); + --text-on-darkgrey: var(--gray-200); + --text-on-dark-gray: var(--gray-200); --text-on-light-gray: var(--gray-100); --text-on-purple: var(--purple-100); @@ -190,4 +196,11 @@ color: var(--text-color); background: var(--gray-500); } + + $indicator-colors: green, cyan, blue, orange, yellow, gray, grey, red, pink, darkgrey, purple, light-blue; + @each $color in $indicator-colors { + .indicator { + --indicator-dot-#{"" + $color}: var(--bg-#{$color}); + } + } } From e2b3dd60c28ba899a717fdc501d995befb47a4b8 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Fri, 20 Jan 2023 19:12:21 +0530 Subject: [PATCH 08/19] fix: Indicator contrast to make it more readable --- frappe/public/scss/common/css_variables.scss | 12 ++++++------ frappe/public/scss/desk/dark.scss | 14 +++++++------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/frappe/public/scss/common/css_variables.scss b/frappe/public/scss/common/css_variables.scss index aa40c9a17f..42d105e60e 100644 --- a/frappe/public/scss/common/css_variables.scss +++ b/frappe/public/scss/common/css_variables.scss @@ -188,15 +188,15 @@ $input-height: 28px !default; --text-on-dark-blue: var(--blue-800); --text-on-green: var(--dark-green-700); --text-on-yellow: var(--yellow-700); - --text-on-orange: var(--orange-600); + --text-on-orange: var(--orange-700); --text-on-red: var(--red-600); - --text-on-gray: var(--gray-600); - --text-on-grey: var(--gray-600); - --text-on-darkgrey: var(--gray-700); - --text-on-dark-gray: var(--gray-700); + --text-on-gray: var(--gray-700); + --text-on-grey: var(--gray-700); + --text-on-darkgrey: var(--gray-800); + --text-on-dark-gray: var(--gray-800); --text-on-light-gray: var(--gray-800); --text-on-purple: var(--purple-700); - --text-on-pink: var(--pink-600); + --text-on-pink: var(--pink-700); --text-on-cyan: var(--cyan-800); // alert colors diff --git a/frappe/public/scss/desk/dark.scss b/frappe/public/scss/desk/dark.scss index 2d37fc3c4b..5970fb509c 100644 --- a/frappe/public/scss/desk/dark.scss +++ b/frappe/public/scss/desk/dark.scss @@ -47,22 +47,22 @@ // Background Text Color Pairs --bg-blue: var(--blue-600); - --bg-light-blue: var(--blue-400); + --bg-light-blue: var(--blue-600); --bg-dark-blue: var(--blue-900); - --bg-green: var(--dark-green-500); - --bg-yellow: var(--yellow-500); - --bg-orange: var(--orange-500); - --bg-red: var(--red-500); + --bg-green: var(--green-800); + --bg-yellow: var(--yellow-700); + --bg-orange: var(--orange-700); + --bg-red: var(--red-600); --bg-gray: var(--gray-600); --bg-grey: var(--gray-600); --bg-darkgrey: var(--gray-800); --bg-dark-gray: var(--gray-800); --bg-light-gray: var(--gray-700); --bg-dark-gray: var(--gray-300); - --bg-purple: var(--purple-600); + --bg-purple: var(--purple-700); --text-on-blue: var(--blue-50); - --text-on-light-blue: var(--blue-100); + --text-on-light-blue: var(--blue-50); --text-on-dark-blue: var(--blue-300); --text-on-green: var(--dark-green-50); --text-on-yellow: var(--yellow-50); From ee3a6fab7d61fd2d35d77c2a2be82baeaccef263 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Fri, 20 Jan 2023 19:24:21 +0530 Subject: [PATCH 09/19] fix: Add cyan and pink indicator color for dark theme - fix gray indicator colors on dark... make them lighter to make it more consistent and visible --- frappe/public/scss/desk/dark.scss | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/frappe/public/scss/desk/dark.scss b/frappe/public/scss/desk/dark.scss index 5970fb509c..e1f210c440 100644 --- a/frappe/public/scss/desk/dark.scss +++ b/frappe/public/scss/desk/dark.scss @@ -53,13 +53,14 @@ --bg-yellow: var(--yellow-700); --bg-orange: var(--orange-700); --bg-red: var(--red-600); - --bg-gray: var(--gray-600); - --bg-grey: var(--gray-600); - --bg-darkgrey: var(--gray-800); - --bg-dark-gray: var(--gray-800); + --bg-gray: var(--gray-400); + --bg-grey: var(--gray-400); + --bg-darkgrey: var(--gray-600); + --bg-dark-gray: var(--gray-600); --bg-light-gray: var(--gray-700); - --bg-dark-gray: var(--gray-300); --bg-purple: var(--purple-700); + --bg-pink: var(--pink-700); + --bg-cyan: var(--cyan-800); --text-on-blue: var(--blue-50); --text-on-light-blue: var(--blue-50); @@ -68,12 +69,14 @@ --text-on-yellow: var(--yellow-50); --text-on-orange: var(--orange-100); --text-on-red: var(--red-50); - --text-on-gray: var(--gray-200); - --text-on-grey: var(--gray-200); + --text-on-gray: var(--gray-50); + --text-on-grey: var(--gray-50); --text-on-darkgrey: var(--gray-200); --text-on-dark-gray: var(--gray-200); --text-on-light-gray: var(--gray-100); --text-on-purple: var(--purple-100); + --text-on-pink: var(--pink-100); + --text-on-cyan: var(--cyan-100); // alert colors --alert-text-danger: var(--red-300); From 64cb507fae19b2fe367a4b19d0987e323a9e8652 Mon Sep 17 00:00:00 2001 From: phot0n Date: Wed, 4 Jan 2023 14:18:00 +0530 Subject: [PATCH 10/19] 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 11/19] 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 12/19] 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 13/19] 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 14/19] 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 15/19] 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 16/19] 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", From 0e6e2609b5cae8a23b5553abcb5c7bd2d3b3677a Mon Sep 17 00:00:00 2001 From: Richard Case <110036763+casesolved-co-uk@users.noreply.github.com> Date: Mon, 23 Jan 2023 09:17:36 +0000 Subject: [PATCH 17/19] fix: unhelpful error message (#19666) --- frappe/utils/password.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frappe/utils/password.py b/frappe/utils/password.py index c033f4682b..fa2e03bde5 100644 --- a/frappe/utils/password.py +++ b/frappe/utils/password.py @@ -62,7 +62,10 @@ def get_decrypted_password(doctype, name, fieldname="password", raise_exception= return decrypt(result[0][0]) elif raise_exception: - frappe.throw(_("Password not found"), frappe.AuthenticationError) + frappe.throw( + _("Password not found for {0} {1} {2}").format(doctype, name, fieldname), + frappe.AuthenticationError, + ) def set_encrypted_password(doctype, name, pwd, fieldname="password"): From 6554919f1e2b2108ddeae55a2c698f10c8ab253d Mon Sep 17 00:00:00 2001 From: Anand Baburajan Date: Mon, 23 Jan 2023 15:00:04 +0530 Subject: [PATCH 18/19] fix: improve invalid naming series message (#19711) * fix: show the invalid naming series in special chars error msg * chore: translations [skip ci] --- frappe/model/naming.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/model/naming.py b/frappe/model/naming.py index 93be2204b4..29831451b0 100644 --- a/frappe/model/naming.py +++ b/frappe/model/naming.py @@ -59,8 +59,8 @@ class NamingSeries: if not NAMING_SERIES_PATTERN.match(self.series): frappe.throw( _( - 'Special Characters except "-", "#", ".", "/", "{" and "}" not allowed in naming series', - ), + "Special Characters except '-', '#', '.', '/', '{{' and '}}' not allowed in naming series {0}" + ).format(frappe.bold(self.series)), exc=InvalidNamingSeriesError, ) From 87561940a4cc70032cfc22ee8b5e5d21287e42c9 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 23 Jan 2023 15:04:43 +0530 Subject: [PATCH 19/19] feat: Interactively add a new patch (#19722) --- frappe/commands/utils.py | 11 +++ frappe/tests/test_boilerplate.py | 34 +++++++++- frappe/utils/boilerplate.py | 111 +++++++++++++++++++++++++++++++ 3 files changed, 154 insertions(+), 2 deletions(-) diff --git a/frappe/commands/utils.py b/frappe/commands/utils.py index bb943c7223..f4f2cd8744 100644 --- a/frappe/commands/utils.py +++ b/frappe/commands/utils.py @@ -1030,6 +1030,16 @@ def make_app(destination, app_name, no_git=False): make_boilerplate(destination, app_name, no_git=no_git) +@click.command("create-patch") +def create_patch(): + "Creates a new patch interactively" + from frappe.utils.boilerplate import PatchCreator + + pc = PatchCreator() + pc.fetch_user_inputs() + pc.create_patch_file() + + @click.command("set-config") @click.argument("key") @click.argument("value") @@ -1176,6 +1186,7 @@ commands = [ data_import, import_doc, make_app, + create_patch, mariadb, postgres, request, diff --git a/frappe/tests/test_boilerplate.py b/frappe/tests/test_boilerplate.py index 8dd29b24af..999c74592e 100644 --- a/frappe/tests/test_boilerplate.py +++ b/frappe/tests/test_boilerplate.py @@ -2,22 +2,25 @@ import ast import copy import glob import os +import pathlib import shutil +import unittest from io import StringIO from unittest.mock import patch import yaml import frappe -from frappe.tests.utils import FrappeTestCase +from frappe.modules.patch_handler import get_all_patches from frappe.utils.boilerplate import ( + PatchCreator, _create_app_boilerplate, _get_user_inputs, github_workflow_template, ) -class TestBoilerPlate(FrappeTestCase): +class TestBoilerPlate(unittest.TestCase): @classmethod def setUpClass(cls): super().setUpClass() @@ -180,3 +183,30 @@ class TestBoilerPlate(FrappeTestCase): ast.parse(p.read()) except Exception as e: self.fail(f"Can't parse python file in new app: {python_file}\n" + str(e)) + + def test_new_patch_util(self): + user_inputs = { + "app_name": "frappe", + "doctype": "User", + "docstring": "Delete all users", + "file_name": "", # Accept default + "patch_folder_confirmation": "Y", + } + + patches_txt = pathlib.Path(pathlib.Path(frappe.get_app_path("frappe", "patches.txt"))) + original_patches = patches_txt.read_text() + + with patch("sys.stdin", self.get_user_input_stream(user_inputs)): + patch_creator = PatchCreator() + patch_creator.fetch_user_inputs() + patch_creator.create_patch_file() + + patches = get_all_patches() + expected_patch = "frappe.core.doctype.user.patches.delete_all_users" + self.assertIn(expected_patch, patches) + + self.assertTrue(patch_creator.patch_file.exists()) + + # Cleanup + shutil.rmtree(patch_creator.patch_file.parents[0]) + patches_txt.write_text(original_patches) diff --git a/frappe/utils/boilerplate.py b/frappe/utils/boilerplate.py index 0963d9fabe..ee75c672a8 100644 --- a/frappe/utils/boilerplate.py +++ b/frappe/utils/boilerplate.py @@ -1,9 +1,13 @@ # Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE +import contextlib +import glob +import json import os import pathlib import re +import textwrap import click import git @@ -162,6 +166,113 @@ def _create_github_workflow_files(dest, hooks): f.write(github_workflow_template.format(**hooks)) +PATCH_TEMPLATE = textwrap.dedent( + ''' + import frappe + + def execute(): + """{docstring}""" + + # Write your patch here. + pass +''' +) + + +class PatchCreator: + def __init__(self): + self.all_apps = frappe.get_all_apps(sites_path=".", with_internal_apps=False) + + self.app = None + self.app_dir = None + self.patch_dir = None + self.filename = None + self.docstring = None + self.patch_file = None + + def fetch_user_inputs(self): + self._ask_app_name() + self._ask_doctype_name() + self._ask_patch_meta_info() + + def _ask_app_name(self): + self.app = click.prompt("Select app for new patch", type=click.Choice(self.all_apps)) + self.app_dir = pathlib.Path(frappe.get_app_path(self.app)) + + def _ask_doctype_name(self): + def _doctype_name(filename): + with contextlib.suppress(Exception): + with open(filename) as f: + return json.load(f).get("name") + + doctype_files = list(glob.glob(f"{self.app_dir}/**/doctype/**/*.json")) + doctype_map = {_doctype_name(file): file for file in doctype_files} + doctype_map.pop(None, None) + + doctype = click.prompt( + "Provide DocType name on which this patch will apply", + type=click.Choice(doctype_map.keys()), + show_choices=False, + ) + self.patch_dir = pathlib.Path(doctype_map[doctype]).parents[0] / "patches" + + def _ask_patch_meta_info(self): + self.docstring = click.prompt("Describe what this patch does", type=str) + default_filename = frappe.scrub(self.docstring) + ".py" + + def _valid_filename(name): + if not name: + return + + match name.partition("."): + case filename, ".", "py" if filename.isidentifier(): + return True + case _: + click.echo(f"{name} is not a valid python file name") + + while not _valid_filename(self.filename): + self.filename = click.prompt( + "Provide filename for this patch", type=str, default=default_filename + ) + + def create_patch_file(self): + self._create_parent_folder_if_not_exists() + + self.patch_file = self.patch_dir / self.filename + + if self.patch_file.exists(): + raise Exception(f"Patch {self.patch_file} already exists") + + *path, _filename = self.patch_file.relative_to(self.app_dir.parents[0]).parts + dotted_path = ".".join(path + [self.patch_file.stem]) + + patches_txt = self.app_dir / "patches.txt" + existing_patches = patches_txt.read_text() + + if dotted_path in existing_patches: + raise Exception(f"Patch {dotted_path} is already present in patches.txt") + + self.patch_file.write_text(PATCH_TEMPLATE.format(docstring=self.docstring)) + + with open(patches_txt, "a+") as f: + if not existing_patches.endswith("\n"): + f.write("\n") # ensure EOF + f.write(dotted_path + "\n") + click.echo(f"Created {self.patch_file} and updated patches.txt") + + def _create_parent_folder_if_not_exists(self): + if not self.patch_dir.exists(): + click.confirm( + f"Patch folder '{self.patch_dir}' doesn't exist, create it?", + abort=True, + default=True, + ) + self.patch_dir.mkdir() + + init_py = self.patch_dir / "__init__.py" + init_py.touch() + + manifest_template = """include MANIFEST.in include requirements.txt include *.json