Skip to content

[T3487] Allow invoicing the contract line amount of pricelist products #289

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
danpa32 wants to merge 1 commit into
base: 18.0
Choose a base branch
from
+13 −8
Open

[T3487] Allow invoicing the contract line amount of pricelist products #289

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion recurring_contract/models/contract_group.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,7 @@ def build_inv_line_data(
contract = contract_line.contract_id
product = contract_line.product_id.with_company(self.company_id.id)
line_name = product.name
if contract_line.pricelist_item_count:
if contract_line.amount_from_pricelist:
price = self.pricelist_id._get_product_price(
product, qty, date=invoicing_date
)
Expand Down
2 changes: 1 addition & 1 deletion recurring_contract/models/move_line.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ def _update_invoice_lines_from_contract(self, modified_contract):
lambda cl, invl=invoice_line: cl.product_id == invl.product_id
)
data_dict = {}
if contract_line.product_id.pricelist_item_count > 0:
if contract_line.amount_from_pricelist:
price = modified_contract.pricelist_id._get_product_price(
contract_line.product_id,
quantity=contract_line.quantity,
Expand Down
9 changes: 9 additions & 0 deletions recurring_contract/models/recurring_contract_line.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ def _compute_display_name(self):
quantity = fields.Integer(default=1, required=True)
subtotal = fields.Float(compute="_compute_subtotal", store=True)
pricelist_item_count = fields.Integer(related="product_id.pricelist_item_count")
amount_from_pricelist = fields.Boolean(
compute="_compute_amount_from_pricelist",
help="The invoiced price comes from the pricelist instead of the line price.",
)
Comment on lines +56 to +59

@greptile-apps greptile-apps Bot Oct 5, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 New price choice lacks tests

There is no direct invoice test for a product-specific pricelist item or an override that uses the contract line amount. The existing fixture uses a global item and defines no test of its own. This is a non-blocking coverage gap, but a later pricing change could restore the wrong sponsorship charge without a test catching it. Add creation and update tests for both price choices.

Knowledge Base Used: Recurring contracts and invoicing

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Artifacts

Invoice price-choice verification script

  • This authored script extracts and executes the pricing methods from either revision with lightweight records, allowing the same scenarios to be compared.

Invoice pricing before the change

  • Running the script against HEAD^ showed that a priced product used 42 for both invoice paths, including the override scenario.

Invoice pricing after the change

  • Running the same script against HEAD showed that the overridden choice used the line amount of 17 for both invoice paths.

Targeted Odoo test collection failure

  • Running the existing test file with pytest exited 2 during import because `dateutil` is missing, so an Odoo transaction run was not possible.

View artifacts

T-Rex Ran code and verified through T-Rex


_sql_constraints = [
(
Expand All @@ -67,6 +71,11 @@ def _compute_subtotal(self):
for contract_line in self:
contract_line.subtotal = contract_line.amount * contract_line.quantity

@api.depends("product_id")
def _compute_amount_from_pricelist(self):
for line in self:
line.amount_from_pricelist = line.pricelist_item_count > 0
Comment on lines +74 to +77

@greptile-apps greptile-apps Bot Oct 5, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Price choice can go stale

If a pricelist item is added or removed after a contract line’s price-choice flag has been read in the same request, and that flag is not invalidated, invoice creation or updates can use the old price source. The flag reads pricelist_item_count but declares only product_id as a dependency. Track the count as a dependency before merging so the price choice updates with it.

Knowledge Base Used: Recurring contracts and invoicing

@danpa32 danpa32 Oct 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In theory: if a pricelist item is added or removed earlier in the same request, the cached value could be outdated. But its fix wouldn't help. Odoo's own pricelist_item_count has no dependencies at all (it just counts records), so declaring it as a dependency triggers nothing. It isn't a regression either: the old code read the same pricelist_item_count with exactly the same caching. In practice, a pricelist rule is never changed and a contract invoiced in the same request.

@greptile-apps greptile-apps Bot Oct 5, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You're right. Adding @api.depends("pricelist_item_count") would not provide meaningful invalidation because the upstream product.pricelist_item_count itself declares no dependencies. This also preserves the same cached read used by the previous invoice computations, so it is not a regression. Since pricelist rules are not changed while an invoice is being created, this is theoretical rather than actionable. I'll withdraw the finding.

Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.


@api.onchange("product_id")
def on_change_product_id(self):
for line in self.filtered("product_id"):
Expand Down
8 changes: 2 additions & 6 deletions recurring_contract/views/recurring_contract_view.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,12 +206,8 @@
<field name="arch" type="xml">
<list editable="bottom">
<field name="product_id" />
<field name="pricelist_item_count" column_invisible="1" />
<field
name="amount"
force_save="1"
readonly="pricelist_item_count > 0"
/>
<field name="amount_from_pricelist" column_invisible="1" />
<field name="amount" force_save="1" readonly="amount_from_pricelist" />
<field name="quantity" />
<field name="subtotal" />
</list>
Expand Down
Loading