[18.0][ADD] project_estimation

[Saran440]

This module is a pre-sales module designed to manage project cost estimation before confirming a Sales Order.

In many service-based businesses, project discussions start with rough calculations, scope alignment, and internal margin validation. Creating a full Project too early can mix pre-sales assumptions with confirmed execution data. This module separates the estimation phase from the execution phase.

Once the estimation is validated and approved, it can be converted into a Sales Order. If the deal does not proceed, the estimation can simply be cancelled without impacting project reporting.

Step to test:

1. Go to Project > Estimation > Estimations
2. Click New, select a Partner, and add cost details to each estimation line

3. Click Confirm > Approve > Create Sale Order. A wizard will open 4. Review the data and click Create Sale Order 5. Verify that a new Sale Order is created correctly from the estimation.

---

Note:

• This module introduces the initial concept of Project Estimation. The logic may evolve based on testing and community feedback.
• Access rights are currently basic. We may consider introducing dedicated permissions for Project Estimation users.

This module is the base of the estimation series. Planned extensions include:

• project_estimation_revision
• project_estimation_discount --> should be include project_estimation or split in new module

[alexey-pelykh]

Nice module, pre-sales estimation is a common need for service businesses. The model structure is solid and the tax computation using _add_tax_details_in_base_lines is the correct approach for 18.0.

CI tests are currently failing -- you'll want to get those sorted. Also the target_sale_price computation returns 0.0 when margin >= 100 -- might be worth adding a help text or validation explaining that edge case to avoid confusion.

Alpha maturity is appropriate for a first iteration. Good call.

[alexey-pelykh]

Review: project_estimation (18.0) -- Draft PR

This PR is marked as Draft, so this review is provided as early feedback to help guide development before it is ready for formal review.

Overall, this is a well-structured new module with a clear business purpose (pre-sales estimation with margin calculation and SO generation). The code follows OCA conventions in most areas. However, there are several issues -- most critically, the test suite will fail due to a missing state value and missing logic in the wizard.

---

Critical Issues

1. Test will fail: state "won" does not exist

The test test_02_workflow_and_wizard asserts estimation.state == "won" after the wizard runs, but the state field selection only defines: draft, confirm, approved, done, cancelled. Additionally, the wizard's action_create_sale_order() method never writes to the estimation state at all.

2. Test expects project_id to be set, but wizard never creates a project

The same test asserts self.assertTrue(estimation.project_id) and checks the project name, but action_create_sale_order() in the wizard only creates a sale.order -- it never creates or links a project.project. This will also cause the test to fail.

These two issues suggest the wizard implementation is incomplete -- the action_create_sale_order method likely needs to:

• Set the estimation state (e.g., to "done" or add a "won" state to the selection)
• Create or link a project if that is the intended behavior

3. Missing record rule for project.estimation.line

There is a multi-company ir.rule for project.estimation but none for project.estimation.line. Since lines inherit company_id from the parent estimation, a similar multi-company record rule should be added to prevent cross-company data leakage.

---

Moderate Issues

4. product_uom_id not propagated to Sale Order

The estimation line has a product_uom_id field, but _get_data_lines() in the wizard does not include it in the dictionary passed to sale.order.line. This means the UoM selected on the estimation will be lost when creating the SO.

5. No state transition validation

All state-changing methods (action_confirm, action_approve, action_done, etc.) simply write the new state without validating the current state. This means action_done() can be called from "draft" state, action_approve() from "cancelled", etc. Consider adding precondition checks, e.g.:

def action_approve(self):
    if any(rec.state != 'confirm' for rec in self):
        raise UserError(_("Only confirmed estimations can be approved."))
    return self.write({"state": "approved"})

6. Inconsistent i18n pattern

The wizard create_sale_order.py uses both self.env._("...") (Odoo 18 new pattern, line calling UserError) and _("Sale Order") (old import-based pattern, in the return dict). The new self.env._() pattern should be used consistently throughout. The from odoo import _ import should be removed if not needed.

