From 99b734bfd7930a624ef4b6af63bc24907e31e3bd Mon Sep 17 00:00:00 2001 From: tundebabzy Date: Wed, 7 Jun 2017 07:32:07 +0100 Subject: [PATCH] Fix #4587: Status does not display "Pending" in report filter (#9104) * adds Material Request to `status_map` * updates eval condition for Partially Ordered in Material Request map * changes material_request doctype to include "pending", "ordered", "partially ordered", "issued", "transferred" as options * adds more options to `validate_status` * adds `set_status` just before saving * adds `check_for_closed_status` in `before_cancel` * adds patch to convert status to material request specific status * adds stricter status update conditions * changes `update_status` to me `set_status` * adds checker such that draft status can only change to pending * renames `check_draft_status` to `status_can_change` * adds Cancelled to Material Request map * makes `status_can_change` block any attempt to change a cancelled document * adds more test cases * updates what `set_status` checks for before adding comment * adds patch to rename the present material request status --- erpnext/controllers/status_updater.py | 13 ++- erpnext/patches.txt | 1 + ...ems_in_status_field_of_material_request.py | 25 ++++++ .../material_request/material_request.json | 28 +++++- .../material_request/material_request.py | 47 ++++++++-- .../material_request/test_material_request.py | 88 +++++++++++++++++++ 6 files changed, 191 insertions(+), 11 deletions(-) create mode 100644 erpnext/patches/v8_0/rename_items_in_status_field_of_material_request.py diff --git a/erpnext/controllers/status_updater.py b/erpnext/controllers/status_updater.py index 42327b94aca..2f54fc01751 100644 --- a/erpnext/controllers/status_updater.py +++ b/erpnext/controllers/status_updater.py @@ -85,6 +85,16 @@ status_map = { ["Completed", "eval:self.per_billed == 100 and self.docstatus == 1"], ["Cancelled", "eval:self.docstatus==2"], ["Closed", "eval:self.status=='Closed'"], + ], + "Material Request": [ + ["Draft", None], + ["Stopped", "eval:self.status == 'Stopped'"], + ["Cancelled", "eval:self.docstatus == 2"], + ["Pending", "eval:self.status != 'Stopped' and self.per_ordered == 0 and self.docstatus == 1"], + ["Partially Ordered", "eval:self.status != 'Stopped' and self.per_ordered < 100 and self.per_ordered > 0 and self.docstatus == 1"], + ["Ordered", "eval:self.status != 'Stopped' and self.per_ordered == 100 and self.docstatus == 1 and self.material_request_type == 'Purchase'"], + ["Transferred", "eval:self.status != 'Stopped' and self.per_ordered == 100 and self.docstatus == 1 and self.material_request_type == 'Material Transfer'"], + ["Issued", "eval:self.status != 'Stopped' and self.per_ordered == 100 and self.docstatus == 1 and self.material_request_type == 'Material Issue'"] ] } @@ -127,7 +137,8 @@ class StatusUpdater(Document): self.status = s[0] break - if self.status != _status and self.status not in ("Submitted", "Cancelled"): + if self.status != _status and self.status not in ("Cancelled", "Partially Ordered", + "Ordered", "Issued", "Transferred"): self.add_comment("Label", _(self.status)) if update: diff --git a/erpnext/patches.txt b/erpnext/patches.txt index 853efa1cc30..0777ab744e2 100644 --- a/erpnext/patches.txt +++ b/erpnext/patches.txt @@ -397,3 +397,4 @@ erpnext.patches.v8_0.rename_total_margin_to_rate_with_margin # 11-05-2017 erpnext.patches.v8_0.fix_status_for_invoices_with_negative_outstanding erpnext.patches.v8_0.make_payments_table_blank_for_non_pos_invoice erpnext.patches.v8_0.delete_schools_depricated_doctypes +erpnext.patches.v8_0.rename_items_in_status_field_of_material_request diff --git a/erpnext/patches/v8_0/rename_items_in_status_field_of_material_request.py b/erpnext/patches/v8_0/rename_items_in_status_field_of_material_request.py new file mode 100644 index 00000000000..5ad862a4362 --- /dev/null +++ b/erpnext/patches/v8_0/rename_items_in_status_field_of_material_request.py @@ -0,0 +1,25 @@ +from __future__ import unicode_literals +import frappe + +def execute(): + frappe.db.sql( + """ + UPDATE `tabMaterial Request` + SET status = CASE + WHEN docstatus = 2 THEN 'Cancelled' + WHEN docstatus = 0 THEN 'Draft' + ELSE CASE + WHEN status = 'Stopped' THEN 'Stopped' + WHEN status != 'Stopped' AND per_ordered = 0 THEN 'Pending' + WHEN per_ordered < 100 AND per_ordered > 0 AND status != 'Stopped' + THEN 'Partially Ordered' + WHEN per_ordered = 100 AND material_request_type = 'Purchase' + AND status != 'Stopped' THEN 'Ordered' + WHEN per_ordered = 100 AND material_request_type = 'Material Transfer' + AND status != 'Stopped' THEN 'Transferred' + WHEN per_ordered = 100 AND material_request_type = 'Material Issue' + AND status != 'Stopped' THEN 'Issued' + END + END + """ + ) \ No newline at end of file diff --git a/erpnext/stock/doctype/material_request/material_request.json b/erpnext/stock/doctype/material_request/material_request.json index 2ab0907fa9d..fc174a46e03 100644 --- a/erpnext/stock/doctype/material_request/material_request.json +++ b/erpnext/stock/doctype/material_request/material_request.json @@ -1,5 +1,6 @@ { "allow_copy": 0, + "allow_guest_to_view": 0, "allow_import": 1, "allow_rename": 0, "autoname": "naming_series:", @@ -12,6 +13,7 @@ "editable_grid": 0, "fields": [ { + "allow_bulk_edit": 0, "allow_on_submit": 0, "bold": 0, "collapsible": 0, @@ -41,6 +43,7 @@ "unique": 0 }, { + "allow_bulk_edit": 0, "allow_on_submit": 1, "bold": 0, "collapsible": 0, @@ -71,6 +74,7 @@ "unique": 0 }, { + "allow_bulk_edit": 0, "allow_on_submit": 0, "bold": 0, "collapsible": 0, @@ -100,6 +104,7 @@ "unique": 0 }, { + "allow_bulk_edit": 0, "allow_on_submit": 0, "bold": 0, "collapsible": 0, @@ -127,6 +132,7 @@ "unique": 0 }, { + "allow_bulk_edit": 0, "allow_on_submit": 0, "bold": 0, "collapsible": 0, @@ -158,6 +164,7 @@ "unique": 0 }, { + "allow_bulk_edit": 0, "allow_on_submit": 0, "bold": 0, "collapsible": 0, @@ -191,6 +198,7 @@ "width": "150px" }, { + "allow_bulk_edit": 0, "allow_on_submit": 0, "bold": 0, "collapsible": 0, @@ -225,6 +233,7 @@ "width": "150px" }, { + "allow_bulk_edit": 0, "allow_on_submit": 0, "bold": 0, "collapsible": 0, @@ -255,6 +264,7 @@ "unique": 0 }, { + "allow_bulk_edit": 0, "allow_on_submit": 0, "bold": 0, "collapsible": 0, @@ -286,6 +296,7 @@ "unique": 0 }, { + "allow_bulk_edit": 0, "allow_on_submit": 0, "bold": 0, "collapsible": 1, @@ -316,6 +327,7 @@ "unique": 0 }, { + "allow_bulk_edit": 0, "allow_on_submit": 0, "bold": 0, "collapsible": 0, @@ -344,6 +356,7 @@ "unique": 0 }, { + "allow_bulk_edit": 0, "allow_on_submit": 0, "bold": 0, "collapsible": 0, @@ -377,6 +390,7 @@ "width": "100px" }, { + "allow_bulk_edit": 0, "allow_on_submit": 0, "bold": 0, "collapsible": 0, @@ -407,6 +421,7 @@ "width": "50%" }, { + "allow_bulk_edit": 0, "allow_on_submit": 0, "bold": 0, "collapsible": 0, @@ -425,7 +440,7 @@ "no_copy": 1, "oldfieldname": "status", "oldfieldtype": "Select", - "options": "\nDraft\nSubmitted\nStopped\nCancelled", + "options": "\nDraft\nSubmitted\nStopped\nCancelled\nPending\nPartially Ordered\nOrdered\nIssued\nTransferred", "permlevel": 0, "print_hide": 1, "print_hide_if_no_value": 0, @@ -440,6 +455,7 @@ "width": "100px" }, { + "allow_bulk_edit": 0, "allow_on_submit": 0, "bold": 0, "collapsible": 0, @@ -471,6 +487,7 @@ "unique": 0 }, { + "allow_bulk_edit": 0, "allow_on_submit": 0, "bold": 0, "collapsible": 1, @@ -500,6 +517,7 @@ "unique": 0 }, { + "allow_bulk_edit": 0, "allow_on_submit": 1, "bold": 0, "collapsible": 0, @@ -531,6 +549,7 @@ "unique": 0 }, { + "allow_bulk_edit": 0, "allow_on_submit": 1, "bold": 0, "collapsible": 0, @@ -560,6 +579,7 @@ "unique": 0 }, { + "allow_bulk_edit": 0, "allow_on_submit": 0, "bold": 0, "collapsible": 1, @@ -591,6 +611,7 @@ "unique": 0 }, { + "allow_bulk_edit": 0, "allow_on_submit": 0, "bold": 0, "collapsible": 0, @@ -622,6 +643,7 @@ "unique": 0 }, { + "allow_bulk_edit": 0, "allow_on_submit": 0, "bold": 0, "collapsible": 0, @@ -652,19 +674,19 @@ "unique": 0 } ], + "has_web_view": 0, "hide_heading": 0, "hide_toolbar": 0, "icon": "fa fa-ticket", "idx": 70, "image_view": 0, "in_create": 0, - "in_dialog": 0, "is_submittable": 1, "issingle": 0, "istable": 0, "max_attachments": 0, "menu_index": 0, - "modified": "2017-02-20 13:29:56.743544", + "modified": "2017-05-31 15:06:44.611826", "modified_by": "Administrator", "module": "Stock", "name": "Material Request", diff --git a/erpnext/stock/doctype/material_request/material_request.py b/erpnext/stock/doctype/material_request/material_request.py index 82c4c19fdf2..65263a0694c 100644 --- a/erpnext/stock/doctype/material_request/material_request.py +++ b/erpnext/stock/doctype/material_request/material_request.py @@ -70,7 +70,9 @@ class MaterialRequest(BuyingController): self.status = "Draft" from erpnext.controllers.status_updater import validate_status - validate_status(self.status, ["Draft", "Submitted", "Stopped", "Cancelled"]) + validate_status(self.status, ["Draft", "Submitted", "Stopped", "Cancelled", "Pending", + "Partially Ordered", "Ordered", "Issued", "Transferred"] + ) validate_for_items(self) @@ -91,9 +93,20 @@ class MaterialRequest(BuyingController): self.title = ', '.join(items) def on_submit(self): - frappe.db.set(self, 'status', 'Submitted') + # frappe.db.set(self, 'status', 'Submitted') self.update_requested_qty() + def before_save(self): + self.set_status(update=True) + + def before_submit(self): + self.set_status(update=True) + + def before_cancel(self): + # if MRQ is already closed, no point saving the document + check_for_closed_status(self.doctype, self.name) + self.set_status(update=True, status='Cancelled') + def check_modified_date(self): mod_db = frappe.db.sql("""select modified from `tabMaterial Request` where name = %s""", self.name) @@ -105,16 +118,36 @@ class MaterialRequest(BuyingController): def update_status(self, status): self.check_modified_date() - frappe.db.set(self, 'status', cstr(status)) + self.status_can_change(status) + self.set_status(update=True, status=status) self.update_requested_qty() + def status_can_change(self, status): + """ + validates that `status` is acceptable for the present controller status + and throws an Exception if otherwise. + """ + if self.status and self.status == 'Cancelled': + # cancelled documents cannot change + if status != self.status: + frappe.throw( + _("{0} {1} is cancelled so the action cannot be completed"). + format(_(self.doctype), self.name), + frappe.InvalidStatusError + ) + + elif self.status and self.status == 'Draft': + # draft document to pending only + if status != 'Pending': + frappe.throw( + _("{0} {1} has not been submitted so the action cannot be completed"). + format(_(self.doctype), self.name), + frappe.InvalidStatusError + ) + def on_cancel(self): - check_for_closed_status(self.doctype, self.name) - self.update_requested_qty() - frappe.db.set(self,'status','Cancelled') - def update_completed_qty(self, mr_items=None, update_modified=True): if self.material_request_type == "Purchase": return diff --git a/erpnext/stock/doctype/material_request/test_material_request.py b/erpnext/stock/doctype/material_request/test_material_request.py index 8f43acdc7b6..c3a2137412b 100644 --- a/erpnext/stock/doctype/material_request/test_material_request.py +++ b/erpnext/stock/doctype/material_request/test_material_request.py @@ -98,6 +98,94 @@ class TestMaterialRequest(unittest.TestCase): se.insert() se.submit() + def test_cannot_stop_cancelled_material_request(self): + mr = frappe.copy_doc(test_records[0]) + mr.insert() + mr.submit() + + mr.load_from_db() + mr.cancel() + self.assertRaises(frappe.ValidationError, mr.update_status, 'Stopped') + + def test_mr_changes_from_stopped_to_pending_after_reopen(self): + mr = frappe.copy_doc(test_records[0]) + mr.insert() + mr.submit() + self.assertEqual('Pending', mr.status) + + mr.update_status('Stopped') + self.assertEqual('Stopped', mr.status) + + mr.update_status('Submitted') + self.assertEqual('Pending', mr.status) + + def test_cannot_submit_cancelled_mr(self): + mr = frappe.copy_doc(test_records[0]) + mr.insert() + mr.submit() + mr.load_from_db() + mr.cancel() + self.assertRaises(frappe.ValidationError, mr.submit) + + def test_mr_changes_from_pending_to_cancelled_after_cancel(self): + mr = frappe.copy_doc(test_records[0]) + mr.insert() + mr.submit() + mr.cancel() + self.assertEqual('Cancelled', mr.status) + + def test_cannot_change_cancelled_mr(self): + mr = frappe.copy_doc(test_records[0]) + mr.insert() + mr.submit() + mr.load_from_db() + mr.cancel() + + self.assertRaises(frappe.InvalidStatusError, mr.update_status, 'Draft') + self.assertRaises(frappe.InvalidStatusError, mr.update_status, 'Stopped') + self.assertRaises(frappe.InvalidStatusError, mr.update_status, 'Ordered') + self.assertRaises(frappe.InvalidStatusError, mr.update_status, 'Issued') + self.assertRaises(frappe.InvalidStatusError, mr.update_status, 'Transferred') + self.assertRaises(frappe.InvalidStatusError, mr.update_status, 'Pending') + + def test_cannot_submit_deleted_material_request(self): + mr = frappe.copy_doc(test_records[0]) + mr.insert() + mr.delete() + + self.assertRaises(frappe.ValidationError, mr.submit) + + def test_cannot_delete_submitted_mr(self): + mr = frappe.copy_doc(test_records[0]) + mr.insert() + mr.submit() + + self.assertRaises(frappe.ValidationError, mr.delete) + + def test_stopped_mr_changes_to_pending_after_reopen(self): + mr = frappe.copy_doc(test_records[0]) + mr.insert() + mr.submit() + mr.load_from_db() + + mr.update_status('Stopped') + mr.update_status('Submitted') + self.assertEqual(mr.status, 'Pending') + + def test_pending_mr_changes_to_stopped_after_stop(self): + mr = frappe.copy_doc(test_records[0]) + mr.insert() + mr.submit() + mr.load_from_db() + + mr.update_status('Stopped') + self.assertEqual(mr.status, 'Stopped') + + def test_cannot_stop_unsubmitted_mr(self): + mr = frappe.copy_doc(test_records[0]) + mr.insert() + self.assertRaises(frappe.InvalidStatusError, mr.update_status, 'Stopped') + def test_completed_qty_for_purchase(self): existing_requested_qty_item1 = self._get_requested_qty("_Test Item Home Desktop 100", "_Test Warehouse - _TC") existing_requested_qty_item2 = self._get_requested_qty("_Test Item Home Desktop 200", "_Test Warehouse - _TC")