Repository: frappe/frappe
Pull request: fix(quick entry): make sure init_callback is always called
  Prior to this PR, as noted in issue #7638, it is not possible with
  frappe.new_doc to initialize certain fields of the new document, such as the
  description of a Task or the posting_date of a Journal Entry (in ERPNext).
  The reason this occurs is that currently the route_options which can be set
  in the second argument to frappe.new_doc() are only allowed to set certain
  field types of a document (namely, Link, Select, Data, and Dynamic Link).

  Although it turns out that it would not work to allow any field type to
  be set in the route options (in particular, attempting to allow one to set
  Table field types in this way is non-functional), it would be reasonable
  simply to try setting other fields that cannot be set in the route_options
  via the callback allowed as the third argument of frappe.new_doc. And
  indeed, this approach works for those DocTypes that have a Quick Entry Form.
  For those DocTypes that do not, however, the callback is never called.

  This PR modifies frappe.ui.form.make_quick_entry() -- which frappe.new_doc
  calls to do most of its work -- so that the callback is called regardless
  of whether the DocType has a Quick Entry Form or not. The only slight
  awkwardness in this is that if there is a Quick Entry, the callback is
  passed the dialog object of that Quick Entry, whereas if there is no
  Quick Entry, the callback is only passed the doc object that is about to be
  edited in the standard Form interface for a new document.

  Nevertheless, in any case, it is now possible to write a callback which
  will initialize any field in the new document being created.

  Resolves #7638.

Problem statement:
[Bug] Cannot set Text Editor's content with frappe.new_doc()
Assume I have a DocType `Task` with these fields:
* `subject` (`Data`)
* `description` (`Text Editor`)

If I call the following in JavaScript, the new `Task`s `subject` will be set to "New task" but the `description` will be left blank.

```js
frappe.new_doc("Task", {"description": "Do stuff", "subject": "New task"})
```
Same problem with `frappe.route_options()`.

This happens with all fields of type `Text Editor`.

Though this works:
```js
frm.doc.description = "Do stuff"
frm.refresh()
```

### Version
Frappe Framework: v11.1.32 (master)

Hints from issue discussion:
This bug still exists in the current develop version. In our case, we are trying to add a custom button which will allow us to create a journal entry (of Voucher Type "Bank Entry") from a Bank Transaction document, and want to also fill in the posting_date (from the date of the Bank Transaction, and one Journal Entry Accounts line in the accounts field (since typically there ought to be a line matching the Bank Transaction). Looking just at the posting date, if we call something like

`frappe.new_doc("Journal Entry", {"voucher_type": "Bank Entry", "posting_date": me.data.date})`

then just like the OP, the voucher_type is pre-set to "Bank Entry" but the posting_date is left to its usual default, which is today's date.

From the code, it is not hard to see why: This second argument is setting the route_options, and the code in frappe.model.get_new_doc (at `frappe/public/js/frappe/model/create_new.js:64` that uses the route options is specifically
allowing only certain field types:
```
         			if(df && in_list(['Link', 'Data', 'Select', 'Dynamic Link'], df.fieldtype) && !df.no_copy) {
		                        doc[fieldname]=value;
                                }
```

Moreover, I tried using the third argument to frappe.new_doc, namely init_callback, like so:

`frappe.new_doc("Journal Entry", {"voucher_type": "Bank Entry"}, dialog => { dialog.doc.posting_date = me.data.date; });`

but that also fails because if the result of the new_doc is not a Quick Entry dialog but rather a new document Form, the init_callback is ignored (cf. `frappe/public/js/frappe/form/quick_entry.js:35`).

I am happy to submit a pull request to fix this bug and resolve this issue, but I see two possible ways to do it:
(A) Remove the limitations on the field types that will be copied from the route_options. I don't understand the rationale for there being a limitation in the first place; looking at the commit (cd0f5e76) that initially created the limitation, although the commit message simply said that route options should not be copied for fields with the no_copy property, the commit also introduced the limitation to only Link and Select field types, without any explanation. So before that, any field type could be set in the route_options, and since then gradually more field types have been allowed. I do not see the difficulty that allowing arbitrary field types (as long as no_copy is not set) would cause, so one possibility for fixing this bug would be to remove the condition on field types altogether. Or maybe there are just a small number of field types it would be best to *disallow* from being set by route_options, so that all others are allowed? If so, what would those problematic field types be?

(B) The function frappe.ui.form.make_quick_entry that is typically being called by frappe.new_doc has a fourth argument called "doc" which at first glance appears to be the initial value of the document to be created. So I could extend the signature of frappe.new_doc to allow this fourth argument as well and if specified pass it on to make_quick_entry. Hopefully that would then allow arbitrary fields to be initialized.

If you could just let me know which of the two above approaches is preferable to the Frappe development team, I will be happy to submit a corresponding pull request to close this issue. Thanks!

Commit message: fix(quick entry): make sure init_callback is always called

  Prior to this PR, as noted in issue #7638, it is not possible with
  frappe.new_doc to initialize certain fields of the new document, such as the
  description of a Task or the posting_date of a Journal Entry (in ERPNext).
  The reason this occurs is that currently the route_options which can be set
  in the second argument to frappe.new_doc() are only allowed to set certain
  field types of a document (namely, Link, Select, Data, and Dynamic Link).

  Although it turns out that it would not work to allow any field type to
  be set in the route options (in particular, attempting to allow one to set
  Table field types in this way is non-functional), it would be reasonable
  simply to try setting other fields that cannot be set in the route_options
  via the callback allowed as the third argument of frappe.new_doc. And
  indeed, this approach works for those DocTypes that have a Quick Entry Form.
  For those DocTypes that do not, however, the callback is never called.

  This PR modifies frappe.ui.form.make_quick_entry() -- which frappe.new_doc
  calls to do most of its work -- so that the callback is called regardless
  of whether the DocType has a Quick Entry Form or not. The only slight
  awkwardness in this is that if there is a Quick Entry, the callback is
  passed the dialog object of that Quick Entry, whereas if there is no
  Quick Entry, the callback is only passed the doc object that is about to be
  edited in the standard Form interface for a new document.

  Nevertheless, in any case, it is now possible to write a callback which
  will initialize any field in the new document being created.

  Resolves #7638.

Patch to review:
diff --git a/frappe/public/js/frappe/form/quick_entry.js b/frappe/public/js/frappe/form/quick_entry.js
index 2da7b8f2364b..0a489e26d6cd 100644
--- a/frappe/public/js/frappe/form/quick_entry.js
+++ b/frappe/public/js/frappe/form/quick_entry.js
@@ -35,7 +35,11 @@ frappe.ui.form.QuickEntryForm = Class.extend({
 				if (this.is_quick_entry() || this.force) {
 					this.render_dialog();
 					resolve(this);
-				} else {
+				} else { // No quick entry, use full Form
+					// but still give callback a shot at the doc
+					if (this.init_callback) {
+						this.init_callback(this.doc);
+					}
 					frappe.quick_entry = null;
 					frappe.set_route('Form', this.doctype, this.doc.name)
 						.then(() => resolve(this));


Code review comments:
diff_hunk: @@ -35,7 +35,11 @@ frappe.ui.form.QuickEntryForm = Class.extend({
 				if (this.is_quick_entry() || this.force) {
 					this.render_dialog();
 					resolve(this);
-				} else {
+				} else { // No quick entry, use full Form
+					// but still give callback a shot at the doc
+					if (this.init_callback) {
+						this.init_callback(this.doc);
+					}
path: frappe/public/js/frappe/form/quick_entry.js
text: @user1:
- Use `if-else if-else` for this, rather than checking callback in else

@author:
Sorry I am not following; if the condition on line 35 is true, we render the dialog (which ultimately will call the callback); but if not, then we have to set the route. In the else clause from that condition on line 35 is our only opportunity to call the callback along that code path, but of course we can't call it if it's not defined. So we need to check that. So I am not sure how you'd like me to restructure the code. I am happy to change it however you like, I am just not clear on exactly what you propose. I look forward to further detail from you, thanks.