Failed attempt to use `override_doctype_class` 😡

MartinHBramwell · March 5, 2021, 10:47pm

Following a suggestion by @peterg …

… I am attempting to use override_doctype_class for the first time. It seems to be completely ignored.

My app directory:

.
├── config
├── docs
├── myApp
    :    :    : 
├── hooks.py
├── overrides
│   └── sales_invoice.py
    :    :    :

My hooks.py:

# along with any modifications made in other Frappe apps
# override_doctype_dashboards = {
#   "Task": "myApp.task.get_dashboard_data"
# }

override_doctype_class = {
    'SalesInvoice': 'app.overrides.sales_invoice.CustomSalesInvoice'
}

My overrides/sales_invoice.py:

import frappe
from erpnext.accounts.doctype.sales_invoice.sales_invoice import SalesInvoice

class CustomSalesInvoice(SalesInvoice):
  def before_save(self):
    print(' === === === ===  BEFORE SAVE  === === === === === ')
    # super(SalesInvoice, self).on_update()

My temporarily altered erpnext/erpnext/accounts/doctype/sales_invoice/sales_invoice.py:

  def before_save(self):
    print(' *** *** *** ***  BEFORE SAVE  *** *** *** *** *** ')
    set_account_for_mode_of_payment(self)

Resulting log file when I save changes to a draft Sales Invoice:

22:41:58 web.1            | 127.0.0.1 - - [05/Mar/2021 22:41:58] "GET / HTTP/1.0" 404 -
22:45:28 web.1            |  * Detected change in '/home/erpdev/frappe-bench-DELS/apps/myApp/myApp/hooks.py', reloading
22:45:29 web.1            |  * Detected change in '/home/erpdev/frappe-bench-DELS/apps/myApp/myApp/hooks.py', reloading
22:45:30 web.1            |  * Restarting with inotify reloader
22:45:32 web.1            |  * Debugger is active!
22:45:32 web.1            |  * Debugger PIN: 105-941-064
22:45:35 web.1            |  * Detected change in '/home/erpdev/frappe-bench-DELS/apps/myApp/myApp/overrides/sales_invoice.py', reloading
22:45:37 web.1            |  * Restarting with inotify reloader
22:45:38 web.1            |  * Debugger is active!
22:45:38 web.1            |  * Debugger PIN: 105-941-064
22:46:03 web.1            | 127.0.0.1 - - [05/Mar/2021 22:46:03] "POST /api/method/frappe.desk.search.search_link HTTP/1.0" 200 -
22:46:03 web.1            |  * Detected change in '/home/erpdev/frappe-bench-DELS/apps/myApp/myApp/__pycache__/hooks.cpython-38.pyc', reloading
22:46:05 web.1            | 127.0.0.1 - - [05/Mar/2021 22:46:05] "GET /api/method/frappe.desk.form.load.getdoctype?doctype=Item&with_parent=1&cached_timestamp=2021-01-25+20%3A49%3A50.222976&_=1614980205184 HTTP/1.0" 200 -
22:46:06 web.1            |  * Restarting with inotify reloader
22:46:07 web.1            |  * Debugger is active!
22:46:07 web.1            |  * Debugger PIN: 105-941-064
22:46:14 web.1            | 127.0.0.1 - - [05/Mar/2021 22:46:14] "GET /api/method/frappe.desk.form.utils.validate_link?value=Biodox+3000+ppm+litro&options=Item&fetch=&_=1614980205185 HTTP/1.0" 200 -
22:46:15 web.1            | 127.0.0.1 - - [05/Mar/2021 22:46:15] "POST /api/method/erpnext.stock.get_item_details.get_item_details HTTP/1.0" 200 -
22:46:19 web.1            |  *** *** *** ***  BEFORE SAVE  *** *** *** *** *** 
22:46:20 web.1            | 127.0.0.1 - - [05/Mar/2021 22:46:20] "POST /api/method/frappe.desk.form.save.savedocs HTTP/1.0" 200 -
22:46:21 web.1            | 127.0.0.1 - - [05/Mar/2021 22:46:21] "GET /api/method/frappe.desk.notifications.get_open_count?doctype=Sales+Invoice&name=001-003-000000001&items=%5B%22Payment+Entry%22%2C%22Payment+Request%22%2C%22Journal+Entry%22%2C%22Invoice+Discounting%22%2C%22Dunning%22%2C%22Timesheet%22%2C%22Delivery+Note%22%2C%22Sales+Order%22%2C%22POS+Invoice%22%2C%22Sales+Invoice%22%2C%22Auto+Repeat%22%5D&_=1614980205186 HTTP/1.0" 200 -
    :    :    :    :    :    :

