Skip to content

Comments

[FIX] account_payment_pro: use journal currency in internal transfers#784

Open
cav-adhoc wants to merge 1 commit intoingadhoc:18.0from
adhoc-dev:18.0-h-101002-cav
Open

[FIX] account_payment_pro: use journal currency in internal transfers#784
cav-adhoc wants to merge 1 commit intoingadhoc:18.0from
adhoc-dev:18.0-h-101002-cav

Conversation

@cav-adhoc
Copy link
Contributor

No description provided.

Copilot AI review requested due to automatic review settings October 3, 2025 20:04
@roboadhoc
Copy link
Contributor

Pull request status dashboard

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This is a work-in-progress pull request that modifies the amount_signed computation logic for internal transfers in the account payment system. The changes appear to handle currency-specific amount calculations for internal transfers.

  • Adds direct amount_signed computation logic within the _prepare_move_line_default_vals method for internal transfers
  • Includes commented-out code that shows an alternative approach using a computed field method

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines 311 to 314
if self.payment_type == "outbound":
self.amount_signed = -amount_in_journal_currency
else:
self.amount_signed = amount_in_journal_currency
Copy link

Copilot AI Oct 3, 2025

Choose a reason for hiding this comment

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

Direct field assignment in a preparation method is unusual and could cause side effects. Consider returning the amount_signed value or using a proper computed field instead of modifying self.amount_signed directly.

Suggested change
if self.payment_type == "outbound":
self.amount_signed = -amount_in_journal_currency
else:
self.amount_signed = amount_in_journal_currency
# Removed direct assignment to self.amount_signed to avoid side effects.

Copilot uses AI. Check for mistakes.
Comment on lines +674 to +688
# @api.depends("amount", "payment_type")
# def _compute_amount_signed(self):
# internal_transfers = self.filtered(lambda p: p.is_internal_transfer)
# non_internal_transfers = self - internal_transfers

# for payment in internal_transfers:
# amount_in_journal_currency = (
# payment.amount if payment.journal_id.currency_id else payment.amount_company_currency
# )
# if payment.payment_type == "outbound":
# payment.amount_signed = -amount_in_journal_currency
# else:
# payment.amount_signed = amount_in_journal_currency

# super(AccountPayment, non_internal_transfers)._compute_amount_signed()
Copy link

Copilot AI Oct 3, 2025

Choose a reason for hiding this comment

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

Large blocks of commented code should be removed before merging. If this alternative implementation is needed for reference, consider moving it to documentation or a separate branch.

Suggested change
# @api.depends("amount", "payment_type")
# def _compute_amount_signed(self):
# internal_transfers = self.filtered(lambda p: p.is_internal_transfer)
# non_internal_transfers = self - internal_transfers
# for payment in internal_transfers:
# amount_in_journal_currency = (
# payment.amount if payment.journal_id.currency_id else payment.amount_company_currency
# )
# if payment.payment_type == "outbound":
# payment.amount_signed = -amount_in_journal_currency
# else:
# payment.amount_signed = amount_in_journal_currency
# super(AccountPayment, non_internal_transfers)._compute_amount_signed()

Copilot uses AI. Check for mistakes.
Ensure internal transfers respect the journal's secondary currency. The first payment's entry amount must match the origin journal's currency.
@cav-adhoc cav-adhoc changed the title [WIP] [FIX] account_payment_pro: use journal currency in internal transfers Oct 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants