Skip to content
Open
Changes from all commits
Commits
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
10 changes: 6 additions & 4 deletions crm_compassion/models/event_compassion.py
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
Contributor

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
Contributor

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,7 @@ 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("balance") or [0])

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