Skip to content

Conversation

@matiasperalta1
Copy link
Contributor

No description provided.

Copilot AI review requested due to automatic review settings December 4, 2025 18:37
@roboadhoc
Copy link

Pull request status dashboard

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Este PR agrega funcionalidad para restringir la selección de UoM (unidades de medida) solo a las definidas en los packagings de un producto. Se introduce un nuevo campo booleano only_packagings en el modelo product.template que, cuando está activo, limita las UoM disponibles en las líneas de pedido de venta.

  • Añade el campo only_packagings al modelo product.template
  • Modifica el método _compute_allowed_uom_ids en sale.order.line para aplicar la restricción
  • Incrementa la versión del módulo de 1.1.0 a 1.2.0 (minor bump apropiado)

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
sale_ux/models/product_template.py Nuevo modelo que agrega el campo booleano only_packagings para controlar la restricción de UoM
sale_ux/models/sale_order_line.py Implementa la lógica de restricción de UoM basada en packagings mediante _compute_allowed_uom_ids (contiene errores críticos)
sale_ux/models/init.py Importa el nuevo modelo product_template y reorganiza imports alfabéticamente
sale_ux/views/product_template_views.xml Añade el campo only_packagings a la vista de formulario de producto (referencia campo inexistente)
sale_ux/manifest.py Incrementa versión a 1.2.0 y registra la nueva vista XML

only_packagings = fields.Boolean(
string="Only Packagings",
default=False,
help="Indicates that this product is only used as packaging and does not use units.",
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

El texto de ayuda "Indicates that this product is only used as packaging and does not use units" es confuso. El campo no indica que el producto "es un packaging", sino que restringe las UoM disponibles a solo las definidas en los packagings del producto.

Sugerencia de mejora:

help="When enabled, only UoMs defined in product packagings will be available for selection in sale orders."
Suggested change
help="Indicates that this product is only used as packaging and does not use units.",
help="When enabled, only UoMs defined in product packagings will be available for selection in sale orders.",

Copilot uses AI. Check for mistakes.
<field name="inherit_id" ref="product.product_template_form_view"/>
<field name="priority">30</field>
<field name="arch" type="xml">
<field name="uom_ids" position="after">
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

La vista asume que existe un campo uom_ids en product.template para poder posicionar el nuevo campo only_packagings después de él. Sin embargo, en Odoo estándar no existe este campo por defecto.

Verifica que:

  1. Otro módulo esté agregando el campo uom_ids a product.template, o
  2. El campo de referencia correcto sea otro (por ejemplo, packaging_ids)

Si uom_ids no existe, el xpath fallará y la vista no se cargará correctamente.

Suggested change
<field name="uom_ids" position="after">
<field name="packaging_ids" position="after">

Copilot uses AI. Check for mistakes.
)
super(SaleOrderLine, self - lines)._compute_discount()

@api.depends("product_template_id.only_packagings")
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

El decorador @api.depends debería depender de product_id.product_tmpl_id.only_packagings en lugar de product_template_id.only_packagings, ya que en la línea 69 se accede al producto a través de line.product_id y luego a product.product_tmpl_id. Esto asegura que el cálculo se active cuando cambie el campo correcto.

Suggested change
@api.depends("product_template_id.only_packagings")
@api.depends("product_id.product_tmpl_id.only_packagings")

Copilot uses AI. Check for mistakes.
for line in self:
product = line.product_id
if product and product.product_tmpl_id.only_packagings:
line.allowed_uom_ids = product.uom_ids
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

La asignación line.allowed_uom_ids = product.uom_ids es incorrecta. El modelo product.product no tiene un campo uom_ids por defecto.

Si la intención es restringir las UoM a las definidas en los packagings del producto, deberías usar:

line.allowed_uom_ids = product.packaging_ids.mapped('product_uom_id')

Alternativamente, si uom_ids es un campo relacionado que extrae las UoM de los packagings, debería estar definido en el modelo product.template o product.product, pero no se encuentra en este diff.

Suggested change
line.allowed_uom_ids = product.uom_ids
line.allowed_uom_ids = product.packaging_ids.mapped('product_uom_id')

Copilot uses AI. Check for mistakes.
Comment on lines 65 to 80
@api.depends("product_template_id.only_packagings")
def _compute_allowed_uom_ids(self):
super()._compute_allowed_uom_ids()
for line in self:
product = line.product_id
if product and product.product_tmpl_id.only_packagings:
line.allowed_uom_ids = product.uom_ids
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

La nueva funcionalidad de restricción de UoM basada en only_packagings no tiene cobertura de tests. Dado que el módulo ya incluye tests (ver tests/test_sale_order_cancel.py), se recomienda agregar tests para verificar:

  1. Que cuando only_packagings=True, el campo allowed_uom_ids se restringe correctamente a las UoM de los packagings
  2. Que cuando only_packagings=False, el comportamiento es el estándar
  3. Que el campo se actualiza correctamente cuando cambia el valor de only_packagings

Copilot uses AI. Check for mistakes.
@matiasperalta1 matiasperalta1 force-pushed the 19.0-t-60815-mnp branch 2 times, most recently from a5429ee to 0bd39d6 Compare January 5, 2026 16:10
"""Override to respect only_packagings configuration"""
for line in self:
if line.product_id.product_tmpl_id.only_packagings and line.product_id.uom_ids:
if line.product_uom_id and line.product_uom_id in line.product_id.uom_ids:
Copy link
Contributor

Choose a reason for hiding this comment

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

Sería necesario este if con el continue?
Creo que pondría solo "line.product_uom_id = line.product_id.uom_ids[0]"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fran ! ese if con el continue evaluo el caso donde el usuario elige manualmente la uom. Entonces si se vuelve a ejecutar el compute no fuerza la uom

)
super(SaleOrderLine, self - lines)._compute_discount()

@api.depends("product_id")
Copy link
Contributor

Choose a reason for hiding this comment

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

Ya está en el super esta dependencia, no hace falta agregarla

Copy link
Contributor Author

Choose a reason for hiding this comment

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

excelente

continue
line.product_uom_id = line.product_id.uom_ids[0]
else:
if not line.product_uom_id or (line.product_id.uom_id.id != line.product_uom_id.id):
Copy link
Contributor

Choose a reason for hiding this comment

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

Acá directamente llamaría a super()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

listoo

@api.depends("product_template_id.only_packagings")
def _compute_allowed_uom_ids(self):
super()._compute_allowed_uom_ids()
for line in self:
Copy link
Contributor

Choose a reason for hiding this comment

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

Capaz quedaria mejor así, qué decís? No recomputas 2 veces las líneas que tienen el only_packagings

def _compute_allowed_uom_ids(self):
lines = self.filtered(lambda x: x.product_id.product_tmpl_id.only_packagings)
for line in lines:
line.allowed_uom_ids = line.product_id.uom_ids
super(SaleOrderLine, self - lines)._compute_allowed_uom_ids()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

si lo veo mejor fran


def _get_product_catalog_lines_data(self, **kwargs):
res = super()._get_product_catalog_lines_data(**kwargs)
if len(self) == 1 and self.product_id.product_tmpl_id.only_packagings:
Copy link
Contributor

Choose a reason for hiding this comment

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

Capaz se ve mejor hacer los 2 if en uno solo agregandole el and al final del primer if

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants