fix: remove item-item group name validation #28392

Alchez · 2021-11-15T11:31:50Z

There's an old validation from ~9 years ago that prevents item names to be the same as an item group's name. The comment against it says that it's to avoid causing issues to build the tree, but it seems like the validation is no longer necessary, considering how much the framework has changed and how treeviews are built now.

codecov · 2021-11-15T11:58:08Z

Codecov Report

Merging #28392 (fe706db) into develop (c0f06bc) will decrease coverage by 0.05%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop   #28392      +/-   ##
===========================================
- Coverage    55.24%   55.19%   -0.06%     
===========================================
  Files         1119     1119              
  Lines        66553    66545       -8     
===========================================
- Hits         36768    36728      -40     
- Misses       29785    29817      +32

Impacted Files	Coverage Δ
erpnext/setup/doctype/item_group/item_group.py	`48.40% <ø> (-0.67%)`	⬇️
erpnext/stock/doctype/item/item.py	`66.84% <ø> (-0.05%)`	⬇️
...work_order_stock_report/work_order_stock_report.py	`50.00% <0.00%> (-50.00%)`	⬇️
...e/asset_value_adjustment/asset_value_adjustment.py	`87.65% <0.00%> (-3.71%)`	⬇️
...eport/item_variant_details/item_variant_details.py	`84.33% <0.00%> (-3.62%)`	⬇️
erpnext/education/doctype/student/student.py	`73.68% <0.00%> (-3.16%)`	⬇️
erpnext/stock/report/stock_ageing/stock_ageing.py	`90.51% <0.00%> (-2.92%)`	⬇️
...ion/doctype/course_enrollment/course_enrollment.py	`44.00% <0.00%> (-2.00%)`	⬇️
...wise_balance_history/batch_wise_balance_history.py	`91.37% <0.00%> (-1.73%)`	⬇️
erpnext/stock/report/stock_ledger/stock_ledger.py	`75.60% <0.00%> (-1.63%)`	⬇️
... and 19 more

ankush · 2021-11-15T13:17:32Z

So it may not be related to NSM tree rebuilding

#4705

Fix tree view generation in reports that have both Item Group and Item

Even then I am not sure which reports are broken by this 😅

Alchez · 2021-11-15T13:24:28Z

@ankush

Oh, good to know lol. But even with reports that use both Item Group and Item, I don't see how having the same name for item and item group can mess it up.

(cherry picked from commit 043e325)

ankush · 2021-11-15T14:20:34Z

For reference, there was some old report which had a common column for item and item group (maybe report filters had the option to pick which doctype to use for report) this used to break. This report doesn't exist anymore.

Even if we see similar error, we will try to change that instead of this validation 😅

(cherry picked from commit 043e325) Co-authored-by: Rohan <Alchez@users.noreply.github.com>

