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

[14.0][IMP] account_financial_report: general ledger account range #1165

Open
wants to merge 1 commit into
base: 14.0
Choose a base branch
from

Conversation

WesleyOliveira98
Copy link

This improvement allows filtering accounts by range in the General Ledger for chart of accounts that use a dotted format, such as 1.1.1.01. Currently, this functionality is not available because accounts with dots fail the isdigit() check.

Brazilian Chart of Accounts Example (https://github.com/OCA/l10n-brazil/blob/14.0/l10n_br_coa_simple/data/account.account.template.csv)
image

General Ledger with Brazilian Accounts
image

When attempting to remove dots from accounts and compare them as integers using the filtered method, the filter does not apply correctly if the accounts have different numbers of digits (when we use the search method, this isn't a problem). So, we implemented a solution where we search for the position of each account in the chart of accounts, finding the position of the start_range account and the end_range account, and then retrieve the accounts between these positions.

image

cc: @marcelsavegnago @kaynnan @Matthwhy

@WesleyOliveira98 WesleyOliveira98 marked this pull request as ready for review May 7, 2024 19:16
@WesleyOliveira98 WesleyOliveira98 force-pushed the 14.0-account_financial_report-imp-general-ledger-account-range branch from 18cfd33 to 9f3cddb Compare May 22, 2024 21:00
@WesleyOliveira98
Copy link
Author

It was necessary to search for the last appearance of the end_range code in case you have the same account code in two or more different companies, the index method returns the first appearance of the code, we also added some unit tests

Copy link

@kaynnan kaynnan left a comment

Choose a reason for hiding this comment

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

LGTM

@WesleyOliveira98
Copy link
Author

ping @OCA/accounting-maintainers

1 similar comment
@marcelsavegnago
Copy link
Sponsor Member

ping @OCA/accounting-maintainers

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@marcelsavegnago
Copy link
Sponsor Member

ping @WesleyOliveira98

@WesleyOliveira98 WesleyOliveira98 force-pushed the 14.0-account_financial_report-imp-general-ledger-account-range branch from 9f3cddb to bad8897 Compare June 4, 2024 17:00
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.

4 participants