frappe / frappe

Low code web framework for real world applications, in Python and Javascript
https://frappeframework.com
MIT License
7.02k stars 3.35k forks source link

Hooks doctype_js and doctype_list_js don't work with Custom DocTypes #16325

Closed zongo811 closed 2 years ago

zongo811 commented 2 years ago

Description of the issue

I created a DocType with the Flag Custom and tried to connect it with client scripts by using doctype_js and doctype_list_js in hooks.py. Unfortunately the scripts are not called. The hooks only work with DocTypes that have property Custom=False.

Context information (for bug reports)

Output of bench version

erpnext 13.22.1
frappe 13.22.1

Steps to reproduce the issue

  1. Create new doctype "Test Scripts", set property "Custom" to false. Create any field you like, these don't matter.
  2. Create scripts and put them into the Assets-folder: test_scripts_list.js:
    frappe.listview_settings['Test Scripts'] = {
    onload(listview) { 
        msgprint("List: Hello from Assets-Script");
    }
    };

    test_scripts.js:

    frappe.ui.form.on('Test Scripts', {
    onload(frm) {
        msgprint("Hello from Assets-Client Script");
    }
    });
  3. Edit hooks.py, choose the path where you put the scripts:
    doctype_js = {"Test Scripts" : "public/js/test_scripts.js"}
    doctype_list_js = {"Test Scripts" : "public/js/test_scripts_list.js"}
  4. Do "bench migrate" and "bench restart" to apply changes.
  5. Check list view and form view of "Test Scripts" if messages are displayed.
  6. In doctype-properties of "Test Scripts" set "Custom=True".
  7. Check list view and form view of "Test Scripts" again if messages are displayed.

Observed result

After setting "Test Scripts" property "Custom=True" messages in list view and form view are not displayed. Scripts are no longer used.

Expected result

Messages in list view and form view are still displayed. Scripts are used independently of property "Custom".

Stacktrace / full error message

Does not occur.

Additional information

OS version / distribution, Frappe install method, etc. Debian buster

ankush commented 2 years ago

why are you adding customization scripts in-app when doctype itself is not in any app but only lives in DB?

Custom doctypes should ideally use client scripts/server scripts instead of hooks that you've mentioned.

This behavior is intentional because custom doctypes don't have JS files on disk; but hooks might be okay to add.

https://github.com/frappe/frappe/blob/e696a8d73a8c3c76d835e9e54f5b7a4c7d87353b/frappe/desk/form/meta.py#L84-L86

zongo811 commented 2 years ago

Thank you for your quick response.

I want to add ClientScript to a DocType in another App that is beyond my control. For reasons unknown the developer of this App created the DocType with Custom=True and refuses to change it.

I just would like it better to have my Client Script in the Filesystem where I can "git" it instead of having it in the DB. doctype_js just seemed like an obvious solution.

I see a "but hooks might be okay" in your answer, would you accept a Change Request for this?

ankush commented 2 years ago

would you accept a Change Request for this?

Sure. It will need a little bit of clean up in code.

Basically for custom doctypes:

  1. Don't attempt to load any standard JS that's implicitly expected on disk inside the app folder.
  2. Do attempt to load JS from explicit hooks like the ones you mentioned.

Right now it's ignoring all of them.

developer of this App created the DocType with Custom=True

This is weird, no app should create doctypes with Custom=True :thinking:

zongo811 commented 2 years ago

Thank you, feature request was submitted.