Changes to Company abbreviation don't propagate properly to Cost Centers, Tax templates, Warehouses etc.

blewsky commented 3 years ago

Description of the issue

Changing the abbreviation for a Company, changes the abbreviations appended in the Chart of Accounts but not to Cost Centers.

Steps to reproduce the issue

Rename company abbreviation using Abbr. Field in Company Doctype. This is the pop-up:
Save changes, wait until changes affect the entries in the database and reload company page.
(Check Chart of Accounts. The abbreviations added to each Account name are changed.)
Go-to Cost Centers. The Cost Centers still have the old abbreviations.

Observed result

Abbreviations in the name of Cost Center are not changed according to the change in Company doctype.
To make things worse: It doesn't work to rename the Cost Centers manually to match the new abbreviations.

Expected result

Changes to the abbreviation in Company doctype should directly propagate correctly and affect the abbreviation used in the name of all Cost Center entries.

Additional information

ERPNext: v13.7.0 (HEAD) Frappe Framework: v13.7.0 (HEAD)

blewsky commented 3 years ago

It seems this problem is bigger than previously thought. The company abbreviations are also not correctly propagated to the Tax Templates and Warehouses. The weird thing is, that some Warehouses etc. are renamed correctly and others not. It seems this "feature" (= renaming a company abbreviation) should be overhauled and checked for bugs. A company can potentially have many Accounts, Warehouses, Tax templates etc. that can be affected by the renaming of the company abbreviation. Not having proper names after a change in the abbreviation, can create confusion or other bugs. Therefore, I think this is a serious issue that should be fixed.

Personally, I think it is not a good practice to add the company abbreviation automatically to all these dependent fields for two reasons:

Propagating changes is very error-prone (as we can see here).
People might want a different naming procedure (e.g. I dislike the space characters between the doctype name and the company abbreviation others might want to hide the company abbreviation because they don't need it.)

blewsky commented 3 years ago

The problem probably stems from the functions `enqueue_replace_abbr()' or 'replace_abbr()' in the Company doctype. It seems that two recent changes to the code in V13.7 (https://github.com/frappe/erpnext/pull/26462) and V13.6 (https://github.com/frappe/erpnext/pull/26203) are responsible for this behavior.

blewsky commented 3 years ago

@nextchamp-saqib : Can you comment on this from the perspective of the two recent PRs?

blewsky commented 3 years ago

Any updates on this? Would be great to get feedback from @nextchamp-saqib ? Otherwise, it could make sense to revert the changes made in https://github.com/frappe/erpnext/pull/26462 and https://github.com/frappe/erpnext/pull/26203 as it seems that they weren't thoroughly tested enough to be rolled out.

nextchamp-saqib commented 3 years ago

@blewsky Sorry for the late response. There seem to be various issues with this feature and I will try to refactor it to address the problems.

However, I can confirm that the problem didn't arise after the 2 recent PRs.

blewsky commented 3 years ago

Hi @nextchamp-saqib, any updates on this issue? Could you track where this bug originates from?

ankush commented 2 years ago

Closing this in line with linked PR ^

This feature is very dangerous on large datasets and doesn't have any real blocking use cases.

blewsky commented 2 years ago

The problem still persists even with the @nextchamp-saqib 's PR. The abbreviation is added automatically after the cost center. ERPnext still adds the old abbreviation it must have still stored somewhere.

frappe / erpnext