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

[Minor Fix] Force toggle display Net Total field when default taxes are fetched #15532

Merged

Conversation

SaiFi0102
Copy link
Collaborator

To reproduce the problem:

  • Create Taxes and Charges Template with some Inclusive Taxes
  • Set it as the default Template so that it will be fetched on transactions automatically
  • Create a new transaction (ex Purchase Order)
  • You will see that Net Total, Net Rate and Net Amount fields are not shown since there is a condition in set_dynamic_fields() that is preventing the AJAX call to toggle its display.

@rmehta
Copy link
Member

rmehta commented Oct 2, 2018

👍 @SaiFi0102 Thanks for fixing this issue.

Please include an animated GIF / screenshots for your PRs. This will greatly help us in reviewing.

Please do through this once:

https://github.com/frappe/erpnext/wiki/Pull-Request-Checklist

…ment

-Fixed problem that discount amount was not recalculated (in mapped document)
-Fixed problem that set_dynamic_fields would work only on certain condition that led to showing Total (Company Currency) field even when using company currency
@SaiFi0102
Copy link
Collaborator Author

Net Total does not show up when an inclusive tax is added in taxes table:
nettotal
After fix:
nettotalfix

My custom feature to create Purchase Invoice from Sales Order shows that item.discount_amount is not calculated in taxes_and_totals.py
discount
It only needed one line to fix this so I'm skipping the GIF of the fix here

Since party_account_currency isn't set from set_missing_values, clicking Make -> Sales/Purchase Invoice shows undefined in place of currency for Outstanding Amount
party_account_currency
After fix:
party_account_currency_fix

I also removed the condition in set_dynamic_labels because then it was creating more problems trying to force reset labels for only some conditions. Like it would show (Company Currency) fields, even though the transaction would be in the company currency:
companycurrency

@stale
Copy link

stale bot commented Oct 13, 2018

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed within a week if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing.

@stale stale bot added the inactive label Oct 13, 2018
@SaiFi0102
Copy link
Collaborator Author

This PR is ready for review

@stale stale bot removed the inactive label Oct 14, 2018
@nabinhait nabinhait merged commit 3c9155e into frappe:staging-fixes Oct 18, 2018
@SaiFi0102 SaiFi0102 deleted the Show-Net-Total-On-Fetch-Default-Taxes branch October 21, 2018 08:51
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.

None yet

4 participants