* perf(minor): general ledger report (backport frappe#27987) * fix: replaced "=" with "in" for multiple statuses in query (backport frappe#28193) * fix: Serial Nos not set in the row after scanning in popup - Avoid whitspaces while calculating length of serial nos (cherry picked from commit 734b57d) * fix: don't make naming series mandatory for items (backport frappe#28394) * fix: Work order creation from sales order (backport frappe#28388) * fix: currency wise pricing rule not working (cherry picked from commit 43aeb54) * fix: remove item-item group name validation (backport frappe#28392) * fix: Default party account getting overriden in invoices (cherry picked from commit 8864857) * fix: Remove warehouse filter on Batch field for Material Receipt (cherry picked from commit 048210a) * fix: Dispatch address details not displayed in v13 * fix: Unable to edit supplier scorecard criteria name once created (backport frappe#28348) * fix: performance to submit the JV (cherry picked from commit 7472760) * fix: Pricing Rule not created against the Promotional Scheme (cherry picked from commit d82910b) * fix: sum of components in salary register (backport frappe#28237) * fix: auto update price list rate * fix: Pull Items that are in JC in Stock Entry against JC - Check if items pulled in stock entry are present in Job Card - Code cleanup and removed redundant checks Co-authored-by: Gavin D'souza <gavin18d@gmail.com> (cherry picked from commit 3da0302) * test: Stock Entry from JC correctness (items mapping and qty) (cherry picked from commit 0aa237f) * fix: Partial Trabsfers against JC - Fixed transferred qty not back updating on JC if partial transfer - Partial transfer not mapping pending qty from JC correctly in SE - tests for above cases - minor code cleanup (cherry picked from commit e8d0c25) * fix: (travis) Production Plan Summary Report breaks if no WO - `get_cached_value` throws a DoesNotExistError if non-existent value, used `get_value` instead - accomodate production plan items that dont have WO/PO against them as well (blank values) - added some None value handling to avoid AttributeError (cherry picked from commit 1eb3ca2) * fix: Server side test - make `tests_that_skip_setup` a tuple (added comma) - remove manual teardown in `test_job_card_material_transfer_correctness` to avoid premature committing - transfer_material_against = "Job Card" while making BOM with mulitple operations (cherry picked from commit bb561ba) * fix: `test_job_card_partial_material_transfer` test - Use a specific BOM for JC tests - Utility to create said BOM - Sider: unused variable (cherry picked from commit a5f8274) Co-authored-by: marination <maricadsouza221197@gmail.com> Co-authored-by: Rohit Waghchaure <rohitw1991@gmail.com> Co-authored-by: Deepesh Garg <deepeshgarg6@gmail.com> Co-authored-by: Subin Tom <subintom2@gmail.com>

* perf(minor): general ledger report (backport frappe#27987) * fix: replaced "=" with "in" for multiple statuses in query (backport frappe#28193) * fix: Serial Nos not set in the row after scanning in popup - Avoid whitspaces while calculating length of serial nos (cherry picked from commit 734b57d) * fix: don't make naming series mandatory for items (backport frappe#28394) * fix: Work order creation from sales order (backport frappe#28388) * fix: currency wise pricing rule not working (cherry picked from commit 43aeb54) * fix: remove item-item group name validation (backport frappe#28392) * fix: Default party account getting overriden in invoices (cherry picked from commit 8864857) * fix: Remove warehouse filter on Batch field for Material Receipt (cherry picked from commit 048210a) * fix: Dispatch address details not displayed in v13 * fix: Unable to edit supplier scorecard criteria name once created (backport frappe#28348) * fix: performance to submit the JV (cherry picked from commit 7472760) * fix: Pricing Rule not created against the Promotional Scheme (cherry picked from commit d82910b) * fix: sum of components in salary register (backport frappe#28237) * fix: auto update price list rate * fix: Pull Items that are in JC in Stock Entry against JC - Check if items pulled in stock entry are present in Job Card - Code cleanup and removed redundant checks Co-authored-by: Gavin D'souza <gavin18d@gmail.com> (cherry picked from commit 3da0302) * test: Stock Entry from JC correctness (items mapping and qty) (cherry picked from commit 0aa237f) * fix: Partial Trabsfers against JC - Fixed transferred qty not back updating on JC if partial transfer - Partial transfer not mapping pending qty from JC correctly in SE - tests for above cases - minor code cleanup (cherry picked from commit e8d0c25) * fix: (travis) Production Plan Summary Report breaks if no WO - `get_cached_value` throws a DoesNotExistError if non-existent value, used `get_value` instead - accomodate production plan items that dont have WO/PO against them as well (blank values) - added some None value handling to avoid AttributeError (cherry picked from commit 1eb3ca2) * fix: Server side test - make `tests_that_skip_setup` a tuple (added comma) - remove manual teardown in `test_job_card_material_transfer_correctness` to avoid premature committing - transfer_material_against = "Job Card" while making BOM with mulitple operations (cherry picked from commit bb561ba) * fix: `test_job_card_partial_material_transfer` test - Use a specific BOM for JC tests - Utility to create said BOM - Sider: unused variable (cherry picked from commit a5f8274) Co-authored-by: marination <maricadsouza221197@gmail.com> Co-authored-by: Rohit Waghchaure <rohitw1991@gmail.com> Co-authored-by: Deepesh Garg <deepeshgarg6@gmail.com> Co-authored-by: Subin Tom <subintom2@gmail.com> Co-authored-by: marination <maricadsouza221197@gmail.com> Co-authored-by: Rohit Waghchaure <rohitw1991@gmail.com> Co-authored-by: Deepesh Garg <deepeshgarg6@gmail.com> Co-authored-by: Subin Tom <subintom2@gmail.com>

* perf(minor): general ledger report (backport frappe#27987) * fix: replaced "=" with "in" for multiple statuses in query (backport frappe#28193) * fix: Serial Nos not set in the row after scanning in popup - Avoid whitspaces while calculating length of serial nos (cherry picked from commit 734b57d) * fix: don't make naming series mandatory for items (backport frappe#28394) * fix: Work order creation from sales order (backport frappe#28388) * fix: currency wise pricing rule not working (cherry picked from commit 43aeb54) * fix: remove item-item group name validation (backport frappe#28392) * fix: Default party account getting overriden in invoices (cherry picked from commit 8864857) * fix: Remove warehouse filter on Batch field for Material Receipt (cherry picked from commit 048210a) * fix: Dispatch address details not displayed in v13 * fix: Unable to edit supplier scorecard criteria name once created (backport frappe#28348) * fix: performance to submit the JV (cherry picked from commit 7472760) * fix: Pricing Rule not created against the Promotional Scheme (cherry picked from commit d82910b) * fix: sum of components in salary register (backport frappe#28237) * fix: auto update price list rate * fix: Pull Items that are in JC in Stock Entry against JC - Check if items pulled in stock entry are present in Job Card - Code cleanup and removed redundant checks Co-authored-by: Gavin D'souza <gavin18d@gmail.com> (cherry picked from commit 3da0302) * test: Stock Entry from JC correctness (items mapping and qty) (cherry picked from commit 0aa237f) * fix: Partial Trabsfers against JC - Fixed transferred qty not back updating on JC if partial transfer - Partial transfer not mapping pending qty from JC correctly in SE - tests for above cases - minor code cleanup (cherry picked from commit e8d0c25) * fix: (travis) Production Plan Summary Report breaks if no WO - `get_cached_value` throws a DoesNotExistError if non-existent value, used `get_value` instead - accomodate production plan items that dont have WO/PO against them as well (blank values) - added some None value handling to avoid AttributeError (cherry picked from commit 1eb3ca2) * fix: Server side test - make `tests_that_skip_setup` a tuple (added comma) - remove manual teardown in `test_job_card_material_transfer_correctness` to avoid premature committing - transfer_material_against = "Job Card" while making BOM with mulitple operations (cherry picked from commit bb561ba) * fix: `test_job_card_partial_material_transfer` test - Use a specific BOM for JC tests - Utility to create said BOM - Sider: unused variable (cherry picked from commit a5f8274) Co-authored-by: marination <maricadsouza221197@gmail.com> Co-authored-by: Rohit Waghchaure <rohitw1991@gmail.com> Co-authored-by: Deepesh Garg <deepeshgarg6@gmail.com> Co-authored-by: Subin Tom <subintom2@gmail.com> Co-authored-by: Francisco Roldán <franciscoproldan@gmail.com> Co-authored-by: marination <maricadsouza221197@gmail.com> Co-authored-by: Rohit Waghchaure <rohitw1991@gmail.com> Co-authored-by: Deepesh Garg <deepeshgarg6@gmail.com> Co-authored-by: Subin Tom <subintom2@gmail.com>

Alchez requested review from ankush, marination and rohitwaghchaure as code owners November 15, 2021 11:31

ankush self-assigned this Nov 15, 2021

fix: remove item-item group name validation

fe706db

Alchez force-pushed the fix-item-group-name-validation branch from d6204b0 to fe706db Compare November 15, 2021 11:37

ankush added the backport version-13-hotfix label Nov 15, 2021

ankush approved these changes Nov 15, 2021

View reviewed changes

ankush merged commit 043e325 into frappe:develop Nov 15, 2021

mergify bot pushed a commit that referenced this pull request Nov 15, 2021

fix: remove item-item group name validation (#28392)

3e69f51

(cherry picked from commit 043e325)

mergify bot mentioned this pull request Nov 15, 2021

fix: remove item-item group name validation (backport #28392) #28399

Merged

ankush pushed a commit that referenced this pull request Nov 15, 2021

fix: remove item-item group name validation (#28392) (#28399)

2c9b995

(cherry picked from commit 043e325) Co-authored-by: Rohan <Alchez@users.noreply.github.com>

rohitwaghchaure mentioned this pull request Nov 22, 2021

chore: change log for v13.15.0 #28508

Merged

conncampbell pushed a commit to conncampbell/erpnext that referenced this pull request Jan 9, 2022

fix: remove item-item group name validation (frappe#28392)

3266cc0

Alchez deleted the fix-item-group-name-validation branch March 2, 2022 11:14

fproldan added a commit to fproldan/erpnext that referenced this pull request Aug 29, 2022

fix: remove item-item group name validation (backport frappe#28392)

c19cc71

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: remove item-item group name validation #28392

fix: remove item-item group name validation #28392

Alchez commented Nov 15, 2021

codecov bot commented Nov 15, 2021

ankush commented Nov 15, 2021 •

edited

Alchez commented Nov 15, 2021

ankush commented Nov 15, 2021

fix: remove item-item group name validation #28392

fix: remove item-item group name validation #28392

Conversation

Alchez commented Nov 15, 2021

codecov bot commented Nov 15, 2021

Codecov Report

ankush commented Nov 15, 2021 • edited

Alchez commented Nov 15, 2021

ankush commented Nov 15, 2021

ankush commented Nov 15, 2021 •

edited