Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(stock): don't reserve qty on sales return. #31271

Merged
merged 25 commits into from
Apr 4, 2023
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
695e2bc
fix: don't reserve qty on sales return.
dj12djdjs May 14, 2022
1d1c975
Merge remote-tracking branch 'upstream/develop' into fix-reserve-qty
dj12djdjs May 16, 2022
47f867b
test: don't reserve qty on sales return.
dj12djdjs May 16, 2022
4d0fa85
fix(test): update_prevdoc_status
dj12djdjs May 16, 2022
5a402a5
fix(test): use unique item in sales order.
dj12djdjs May 17, 2022
f39422a
Merge branch 'develop' into fix-reserve-qty
dj12djdjs Jun 17, 2022
d259c2e
fix: merge conflict
dj12djdjs Jun 17, 2022
f3f26d2
refactor: get_reserved_qty query builder
dj12djdjs Jun 17, 2022
494bbf0
feat: add checkbox to reserve qty on sales return
dj12djdjs Jun 17, 2022
7855a59
Merge branch 'develop' into fix-reserve-qty
dj12djdjs Jun 17, 2022
0328874
fix: syntax missing )
dj12djdjs Jun 17, 2022
591b591
fix: re-assign after append query
dj12djdjs Jun 17, 2022
179e2d2
fix: reset selling setting
dj12djdjs Jun 17, 2022
6a6c560
fix: move to transcation settings
dj12djdjs Jun 17, 2022
199b8dd
Merge branch 'develop' into fix-reserve-qty
dj12djdjs Jun 22, 2022
ef64808
chore: linter
dj12djdjs Jun 22, 2022
5685492
Merge remote-tracking branch 'frappe/develop' into fix-reserve-qty
dj12djdjs Feb 9, 2023
4c5147f
Merge: upstream/develop into pps190/fix-reserve-qty
dj12djdjs Feb 14, 2023
46afbb5
Merge branch 'develop' into fix-reserve-qty
s-aga-r Feb 15, 2023
2959285
Merge branch 'develop' into fix-reserve-qty
s-aga-r Feb 19, 2023
6190b4c
chore: revert unrelated changes.
dj12djdjs Mar 3, 2023
1142e69
Merge branch 'develop' into fix-reserve-qty
Mar 25, 2023
17f8080
chore: revert get_reserve_qty method
dj12djdjs Mar 25, 2023
3c553b0
feat: don't reserve qty on sales return
dj12djdjs Mar 27, 2023
edb0666
Merge branch 'develop' into fix-reserve-qty
rohitwaghchaure Mar 30, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
"allow_multiple_items",
"allow_against_multiple_purchase_orders",
"allow_sales_order_creation_for_expired_quotation",
"dont_reserve_sales_order_qty_on_sales_return",
"hide_tax_id",
"enable_discount_accounting"
],
Expand Down Expand Up @@ -179,6 +180,12 @@
"fieldname": "allow_sales_order_creation_for_expired_quotation",
"fieldtype": "Check",
"label": "Allow Sales Order Creation For Expired Quotation"
},
{
"default": "0",
"fieldname": "dont_reserve_sales_order_qty_on_sales_return",
"fieldtype": "Check",
"label": "Don't Reserve Sales Order Qty on Sales Return"
}
],
"icon": "fa fa-cog",
Expand Down Expand Up @@ -215,4 +222,4 @@
"sort_order": "DESC",
"states": [],
"track_changes": 1
}
}
47 changes: 47 additions & 0 deletions erpnext/stock/doctype/delivery_note/test_delivery_note.py
Original file line number Diff line number Diff line change
Expand Up @@ -1180,6 +1180,53 @@ def test_batch_expiry_for_delivery_note(self):

self.assertTrue(return_dn.docstatus == 1)

def test_reserve_qty_on_sales_return(self):
frappe.db.set_single_value("Selling Settings", "dont_reserve_sales_order_qty_on_sales_return", 0)
self.reserved_qty_check()

def test_dont_reserve_qty_on_sales_return(self):
frappe.db.set_single_value("Selling Settings", "dont_reserve_sales_order_qty_on_sales_return", 1)
self.reserved_qty_check()

def reserved_qty_check(self):
from erpnext.controllers.sales_and_purchase_return import make_return_doc
from erpnext.selling.doctype.sales_order.sales_order import make_delivery_note
from erpnext.stock.stock_balance import get_reserved_qty

dont_reserve_qty = frappe.db.get_single_value(
"Selling Settings", "dont_reserve_sales_order_qty_on_sales_return"
)

item = make_item().name
warehouse = "_Test Warehouse - _TC"
qty_to_reserve = 5

so = make_sales_order(item_code=item, qty=qty_to_reserve)

# Make qty avl for test.
make_stock_entry(item_code=item, to_warehouse=warehouse, qty=10, basic_rate=100)

# Test that item qty has been reserved on submit of sales order.
self.assertEqual(get_reserved_qty(item, warehouse), qty_to_reserve)

dn = make_delivery_note(so.name)
dn.save().submit()

# Test that item qty is no longer reserved since qty has been delivered.
self.assertEqual(get_reserved_qty(item, warehouse), 0)

dn_return = make_return_doc("Delivery Note", dn.name)
dn_return.save().submit()

returned = frappe.get_doc("Delivery Note", dn_return.name)
returned.update_prevdoc_status()

# Test that item qty is not reserved on sales return, if selling setting don't reserve qty is checked.
self.assertEqual(get_reserved_qty(item, warehouse), 0 if dont_reserve_qty else qty_to_reserve)

def tearDown(self):
frappe.db.set_single_value("Selling Settings", "dont_reserve_sales_order_qty_on_sales_return", 0)


def create_delivery_note(**args):
dn = frappe.new_doc("Delivery Note")
Expand Down
122 changes: 80 additions & 42 deletions erpnext/stock/stock_balance.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@


import frappe
from frappe.query_builder import DocType
from frappe.query_builder.functions import Sum
from frappe.query_builder.utils import Table
from frappe.utils import cstr, flt, now, nowdate, nowtime
from pypika.queries import QueryBuilder

from erpnext.controllers.stock_controller import create_repost_item_valuation_entry

Expand Down Expand Up @@ -94,51 +98,85 @@ def get_balance_qty_from_sle(item_code, warehouse):


def get_reserved_qty(item_code, warehouse):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function looks very complex and difficult to read, could you split the code into multiple methods?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rohitwaghchaure Honestly, I don't even fully understand the query. I just reverted it back to the previous query and used f string to inject the condition.

reserved_qty = frappe.db.sql(
"""
select
sum(dnpi_qty * ((so_item_qty - so_item_delivered_qty) / so_item_qty))
from
(
(select
qty as dnpi_qty,
(
select qty from `tabSales Order Item`
where name = dnpi.parent_detail_docname
and (delivered_by_supplier is null or delivered_by_supplier = 0)
) as so_item_qty,
(
select delivered_qty from `tabSales Order Item`
where name = dnpi.parent_detail_docname
and delivered_by_supplier = 0
) as so_item_delivered_qty,
parent, name
from
def append_open_so_query(q: QueryBuilder, child_table: Table) -> QueryBuilder:
return (
q.inner_join(SalesOrder)
.on(SalesOrder.name == child_table.parent)
.where(SalesOrder.docstatus == 1)
.where(SalesOrder.status.notin(["On Hold", "Closed"]))
)

SalesOrder = DocType("Sales Order")
SalesOrderItem = DocType("Sales Order Item")
PackedItem = DocType("Packed Item")

dont_reserve_qty_on_sales_return = frappe.db.get_single_value(
"Selling Settings", "dont_reserve_sales_order_qty_on_sales_return"
)

tab = (
frappe.qb.from_(SalesOrderItem)
.where(SalesOrderItem.item_code == item_code)
.where(SalesOrderItem.warehouse == warehouse)
)
for field, cond in [
(SalesOrderItem.stock_qty.as_("dnpi_qty"), 1),
(SalesOrderItem.qty.as_("so_item_qty"), 1),
(SalesOrderItem.delivered_qty.as_("so_item_delivered_qty"), 1),
(SalesOrderItem.returned_qty.as_("so_item_returned_qty"), dont_reserve_qty_on_sales_return),
(SalesOrderItem.parent, 1),
(SalesOrderItem.name, 1),
]:
if cond:
tab = tab.select(field)
tab = append_open_so_query(tab, SalesOrderItem)

dnpi = (
frappe.qb.from_(PackedItem)
.select(PackedItem.qty, PackedItem.parent_detail_docname, PackedItem.parent, PackedItem.name)
.where(PackedItem.item_code == item_code)
.where(PackedItem.warehouse == warehouse)
)
dnpi = append_open_so_query(dnpi, PackedItem)

dnpi_parent = frappe.qb.from_(dnpi).select(dnpi.qty.as_("dnpi_qty"))
for key, so_item_field, cond in [
("so_item_qty", "qty", 1),
("so_item_delivered_qty", "delivered_qty", 1),
("so_item_returned_qty", "returned_qty", dont_reserve_qty_on_sales_return),
]:
if cond:
dnpi_parent = dnpi_parent.select(
(
select qty, parent_detail_docname, parent, name
from `tabPacked Item` dnpi_in
where item_code = %s and warehouse = %s
and parenttype='Sales Order'
and item_code != parent_item
and exists (select * from `tabSales Order` so
where name = dnpi_in.parent and docstatus = 1 and status not in ('On Hold', 'Closed'))
) dnpi)
union
(select stock_qty as dnpi_qty, qty as so_item_qty,
delivered_qty as so_item_delivered_qty, parent, name
from `tabSales Order Item` so_item
where item_code = %s and warehouse = %s
and (so_item.delivered_by_supplier is null or so_item.delivered_by_supplier = 0)
and exists(select * from `tabSales Order` so
where so.name = so_item.parent and so.docstatus = 1
and so.status not in ('On Hold', 'Closed')))
) tab
where
so_item_qty >= so_item_delivered_qty
""",
(item_code, warehouse, item_code, warehouse),
frappe.qb.from_(SalesOrderItem)
.select(SalesOrderItem[so_item_field])
.where(SalesOrderItem.name == dnpi.parent_detail_docname)
.where(SalesOrderItem.delivered_by_supplier == 0)
).as_(key)
)
dnpi_parent = dnpi_parent.select(dnpi.parent, dnpi.name)

dnpi_parent = dnpi_parent + tab

q = (
frappe.qb.from_(dnpi_parent)
.select(
Sum(
dnpi_parent.dnpi_qty
* (
(
dnpi_parent.so_item_qty
- dnpi_parent.so_item_delivered_qty
- (dnpi_parent.so_item_returned_qty if dont_reserve_qty_on_sales_return else 0)
)
/ dnpi_parent.so_item_qty
)
)
)
.where(dnpi_parent.so_item_qty >= dnpi_parent.so_item_delivered_qty)
)

reserved_qty = q.run()
return flt(reserved_qty[0][0]) if reserved_qty else 0


Expand Down