Skip to content

[18.0][IMP] l10n_es_aeat_mod347: Field Not in mod347 in partner converted to company dependent #4334

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.

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
wants to merge 1 commit into
base: 18.0
Choose a base branch
from

Conversation

Andrii9090
Copy link

This is a straightforward cherry-pick of commit 217854a

MT-10026 @moduon @rafaelbn @yajo @EmilioPascual @u0f please review if you want 😄

@Andrii9090 Andrii9090 force-pushed the 18.0-fw_p-217854ac76110619a323660dce2e9180b977f8d3 branch from 607a8ab to 882338c Compare August 1, 2025 13:18
@Andrii9090 Andrii9090 changed the title [18.0][IMP] fw_p cherry-pick commit 217854ac76110619a323660dce2e9180b977f8d3 [18.0][IMP] l10n_es_aeat_mod347: Field Not in mod347 in partner converted to company dependent Aug 1, 2025
Copy link
Contributor

@EmilioPascual EmilioPascual left a comment

Choose a reason for hiding this comment

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

Gracias @Andrii9090 por la migración de esta funcionalidad, durante la migración en #4256 se olvido incluir este commit.

Activado en una compañía
image

Desactivado en una compañía distinta
image

Copy link
Contributor

@ArantxaSudon ArantxaSudon left a comment

Choose a reason for hiding this comment

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

Revisado Funcionalmente

Un contacto que marcas como no incluir en el 347 en una compañía en las otras no queda marcado

Gracias @EmilioPascual @Andrii9090

Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

Ver comentario inline.


def migrate(cr, version):
env = api.Environment(cr, SUPERUSER_ID, {})
openupgrade.convert_to_company_dependent(
Copy link
Member

Choose a reason for hiding this comment

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

Esto debe detectar si viene de 16.0, donde ya está convertido a dependiente de la compañía, y entonces daría error.

Copy link
Contributor

Choose a reason for hiding this comment

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

Correcto.
@Andrii9090 debes usar parse_version importandolo desde odoo.tools
Algo parecido a:

from odoo.tools import parse_version

...

def migrate(cr, version):
      if parse_version(version) == parse_version(16.0):
            return
      ....

Además, debería también comprobarse en el pre-migration

Copy link
Member

Choose a reason for hiding this comment

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

Bueno, mejor que con eso de la versión, que además, puedes venir de la 17, no de la 16, se debería comprobar por datos. Por ejemplo, si existe la columna en la BD.

@pedrobaeza pedrobaeza added this to the 18.0 milestone Aug 4, 2025
@pedrobaeza
Copy link
Member

Faltaría también el fw-port de la 17, ¿no?

@EmilioPascual
Copy link
Contributor

Faltaría también el fw-port de la 17, ¿no?

Se intentará, pero a veces el tiempo no da para más y al final toca priorizar.

@pedrobaeza
Copy link
Member

El fw-port de la 17 debe ser igual que el de la 18, y ésta no se puede aceptar sin tener la anterior.

Copy link

@Xino61122 Xino61122 left a comment

Choose a reason for hiding this comment

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

Revisado Funcionalmente

Un contacto que marcas como no incluir en el 347 en una compañía en las otras no queda marcado 👌.

Copy link

@u0f u0f left a comment

Choose a reason for hiding this comment

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

Gracias @Andrii9090 por migrar esta funcionalidad 😄. Testeado funcionalmente y todo correcto.

image

@sabrinaRMartin
Copy link

Gracias @Andrii9090 !!
Respecto al código a parte de la sugerencia de Emilio lo veo todo bien.
Lo he probado a nivel funcional también y funciona perfectamente.

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.

9 participants