[16.0][IMP]commission, account_commission: settlements grouped by payment date

[matteonext]

This pr allows to generate commissions based on the payment date.
For example, if you have three invoices from three different months but with a payment date in the same month, the current behavior generates three separate period, one for each month. With this change, however, all commissions are assigned to a single period.
It covers a similar scope but is managed in a different way #581

[OCA-git-bot]

Hi @pedrobaeza,
some modules you are maintaining are being modified, check this out!

[matteonext]

Hi @pedrobaeza, @etobella sorry for bother!
I just opened this pr, I maybe need to add some tests and split in two commits, one for each module. What do you think about this changes, would you consider including it? Thanks a lot!

[VBNext]

Functionality tested. LGTM

[matteonext]

Hi @pedrobaeza, @etobella sorry for bother!
I just opened this pr, I maybe need to add some tests and split in two commits, one for each module. What do you think about this changes, would you consider including it? Thanks a lot!

[etobella]

From my side, the code looks ok,

However, I don't know if it makes sense to split in an extra module and add the necessary hooks in account_comission 🤔

@pedrobaeza WDYT?

[pedrobaeza]

For not forcing a new module being an small configurable addition, and having a test, let's keep it here, but please remove the empty lines inside methods, following the same style as the rest, and check the comments inline.

Another thing to think is to put a generic field "Group by", including this option and the standard one, and leaving the door opened for other groupings.

[pedrobaeza]

Why are you removing the order here?

[pedrobaeza]

Comments in English only

[pedrobaeza]

Better readability doing the contrary:

Suggested change

	if line.agent_id.commission_id.settled_dates_based_on != "payment":
	return super().get_period_date(line)
	return self.get_latest_payment_date(line.invoice_id)
	if line.agent_id.commission_id.settled_dates_based_on == "payment":
	return self.get_latest_payment_date(line.invoice_id)
	return super().get_period_date(line)

[pedrobaeza]

Avoid excessive lines:

Suggested change

	for (
	_partial,
	_amount,
	counterpart_line,
	) in invoice_partials:
	for (_p, _a, counterpart_line) in invoice_partials:

[pedrobaeza]

Incorrect indentation.

[pedrobaeza]

I would prefer to remove this.

[marcos-mendez]

Automated Review -- Tests Passed

All tests for account_commission,commission passed successfully on Odoo 16.0.
Environment: Minikube + K8s Job + oca-ci/py3.10-odoo16.0

---

Test Coverage Suggestions

Coverage Gaps

The new feature introduces a settled_dates_based_on field and logic to group settlements by Payment Date instead of Invoice Date. While test_groupby_payment_date_settlement exists, it's quite specific and doesn't cover:

1. Edge cases with no payments: What happens when an invoice is paid but not fully reconciled or has no payment dates?
2. Mixed invoice states: Behavior when some invoices are paid and others are not within the same settlement period.
3. Different date formats or invalid data: Robustness of get_latest_payment_date() in unusual scenarios.
4. Backward compatibility: Ensuring invoice_date is still used when settled_dates_based_on is not set to payment.

Suggested Test Cases

def test_get_latest_payment_date_edge_cases(self):
    """Test get_latest_payment_date with invoices that have no payments or partial payments."""

def test_mixed_invoice_states_settlement(self):
    """Test settlements with a mix of paid and unpaid invoices in same period."""

def test_commission_grouped_by_field_display(self):
    """Verify commission_grouped_by field displays correct value in UI (tree/form views)."""

Codecov Risk

• get_latest_payment_date() in commission_make_settle.py – new method, likely to reduce coverage if not tested.
• _get_agent_lines() in commission_make_settle.py – modified logic with new conditional path, requires explicit testing of the payment branch.
• get_period_date() override – new override; ensures correct usage in period calculation logic.

Note: The existing tests cover core functionality, but edge cases in payment reconciliation and field display are not covered. These new tests would improve robustness and coverage.

---

⏰ PR Aging Alert

This PR by @matteonext has been open for 158 days (5 months).
💤 Last activity was 40 days ago.

Every ignored PR is a contributor who might not come back. Review time matters. (OCA Aging Report)

---

Reciprocal Review Request

Hi everyone! Thanks for your contribution to OCA. I reviewed and approved this PR. If any of you have a moment, I would really appreciate a review on my open PR(s):

My open PRs across OCA:

• server-tools#3548 [18.0][MIG] base_kanban_stage: Migration to 18.0
• hr-attendance#262 [16.0][ADD] Hr_attendance_idsecure: iDSecure (ControliD) attendance integration
• stock-logistics-workflow#2276 [16.0][ADD] stock_move_line_devaluation
• stock-logistics-workflow#2275 [16.0][ADD] Stock move line analytic account
• stock-logistics-workflow#2268 [16.0][ADD] stock_move_line_picking_partner
• purchase-workflow#2694 [16.0][IMP]Purchase workflow added to review state & exception fix

Reviewing each other's work helps the whole community move forward faster. Thank you!

---

Automated review by OCA Neural Reviewer + qwen3-coder:30b