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

fix(Expense Claim): get_taxes condition #2077

Merged
merged 1 commit into from
Aug 14, 2024

Conversation

krantheman
Copy link
Member

Issue

After adding a few rows in the expenses table, new ones randomly become uneditable.

a20um1Z

Investigation

It seems to occur because of the following code, which is executed every time the sanctioned amount in an expense is updated. Not entirely sure why, but turns out that making any server side call causes this to happen.

	get_taxes: function (frm) {
		if (frm.doc.taxes) {
			frappe.call({
				method: "calculate_taxes",
				doc: frm.doc,
				callback: () => {
					refresh_field("taxes");
					frm.trigger("update_employee_advance_claimed_amount");
				},
			});
		}
	},

Temporary Solution

The get_taxes function is meant to run only if there are taxes added in the expense claim document. However, an empty array being a truthy value, frm.doc.taxes in if (frm.doc.taxes) always returns true and this call is made for every expense.

Fixing this condition, at the very least, prevents this issue from occurring for most users (who I'm guessing add their taxes after their expenses). However, a more universal fix, involving business logic refactoring, may be in order.

@krantheman krantheman merged commit 6b647a4 into frappe:develop Aug 14, 2024
5 checks passed
@krantheman krantheman deleted the fix-expense-claim branch August 14, 2024 09:35
krantheman added a commit that referenced this pull request Aug 14, 2024
…2077

fix(Expense Claim): get_taxes condition (backport #2077)
krantheman added a commit that referenced this pull request Aug 14, 2024
…2077

fix(Expense Claim): get_taxes condition (backport #2077)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant