Skip to content
Closed
Show file tree
Hide file tree
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
27 changes: 17 additions & 10 deletions account_ebics_payment_return/models/account_payment_return.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ def _on_error_parse_xml_and_cancel(self, err_message):
[("name", "=", po_name)]
)
_logger.info("PAIN002 payment_order: %s", payment_order)
if payment_order.state == "generated":
if payment_order.state in ("generated","uploaded"):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

For better readability and adherence to PEP 8 style guidelines, it's recommended to add a space after the comma within the tuple.

Suggested change
if payment_order.state in ("generated","uploaded"):
if payment_order.state in ("generated", "uploaded"):
References
  1. PEP 8 suggests adding a space after commas in tuples, dicts, and lists to improve readability. (link)

if po_state == "RJCT":
_logger.info(
"RJCT payment order %s with the folowing err: %s",
Expand All @@ -121,13 +121,11 @@ def _on_error_parse_xml_and_cancel(self, err_message):
for t in tx:
if t.find("./ns:TxSts", namespaces={"ns": ns}).text == "RJCT":
# search for payment line
payment_line_ids = payment_order.bank_line_ids.filtered(
lambda r, transaction=t: r.name
== transaction.find(
"./ns:OrgnlEndToEndId", namespaces={"ns": ns}
).text
)[0].payment_line_ids
_logger.info("PAIN002 payment_line_ids: %s", payment_line_ids)
endtoend_id=t.find("./ns:OrgnlEndToEndId", namespaces={"ns": ns}).text
payment_id = payment_order.payment_ids.filtered(
lambda l: l.move_id.id == int(endtoend_id))
payment_line_ids = payment_order.payment_line_ids.filtered(lambda l: l.payment_ids.id ==payment_id.id)
_logger.info(f"PAIN002 payment_line_ids: {payment_id.name} with endtoend_id: {endtoend_id}", )
Comment on lines +124 to +128
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

The logic for finding payment_line_ids has several critical issues that could lead to runtime errors and incorrect behavior:

  1. int(endtoend_id) will raise a ValueError if endtoend_id from the XML is not a string representing an integer. This will crash the import process and should be handled gracefully.
  2. The result of filtered, payment_id, is a recordset. If no payment is found, it will be empty. If multiple are found, it will contain multiple records. Accessing attributes like .id or .name will fail if payment_id is not a singleton recordset. This needs to be handled, for example by using ensure_one().
  3. The expression l.payment_ids.id == payment_id.id is incorrect. l.payment_ids is a Many2many field, so it's a recordset and doesn't have an .id attribute. You should check for membership, e.g., payment in l.payment_ids.
  4. The log message is confusing as it says payment_line_ids but logs payment_id.name.
                        endtoend_id = t.find("./ns:OrgnlEndToEndId", namespaces={"ns": ns}).text
                        try:
                            move_id = int(endtoend_id)
                        except (ValueError, TypeError):
                            _logger.warning(f"PAIN002: Invalid OrgnlEndToEndId '{endtoend_id}'.")
                            continue
                        payment = payment_order.payment_ids.filtered(
                            lambda p: p.move_id.id == move_id
                        )
                        if not payment:
                            _logger.warning(f"PAIN002: No payment found for move_id {move_id}.")
                            continue
                        payment.ensure_one()
                        payment_line_ids = payment_order.payment_line_ids.filtered(
                            lambda l: payment in l.payment_ids
                        )
                        _logger.info(
                            f"PAIN002 found payment lines {payment_line_ids.ids} for payment {payment.name} with endtoend_id: {endtoend_id}"
                        )


# free line with message
rsn = t.findall(
Expand All @@ -136,9 +134,18 @@ def _on_error_parse_xml_and_cancel(self, err_message):
rsn_text = []
for r in rsn:
rsn_text.append(r.text)
payment_line_ids.free_line(" ".join(rsn_text))
rsn_txt = " ".join(rsn_text)
_logger.info(f"PAIN002 line free: {rsn_txt} for lines {payment_line_ids}", )
for b in payment_line_ids:
try:
b.free_line(rsn_txt)
_logger.info(f"PAIN002 line free: {rsn_txt}" ,)
except Exception as e:
_logger.info(f"error line free: {e}" ,)
Comment on lines +139 to +144
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The try...except block is a good addition for robustness. However, it can be improved:

  1. Catching the generic Exception is too broad. The free_line method raises odoo.exceptions.UserError. It's better to catch this specific exception to avoid hiding other unexpected errors.
  2. When an error occurs, logging it with _logger.info is not ideal for an error condition. _logger.warning or _logger.error would be more appropriate to highlight the problem. The log message should also include more context, like which payment line failed.
  3. The trailing comma in the logger calls is unnecessary and can be removed for cleaner code.
                        for b in payment_line_ids:
                            try:
                                b.free_line(rsn_txt)
                                _logger.info(f"PAIN002 line free: {rsn_txt}")
                            except UserError as e:
                                _logger.error(f"Error freeing line {b.id}: {e}")


payment_order.generated2uploaded()

if payment_order.state == "generated":
payment_order.generated2uploaded()
self.write({"state": "done", "note_process": err_message})

def _unlink_pain002(self):
Expand Down
2 changes: 1 addition & 1 deletion recurring_contract/models/contract_group.py
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@ def build_inv_line_data(
elif gift_wizard:
product = gift_wizard.product_id
qty = gift_wizard.quantity
contract = gift_wizard.contract_id
contract = gift_wizard.current_contract_id
price = gift_wizard.amount
line_name = gift_wizard.description or product.name
else:
Expand Down
2 changes: 0 additions & 2 deletions setup/.setuptools-odoo-make-default-ignore

This file was deleted.

2 changes: 0 additions & 2 deletions setup/README

This file was deleted.

This file was deleted.

6 changes: 0 additions & 6 deletions setup/account_analytic_attribution/setup.py

This file was deleted.

This file was deleted.

6 changes: 0 additions & 6 deletions setup/account_analytic_compassion/setup.py

This file was deleted.

1 change: 0 additions & 1 deletion setup/account_ebics_CH/odoo/addons/account_ebics_CH

This file was deleted.

6 changes: 0 additions & 6 deletions setup/account_ebics_CH/setup.py

This file was deleted.

This file was deleted.

6 changes: 0 additions & 6 deletions setup/account_ebics_payment_return/setup.py

This file was deleted.

This file was deleted.

6 changes: 0 additions & 6 deletions setup/account_invoice_export_grouped/setup.py

This file was deleted.

This file was deleted.

6 changes: 0 additions & 6 deletions setup/account_invoice_split_invoice/setup.py

This file was deleted.

This file was deleted.

6 changes: 0 additions & 6 deletions setup/account_move_periodic_accounting_transfer/setup.py

This file was deleted.

This file was deleted.

6 changes: 0 additions & 6 deletions setup/account_offbalance_sponsorship/setup.py

This file was deleted.

This file was deleted.

6 changes: 0 additions & 6 deletions setup/account_payment_line_free/setup.py

This file was deleted.

This file was deleted.

6 changes: 0 additions & 6 deletions setup/account_statement_completion/setup.py

This file was deleted.

This file was deleted.

6 changes: 0 additions & 6 deletions setup/compassion_sub_chart_account/setup.py

This file was deleted.

This file was deleted.

6 changes: 0 additions & 6 deletions setup/donation_report_compassion/setup.py

This file was deleted.

This file was deleted.

6 changes: 0 additions & 6 deletions setup/invoice_restrictions/setup.py

This file was deleted.

1 change: 0 additions & 1 deletion setup/recurring_contract/odoo/addons/recurring_contract

This file was deleted.

6 changes: 0 additions & 6 deletions setup/recurring_contract/setup.py

This file was deleted.

Loading