Skip to content

Conversation

@Navin-S-R
Copy link
Contributor

Issue: During asset creation if the purchase Invoice is linked, system sets Supplier Invoice Date as the Purchase Date instead of Posting Date.

Ref: 54340

Backport Needed: Version-15

@github-actions github-actions bot added the needs-tests This PR needs automated unit-tests. label Nov 27, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 27, 2025

📝 Walkthrough

Walkthrough

A modification to the get_values_from_purchase_doc method in the Asset doctype removes bill_date as a fallback source for calculating purchase_date. The method now derives purchase_date exclusively from posting_date, eliminating conditional logic that previously prioritized bill_date when available. This change affects scenarios where bill_date and posting_date differ on the purchase document.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Single file and method modified with straightforward logic reduction
  • Change removes a fallback condition rather than introducing new complexity
  • Potential areas requiring attention:
    • Verify the business rationale for eliminating bill_date as a source (e.g., whether this impacts existing asset valuation or date tracking)
    • Confirm no dependent code relies on the previous fallback behavior
    • Check if this aligns with accounting standards for asset purchase date determination

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: switching from using bill_date to posting_date when deriving purchase_date from purchase invoices.
Description check ✅ Passed The description clearly explains the issue being fixed, references the support ticket, and mentions backport requirements, all directly related to the changeset.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


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

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
erpnext/assets/doctype/asset/asset.py (1)

1213-1222: Aligning purchase_date with posting_date looks correct; consider adding a regression test

Using purchase_doc.get("posting_date") for purchase_date matches the requirement to stop using supplier bill date and ties the asset’s purchase date to the actual posting date of the source document. This is consistent with other accounting flows that treat posting date as canonical. Based on learnings, this should also remain non‑null for Purchase Invoices/Receipts.

To guard against regressions (e.g. when bill_date != posting_date on a Purchase Invoice), consider adding a small test that:

  • Creates a Purchase Invoice/Receipt with differing bill_date and posting_date
  • Calls get_values_from_purchase_doc
  • Asserts that purchase_date equals the document’s posting_date.
📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8a5fd5f and 145d40d.

📒 Files selected for processing (1)
  • erpnext/assets/doctype/asset/asset.py (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: ljain112
Repo: frappe/erpnext PR: 48864
File: erpnext/controllers/accounts_controller.py:2586-2596
Timestamp: 2025-08-20T11:58:32.385Z
Learning: In erpnext/controllers/accounts_controller.py, the posting_date parameter passed to get_due_date() and get_discount_date() is mandatory and calculated from bill_date/posting_date/transaction_date, so it should not be None.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: Python Unit Tests (3)
  • GitHub Check: Python Unit Tests (4)
  • GitHub Check: Python Unit Tests (1)
  • GitHub Check: Python Unit Tests (2)
  • GitHub Check: Patch Test
  • GitHub Check: Summary

@khushi8112 khushi8112 self-assigned this Nov 28, 2025
@khushi8112 khushi8112 merged commit e42b751 into frappe:develop Nov 28, 2025
14 checks passed
Jatin3128 pushed a commit to Jatin3128/erpnext that referenced this pull request Nov 28, 2025
fix: use posting_date instead of bill_date from purchase invoice
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport version-15-hotfix needs-tests This PR needs automated unit-tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants