From b274c45992419aee34dfb9757bc65a24e38e63c8 Mon Sep 17 00:00:00 2001 From: Rutwik Hiwalkar Date: Tue, 16 Apr 2024 19:46:34 +0530 Subject: [PATCH 01/10] feat: discard draft transactions --- frappe/desk/form/save.py | 11 ++++++ frappe/model/document.py | 15 ++++++-- frappe/public/js/frappe/form/form.js | 46 +++++++++++++++++++++++++ frappe/public/js/frappe/form/save.js | 17 +++++++++ frappe/public/js/frappe/form/toolbar.js | 10 ++++++ 5 files changed, 96 insertions(+), 3 deletions(-) diff --git a/frappe/desk/form/save.py b/frappe/desk/form/save.py index 795a7deeb9..1751113df7 100644 --- a/frappe/desk/form/save.py +++ b/frappe/desk/form/save.py @@ -59,6 +59,17 @@ def cancel(doctype=None, name=None, workflow_state_fieldname=None, workflow_stat frappe.msgprint(frappe._("Cancelled"), indicator="red", alert=True) +@frappe.whitelist() +def discard(doctype=None, name=None): + """discard a doclist""" + doc = frappe.get_doc(doctype, name) + capture_doc(doc, "Discard") + + doc.discard() + send_updated_docs(doc) + frappe.msgprint(frappe._("Discarded"), indicator="red", alert=True) + + def send_updated_docs(doc): from .load import get_docinfo diff --git a/frappe/model/document.py b/frappe/model/document.py index 17fcaf50da..a6a11a07cc 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -848,9 +848,7 @@ class Document(BaseDocument): self._action = "submit" self.check_permission("submit") elif self.docstatus.is_cancelled(): - raise frappe.DocstatusTransitionError( - _("Cannot change docstatus from 0 (Draft) to 2 (Cancelled)") - ) + self.check_permission("cancel") else: raise frappe.ValidationError(_("Invalid docstatus"), self.docstatus) @@ -1058,6 +1056,17 @@ class Document(BaseDocument): """Cancel the document. Sets `docstatus` = 2, then saves.""" return self._cancel() + @frappe.whitelist() + def discard(self): + """Discard the draft document. Sets `docstatus` = 2, then saves.""" + self.check_if_locked() + self.set_user_and_timestamp() + self.check_if_latest() + + self.run_method("before_discard") + self.db_set("docstatus", DocStatus.cancelled()) + self.run_method("on_discard") + @frappe.whitelist() def rename(self, name: str, merge=False, force=False, validate_rename=True): """Rename the document to `name`. This transforms the current object.""" diff --git a/frappe/public/js/frappe/form/form.js b/frappe/public/js/frappe/form/form.js index 854d60d907..52cd945eea 100644 --- a/frappe/public/js/frappe/form/form.js +++ b/frappe/public/js/frappe/form/form.js @@ -833,6 +833,18 @@ frappe.ui.form.Form = class FrappeForm { } } + discard(btn, callback, on_error) { + var me = this; + return new Promise((resolve) => { + // this.validate_form_action("Discard") // ? + frappe.confirm(__("Discard {0}", [this.docname]), function () { + me.script_manager.trigger("before_discard").then(function () { + return me._discard(btn, callback, on_error, false); // ? + }); + }); + }); + } + savesubmit(btn, callback, on_error) { var me = this; return new Promise((resolve) => { @@ -1015,6 +1027,40 @@ frappe.ui.form.Form = class FrappeForm { } } + _discard(btn, on_error, skip_confirm) { + const me = this; + const discard_doc = () => { + frappe.validated = true; + me.script_manager.trigger("before_discard").then(() => { + if (!frappe.validated) { + return me.handle_save_fail(btn, on_error); + } + + var after_discard = function (r) { + if (r.exc) { + me.handle_save_fail(btn, on_error); + } else { + frappe.utils.play_sound("cancel"); + me.refresh(); + me.script_manager.trigger("after_discard"); + } + me.reload_doc(); + }; + frappe.ui.form.discard(me, after_discard, btn); + }); + }; + + if (skip_confirm) { + discard_doc(); + } else { + frappe.confirm( + __("Permanently Discard {0}?", [this.docname]), + discard_doc, + me.handle_save_fail(btn, on_error) + ); + } + } + savetrash() { this.validate_form_action("Delete"); frappe.model.delete_doc(this.doctype, this.docname, function () { diff --git a/frappe/public/js/frappe/form/save.js b/frappe/public/js/frappe/form/save.js index 157767d20d..c4e9903c8e 100644 --- a/frappe/public/js/frappe/form/save.js +++ b/frappe/public/js/frappe/form/save.js @@ -274,6 +274,23 @@ frappe.ui.form.save = function (frm, action, callback, btn) { } }; +frappe.ui.form.discard = function (frm, callback, btn) { + var args = { + doctype: frm.doc.doctype, + name: frm.doc.name, + }; + + return frappe.call({ + freeze: true, + method: "frappe.desk.form.save.discard", + args: args, + btn: btn, + callback: function (r) { + callback && callback(r); + }, + }); +}; + frappe.ui.form.remove_old_form_route = () => { let current_route = frappe.get_route().join("/"); frappe.route_history = frappe.route_history.filter( diff --git a/frappe/public/js/frappe/form/toolbar.js b/frappe/public/js/frappe/form/toolbar.js index 0fc3b114fb..28c8c02cf9 100644 --- a/frappe/public/js/frappe/form/toolbar.js +++ b/frappe/public/js/frappe/form/toolbar.js @@ -310,6 +310,16 @@ frappe.ui.form.Toolbar = class Toolbar { const allow_print_for_draft = cint(print_settings.allow_print_for_draft); const allow_print_for_cancelled = cint(print_settings.allow_print_for_cancelled); + if (is_submittable && docstatus == 0) { + this.page.add_menu_item( + __("Discard"), + function () { + me.frm._discard(); + }, + true + ); + } + if ( !is_submittable || docstatus == 1 || From b6418c6903d4227be00e07efa9b0408ebdfc469f Mon Sep 17 00:00:00 2001 From: Rutwik Hiwalkar Date: Tue, 16 Apr 2024 19:48:57 +0530 Subject: [PATCH 02/10] chore: add discard events for server script --- frappe/core/doctype/server_script/server_script.json | 4 ++-- frappe/core/doctype/server_script/server_script.py | 2 ++ frappe/core/doctype/server_script/server_script_utils.py | 2 ++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/server_script/server_script.json b/frappe/core/doctype/server_script/server_script.json index a88bc99f0a..4e5d0d606b 100644 --- a/frappe/core/doctype/server_script/server_script.json +++ b/frappe/core/doctype/server_script/server_script.json @@ -57,7 +57,7 @@ "fieldname": "doctype_event", "fieldtype": "Select", "label": "DocType Event", - "options": "Before Insert\nBefore Validate\nBefore Save\nAfter Insert\nAfter Save\nBefore Rename\nAfter Rename\nBefore Submit\nAfter Submit\nBefore Cancel\nAfter Cancel\nBefore Delete\nAfter Delete\nBefore Save (Submitted Document)\nAfter Save (Submitted Document)\nBefore Print\nOn Payment Authorization\nOn Payment Paid\nOn Payment Failed" + "options": "Before Insert\nBefore Validate\nBefore Save\nAfter Insert\nAfter Save\nBefore Rename\nAfter Rename\nBefore Submit\nAfter Submit\nBefore Cancel\nAfter Cancel\nBefore Discard\nAfter Discard\nBefore Delete\nAfter Delete\nBefore Save (Submitted Document)\nAfter Save (Submitted Document)\nBefore Print\nOn Payment Authorization\nOn Payment Paid\nOn Payment Failed" }, { "depends_on": "eval:doc.script_type==='API'", @@ -151,7 +151,7 @@ "link_fieldname": "server_script" } ], - "modified": "2024-04-08 16:18:52.901097", + "modified": "2024-04-15 20:12:41.971315", "modified_by": "Administrator", "module": "Core", "name": "Server Script", diff --git a/frappe/core/doctype/server_script/server_script.py b/frappe/core/doctype/server_script/server_script.py index f5b4e518bf..d3af5d1fc6 100644 --- a/frappe/core/doctype/server_script/server_script.py +++ b/frappe/core/doctype/server_script/server_script.py @@ -42,6 +42,8 @@ class ServerScript(Document): "After Submit", "Before Cancel", "After Cancel", + "Before Discard", + "After Discard", "Before Delete", "After Delete", "Before Save (Submitted Document)", diff --git a/frappe/core/doctype/server_script/server_script_utils.py b/frappe/core/doctype/server_script/server_script_utils.py index ebc5fe9e9d..418f3aea83 100644 --- a/frappe/core/doctype/server_script/server_script_utils.py +++ b/frappe/core/doctype/server_script/server_script_utils.py @@ -15,6 +15,8 @@ EVENT_MAP = { "on_submit": "After Submit", "before_cancel": "Before Cancel", "on_cancel": "After Cancel", + "before_discard": "Before Discard", + "on_discard": "After Discard", "on_trash": "Before Delete", "after_delete": "After Delete", "before_update_after_submit": "Before Save (Submitted Document)", From c341360e363c002301f912c8bde984b8f9445946 Mon Sep 17 00:00:00 2001 From: Rutwik Hiwalkar Date: Tue, 16 Apr 2024 20:36:04 +0530 Subject: [PATCH 03/10] test: add test for discard action --- frappe/tests/test_document.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/frappe/tests/test_document.py b/frappe/tests/test_document.py index dd76970903..2dff349b1f 100644 --- a/frappe/tests/test_document.py +++ b/frappe/tests/test_document.py @@ -98,6 +98,13 @@ class TestDocument(FrappeTestCase): self.assertEqual(frappe.db.get_value(d.doctype, d.name, "subject"), "subject changed") + def test_discard(self): + d = self.test_insert() + self.assertEqual(d.docstatus, 0) + + d.discard() + self.assertEqual(d.docstatus, 2) + def test_value_changed(self): d = self.test_insert() d.subject = "subject changed again" From 82d61b32e4d88aa360997e0dd3be803edca26398 Mon Sep 17 00:00:00 2001 From: Rutwik Hiwalkar Date: Wed, 17 Apr 2024 12:32:34 +0530 Subject: [PATCH 04/10] fix: misc fixes for discard action * use write perms instead of cancel * update docstring * remove discard from global namespace --- frappe/desk/form/save.py | 4 ++-- frappe/model/document.py | 2 +- frappe/public/js/frappe/form/form.js | 16 ++++++++++++++-- frappe/public/js/frappe/form/save.js | 17 ----------------- 4 files changed, 17 insertions(+), 22 deletions(-) diff --git a/frappe/desk/form/save.py b/frappe/desk/form/save.py index 1751113df7..b531559c25 100644 --- a/frappe/desk/form/save.py +++ b/frappe/desk/form/save.py @@ -60,8 +60,8 @@ def cancel(doctype=None, name=None, workflow_state_fieldname=None, workflow_stat @frappe.whitelist() -def discard(doctype=None, name=None): - """discard a doclist""" +def discard(doctype: str, name: str | int): + """dicard a draft document""" doc = frappe.get_doc(doctype, name) capture_doc(doc, "Discard") diff --git a/frappe/model/document.py b/frappe/model/document.py index a6a11a07cc..ffba894e2a 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -848,7 +848,7 @@ class Document(BaseDocument): self._action = "submit" self.check_permission("submit") elif self.docstatus.is_cancelled(): - self.check_permission("cancel") + self.check_permission("write") else: raise frappe.ValidationError(_("Invalid docstatus"), self.docstatus) diff --git a/frappe/public/js/frappe/form/form.js b/frappe/public/js/frappe/form/form.js index 52cd945eea..c415cb09ce 100644 --- a/frappe/public/js/frappe/form/form.js +++ b/frappe/public/js/frappe/form/form.js @@ -834,7 +834,7 @@ frappe.ui.form.Form = class FrappeForm { } discard(btn, callback, on_error) { - var me = this; + const me = this; return new Promise((resolve) => { // this.validate_form_action("Discard") // ? frappe.confirm(__("Discard {0}", [this.docname]), function () { @@ -1046,7 +1046,19 @@ frappe.ui.form.Form = class FrappeForm { } me.reload_doc(); }; - frappe.ui.form.discard(me, after_discard, btn); + //frappe.ui.form.discard(me, after_discard, btn); + frappe.call({ + freeze: true, + method: "frappe.desk.form.save.discard", + args: { + doctype: me.doc.doctype, + name: me.doc.name, + }, + btn: btn, + callback: function (r) { + after_discard(r); + }, + }); }); }; diff --git a/frappe/public/js/frappe/form/save.js b/frappe/public/js/frappe/form/save.js index c4e9903c8e..157767d20d 100644 --- a/frappe/public/js/frappe/form/save.js +++ b/frappe/public/js/frappe/form/save.js @@ -274,23 +274,6 @@ frappe.ui.form.save = function (frm, action, callback, btn) { } }; -frappe.ui.form.discard = function (frm, callback, btn) { - var args = { - doctype: frm.doc.doctype, - name: frm.doc.name, - }; - - return frappe.call({ - freeze: true, - method: "frappe.desk.form.save.discard", - args: args, - btn: btn, - callback: function (r) { - callback && callback(r); - }, - }); -}; - frappe.ui.form.remove_old_form_route = () => { let current_route = frappe.get_route().join("/"); frappe.route_history = frappe.route_history.filter( From 5335d6c19cdbb93a4b3d576b896b690b0f061a2d Mon Sep 17 00:00:00 2001 From: Rutwik Hiwalkar Date: Wed, 17 Apr 2024 20:51:04 +0530 Subject: [PATCH 05/10] chore: revert transition rule for 0 to 2 doing explicit transition check for discard because, * there's only one transition check that is required * draft(0) > cancelled(2) and submitted(1) > cancelled(2) are valid checkes for save so it doesn't make sense editing check_docstatus_transition --- frappe/model/document.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/frappe/model/document.py b/frappe/model/document.py index ffba894e2a..478f7e4dda 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -848,7 +848,9 @@ class Document(BaseDocument): self._action = "submit" self.check_permission("submit") elif self.docstatus.is_cancelled(): - self.check_permission("write") + raise frappe.DocstatusTransitionError( + _("Cannot change docstatus from 0 (Draft) to 2 (Cancelled)") + ) else: raise frappe.ValidationError(_("Invalid docstatus"), self.docstatus) @@ -1058,10 +1060,14 @@ class Document(BaseDocument): @frappe.whitelist() def discard(self): - """Discard the draft document. Sets `docstatus` = 2, then saves.""" + """Discard the draft document. Sets `docstatus` = 2 with db_set.""" self.check_if_locked() self.set_user_and_timestamp() - self.check_if_latest() + + if not self.docstatus == DocStatus.draft(): + raise frappe.ValidationError(_("Only draft documents can be discarded"), self.docstatus) + + self.check_permission("write") self.run_method("before_discard") self.db_set("docstatus", DocStatus.cancelled()) From b1e6f691a75589f59d3e2042fc369703765ce779 Mon Sep 17 00:00:00 2001 From: Rutwik Hiwalkar Date: Wed, 17 Apr 2024 20:54:15 +0530 Subject: [PATCH 06/10] test: add more tests for discard --- frappe/tests/test_document.py | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/frappe/tests/test_document.py b/frappe/tests/test_document.py index 2dff349b1f..5f7c0a5efb 100644 --- a/frappe/tests/test_document.py +++ b/frappe/tests/test_document.py @@ -98,12 +98,36 @@ class TestDocument(FrappeTestCase): self.assertEqual(frappe.db.get_value(d.doctype, d.name, "subject"), "subject changed") - def test_discard(self): + def test_discard_transitions(self): d = self.test_insert() self.assertEqual(d.docstatus, 0) - d.discard() - self.assertEqual(d.docstatus, 2) + # invalid: Submit > Discard, Cancel > Discard + d.submit() + self.assertRaises(frappe.ValidationError, d.discard) + d.reload() + + d.cancel() + self.assertRaises(frappe.ValidationError, d.discard) + + # valid: Draft > Discard + d2 = self.test_insert() + d2.discard() + self.assertEqual(d2.docstatus, 2) + + def test_save_on_discard_throws(self): + from frappe.desk.doctype.event.event import Event + + d3 = self.test_insert() + + def test_on_discard(d3): + d3.subject = d3.subject + "update" + d3.save() + + d3.on_discard = (test_on_discard)(d3) + d3.on_discard = test_on_discard.__get__(d3, Event) + + self.assertRaises(frappe.ValidationError, d3.discard) def test_value_changed(self): d = self.test_insert() From c195b3b64939321dc394cc0b8706c9f21a89a0f8 Mon Sep 17 00:00:00 2001 From: Rutwik Hiwalkar Date: Wed, 17 Apr 2024 22:05:46 +0530 Subject: [PATCH 07/10] fix: typo --- frappe/desk/form/save.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/desk/form/save.py b/frappe/desk/form/save.py index b531559c25..49a060ad46 100644 --- a/frappe/desk/form/save.py +++ b/frappe/desk/form/save.py @@ -61,7 +61,7 @@ def cancel(doctype=None, name=None, workflow_state_fieldname=None, workflow_stat @frappe.whitelist() def discard(doctype: str, name: str | int): - """dicard a draft document""" + """discard a draft document""" doc = frappe.get_doc(doctype, name) capture_doc(doc, "Discard") From 9492f88018db423ba32ec5841addfa5aab10cd92 Mon Sep 17 00:00:00 2001 From: Rutwik Hiwalkar Date: Thu, 18 Apr 2024 15:03:54 +0530 Subject: [PATCH 08/10] chore: hide discard button if workflow is active and reuse frappe.model.has_workflow --- frappe/public/js/frappe/form/toolbar.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/frappe/public/js/frappe/form/toolbar.js b/frappe/public/js/frappe/form/toolbar.js index 28c8c02cf9..1183d04d03 100644 --- a/frappe/public/js/frappe/form/toolbar.js +++ b/frappe/public/js/frappe/form/toolbar.js @@ -310,7 +310,7 @@ frappe.ui.form.Toolbar = class Toolbar { const allow_print_for_draft = cint(print_settings.allow_print_for_draft); const allow_print_for_cancelled = cint(print_settings.allow_print_for_cancelled); - if (is_submittable && docstatus == 0) { + if (is_submittable && docstatus == 0 && !this.has_workflow()) { this.page.add_menu_item( __("Discard"), function () { @@ -594,9 +594,7 @@ frappe.ui.form.Toolbar = class Toolbar { } has_workflow() { if (this._has_workflow === undefined) - this._has_workflow = frappe.get_list("Workflow", { - document_type: this.frm.doctype, - }).length; + this._has_workflow = frappe.model.has_workflow(this.frm.doctype); return this._has_workflow; } get_docstatus() { From fa18de6302b26f384e82769ce6d1109b1d98d31e Mon Sep 17 00:00:00 2001 From: Rutwik Hiwalkar Date: Tue, 23 Apr 2024 20:35:36 +0530 Subject: [PATCH 09/10] chore: check_if_latest for discard action --- frappe/model/document.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/frappe/model/document.py b/frappe/model/document.py index 478f7e4dda..92e8af2cdc 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -809,11 +809,12 @@ class Document(BaseDocument): self.load_doc_before_save(raise_exception=True) - self._action = "save" - previous = self._doc_before_save + if not hasattr(self, "_action"): + self._action = "save" + previous = self._doc_before_save # previous is None for new document insert - if not previous: + if not previous and self._action != "discard": self.check_docstatus_transition(0) return @@ -825,7 +826,7 @@ class Document(BaseDocument): raise_exception=frappe.TimestampMismatchError, ) - if not self.meta.issingle: + if not self.meta.issingle and self._action != "discard": self.check_docstatus_transition(previous.docstatus) def check_docstatus_transition(self, to_docstatus): @@ -1061,8 +1062,11 @@ class Document(BaseDocument): @frappe.whitelist() def discard(self): """Discard the draft document. Sets `docstatus` = 2 with db_set.""" + self._action = "discard" + self.check_if_locked() self.set_user_and_timestamp() + self.check_if_latest() if not self.docstatus == DocStatus.draft(): raise frappe.ValidationError(_("Only draft documents can be discarded"), self.docstatus) From 657faea60d8dfe570c37d089b6bca5453e9ea0da Mon Sep 17 00:00:00 2001 From: Rutwik Hiwalkar Date: Tue, 23 Apr 2024 20:36:02 +0530 Subject: [PATCH 10/10] chore: drop dead comment --- frappe/model/document.py | 1 + frappe/public/js/frappe/form/form.js | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/model/document.py b/frappe/model/document.py index 92e8af2cdc..f9c9ae7ae1 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -1075,6 +1075,7 @@ class Document(BaseDocument): self.run_method("before_discard") self.db_set("docstatus", DocStatus.cancelled()) + delattr(self, "_action") self.run_method("on_discard") @frappe.whitelist() diff --git a/frappe/public/js/frappe/form/form.js b/frappe/public/js/frappe/form/form.js index c415cb09ce..252010d36f 100644 --- a/frappe/public/js/frappe/form/form.js +++ b/frappe/public/js/frappe/form/form.js @@ -836,7 +836,6 @@ frappe.ui.form.Form = class FrappeForm { discard(btn, callback, on_error) { const me = this; return new Promise((resolve) => { - // this.validate_form_action("Discard") // ? frappe.confirm(__("Discard {0}", [this.docname]), function () { me.script_manager.trigger("before_discard").then(function () { return me._discard(btn, callback, on_error, false); // ?