7. Cancel button not available from "confirm" state

The Cancel button has invisible="state not in ('draft', 'approved')", which means it is hidden in the "confirm" state. A confirmed estimation cannot be cancelled from the UI, only via direct method call. This seems like an oversight -- consider adding 'confirm' to the visible states.

---

Minor Suggestions

8. Commented-out code should be removed

In project_estimation_line.py, _prepare_base_line_for_taxes_computation contains:

# 'rate': self.estimation_id.currency_rate,

Remove this before merging -- commented-out code adds noise.

9. Sequence prefix PEST

The sequence prefix in data/sequence.xml is PEST. While technically just an abbreviation of "Project ESTimation", consider whether PE or EST might be a better choice to avoid the unfortunate English word association.

10. _compute_target_sale_price edge case with negative margins

When target_margin is negative (which the field allows), the formula total_cost / (1 - margin / 100) will produce a value less than total_cost, which is mathematically correct but may be confusing. When target_margin is 0, target_sale_price equals total_cost (0% margin means selling at cost) -- the elif margin > 0 branch skips this case and falls to the else. This works but the intent could be clearer.

11. Test skips action_confirm step

In test_02_workflow_and_wizard, the test goes directly from "draft" to action_approve(), skipping action_confirm(). While this works due to the lack of state validation (issue #5), it does not test the intended workflow.

---

What Looks Good

• Clean OCA module structure with proper __manifest__.py, pyproject.toml, and readme files
• Good use of mail.thread and mail.activity.mixin for chatter integration
• Proper noupdate="1" on sequence data
• Tax computation follows Odoo 18 account.tax computation pattern correctly
• Section/note line support with display_type and proper filtering in computations
• Tests exist and cover computations, workflow, and section lines
• Proper use of copy=False on sequence name field

---

Review posted via CorporateHub OCA review campaign

[alexey-pelykh]

Bug: This assertion will fail. The state field selection does not include "won" -- it only defines draft, confirm, approved, done, cancelled. Additionally, the wizard's action_create_sale_order() never writes to the estimation state.

Either:

• Add "won" to the state selection and have the wizard set it, or
• Change this to "done" and have the wizard call action_done() after creating the SO.

[alexey-pelykh]

Bug: This will also fail. The wizard action_create_sale_order() only creates a sale.order -- it never creates or assigns a project.project to the estimation. The project_id field will remain empty.

If the intended behavior is to auto-create a project, the wizard logic needs to be extended.

[alexey-pelykh]

This uses the old-style _("Sale Order") pattern (imported from from odoo import _), while UserError earlier uses the new self.env._("...") pattern. In Odoo 18, prefer self.env._("Sale Order") consistently.

Also, the wizard should probably update the estimation state after creating the SO (see test expectations).

[alexey-pelykh]

The product_uom_id from the estimation line is not propagated to the sale order line. Consider adding:

"product_uom": line.product_uom_id.id,

to preserve the UoM when creating the SO.

[alexey-pelykh]

There is a multi-company record rule for project.estimation, but project.estimation.line also has a company_id field and is missing an equivalent rule. Consider adding a multi-company rule for the line model as well.

[alexey-pelykh]

State transition methods have no precondition checks. For example, action_approve() can be called regardless of the current state. Consider validating the current state before transitioning, e.g.:

def action_approve(self):
    for rec in self:
        if rec.state != 'confirm':
            raise UserError(self.env._("Only confirmed estimations can be approved."))
    return self.write({"state": "approved"})

[alexey-pelykh]

Nit: Remove this commented-out line before merging. Commented-out code adds maintenance noise. If currency rate support is planned for a future module, document it in the README or a TODO instead.

[alexey-pelykh]

The Cancel button is hidden when state not in ('draft', 'approved'), which means it is not visible in the "confirm" state. Consider adding 'confirm' so users can cancel a confirmed (but not yet approved) estimation.