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

[16.0][IMP] account-financial-tools: ADD account_loan_analytic_account #1928

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

Conversation

lmarion-source
Copy link

@lmarion-source lmarion-source commented Aug 29, 2024

This module allows to add analytic distributions on loans and propagate them to moves when the loan is posted

@lmarion-source lmarion-source force-pushed the 16.0-analytic_account_account_loan branch 3 times, most recently from fc2067c to a9e1122 Compare September 5, 2024 08:38
@lmarion-source lmarion-source changed the title [16.0][ADD] Add analytic accounts on account.loan [16.0][IMP] account-financial-tools: ADD account_loan_analytic_account Sep 5, 2024
@lmarion-source lmarion-source force-pushed the 16.0-analytic_account_account_loan branch 3 times, most recently from 6950936 to 6face17 Compare September 5, 2024 12:22
and loan
and vals["account_id"] == loan.interest_expenses_account_id.id
):
vals["analytic_distribution"] = loan.analytic_distribution

Choose a reason for hiding this comment

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

And what if the user already define some analytic_distribution? You completely overwrite filled values.

Copy link
Author

Choose a reason for hiding this comment

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

The analytic_distribution on the move_line must follow the one on the loan : if the distribution changes on the loan, the distribution should be erased on the move_line

and loan
and vals["account_id"] == loan.interest_expenses_account_id.id
):
vals["analytic_distribution"] = loan.analytic_distribution

Choose a reason for hiding this comment

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

Same here, during the write.

Copy link
Author

@lmarion-source lmarion-source Sep 10, 2024

Choose a reason for hiding this comment

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

same here than for the create, the user should not be able to change the distribution on the move_line independently of the loan

@lmarion-source lmarion-source force-pushed the 16.0-analytic_account_account_loan branch from 6face17 to 50232bf Compare September 25, 2024 11:01
Copy link
Contributor

@AnizR AnizR left a comment

Choose a reason for hiding this comment

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

Code LGTM

    This module allows to add analytic distributions on loans and propagate them
    to moves when the loan is posted
@lmarion-source lmarion-source force-pushed the 16.0-analytic_account_account_loan branch from 50232bf to daa9e53 Compare October 7, 2024 09:53
@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). 🤖

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