Skip to content

Conversation

@chienandalu
Copy link
Member

  • Get along with other modules adding automatic route mechanisms, like sale_order_type or stock_customer_deposit
  • Now when we unset the elaborations, the elaboration routes are removed.

Pending:

  • Add test cases for routes

cc @moduon MT-11838

- Get along with other modules adding automatic route mechanisms,
  like sale_order_type or stock_customer_deposit
- Now when we unset the elaborations, the elaboration routes are
  removed.
@OCA-git-bot
Copy link
Contributor

Hi @rafaelbn, @CarlosRoca13, @sergio-teruel, @yajo,
some modules you are maintaining are being modified, check this out!

@rousseldenis
Copy link
Contributor

IMHO a glue module should be more appropriate

@chienandalu
Copy link
Member Author

IMHO a glue module should be more appropriate

Not exactly. There are several modules that use that declare that method for their own purpuses. The thing is to let each one do their thing. We want't avoid that the last one to come squash the declaration.

Additionally, glue modules might be needed, but the main thing is to try to stablish a chain of inheritance. See that sale_order_type already does it for this same method, so if we want them to coexist, this is necessary:

route_id = fields.Many2one(compute="_compute_route_id", store=True, readonly=False)
@api.depends("order_id.type_id")
def _compute_route_id(self):
res = None
if hasattr(super(), "_compute_route_id"):
res = super()._compute_route_id()
for line in self.filtered("order_id.type_id"):
order_type = line.order_id.type_id
if order_type.route_id:
line.route_id = order_type.route_id
return res

Copy link

@rrebollo rrebollo left a comment

Choose a reason for hiding this comment

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

Code Review. LGTM! Didactic.
📋 Strong Recommendation
I would strongly recommend implementing the tests mentioned in your PR description before merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants