feat: add opening stock dialog for stock items#54570
feat: add opening stock dialog for stock items#54570khushi8112 wants to merge 3 commits intofrappe:developfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #54570 +/- ##
===========================================
- Coverage 79.74% 79.71% -0.03%
===========================================
Files 1160 1160
Lines 126362 126427 +65
===========================================
+ Hits 100768 100787 +19
- Misses 25594 25640 +46
🚀 New features to boost your workflow:
|
khushi8112
force-pushed
the
item-opening-stock-dialog
branch
from
76628d0 to
b7d6ed5
Compare
khushi8112
requested review from
mihir-kandoi and
rohitwaghchaure
as code owners
📝 WalkthroughWalkthroughThe PR replaces the conditional Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@erpnext/stock/doctype/item/item.js`:
- Around line 162-173: The "Set Opening Stock" button is being added for
unsaved/new items; update the condition that wraps frm.add_custom_button so it
only runs for persisted items by checking the doc is not local (e.g., require
!frm.doc.__islocal or an equivalent saved-doc check). In practice, modify the
existing if that uses can_create_stock_entry and has_existing_stock to also
require the item be saved (reference the can_create_stock_entry and
has_existing_stock variables and the call to
erpnext.item.show_opening_stock_dialog) so the button is only added for saved
items.
- Around line 708-720: The current show_opening_stock_dialog function aborts
when frm.doc.item_defaults is empty; remove the early msgprint/return and
instead allow the dialog flow to continue even if companies array is empty so
the server-side fallback (Stock Settings/"Stores") can resolve a warehouse.
Specifically, update show_opening_stock_dialog to stop treating
(frm.doc.item_defaults || []).map(...) producing no companies as a hard
error—either omit the msgprint/return or replace it by continuing with companies
= [] and ensuring the subsequent dialog/request uses that empty companies value
so backend resolution can occur.
In `@erpnext/stock/doctype/item/item.json`:
- Around line 243-245: The tour step in erpnext/stock/doctype/item/item.js
currently targets the now-hidden field "opening_stock" and must be updated:
locate the Item tour definition (the TOUR_STEPS or initTour block around the
step targeting "opening_stock") and either change its selector/target to the new
"Set Opening Stock" action element (use the action's CSS selector or data-action
attribute) or remove that step entirely; ensure the step text/context still
makes sense after changing the target and that the tour initialization (function
names like init_item_tour or the TOUR_STEPS array) is updated accordingly.
In `@erpnext/stock/doctype/item/item.py`:
- Around line 1565-1574: The Stock Settings fallback used by
get_default_warehouse_for_opening_stock() must be company-safe: when reading
frappe.get_single_value("Stock Settings", "default_warehouse") check that the
returned warehouse's company matches the target company (use
frappe.db.get_value("Warehouse", default_wh, "company") or similar); if it
belongs to a different company, ignore it and continue to the existing fallback
(frappe.db.get_value("Warehouse", {"warehouse_name": _("Stores"), "company":
company})). Ensure this logic is applied where
get_default_warehouse_for_opening_stock() is used (and mirrors the previous
Item.set_opening_stock() behaviour) so opening stock never resolves to a
warehouse in another company.
- Around line 1489-1493: Before allowing an Opening Stock entry to be created,
re-check the item's ledger state by calling item.stock_ledger_created() after
loading the Item doc (the existing code uses item = frappe.get_doc("Item",
item_code) and currently only checks item.is_stock_item); if
item.stock_ledger_created() returns True, raise an exception (frappe.throw)
rejecting the Opening Stock creation with an appropriate message so backend
cannot be bypassed by stale client state or direct RPC.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: b608296b-c899-45e4-b183-a67148f7e849
📒 Files selected for processing (3)
erpnext/stock/doctype/item/item.jserpnext/stock/doctype/item/item.jsonerpnext/stock/doctype/item/item.py
| show_opening_stock_dialog: function (frm) { | ||
| const companies = (frm.doc.item_defaults || []).map((d) => d.company).filter(Boolean); | ||
|
|
||
| if (!companies.length) { | ||
| frappe.msgprint({ | ||
| title: __("No Company Found"), | ||
| message: __( | ||
| "Please add at least one row in Item Defaults with a Company before setting opening stock." | ||
| ), | ||
| indicator: "orange", | ||
| }); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Don't make Item Defaults mandatory for this workflow.
The backend already falls back from Item Defaults to Stock Settings / "Stores", but the dialog exits immediately unless at least one item_defaults row exists. That means saved stock items without Item Defaults never get the new opening-stock flow even though the server path can resolve a warehouse for them.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@erpnext/stock/doctype/item/item.js` around lines 708 - 720, The current
show_opening_stock_dialog function aborts when frm.doc.item_defaults is empty;
remove the early msgprint/return and instead allow the dialog flow to continue
even if companies array is empty so the server-side fallback (Stock
Settings/"Stores") can resolve a warehouse. Specifically, update
show_opening_stock_dialog to stop treating (frm.doc.item_defaults ||
[]).map(...) producing no companies as a hard error—either omit the
msgprint/return or replace it by continuing with companies = [] and ensuring the
subsequent dialog/request uses that empty companies value so backend resolution
can occur.
| item = frappe.get_doc("Item", item_code) | ||
|
|
||
| if not item.is_stock_item: | ||
| frappe.throw(_("Opening Stock can only be set for stock items.")) | ||
|
|
There was a problem hiding this comment.
Reject "opening stock" once ledger activity already exists.
This endpoint trusts the client-side stock_exists gate, so a stale form or direct RPC can still create another "Opening Stock" material receipt after transactions already exist for the item. Please recheck item.stock_ledger_created() here before persisting fields or inserting the stock entry.
Suggested fix
item = frappe.get_doc("Item", item_code)
if not item.is_stock_item:
frappe.throw(_("Opening Stock can only be set for stock items."))
+
+ if item.stock_ledger_created():
+ frappe.throw(_("Opening stock can only be set before the first stock transaction."))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| item = frappe.get_doc("Item", item_code) | |
| if not item.is_stock_item: | |
| frappe.throw(_("Opening Stock can only be set for stock items.")) | |
| item = frappe.get_doc("Item", item_code) | |
| if not item.is_stock_item: | |
| frappe.throw(_("Opening Stock can only be set for stock items.")) | |
| if item.stock_ledger_created(): | |
| frappe.throw(_("Opening stock can only be set before the first stock transaction.")) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@erpnext/stock/doctype/item/item.py` around lines 1489 - 1493, Before allowing
an Opening Stock entry to be created, re-check the item's ledger state by
calling item.stock_ledger_created() after loading the Item doc (the existing
code uses item = frappe.get_doc("Item", item_code) and currently only checks
item.is_stock_item); if item.stock_ledger_created() returns True, raise an
exception (frappe.throw) rejecting the Opening Stock creation with an
appropriate message so backend cannot be bypassed by stale client state or
direct RPC.
| target = frappe.get_single_value("Stock Settings", "default_warehouse") or frappe.db.get_value( | ||
| "Warehouse", {"warehouse_name": _("Stores"), "company": company} | ||
| ) | ||
|
|
||
| if not target: | ||
| frappe.throw( | ||
| _( | ||
| "No warehouse found for company {0}. Please set a Default Warehouse in Item Defaults or Stock Settings." | ||
| ).format(frappe.bold(company)) | ||
| ) |
There was a problem hiding this comment.
Keep the Stock Settings fallback company-safe.
get_default_warehouse_for_opening_stock() now accepts Stock Settings.default_warehouse without checking its company. In a multi-company setup, that can point opening stock for Company B at Company A's warehouse and fail the transaction, whereas Item.set_opening_stock() previously fell back to that company's "Stores" warehouse in this case.
Suggested fix
- target = frappe.get_single_value("Stock Settings", "default_warehouse") or frappe.db.get_value(
- "Warehouse", {"warehouse_name": _("Stores"), "company": company}
- )
+ target = frappe.get_single_value("Stock Settings", "default_warehouse")
+ if target and frappe.db.get_value("Warehouse", target, "company") != company:
+ target = None
+
+ if not target:
+ target = frappe.db.get_value("Warehouse", {"warehouse_name": _("Stores"), "company": company})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| target = frappe.get_single_value("Stock Settings", "default_warehouse") or frappe.db.get_value( | |
| "Warehouse", {"warehouse_name": _("Stores"), "company": company} | |
| ) | |
| if not target: | |
| frappe.throw( | |
| _( | |
| "No warehouse found for company {0}. Please set a Default Warehouse in Item Defaults or Stock Settings." | |
| ).format(frappe.bold(company)) | |
| ) | |
| target = frappe.get_single_value("Stock Settings", "default_warehouse") | |
| if target and frappe.db.get_value("Warehouse", target, "company") != company: | |
| target = None | |
| if not target: | |
| target = frappe.db.get_value("Warehouse", {"warehouse_name": _("Stores"), "company": company}) | |
| if not target: | |
| frappe.throw( | |
| _( | |
| "No warehouse found for company {0}. Please set a Default Warehouse in Item Defaults or Stock Settings." | |
| ).format(frappe.bold(company)) | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@erpnext/stock/doctype/item/item.py` around lines 1565 - 1574, The Stock
Settings fallback used by get_default_warehouse_for_opening_stock() must be
company-safe: when reading frappe.get_single_value("Stock Settings",
"default_warehouse") check that the returned warehouse's company matches the
target company (use frappe.db.get_value("Warehouse", default_wh, "company") or
similar); if it belongs to a different company, ignore it and continue to the
existing fallback (frappe.db.get_value("Warehouse", {"warehouse_name":
_("Stores"), "company": company})). Ensure this logic is applied where
get_default_warehouse_for_opening_stock() is used (and mirrors the previous
Item.set_opening_stock() behaviour) so opening stock never resolves to a
warehouse in another company.
|
@khushi8112 Opening stock is a one-time activity and after that the normal flow will be used. We have added the opening_stock field so that the opening stock entry can be created automatically during the item creation (either manually or through data import) Even for the people who forgot to add opening stock while creating the item can use the Stock Reconciliation feature to add opening stock. 
|
I get the idea, but this still has some issues. There’s no company or warehouse context here, which matters even for opening stock. |
|
@khushi8112 The pop-up still has issues because it does not allow users to add serial numbers or batches manually. It would be better to redirect users to the Stock Reconciliation form when they click on "Set / Add Opening Stock", with the item details pre-filled and the purpose set to "Opening Stock". |
khushi8112
force-pushed
the
item-opening-stock-dialog
branch
from
b7d6ed5 to
1890ccd
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
erpnext/stock/doctype/item/item.js (1)
714-726:⚠️ Potential issue | 🟠 MajorDon’t block the dialog when
item_defaultsis empty.This hard return still makes the feature unusable for stock items that have no Item Defaults row. The server already has warehouse fallback logic in
erpnext/stock/doctype/item/item.py:get_default_warehouse_for_opening_stock, so the dialog should sourcecompanyindependently instead of treating Item Defaults as a prerequisite.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@erpnext/stock/doctype/item/item.js` around lines 714 - 726, The current show_opening_stock_dialog function aborts when frm.doc.item_defaults yields no companies; instead remove the early return and determine company independently: if companies is empty, set companies to a single-entry array from frm.doc.company (if present) or invoke the existing server fallback (call the server method get_default_warehouse_for_opening_stock in erpnext/stock/doctype/item/item.py) to obtain the appropriate warehouse/company to use; then continue with the dialog flow. Ensure you update the logic around the companies variable in show_opening_stock_dialog so the dialog is not blocked when item_defaults is empty.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@erpnext/stock/doctype/item/item.js`:
- Around line 803-838: The dialog always sets create_new_batch to 1 and passes
it into make_opening_stock_entry which causes
persist_serial_batch_fields_for_opening_stock to update the Item record
permanently; change the dialog handling so it does not persist one-off choices:
pass create_new_batch and batch_number_series only as transient args for the
stock entry (avoid calling persist_*), or send a flag like
persist_serial_batch_fields=false to make_opening_stock_entry and ensure
make_opening_stock_entry (and persist_serial_batch_fields_for_opening_stock)
only persists when an explicit “save default” action is taken; update references
to create_new_batch, batch_number_series, make_opening_stock_entry and
persist_serial_batch_fields_for_opening_stock accordingly to prevent silent
Item-level changes from this dialog.
In `@erpnext/stock/doctype/item/item.json`:
- Around line 256-258: The opening_stock field in item.json is hidden
unconditionally, which removes it from the create form even though after_insert
-> set_opening_stock() in erpnext/stock/doctype/item/item.py still expects it;
change the field to be visible on new items and only hidden after the Item is
saved—remove or replace "hidden": 1 with logic that toggles visibility based on
document state (e.g., use "read_only": 1 or a form script/depends_on that hides
the field when docname exists), and ensure references to opening_stock in
set_opening_stock() and after_insert continue to work.
---
Duplicate comments:
In `@erpnext/stock/doctype/item/item.js`:
- Around line 714-726: The current show_opening_stock_dialog function aborts
when frm.doc.item_defaults yields no companies; instead remove the early return
and determine company independently: if companies is empty, set companies to a
single-entry array from frm.doc.company (if present) or invoke the existing
server fallback (call the server method get_default_warehouse_for_opening_stock
in erpnext/stock/doctype/item/item.py) to obtain the appropriate
warehouse/company to use; then continue with the dialog flow. Ensure you update
the logic around the companies variable in show_opening_stock_dialog so the
dialog is not blocked when item_defaults is empty.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 277920e8-c559-4809-a42f-ea2aebacd7c3
📒 Files selected for processing (3)
erpnext/stock/doctype/item/item.jserpnext/stock/doctype/item/item.jsonerpnext/stock/doctype/item/item.py
🚧 Files skipped from review as they are similar to previous changes (1)
- erpnext/stock/doctype/item/item.py
| { | ||
| label: __("Automatically Create New Batch"), | ||
| fieldname: "create_new_batch", | ||
| fieldtype: "Check", | ||
| default: 1, | ||
| read_only: 1, | ||
| }, | ||
| { | ||
| label: __("Batch Number Series"), | ||
| fieldname: "batch_number_series", | ||
| fieldtype: "Data", | ||
| default: frm.doc.batch_number_series || "", | ||
| reqd: 1, | ||
| description: __( | ||
| "Example: BATCH-.YYYY.-.#####. - A new batch will be auto-created from this series." | ||
| ), | ||
| } | ||
| ); | ||
| } | ||
|
|
||
| const dialog = new frappe.ui.Dialog({ | ||
| title: __("Set Opening Stock"), | ||
| fields: fields, | ||
| primary_action_label: __("Create Stock Entry"), | ||
| primary_action: function (values) { | ||
| frappe.call({ | ||
| method: "erpnext.stock.doctype.item.item.make_opening_stock_entry", | ||
| args: { | ||
| item_code: frm.doc.name, | ||
| company: values.company, | ||
| qty: values.qty, | ||
| valuation_rate: values.valuation_rate || 0, | ||
| warehouse: values.warehouse || null, | ||
| serial_no_series: values.serial_no_series || null, | ||
| create_new_batch: values.create_new_batch ? 1 : 0, | ||
| batch_number_series: values.batch_number_series || null, |
There was a problem hiding this comment.
Avoid permanently enabling auto batch creation from this one-off dialog.
For batch items this always submits create_new_batch: 1, and make_opening_stock_entry() persists that back to the Item via persist_serial_batch_fields_for_opening_stock(...). Using a one-time opening-stock action will therefore silently change future batch behavior for the Item, which is a much bigger side effect than creating the initial stock entry.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@erpnext/stock/doctype/item/item.js` around lines 803 - 838, The dialog always
sets create_new_batch to 1 and passes it into make_opening_stock_entry which
causes persist_serial_batch_fields_for_opening_stock to update the Item record
permanently; change the dialog handling so it does not persist one-off choices:
pass create_new_batch and batch_number_series only as transient args for the
stock entry (avoid calling persist_*), or send a flag like
persist_serial_batch_fields=false to make_opening_stock_entry and ensure
make_opening_stock_entry (and persist_serial_batch_fields_for_opening_stock)
only persists when an explicit “save default” action is taken; update references
to create_new_batch, batch_number_series, make_opening_stock_entry and
persist_serial_batch_fields_for_opening_stock accordingly to prevent silent
Item-level changes from this dialog.
| "fieldname": "opening_stock", | ||
| "fieldtype": "Float", | ||
| "hidden": 1, |
There was a problem hiding this comment.
Keep opening_stock available on new Items.
"hidden": 1 removes the inline field during initial Item creation too, so users lose the existing create-time opening-stock flow even though after_insert / set_opening_stock() still consumes opening_stock in erpnext/stock/doctype/item/item.py. This should be hidden only after the Item has been saved.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@erpnext/stock/doctype/item/item.json` around lines 256 - 258, The
opening_stock field in item.json is hidden unconditionally, which removes it
from the create form even though after_insert -> set_opening_stock() in
erpnext/stock/doctype/item/item.py still expects it; change the field to be
visible on new items and only hidden after the Item is saved—remove or replace
"hidden": 1 with logic that toggles visibility based on document state (e.g.,
use "read_only": 1 or a form script/depends_on that hides the field when docname
exists), and ensure references to opening_stock in set_opening_stock() and
after_insert continue to work.
2 participants
Why this change:
The inline field was limited, it only worked at item creation, had no company/warehouse visibility, and completely broke for serial/batch items. Users who forgot to set opening stock had no way to do it from the item form without manually creating a Stock Entry.
What changed
Screen.Recording.2026-04-29.at.1.10.39.AM.mov
no-docs