fix(sales_and_purchase_return): consider net_rate instead of rate for return validation

[diptanilsaha]

Considering net_rate instead of rate to validate item return.

Problem with the existing flow:

• If the Sales Invoice has some discounting, it won't affect the rate, but the total and other amounts are affected. This is a bit problematic, as in the case of return, it was only validating with rate, which allows a larger amount in the Credit Note than what is received during the Sales transaction.

Todo:

• Add Tests

[coderabbitai]

📝 Walkthrough

Walkthrough

The changes modify item-level reference-rate validation in return handling by replacing rate with net_rate throughout erpnext/controllers/sales_and_purchase_return.py. The SQL selection now fetches net_rate, the returned-items comparison checks d.net_rate against ref.net_rate, and the per-item aggregation in get_ref_item_dict tracks net_rate maxima. The validation scope is expanded to include POS Invoice alongside Delivery Note and Sales Invoice. No function or class signatures are modified.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: switching from 'rate' to 'net_rate' for return validation, which is the core modification throughout the changeset.
Description check	✅ Passed	The description is directly related to the changeset, explaining the problem (discounts not affecting rate validation) and the solution (using net_rate instead), which aligns with the actual code changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

erpnext/controllers/sales_and_purchase_return.py (1)

89-89: Add a safe fallback to rate when net_rate is missing/zero.

For resilience with legacy/edge data, consider falling back to rate in both reference extraction and comparison so validation is not silently skipped when net_rate is unavailable.

💡 Proposed hardening diff

-	select_fields = "item_code, qty, stock_qty, net_rate, parenttype, conversion_factor, name"
+	select_fields = "item_code, qty, stock_qty, net_rate, rate, parenttype, conversion_factor, name"
@@
-				if (
-					ref.net_rate
-					and flt(d.net_rate) > ref.net_rate
+				ref_net_rate = flt(ref.get("net_rate") or ref.get("rate"))
+				row_net_rate = flt(d.get("net_rate") or d.get("rate"))
+				if (
+					ref_net_rate
+					and row_net_rate > ref_net_rate
 					and doc.doctype in ("Delivery Note", "Sales Invoice", "POS Invoice")
 					and get_valuation_method(ref.item_code, doc.company) != "Moving Average"
 				):
@@
-	if ref_item_row.get("net_rate", 0) > item_dict["net_rate"]:
-		item_dict["net_rate"] = ref_item_row.get("net_rate", 0)
+	ref_net_rate = flt(ref_item_row.get("net_rate") or ref_item_row.get("rate"))
+	if ref_net_rate > item_dict["net_rate"]:
+		item_dict["net_rate"] = ref_net_rate

Also applies to: 144-147, 257-258

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@erpnext/controllers/sales_and_purchase_return.py` at line 89, The code
currently selects net_rate and uses it directly; update the logic to include a
safe fallback to rate: add "rate" to the select_fields string (so both net_rate
and rate are fetched) and, wherever the code reads net_rate (reference
extraction and comparison logic in the functions that handle return item rate
lookup and validation), replace direct net_rate usage with a safe value
expression like: use net_rate if truthy/non-zero else use rate (and coerce to
numeric safely). Apply this change at the three sites mentioned (the
select_fields definition and the two places that perform reference
extraction/comparison around lines 144-147 and 257-258), ensuring comparisons
and any zero-checks use the fallback value so validation isn’t skipped when
net_rate is missing or zero.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@erpnext/controllers/sales_and_purchase_return.py`:
- Line 89: The code currently selects net_rate and uses it directly; update the
logic to include a safe fallback to rate: add "rate" to the select_fields string
(so both net_rate and rate are fetched) and, wherever the code reads net_rate
(reference extraction and comparison logic in the functions that handle return
item rate lookup and validation), replace direct net_rate usage with a safe
value expression like: use net_rate if truthy/non-zero else use rate (and coerce
to numeric safely). Apply this change at the three sites mentioned (the
select_fields definition and the two places that perform reference
extraction/comparison around lines 144-147 and 257-258), ensuring comparisons
and any zero-checks use the fallback value so validation isn’t skipped when
net_rate is missing or zero.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 226d5083-03a7-423f-97c2-c935d79ac653

📥 Commits

Reviewing files that changed from the base of the PR and between 3542087 and fb8ac97.

📒 Files selected for processing (1)

• erpnext/controllers/sales_and_purchase_return.py

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.74%. Comparing base (2b3e047) to head (fb8ac97).
⚠️ Report is 30 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #54628      +/-   ##
===========================================
+ Coverage    79.44%   79.74%   +0.30%     
===========================================
  Files         1160     1160              
  Lines       126161   126372     +211     
===========================================
+ Hits        100224   100776     +552     
+ Misses       25937    25596     -341     

Files with missing lines	Coverage Δ
erpnext/controllers/sales_and_purchase_return.py	87.77% <100.00%> (ø)

... and 14 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[diptanilsaha]

@rohitwaghchaure