Skip to content

[19.0][MIG] agreement_rebate: Migration to 19.0#108

Open
gdgellatly wants to merge 38 commits intoOCA:19.0from
gdgellatly:19.0-mig-agreement_rebate
Open

[19.0][MIG] agreement_rebate: Migration to 19.0#108
gdgellatly wants to merge 38 commits intoOCA:19.0from
gdgellatly:19.0-mig-agreement_rebate

Conversation

@gdgellatly
Copy link

@gdgellatly gdgellatly commented Feb 28, 2026

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

sergio-teruel and others added 30 commits February 28, 2026 21:16
Currently translated at 100.0% (116 of 116 strings)

Translation: agreement-16.0/agreement-16.0-agreement_rebate
Translate-URL: https://translation.odoo-community.org/projects/agreement-16-0/agreement-16-0-agreement_rebate/es/
…d twice when process the settlement in tow steps

TT52964
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: agreement-18.0/agreement-18.0-agreement_rebate
Translate-URL: https://translation.odoo-community.org/projects/agreement-18-0/agreement-18-0-agreement_rebate/
1. Use `default_*` context keys instead of custom ones.
   This actually solves an issue with the CI.

2. Drop the virtual records to prepare invoice values. That was needed to play
   onchanges but now all interesting fields are computed.

3. Refactor the invoice creation method to process groups in a cleaner way.
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: agreement-18.0/agreement-18.0-agreement_rebate
Translate-URL: https://translation.odoo-community.org/projects/agreement-18-0/agreement-18-0-agreement_rebate/
Currently translated at 100.0% (114 of 114 strings)

Translation: agreement-18.0/agreement-18.0-agreement_rebate
Translate-URL: https://translation.odoo-community.org/projects/agreement-18-0/agreement-18-0-agreement_rebate/it/
jakobkrabbe and others added 4 commits February 28, 2026 21:16
Currently translated at 100.0% (114 of 114 strings)

Translation: agreement-18.0/agreement-18.0-agreement_rebate
Translate-URL: https://translation.odoo-community.org/projects/agreement-18-0/agreement-18-0-agreement_rebate/sv/
… taking care about it

Before these changes, the value set in the “group invoices by” field was not taken into account when creating invoices.

After these changes, invoices are created normally, grouped according to the selected option. In addition, an improvement has been added so that if an invoice is forced to a specific person, that value is used for grouping. When a value is selected for “Force invoice to”, the “group invoices by” option will be hidden.
@gdgellatly gdgellatly force-pushed the 19.0-mig-agreement_rebate branch from 552bcf6 to 41f30af Compare March 1, 2026 07:12
@gdgellatly gdgellatly force-pushed the 19.0-mig-agreement_rebate branch from 41f30af to 4422f17 Compare March 2, 2026 19:29
Move the conditional item selection fields (products, templates,
categories, condition, domain) into the same group as rebate_target.

The separate "Select items" group with col="1" caused visual
misalignment with the selection dropdown above it. Placing them
in the same group keeps them properly aligned and is equally
functional.

Made-with: Cursor
@gdgellatly
Copy link
Author

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

@gdgellatly gdgellatly marked this pull request as ready for review March 3, 2026 22:09
Copy link

@dannyadair dannyadair left a comment

Choose a reason for hiding this comment

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

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

Add standard multi-company ir.rule records for agreement.rebate.settlement
and agreement.rebate.settlement.line models, restricting access to records
matching the user's allowed companies.

Made-with: Cursor
domain = self._target_line_domain(
agreement_domain, agreement, line=line
)
groups = target_model.read_group(
Copy link

@yankinmax yankinmax Mar 13, 2026

Choose a reason for hiding this comment

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

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,
}
  1. 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()
  1. 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?

Choose a reason for hiding this comment

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

Hello @gdgellatly , can you pls update this PR?

Copy link
Author

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

@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.

@gdgellatly gdgellatly force-pushed the 19.0-mig-agreement_rebate branch from 7ff3887 to ab1a9d9 Compare March 15, 2026 22:06
Odoo 19 removed the public read_group API. Migrate to _read_group with
the new (domain, groupby, aggregates) signature. A helper method
converts tuple results back to dicts so _prepare_settlement_line keeps
its existing public interface for downstream overrides.

Also fix partner_id access: _read_group returns a recordset for
many2one groupby fields, so use .id instead of [0].

Made-with: Cursor
@yankinmax
Copy link

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

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.