[16.0] [ADD] project_required_field_by_stage: New module by gfcapalbo · Pull Request #1650 · OCA/project

gfcapalbo · 2026-01-09T16:13:15Z

reopening #1341 with original commits.
Added changes requested in that original MR-.

ddejong-therp · 2026-01-21T17:17:11Z

Code LGTM.
I've tested it, and run the tests as well.
All looks good.

UrzaBlock · 2026-01-22T09:40:20Z

Tested its functionality on runboat, and it does what it's supposed to do: When you have added fields in the Required Field section of a stage, a task that gets put in that stage will have those fields highlighted as required.

NL66278 · 2026-01-22T09:46:01Z

+
+    @api.constrains("stage_id")
+    def _check_stage_id_(self):
+        for this in self:


Maybe I miss something, but I do not see why the inner loops are necessary. A project can only be in one stage, so why would this be wrong?

@api.constrains("stage_id") def _check_stage_id_(self): for this in self: required_fields = this.stage_id.required_field_ids if not required_fields: continue for field in required_fields: if hasattr(this, field.name) and not getattr(this, field.name): raise UserError( _( "Field '%(field)s' is mandatory in stage '%(stage)s'." ) % ( { "field": field.display_name.split(" (")[0], "stage": this.stage_id.display_name, } ) )

Ah yes I agree, I see that stage_id is a Many2one field

Every record has one stage. we only need to check mandatory fields for that one stage, not all stages every time. thank you @NL66278 , i will simplify this , by removing the loop.

I re-reviewed this. @NL66278

stages = this.stage_id.filtered(lambda x: x.required_field_ids) makes no sense. and some other filtering makes no sense either.

All we want in this costrains is to verify that on stage change all mandatory fields specified for that stage are there.

Added a fix commit, please re-check.

luisDIXMIT

When I remove required fields from a stage, it still requires those fields.

luisDIXMIT · 2026-02-19T08:58:45Z

