Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PR #38608 breaks Pricing Rules ("don't update qty blindly") #38724

Closed
cogk opened this issue Dec 13, 2023 · 3 comments
Closed

PR #38608 breaks Pricing Rules ("don't update qty blindly") #38724

cogk opened this issue Dec 13, 2023 · 3 comments
Assignees
Labels

Comments

@cogk
Copy link
Contributor

cogk commented Dec 13, 2023

@rtdany10 is working on a fix right now if I'm not mistaken? (just a revert)

/cc @s-aga-r


Information about bug

In a Quotation, when adding an Item row, the get_item_details method is called.
When Pricing Rules exist for the given item_code, a list of their names is loaded into a property called pricing_rules.

r.message = {
  ...,
  "pricing_rules": "['my rule name']",
}

However, there is also a pricing_rules field (table) in the parent document (Quotation).

Because the child: item, parameter was removed in #38608 from the frm.call params, this means that the else branch of the callback is executed, which calls frm.set_value({ ..., "pricing_rules": "..." }). This breaks the pricing_rules table in the parent document, silently with an error visible in the console.

https://github.com/frappe/frappe/blob/174e24f1597e6c3834a6888c2e7d2a7cabe69065/frappe/public/js/frappe/form/form.js#L1755-L1758

if ($.isPlainObject(r.message)) {
	if (opts.child) {
		// Copy all fields, even the qty field that PR #38608 wants to preserve.
	} else {
		// update parent doc
		me.set_value(r.message); // This sets the value of the `pricing_rules` TABLE with a string value (bad)
	}

0156339

Module

buying, selling

Version

develop

@cogk cogk added the bug label Dec 13, 2023
@cogk cogk changed the title PR #38608 breaks Pricing Rules PR #38608 breaks Pricing Rules ("don't update qty blindly") Dec 13, 2023
@s-aga-r
Copy link
Contributor

s-aga-r commented Dec 13, 2023

@rtdany10 try replacing the this.frm.call with frappe.call?

@s-aga-r s-aga-r self-assigned this Dec 15, 2023
@s-aga-r
Copy link
Contributor

s-aga-r commented Dec 15, 2023

#38728

@s-aga-r s-aga-r closed this as completed Dec 15, 2023
@rtdany10
Copy link
Contributor

@rtdany10 try replacing the this.frm.call with frappe.call?

That should probably take care of it, let me check in the evening.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants