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: AP report does not show expense claim payables #36333

Merged
merged 4 commits into from
Aug 4, 2023

Conversation

GursheenK
Copy link
Member

Problem
When an Expense Claim is created using the Expense Claim Doctype from the hrms app, the Accounts Payable Report does not fetch the Payment Ledger Entries for the payable expense. This leads to a mismatch between the General Ledger Report balance amount and the Accounts Payable total amount.

Accounting Ledger for an Expense Claim of ₹1000

Screenshot 2023-07-26 at 4 54 26 PM

Accounts Payable Report ( Payable for the Expense Claim not recorded ) - Outstanding ₹1000

Screenshot 2023-07-26 at 4 58 32 PM

General Ledger for the Creditors Account - Balance -₹2000

Screenshot 2023-07-26 at 5 01 47 PM


Solution

  • Add an extra column for showing Party Type.
  • Fetch Payment Ledger Entries with the party type Employee along with Supplier in AP Report.

@github-actions github-actions bot added the needs-tests This PR needs automated unit-tests. label Jul 26, 2023
@deepeshgarg007 deepeshgarg007 self-assigned this Jul 26, 2023
@codecov
Copy link

codecov bot commented Jul 26, 2023

Codecov Report

Merging #36333 (f5761e7) into develop (8c410c6) will increase coverage by 0.11%.
Report is 72 commits behind head on develop.
The diff coverage is 37.36%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #36333      +/-   ##
===========================================
+ Coverage    64.11%   64.22%   +0.11%     
===========================================
  Files          785      785              
  Lines        60780    61004     +224     
===========================================
+ Hits         38970    39182     +212     
- Misses       21810    21822      +12     
Files Changed Coverage Δ
...counts/report/accounts_payable/accounts_payable.py 100.00% <ø> (ø)
...counts_payable_summary/accounts_payable_summary.py 0.00% <ø> (ø)
..._receivable_summary/accounts_receivable_summary.py 0.00% <0.00%> (ø)
...t/crm/doctype/twitter_settings/twitter_settings.py 0.00% <0.00%> (ø)
erpnext/accounts/party.py 77.40% <9.09%> (-0.76%) ⬇️
.../report/accounts_receivable/accounts_receivable.py 70.64% <60.00%> (-1.60%) ⬇️

... and 19 files with indirect coverage changes

Copy link
Member

@deepeshgarg007 deepeshgarg007 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we can fix this for all party types rather than just for employees, in case any new party type gets introduced

This report accepts some args and party_type is one of them, from my understanding, this is right now to make out if its an accounts receivable or payable report.

The party_type arg can be replaced with report_type or account_type and that can be used to fetch all the party types from the "Party Type" master that have that account type and make other necessary changes in the report

Also similar changes will be needed in Accounts Receivable/Payable summary reports

@deepeshgarg007 deepeshgarg007 added the backport version-14-hotfix backport to version 14 label Aug 4, 2023
@deepeshgarg007 deepeshgarg007 merged commit 17585f0 into frappe:develop Aug 4, 2023
12 checks passed
deepeshgarg007 added a commit that referenced this pull request Aug 4, 2023
…-36333

fix: AP report does not show expense claim payables (#36333)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport version-14-hotfix backport to version 14 needs-tests This PR needs automated unit-tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants