Skip to content

Update event_compassion.py #2071

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.

Sign up for GitHub

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

Open
davidwul wants to merge 2 commits into
base: 14.0
Choose a base branch
from
Open

Update event_compassion.py #2071

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9afce1f
Update event_compassion.py
davidwul Jan 28, 2026
35ceac7
Update event_compassion.py
davidwul Jan 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions crm_compassion/models/event_compassion.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,9 +163,11 @@ def _compute_expense_lines(self):
def _compute_income_lines(self):
for event in self:
event.income_line_ids = event.invoice_line_ids.filtered(
lambda line: line.payment_state == "paid"
and not line.contract_id
and line.move_id.move_type == "out_invoice"
lambda line: not line.contract_id
and line.account_id.user_type_id.name == "Income"
and ((line.payment_state == "paid"
and line.move_id.move_type == "out_invoice") or
(line.move_id.move_type == "entry"))
)
Comment on lines 165 to 171
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Jan 28, 2026

Choose a reason for hiding this comment

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

high

The logic inside this filtered call introduces new dependencies on account.move.line fields (account_id, move_id.move_type). Since total_income is a stored computed field that depends on this, its @api.depends decorator needs to be updated to reflect these new dependencies. Otherwise, total_income will not be recomputed when these fields change, leading to incorrect data.

The decorator for _compute_income should be updated. A better long-term solution would be to add a proper @api.depends to _compute_income_lines itself, and then make _compute_income depend on income_line_ids.

Comment on lines 163 to 171
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Jan 28, 2026

Choose a reason for hiding this comment

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

medium

Using a hardcoded string "Income" to identify the account type is fragile. It can break if the name is changed or translated. It's better to use an XML ID to get a reference to the record. This makes the code more robust and maintainable.

Suggested change
def _compute_income_lines(self):
for event in self:
event.income_line_ids = event.invoice_line_ids.filtered(
lambda line: line.payment_state == "paid"
and not line.contract_id
and line.move_id.move_type == "out_invoice"
lambda line: not line.contract_id
and line.account_id.user_type_id.name == "Income"
and ((line.payment_state == "paid"
and line.move_id.move_type == "out_invoice") or
(line.move_id.move_type == "entry"))
)
def _compute_income_lines(self):
income_account_type = self.env.ref("account.data_account_type_income", raise_if_not_found=False)
for event in self:
if not income_account_type:
event.income_line_ids = self.env['account.move.line']
continue
event.income_line_ids = event.invoice_line_ids.filtered(
lambda line: not line.contract_id
and line.account_id.user_type_id == income_account_type
and ((line.payment_state == "paid"
and line.move_id.move_type == "out_invoice") or
(line.move_id.move_type == "entry"))
)


@api.depends("analytic_id.line_ids")
Expand All @@ -178,7 +180,8 @@ def _compute_expense(self):
def _compute_income(self):
for event in self:
incomes = event.income_line_ids
event.total_income = sum(incomes.mapped("price_subtotal") or [0])
event.total_income = (sum(incomes.mapped("credit") or [0])
-sum(incomes.mapped("debit") or [0]))
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Jan 28, 2026

Choose a reason for hiding this comment

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

medium

The or [0] is redundant here. sum() on an empty list (which mapped() returns for an empty recordset) correctly returns 0. Removing the or [0] makes the code cleaner.

            event.total_income = sum(incomes.mapped("credit")) - sum(incomes.mapped("debit"))


@api.depends("total_income", "total_expense")
def _compute_balance(self):
Expand Down
Loading