Uninstall-app does not remove app related custom fields from site

[blewsky]

Description of the issue

Uninstall-app does not remove app related custom fields from site.

Context information (for bug reports)

This topic was discussed but not addressed in the last refactoring of uninstall-apps #10871 (comment)
This bug renders ERPnext installations broken. In our case, we installed POS-Awesome but don't use it anymore. Removing the app leaves the custom fields in our site.

Steps to reproduce the issue

1. Install app (e.g. POS-Awesome)
2. Add some data using the app
3. Remove app
4. Look into Custom fields where you will still see the app related custom fields. This breaks certain doctypes where the custom fields are still present. The custom fields are required but the module is already deinstalled.

Expected behavior

Removing the app should also remove the custom fields in all doctypes. Otherwise, it will break these doctypes.

Additional information

Frappe Framework: v14.62.1 (HEAD) on Frappe Cloud

[blewsky]

As addendum, the custom fields can't even be manually removed from standard doctypes such as sales order. This is a big problem, since this removes all options to manually fix the issue. We are hosting on Frappe Cloud and cannot manually edit the core doctypes.

[ankush]

Apps are supposed to handle customization they add. I don't think we can magically guess who added what and remove it. Also it might not make sense to always remove custom fields (?)

Example: https://github.com/resilient-tech/india-compliance/blob/develop/india_compliance/uninstall.py

[blewsky]

Not sure I fully understand you. This approach sounds to me like a recipe for potential future problems. It would be better if the Frappe framework kept track of the custom changes made by apps so it can properly uninstall them. Otherwise, it would be expected from users that they vet all the custom apps they install (e.g. from Frappe Cloud Marketplace) that these apps handle their customizations well. It would be better if there was a process in place where all customizations are stored and removed when uninstalling the app. Also this system should track if there are any conflicting customization when installing an app making sure that they don't overwrite each others customizations. I am not really familiar with the details of customizations and app development so my comment might be shooting past the target.

[ankush]

@blewsky how do we keep track of it though? Apps can run arbitrary code any time to create customizations 😅

here's example of customizations being added at runtime when user does something: https://github.com/frappe/ecommerce_integrations/blob/develop/ecommerce_integrations/shopify/doctype/shopify_setting/shopify_setting.py#L50

[cogk]

Maybe we could remove Custom Fields and Property Setters that are marked is_system_generated on migrate, but this means that apps must ensure that all fields added on_migrate have is_system_generated: 1, and only them? This does not sound good 😅

Does Module Def get deleted during uninstall of an app (I'm not sure)? Maybe it should be done (if it's not bad to do…), and so custom fields could be removed on_trash of Module Def?

Maybe it's just easier if apps clean-up after themselves 😄

[ankush]

@cogk but how to know which app created it?

Maybe we can start asking app devs to associate their custom field with module and then with deletion of module the field also goes away automatically.

Nothing I can think of doesn't require following some hygiene from app devs 😬

[ankush]

With great power of running arbitrary things comes the responsibility of cleaning up after yourself 🤣

[blewsky]

I still see an issue here if this is left entirely left at the mercy of app developers. The higher the standard is that frappe enforces, the better the quality will be long-term and the more it helps the brand image. It will only hurt the Frappe ecosystem if there is no enforced quality control and apps can leave ERPnext systems broken how I experienced it anecdotally.

[sharad740]

@blewsky Make sure to export custom field as export fixtures command inside your custom app and implement trigger remove fields in app uninstall hook.

[blewsky]

@sharad740 This is the responsibility of the app developers who publish their apps on the market place, not the users/customers who end up using the apps.
(and by the way, this is not possible on public benches on frappe cloud).