+{
+    "name": "Project Required Field By Stage",
+    "summary": """
+        KMEE""",


Can you add a more detailed summary, please?

Done. thank you for reviewing. if module is satisfactory please leave an approval.

luisDIXMIT · 2026-02-19T09:37:54Z

PR tittle must be "[16.0] [ADD] project_required_field_by_stage: New module"

NL66278 · 2026-02-27T14:37:39Z

@@ -0,0 +1,3 @@
+* KMEE (https://kmee.com.br/):
+
+  * Tiago Amaral


Enough enhancements to add yourself to contributors list

NL66278 · 2026-02-27T16:22:17Z

+                    if attrs:
+                        if attrs.get("required"):
+                            attrs["required"] = [
+                                "|",
+                                ("stage_id", "in", stages_with_field.ids),
+                            ] + attrs["required"]
+                        else:
+                            attrs["required"] = [
+                                ("stage_id", "in", stages_with_field.ids)
+                            ]
+                    else:
+                        attrs["required"] = [("stage_id", "in", stages_with_field.ids)]


All this can be done much more concise (and without repeating three times the same condition added):
` ``
from odoo.osv.expression import OR # Obviously this at the top of the python file
...
required_domain = attrs.get("required", [])
attrs["required"] = OR([required_domain, [("stage_id", "in", stages_with_field.ids)]])

No `if` or `else` needed!

NL66278 · 2026-02-27T16:26:35Z

+        mandatory fields associated.""",
+    "version": "16.0.1.0.0",
+    "license": "AGPL-3",
+    "author": "KMEE,Odoo Community Association (OCA)",


There is as yet no maintainer defined. Maybe you can add yourself as maintainer? This will give the possibility to merge stuff, even if you are not a PSC member for project and you will be notified of PR's. Of course this brings responsibility as well.

alexey-pelykh

Thanks for reviving this from #1341 and for the work on tests and the view cache key -- this is a useful module. A few things need attention before merge:

1. _get_view attrs logic -- apply NL66278's suggestion

As @NL66278 pointed out, the three-branch if/elif/else for building the required domain can be collapsed using odoo.osv.expression.OR:

from odoo.osv.expression import OR
...
required_domain = attrs.get("required", [])
attrs["required"] = OR([required_domain, [("stage_id", "in", stages_with_field.ids)]])

This removes all the branching and is the idiomatic Odoo way to combine domains.

2. _check_stage_id_ -- redundant search

this.stage_id.required_field_ids is already a recordset of ir.model.fields. The .sudo().search(["id", "in", ...ids]) re-queries the same records for no reason. Simplify to:

for field in this.stage_id.required_field_ids:

The .sudo() is also unnecessary here since you already have access to the stage through the task.

3. Falsy check fails on numeric zero

not getattr(this, field.name) treats 0 and 0.0 as empty. If someone marks an integer or float field as required and sets it to 0, the constraint will incorrectly reject it. Consider checking the field type: for relational fields check not value, for others check value is False (which is how Odoo represents "not set" for scalar fields).

4. View cache invalidation bug

@luisDIXMIT reported that removing required fields from a stage still enforces them. The _get_view_cache_key only includes field names but not which stages they belong to. If stage A requires user_ids and you remove it, but stage B also requires user_ids, the cache key is unchanged and the stale view is served. Include stage IDs in the cache key.

5. Minor items

import json as simplejson -- misleading alias, just use import json and json.dumps.
Method name _check_stage_id_ has a trailing underscore -- likely a typo. Rename to _check_required_fields_by_stage for clarity.
Missing "installable": True in manifest (OCA convention).

Overall the concept is solid and the test coverage is decent. Looking forward to the next iteration.

alexey-pelykh · 2026-03-01T14:21:47Z

+# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl).
+
+import ast
+import json as simplejson


import json as simplejson is a misleading alias (simplejson was a different library). Just use import json and call json.dumps below.

alexey-pelykh · 2026-03-01T14:21:47Z

+                )
+                for node in arch.xpath("//field[@name='%s']" % field.name):
+                    attrs = ast.literal_eval(node.attrib.get("attrs", "{}"))
+                    if attrs:


As @NL66278 suggested -- this whole if/elif/else block can be replaced with:

from odoo.osv.expression import OR ... required_domain = attrs.get("required", []) attrs["required"] = OR([required_domain, [("stage_id", "in", stages_with_field.ids)]])

No branching needed.

yes attrs , previously in node.attrib.get defaults to {} so it is safe to remove all branches.

alexey-pelykh · 2026-03-01T14:21:47Z

+
+    @api.constrains("stage_id")
+    def _check_stage_id_(self):
+        for this in self:


Trailing underscore on _check_stage_id_ looks like a typo. Consider _check_required_fields_by_stage which also better describes the intent.

alexey-pelykh · 2026-03-01T14:21:47Z

+    def _check_stage_id_(self):
+        for this in self:
+            if this.stage_id.required_field_ids:
+                fields = (


This search is redundant -- this.stage_id.required_field_ids is already the ir.model.fields recordset you want. The .sudo().search(...) just re-fetches the same records. Replace the whole block with:

for field in this.stage_id.required_field_ids:

alexey-pelykh · 2026-03-01T14:21:47Z

+                    .search([("id", "in", this.stage_id.required_field_ids.ids)])
+                )
+                for field in fields:
+                    if hasattr(this, field.name) and not getattr(this, field.name):


not getattr(this, field.name) is falsy for 0 and 0.0, so a numeric field set to zero would incorrectly fail validation. For scalar fields, Odoo uses False to mean "not set" -- check getattr(this, field.name) is False for non-relational types, or inspect field.ttype to pick the right emptiness check.

Managed this in refactored is_empty function for clarity, where i check the ttype and do the appropriate check.

alexey-pelykh · 2026-03-01T14:21:47Z

+            self.env["project.task.type"]
+            .search([("required_field_ids", "!=", False)])
+            .mapped("required_field_ids.name")
+        )


Cache key only includes field names, not which stages they belong to. Removing a required field from one stage won't invalidate the cache if another stage still requires the same field name. Include stage IDs or write_date in the key to ensure proper invalidation on config changes.

gfcapalbo · 2026-03-02T13:11:07Z

@alexey-pelykh @NL66278 all review comments and simplifications implemented specifically in commit 4f16056 .

Added a function that correctly evaluates what "not set" means for the different types of fields. Added tests to cover those code branches too and keep codeCov coverage percentage from dipping.
I made a choice: Adding booleans is not a noop operation, a boolean is always "set" ( true or false), so i semantically interpret booleanfield = false as "not set" and true as "set" i think is the right interpretation .

if i had made {boolean_field} = true or false = set that would had made adding booleans to restrictions on stages effectively useless.

alexey-pelykh

All review comments from the previous round have been addressed in commit 4f16056:

import json as simplejson -- replaced with clean import json.
_get_view attrs logic -- collapsed to OR([required_domain, ...]) as suggested by @NL66278.
Redundant search removed -- constraint now iterates this.stage_id.required_field_ids directly.
Method renamed -- _check_stage_id_ is now _check_required_fields_by_stage.
Type-aware emptiness check -- new _is_empty() function correctly differentiates by field.ttype. The boolean interpretation (False = "not set") is reasonable for this use case.
Cache key fix -- _get_view_cache_key now includes (stage.id, field.name) tuples, so config changes properly invalidate the cached view.
"installable": True added to manifest.

Tests cover the constraint, view injection for multiple field types, and the OR domain merging. Looks good.

OCA-git-bot · 2026-03-03T10:25:23Z

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

NL66278 · 2026-03-04T15:09:13Z

+        )
+        self.project_task_1.write(
+            {
+                "stage_id": self.project_task_type_2.id,


This write can be combined with the previous write, which actually would make the test stronger, as writing the required field for a stage together with the stage should succeed.

NL66278 · 2026-03-04T15:59:26Z

+                "name": "Project Stage 1",
+                "project_ids": [(4, self.project_project_1.id)],
+            }
+        )


Just a suggestion for more concise code:

from odoo.fields import Command ... required_field_names = [ "planned_hours", # integer "project_id", # m20 "user_ids", # m2m "active", # bool ] .... "required_fields": [ Command.link(self.env.ref("project.field_project_task__%s" % field_name).id) for field_name in required_field_names ]

NL66278 · 2026-03-04T16:00:34Z

@gfcapalbo Please squash your own commits into one.

OCA-git-bot · 2026-03-05T12:34:00Z

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

leemannd · 2026-03-05T12:34:49Z

@NL66278 I missed your comment before starting the merge of the module : /

OCA-git-bot · 2026-03-05T12:44:14Z

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

gfcapalbo force-pushed the 16.0-add-project_required_field_by_stage branch 2 times, most recently from f51f3e4 to 709ab1c Compare January 9, 2026 16:38

gfcapalbo mentioned this pull request Jan 9, 2026

[16.0][ADD] Addon: project_required_field_by_stage #1341

Closed

gfcapalbo force-pushed the 16.0-add-project_required_field_by_stage branch 3 times, most recently from d973054 to 60b2afd Compare January 12, 2026 16:05

ddejong-therp approved these changes Jan 21, 2026

View reviewed changes

NL66278 reviewed Jan 22, 2026

View reviewed changes

luisDIXMIT reviewed Feb 19, 2026

View reviewed changes

gfcapalbo changed the title ~~[ADD] Addon: project_required_field_by_stage~~ [16.0] [ADD] project_required_field_by_stage: New module Feb 27, 2026

gfcapalbo force-pushed the 16.0-add-project_required_field_by_stage branch 3 times, most recently from 0631016 to a880627 Compare February 27, 2026 14:30

NL66278 reviewed Feb 27, 2026

View reviewed changes

alexey-pelykh requested changes Mar 1, 2026

View reviewed changes

gfcapalbo force-pushed the 16.0-add-project_required_field_by_stage branch 9 times, most recently from 3773f27 to 035ae86 Compare March 2, 2026 13:04

gfcapalbo force-pushed the 16.0-add-project_required_field_by_stage branch 2 times, most recently from 067b7a3 to b67aa25 Compare March 2, 2026 13:23

Tiago Amaral and others added 4 commits March 2, 2026 14:23

[ADD] Addon: project_required_field_by_stage

bbe2d0f

[IMP] review improvements

dda7ee0

[IMP] extend tests

2775524

[FIX] review improvements. tests extended

4f16056

gfcapalbo force-pushed the 16.0-add-project_required_field_by_stage branch from b67aa25 to 4f16056 Compare March 2, 2026 13:23

alexey-pelykh approved these changes Mar 3, 2026

View reviewed changes

OCA-git-bot added approved ready to merge labels Mar 3, 2026

NL66278 reviewed Mar 4, 2026

View reviewed changes

OCA-git-bot added the bot is merging ⏳ label Mar 5, 2026

OCA-git-bot merged commit 9334bfc into OCA:16.0 Mar 5, 2026
7 checks passed

OCA-git-bot added merged 🎉 and removed bot is merging ⏳ labels Mar 5, 2026

Uh oh!

Conversation

gfcapalbo commented Jan 9, 2026

Uh oh!

ddejong-therp commented Jan 21, 2026

Uh oh!

UrzaBlock commented Jan 22, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luisDIXMIT left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gfcapalbo Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luisDIXMIT commented Feb 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexey-pelykh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gfcapalbo commented Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexey-pelykh left a comment

Choose a reason for hiding this comment

Uh oh!

OCA-git-bot commented Mar 3, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NL66278 commented Mar 4, 2026

Uh oh!

OCA-git-bot commented Mar 5, 2026

Uh oh!

leemannd commented Mar 5, 2026

Uh oh!

Uh oh!

OCA-git-bot commented Mar 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

gfcapalbo Feb 27, 2026 •

edited

Loading

gfcapalbo commented Mar 2, 2026 •

edited

Loading