Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: department creation #31548

Merged
merged 1 commit into from
Jul 7, 2022
Merged

Conversation

ankush
Copy link
Member

@ankush ankush commented Jul 7, 2022

  • all department creation always fails after first company, this is
    handled in exception handling code but better to not attempt this in
    first place.
  • move department creation to company.py; this has nothing to do with
    setup and previous function signature made no sense.

This eliminates the misleading error message in CI:

Traceback (most recent call last):
  File "apps/frappe/frappe/desk/page/setup_wizard/setup_wizard.py", line 437, in make_records
    doc.insert(ignore_permissions=True, ignore_if_duplicate=True)
  File "apps/frappe/frappe/model/document.py", line 278, in insert
    self.run_post_save_methods()
  File "apps/frappe/frappe/model/document.py", line 1084, in run_post_save_methods
    self.run_method("on_update")
  File "apps/frappe/frappe/model/document.py", line 925, in run_method
    out = Document.hook(fn)(self, *args, **kwargs)
  File "apps/frappe/frappe/model/document.py", line 1263, in composer
    return composed(self, method, *args, **kwargs)
  File "apps/frappe/frappe/model/document.py", line 1245, in runner
    add_to_return_value(self, fn(self, *args, **kwargs))
  File "apps/frappe/frappe/model/document.py", line 9[22](https://github.com/frappe/erpnext/runs/7226846518?check_suite_focus=true#step:12:23), in fn
    return method_object(*args, **kwargs)
  File "apps/erpnext/erpnext/hr/doctype/department/department.py", line 36, in on_update
    super(Department, self).on_update()
  File "apps/frappe/frappe/utils/nestedset.py", line 246, in on_update
    update_nsm(self)
  File "apps/frappe/frappe/utils/nestedset.py", line 55, in update_nsm
    update_add_node(doc, parent or "", parent_field)
  File "apps/frappe/frappe/utils/nestedset.py", line 77, in update_add_node
    validate_loop(doc.doctype, doc.name, left, right)
  File "apps/frappe/frappe/utils/nestedset.py", line [23](https://github.com/frappe/erpnext/runs/7226846518?check_suite_focus=true#step:12:24)7, in validate_loop
    frappe.throw(_("Item cannot be added to its own descendents"), NestedSetRecursionError)
  File "apps/frappe/frappe/__init__.py", line 516, in throw
    msgprint(
  File "apps/frappe/frappe/__init__.py", line 484, in msgprint
    _raise_exception()
  File "apps/frappe/frappe/__init__.py", line 4[36](https://github.com/frappe/erpnext/runs/7226846518?check_suite_focus=true#step:12:37), in _raise_exception
    raise raise_exception(msg)
frappe.utils.nestedset.NestedSetRecursionError: Item cannot be added to its own descendents

- all department creation always fails after first company, this is
handled in exception handling code but better to not attempt this in
first place.
- move department creation to company.py this has nothing to do with
  setup and previous function signature made no sense.
@codecov
Copy link

codecov bot commented Jul 7, 2022

Codecov Report

Merging #31548 (07f6ce6) into develop (c9dc902) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop   #31548      +/-   ##
===========================================
+ Coverage    64.57%   64.59%   +0.02%     
===========================================
  Files          965      965              
  Lines        65153    65152       -1     
===========================================
+ Hits         42070    42084      +14     
+ Misses       23083    23068      -15     
Impacted Files Coverage Δ
.../setup/setup_wizard/operations/install_fixtures.py 92.46% <ø> (-0.40%) ⬇️
erpnext/setup/doctype/company/company.py 73.99% <100.00%> (+0.57%) ⬆️
erpnext/accounts/doctype/bank/bank.py 71.42% <0.00%> (-14.29%) ⬇️
...enefit_application/employee_benefit_application.py 64.49% <0.00%> (-8.29%) ⬇️
...urity_shortfall/process_loan_security_shortfall.py 93.75% <0.00%> (-6.25%) ⬇️
...pnext/setup/doctype/sales_partner/sales_partner.py 65.21% <0.00%> (-4.35%) ⬇️
erpnext/hr/doctype/job_applicant/job_applicant.py 63.93% <0.00%> (-3.28%) ⬇️
erpnext/assets/doctype/asset/depreciation.py 85.23% <0.00%> (-2.69%) ⬇️
erpnext/stock/reorder_item.py 73.33% <0.00%> (-2.50%) ⬇️
...next/accounts/doctype/bank_account/bank_account.py 77.27% <0.00%> (-2.28%) ⬇️
... and 23 more

@ankush ankush merged commit bc3f993 into frappe:develop Jul 7, 2022
@ankush ankush deleted the department_creation branch July 7, 2022 10:35
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.

None yet

1 participant