[19.0][MIG] agreement_rebate: Migration to 19.0

[gdgellatly]

Depends on #99

Summary

Migration of agreement_rebate module to 19.0.

• Version bump to 19.0.1.0.0
• Removed category_id from res.groups XML (field removed in v19)
• Fixed groups_id -> group_ids rename in tests
• Fixed search view: removed invalid string/expand on <group>, added domain="[]" to group-by filters, renamed duplicate filter name
• Fixed translation positional formatting to use named placeholders
• Replaced product.product_category_all ref in tests (removed in v19)

Test plan

• Module installs successfully
• 9/9 tests pass with --test-enable
• Pre-commit passes

[gdgellatly]

Made one visual improvement, left as seperate commit so we can remove/change or backport more easily.

[dannyadair]

Tested this, also with an extensive test suite for production data. Everything looks good.

[yankinmax]

I've an issue on another migration due to this issue.
Can you pls change read_group to _read_group?

WARNING odoo py.warnings: /opt/odoo-venv/lib/python3.10/site-packages/odoo/addons/agreement_rebate/wizards/settlement_create.py:296: DeprecationWarning: Since 19.0, read_group is deprecated. Please use _read_group in the backend code or formatted_read_group for a complete formatted result
....
File "/__w/contract/contract/agreement_rebate_partner_company_group/tests/test_agreement_rebate.py", line 33, in test_create_settlement_wo_filters_global_company_group
    settlement_wiz.action_create_settlement()
  File "/opt/odoo-venv/lib/python3.10/site-packages/odoo/addons/agreement_rebate/wizards/settlement_create.py", line 296, in action_create_settlement
    groups = target_model.read_group(

To fix the migration I would apply such fix (tested on OCA/contract#1405 migration):

1. in _prepare_settlement_line:

vals = {
    "agreement_id": agreement.id,
    "partner_id": group["partner_id"][0]
    if "partner_id" in group
    else agreement.partner_id.id,
}

should be

vals = {
    "agreement_id": agreement.id,
    "partner_id": group["partner_id"]
    if "partner_id" in group
    else agreement.partner_id.id,
}

2. action_create_settlement I would rewrite modern Odoo 19 way:

def action_create_settlement(self):
    self.ensure_one()
    Agreement = self.env["agreement"]
    target_model = self._get_target_model()
    orig_domain = self._prepare_target_domain()
    settlement_dic = defaultdict(lambda: {"lines": []})
    agreements = Agreement.search(self._prepare_agreement_domain())
    for agreement in agreements:
        key = self.get_settlement_key(agreement)
        if key not in settlement_dic:
            settlement_dic[key]["amount_rebate"] = 0.0
            settlement_dic[key]["amount_invoiced"] = 0.0
            settlement_dic[key]["partner_id"] = agreement.partner_id.id
        agreement_domain = orig_domain + self._partner_domain(agreement)
        if agreement.rebate_type == "line":
            if not agreement.rebate_line_ids:
                continue
            for line in agreement.rebate_line_ids:
                domain = self._target_line_domain(
                    agreement_domain, agreement, line=line
                )
                groups = target_model._read_group(
                    domain,
                    groupby=["partner_id"],
                    aggregates=["price_subtotal:sum", "__count"],
                )
                if not groups or (not groups[0][2] and not agreement.additional_consumption):
                    continue

                for partner, amount_invoiced, count in groups:
                    group = {
                        "partner_id": partner.id,
                        "price_subtotal": amount_invoiced,
                        "__count": count,
                    }
                    vals = self._prepare_settlement_line(
                        domain, group, agreement, line=line
                    )
                    settlement_dic[key]["amount_rebate"] += vals["amount_rebate"]
                    settlement_dic[key]["amount_invoiced"] += vals["amount_invoiced"]
                    settlement_dic[key]["lines"].append((0, 0, vals))
        elif agreement.rebate_type == "section_prorated":
            domain = self._target_line_domain(agreement_domain, agreement)
            groups = target_model._read_group(
                domain,
                groupby=["partner_id"],
                aggregates=["price_subtotal:sum", "__count"],
            )
            if not groups or (not groups[0][2] and not agreement.additional_consumption):
                continue

            amount = groups[0][1] if groups else 0.0
            for section in agreement.rebate_section_ids:
                if amount < section.amount_to and amount < section.amount_from:
                    break
                for partner, amount_invoiced, count in groups:
                    group = {
                        "partner_id": partner.id,
                        "price_subtotal": amount_invoiced,
                        "__count": count,
                    }
                    vals = self._prepare_settlement_line(
                        domain, group, agreement, section=section
                    )
                    settlement_dic[key]["amount_rebate"] += vals["amount_rebate"]
                    settlement_dic[key]["lines"].append((0, 0, vals))
            settlement_dic[key]["amount_invoiced"] += amount
        else:
            domain = self._target_line_domain(agreement_domain, agreement)
            groups = target_model._read_group(
                domain,
                groupby=["partner_id"],
                aggregates=["price_subtotal:sum", "__count"],
            )
            if not groups or (not groups[0][2] and not agreement.additional_consumption):
                continue

            for partner, amount_invoiced, count in groups:
                group = {
                    "partner_id": partner.id,
                    "price_subtotal": amount_invoiced,
                    "__count": count,
                }
                vals = self._prepare_settlement_line(domain, group, agreement)
                settlement_dic[key]["lines"].append((0, 0, vals))
                settlement_dic[key]["amount_rebate"] += vals["amount_rebate"]
                settlement_dic[key]["amount_invoiced"] += vals["amount_invoiced"]
    settlements = self._create_settlement(settlement_dic)
    return settlements.action_show_settlement()

3. Remove get_agregate_fields and _settlement_line_break_fields. It's much easier to read the _read_group arguments and is there any possibility these methods would be ever overriden? Why do we need those two helpers?

[yankinmax]

Hello @gdgellatly , can you pls update this PR?

[gdgellatly]

Hi @yankinmax ,

Thank you for the review.

Firstly there is extensive reasons we would need to override those methods. The most common is a quantity based rebate. In terms of break, if you do a lot with multicompany and company groups often you need to break your lines differently.

I am extremely hesitant to update on these suggestions, they will hamper back and forward port and with no diff, honestly I don't even really know what is changed. But from what I could tell, they were breaking changes anyway that should not be taken up.

However I have done the obvious pieces.

[gdgellatly]

@yankinmax The obvious pieces broke tests, so have reverted for now. I am going to need a lot more clarity here. For your own clarity, those helper methods must remain.

[yankinmax]

Hello @gdgellatly from what I see you just need to include test-requirements.txt here as a separate commit.
It should contain such line:

odoo-addon-agreement @ git+https://github.com/OCA/agreement.git@refs/pull/99/head#subdirectory=agreement

This is an OCA dependency.
Thanks in advance for update

[yankinmax]

Hello @gdgellatly
The dependent agreement PR was merged:

• [19.0][MIG] agreement: Migration to 19.0 #99

Can you pls update this one and fix the tests?

[gdgellatly]

/ocabot rebase

[OCA-git-bot]

Sorry @gdgellatly you are not allowed to rebase.

To do so you must either have push permissions on the repository, or be a declared maintainer of all modified addons.

If you wish to adopt an addon and become it's maintainer, open a pull request to add your GitHub login to the maintainers key of its manifest.

[yankinmax]

Hello @OCA/project-service-maintainers can you pls take a look and possibly trigger merge?

[leemannd]

/ocabot merge nobump

[OCA-git-bot]

On my way to merge this fine PR!
Prepared branch 19.0-ocabot-merge-pr-108-by-leemannd-bump-nobump, awaiting test results.

[OCA-git-bot]

Congratulations, your PR was merged at 0f0d139. Thanks a lot for contributing to OCA. ❤️