I would expect the logged print statement to be :

22:46:19 web.1            |  === === === ===  BEFORE SAVE  === === === === ===

It should not be …

22:46:19 web.1            |  *** *** *** ***  BEFORE SAVE  *** *** *** *** ***

… because the call to super() is disabled.

I’ll be grateful for any hints as to how to get this to work

Questions

Is there a “please_dont_ignore_my_hooks()” command?

Notes

erpdev@loso:~/frappe-bench-DELS$ bench version
myApp 0.0.1
erpnext 13.0.0-beta.12
frappe 13.0.0-beta.11

erpdev@loso:~/frappe-bench-DELS$ lsb_release -a
Description:  Ubuntu 20.04.2 LTS
Codename: focal

erpdev@loso:~/frappe-bench-DELS$ bench --site dev.erpnext.host clear-cache
erpdev@loso:~/frappe-bench-DELS$ bench --site dev.erpnext.host migrate
Migrating dev.erpnext.host
Updating DocTypes for frappe        : [=============================] 100%
Updating DocTypes for erpnext       : [============================] 100%
Updating DocTypes for myApp    : [=============================] 100%
Updating Dashboard for frappe
Updating Dashboard for erpnext
Updating Dashboard for myApp
Updating customizations for Address
Updating customizations for Contact
Building search index for dev.erpnext.host
erpdev@loso:~/frappe-bench-DELS$

peterg · March 6, 2021, 1:37am

It looks like an addressing issue. Is your app is called “myApp”?

In that case, the “app” in ‘app.overrides.sales_invoice.CustomSalesInvoice’ doesn’t look right.

MartinHBramwell · March 6, 2021, 2:27am

Oh God! The perils of copy’n paste coding.

Thanks!

MartinHBramwell · March 6, 2021, 1:42pm

Despite help from @peterg this is still not working.

I altered my hooks.py thusly …

override_doctype_class = {
    'SalesInvoice': 'myApp.overrides.sales_invoice.CustomSalesInvoice'
}

… replacing app with myApp.

It did not solve the problem!

My full app directory looks like this:

erpdev@loso:~/frappe-bench-DELS$ tree -L 2 apps/myApp/
apps/myApp/
├── customizations
├── myApp
│   ├── config
│   ├── docs
│   ├── myApp
│   ├── fixtures
│   ├── hooks.py
│   ├── __init__.py
│   ├── modules.txt
│   ├── overrides
│   ├── patches.txt
│   ├── __pycache__
│   └── templates
├── myApp.egg-info
├── LICENSE
├── license.txt
├── MANIFEST.in
├── README.md
├── requirements.txt
├── services
│   ├── einv_svc
│   └── rSYNC.sh
└── setup.py

19 directories, 38 files
erpdev@loso:~/frappe-bench-DELS$

So I altered my hooks.py again…

override_doctype_class = {
    'SalesInvoice': 'myApp.myApp.overrides.sales_invoice.CustomSalesInvoice'
}

… replacing myApp with myApp.myApp.

It made no difference, either.

I have no idea how to determine which of those two is correct. What’s the rule here?

It occurred to me that trying to intercept before_save is a bad idea, because it can be intercepted separately as a Document Event hook, so I tried instead to override set_status, the same way.

That also did not work.

I then got suspicious that the hooks.py for my app was not working at all.

I added a doc_event hook:

doc_events = {
	"SalesInvoice": {
		"before_save": "electronic_invoice.crud_events.before_save"
	}
}

That ALSO did not work; so I then tried:

doc_events = {
	"*": {
		"before_save": "electronic_invoice.crud_events.before_save"
	}
}

Yay! It works!

Wut? Wait a sec …

doc_events = {
	"Sales Invoice": {
		"before_save": "electronic_invoice.crud_events.before_save"
	}
}

Gees whiz. Works too!

Now my, class override …

override_doctype_class = {
    'Sales Invoice': 'electronic_invoice.overrides.sales_invoice.CustomSalesInvoice'
}

Well, golly, gosh, darn that works too.

So the documentation clearly shows that DocType type names in hooks must not have spaces. Correct?

The name of the class is SalesInvoice – spaces removed:

class SalesInvoice(SellingController):

The DocType ToDo does not have spaces in its name, so spaces are to be removed … OBVIOUSLY!

*  *  *    WRONG!  *  *  *

Question:

Why does ERPNext documentation HAVE to be filled with land mines?

Any way, on to the next step.

Now I have to add the call to super. Do I do…

    super(SalesInvoice, self).before_save()

… or do I do …

    super(Sales Invoice, self).before_save()

Who the hell knows!

Welcome to the ERPNext guessing game!

(ji$u$ f&k$#g @H$t )

p.s.

What works is:

    super().before_save()

peterg · March 6, 2021, 3:49pm

Martin, I appreciate that you’re frustrated, but the tone of your response here isn’t very helpful. Ironically, your post reveals some significant misunderstandings and gaps, not of ERPNext but just plain old python. That includes myApp vs. myApp.myApp, “Sales Invoice” vs. SalesInvoice, and super(). None of these are frappe design decisions. It’s all just vanilla python. I’m sure many people here, including myself, would be more than happy to explain things that aren’t clear to you as best we can, but the sarcasm is kind of exhausting.

MartinHBramwell · March 6, 2021, 9:55pm

I am sorry to have annoyed you.

I also regret distracting from the major issue, by mentioning the minor issues that you focused on.

The real problem:

In different contexts DocType names are sometimes concatenated and sometimes not.

So:

To use a multiple name DocType for code examples, and make it clear when and when not to concatenate, would be ideal.
To use a single name DocType for code examples would be ok, but not great.
To use a two name DocType that also happens to be the only concatenated two word name is horrible.

Worse still, it’s used in a code example that shows NO error messages of any kind if you get it wrong. Getting it right is like a combination padlock: when everything is correct you get something; anything else, you get nothing.

I have been told off before for being sarcastic and typically I’m told, “if you don’t like it, fix it!” Well, guess what … I do … over and over again, filling in the gaping blanks in the documentation, but getting very little thanks or recognition…

… here: How to fully submit a DocType from Python?

and here: How to create an "Absolute Minimum Script Report"?

and here: [solved] cURL example of an attachment upload RPC call?

and here: How to set a "Naming Series" sequence start number through the RPC API?

and here: How to remove the "Publications" card from the landing page?

and here: How to get 'site_config.json' settings in Python code?

and here: How to call "Chart of Accounts Importer" through RPC API? - #7 by MartinHBramwell

and here: Can't use port 8000 except as NGinx upstream. What am I missing? - #5 by MartinHBramwell

and here: [Solved] What's the right way to version control my customizations? - #2 by MartinHBramwell

and here: How to create an "Absolute Minimum Script Report"?

All of those represent cases where I found the docs to be like product brochure feature lists, “Hey, here’s another cool thing ERPNext can do!”. They’re not written to fully armed the developer to go in and solve any problem. They’re more like, “When you figure out how, you will be able to …”

Very often, the examples only show the trivial case, like here: REST API – RPC, where the typical case is a call with parameters, but the example only shows how it’s done without parameters. Once again, I made the effort to fill in the hole: [Solved] cURL example of a whitelist method RPC call with parameters?

peterg · March 7, 2021, 12:26am

This is the misunderstanding I was talking about. The DocType name is “Sales Invoice”. “SalesInvoice”, on the other hand, is the name of a python class. Structurally, these are very different things. There’s no space because Python class names can’t have spaces. The fact that they have similar but slightly different names shouldn’t be confusing because there’s really no context where it’s structurally unclear which you should be using.

This, for example:

…is obviously wrong. Using a class name in this way is not valid Python. A class name would never be in quotes, and more fundamentally it would be extremely odd to use one as a key in a dict structure. I’m not sure how exactly the documentation frames the issue, but there really isn’t any ambiguity in this context.

MartinHBramwell · March 7, 2021, 12:54am

Not if it is what you do all day long.

For a newcomer, context can be a very hard thing to determine. That’s one of the main problems with the documentation – it is rarely explicit about context, code examples just float in space and it can be a real puzzle to know where to apply the example and where not.

override_doctype_class = {
    'SalesInvoice': 'myApp.myApp.overrides.sales_invoice.CustomSalesInvoice'
}

Oh, really?

I got that directly from the code example I have been complaining about: Override DocType Class

override_doctype_class = {
    'ToDo': 'app.overrides.todo.CustomToDo'
}

Furthermore, it is wrong, but not in the way you say. To get it work I had to do:

override_doctype_class = {
    'Sales Invoice': 'myApp.overrides.sales_invoice.CustomSalesInvoice'
}

Because, it isn’t a class name, it’s a DocType!

peterg · March 7, 2021, 1:08am

Yes. The structure here is straightforward and identical to other parallel structures in hooks:

override_doctype_class = {
    'Doctype Name': 'path.to.python.package'
}

Exactly. That is what I am saying. In hooks.py, it’s always DocType names. Never objects.

Is the thing confusing you the fact that there is no space in “ToDo”? That’s because the name of the doctype is “ToDo”. There’s no such thing as “To Do” in Frappe.

MartinHBramwell · March 7, 2021, 1:26am

I already made clear the main issue that caused me so much trouble:

I should have been more accurate, instead of stating, “In different contexts DocType names are sometimes concatenated and sometimes not.” , I should have said:

DocType names and the names of their Python handlers can be very similar. Eg, `SalesInvoice’ is the Python handler for the “Sales Invoice” DocType.

However that still sidesteps the booby trap I fell into. This is the boobytrap:

I know that there’s no such thing as “To Do” in Frappe … now, after hours of bashing my head against a wall!

It’s the fact that it is the only concatenated two word DocType name in all of Frappe and ERPNext that makes it the absolute worst DocType to use where DocType names and Python class names are mixed in the same code examples.

peterg · March 7, 2021, 1:47am

I’m not quite clear on your goals in this thread, but I’m glad you seem to have figured out your original question now. I’m sure you’re right that the documentation is sometimes confusing to people new to python. Documentation writing is tedious work. PRs always welcome!

saurabhn · January 31, 2024, 9:19am

Just wanted to thank you for pointing this out. I too was breaking my head due to the whitespace in DocType name.

MartinHBramwell · January 31, 2024, 1:01pm

I am delighted that my comments were helpful.

However, I continue to think that the principal stakeholders in ERPNext fail to appreciate the number of new users who give up and go away because of these sorts of anti-personnel mines scattered around the code and documentation.

TurkerTunali · February 1, 2024, 11:38am

Thank you for your efforts @MartinHBramwell . Do we still need to update the documantation regard to your issue?

MartinHBramwell · February 4, 2024, 12:12pm

Yes!

The anti-personnel land mine is still there in the documentation : Document Hooks

In the entire list of DocTypes there is exactly one who’s name is concatenated: ToDo

Therefore, logically, to a newcomer that means that if this is correct …

override_doctype_class = {
    'ToDo': 'app.overrides.todo.CustomToDo'
}

… then that definitely means that this is correct …

override_doctype_class = {
    'SalesInvoice': 'myApp.overrides.sales_invoice.CustomSalesInvoice'
}

The only thing that will save a newcomer from hours of head scratching is for that bit of documentation to have a bold, explicit note saying “WATCH OUT : you MUST respect spaces in names of DocTypes. The ToDo DocType is one of the very few who’s name has no space in it, most of the others use normal spacing”.

Better would be NEVER to use ToDo in any coding example.

TurkerTunali · February 5, 2024, 12:16pm

I believe that the general idea is:

If it’s written in capital letters (ie ToDo) that string must be same as Doctype name. If it has a space, then there should be a space.
If it’s written in lower case then it should be python class name and it should be a pythonic.

MartinHBramwell · February 5, 2024, 3:02pm

I cannot understand why I am unable to convey the simple, simple, simple issue that concerns me here.

If you look back over this entire thread I have been trying over and over again to say to various different people, “Yes you’re right about technical issue xyz. But it’s not what I’m talking about.”

This is last post I make on this topic because I must be in the wrong but just cannot see why. I have been trying to get everyone to see this single little point:

          Using the ToDo DocType in coding examples
           is a HORRIBLE booby trap for newcomers.
      Any other DocType with spaces in their name would be fine.

That’s my entire point.
Close this thread.
Please.

peterg · February 5, 2024, 3:21pm

Will do.