Skip to content

Conversation

@PugazhendhiVelu
Copy link
Contributor

Issue : When the company is changed, the existing billing and shipping addresses are retained even when the new company has no linked addresses.

Ref : #52293

Before :

Before.mp4

After :

After.mp4

Backport needed: v15

@PugazhendhiVelu PugazhendhiVelu marked this pull request as ready for review November 6, 2025 13:05
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 6, 2025

📝 Walkthrough

Walkthrough

The company() callback in the buying controller has been refactored to change how billing and shipping addresses are updated. The method now includes an early exit if company message data is missing, always updates the billing address field when company data is received, and simplifies the shipping address update logic to depend solely on field existence rather than multiple combined conditions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Impact on address population logic when switching companies—ensure business requirements are met with the new always-update approach for billing address
  • Verify that the simplified shipping address condition (field existence only) doesn't introduce regressions or unintended behavior
  • Confirm the early exit for missing company message doesn't bypass necessary initialization

Possibly related PRs

  • fix: preserve address if present #49939: Modifies the same company() callback in buying.js but implements opposite behavior—preserves existing addresses rather than forcing updates, representing a conflicting change that may need coordination.

Suggested labels

backport version-15-hotfix

Suggested reviewers

  • diptanilsaha

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: fixing the behavior where billing and shipping addresses are reset when the company changes.
Description check ✅ Passed The description clearly relates to the changeset, explaining the issue being fixed, providing a reference ticket, and showing before/after comparisons of the fix.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent 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 4d26a79 and 0510f7e.

📒 Files selected for processing (1)
  • erpnext/public/js/controllers/buying.js (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
erpnext/public/js/controllers/buying.js (1)
erpnext/accounts/doctype/pos_invoice/pos_invoice.js (1)
  • r (122-126)
⏰ 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). (1)
  • GitHub Check: Summary
🔇 Additional comments (1)
erpnext/public/js/controllers/buying.js (1)

177-183: Fix is correct; early exit properly handles API failure scenarios.

Based on verification of the server-side code, the get_default_company_address method returns None when no addresses are found, and the get_billing_shipping_address method always returns a dictionary with primary_address and shipping_address keys.

When a company has no linked addresses:

  • The server returns {"primary_address": null, "shipping_address": null} (in JSON)
  • The early exit on line 177 only triggers if r.message is completely missing (true API failure)
  • The fallback || operators on lines 179 and 182 properly convert null values to empty strings

The implementation correctly prevents stale addresses from persisting while protecting against actual API failures.


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.

@diptanilsaha diptanilsaha self-assigned this Nov 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants