Skip to content
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

[11.0] [MIG] l10n_es_dua_sii #944

Closed
wants to merge 8 commits into from

Conversation

angelmoya
Copy link
Member

No description provided.

@pedrobaeza pedrobaeza added this to the 11.0 milestone Nov 8, 2018
@pedrobaeza
Copy link
Member

Ángel, no has hecho lo que te dije de buscar por el XML-ID según veo, ¿no?

@pedrobaeza pedrobaeza mentioned this pull request Nov 8, 2018
32 tasks
@angelmoya
Copy link
Member Author

angelmoya commented Nov 8, 2018 via email

@pedrobaeza
Copy link
Member

¿Pero lo vas a hacer?

P.D.: Si contestas por correo, por favor elimina el texto citado que es muy incómodo de ver en lo que llega por correo de GitHub.

@angelmoya
Copy link
Member Author

Ok, lo de responder, lo hice desde el móvil y no pensé en eso.

Sobre el XML-ID.... te pongo el caso que tengo, que quería revisarlo primero.

En la implantación en v11 cuando instalamos el módulo de DUA no estaba disponible el account-chart-update, así que creamos a mano los impuestos y la posición fiscal, así que el XML-ID no se corresponde.

Ahora que el account_chart_update ya tengo la posición fiscal creada y no me lo actualiza, pero si le cambio el nombre a la que creamos manualmente, si me crea la nueva, y la anterior la puedo archivar.

Lo único es que habría que poner un aviso en el módulo, para tener en cuenta esto.

@pedrobaeza
Copy link
Member

@angelmoya esto lo he estado pensando también para la AEAT. Se puede tener una verificación fallback si no se encuentra XML-ID. Pero también se podría incluir en el módulo account_chart_update el que le añada el XML-ID si no existe. (off-topic: ¿a que molan los últimos cambios que le he metido para selección de campos y demás, jeje?).

Yo tengo todas las BDs que he migrado desde v8-v9 sin XML-IDs, y el método más directo sería ése (aunque también se puede intentar poner en OpenUpgrade).

@angelmoya
Copy link
Member Author

Tienes razón, echale un ojo ahora... aunque todavía no lo he probado, pero por ir afinando

@angelmoya
Copy link
Member Author

El error de travis parece que es por la conexión con egoitza.gipuzkoa.eus

@pedroortega-punt
Copy link

Queda mucho para el merge?

Gracias.

@angelmoya
Copy link
Member Author

Hace falta que alguien revise y valide.

Hice las modificaciones que me solicitó @pedrobaeza , pero falta validar.

@pedrobaeza
Copy link
Member

@angelmoya el error de Travis es en este módulo. He hecho rebase y squash para fusionar y resulta en ese error.

@angelmoya
Copy link
Member Author

OK, gracias @pedrobaeza , no había visto bien el error, que saltaba con el modulo de facturae, pero es por la comprobación de la posición fiscal. Voy a verlo.

@pedroortega-punt
Copy link

Se puede instalar el módulo para probar?

Gracias

@pedrobaeza
Copy link
Member

@vipvalen este módulo provoca un error como denota Travis, así que mientras no esté arreglado, yo no lo recomendaría.

@pedroortega-punt
Copy link

Y que previsión hay para revisar?, se me ha ocurrido migrar de v10 a v11 y ahora tengo un problema, no pueden presentar las facturas de DUA a sii.
Gracias.

@pedrobaeza
Copy link
Member

Pues eso díselo al autor del PR. Por mi parte ninguna, porque no tengo esta necesidad por el momento.

@pedroortega-punt
Copy link

@angelmoya
Cabe la posibilidad de que se revise los errores de Travis?, que previsión puedo tener?

Gracias.

@angelmoya
Copy link
Member Author

@vipvalen @pedrobaeza el problema estaba en los test unitarios, al buscar la posición fiscal en la base de datos demo, hemos añadido en los datos demo una posición fiscal que se llama 'Importación con DUA' para que no de el error.

@pedrobaeza
Copy link
Member

pedrobaeza commented Mar 4, 2019

@angelmoya pero si se instala el plan contable español eso no debe ser necesario. Y es más, es incorrecto añadir los datos demo de esa forma.

@angelmoya
Copy link
Member Author

Ok @pedrobaeza , ¿de que forma podemos hacerlo para que no de error en test?

Porque cuando se ejecutan los test el plan contable no está instalado.

@pedrobaeza
Copy link
Member

Es hacer como en otros tests de este repositorio:

  • Crear compañía
  • Instalar plan contable español en esa compañía
  • Hacer los tests en esa compañía.

De hecho, en el l10n_es_aeat_sii creo que se hace así. En l10n_es_account_invoice_sequence seguro que se hace así.

@angelmoya
Copy link
Member Author

Ok, he vuelto al estado anterior, el error lo está dando en el módulo facturae:

ERROR openerp_test odoo.addons.l10n_es_facturae.tests.test_l10n_es_facturae: for invoice in self.filtered('sii_enabled'):
`

Tendría entonces que reescribir los tests de l10n_es_facturae para que no falle mi módulo.

Ok, lo reviso entonces.

@pedrobaeza
Copy link
Member

No es necesario, Ángel, y de hecho el problema no es ése: es el error incondicional que has puesto. Imagina que tienes una BD con una compañía francesa y una española. La francesa nunca va a tener una posición fiscal DUA.

¿Qué tienes que hacer?

  1. No buscar eso si el plan contable de la compañía company_id.chart_template_id no es uno de los españoles que tenemos controlados (pyme, full, assoc).
  2. Inhibir el error si estás en modo test y no estás testeando precisamente el DUA.
  3. Otros errores que ahora te menciono inline.

@angelmoya
Copy link
Member Author

OK, gracias Pedro, bien visto

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.

No entiendo por qué añades ese mapeo del tax line y además, en todo caso necesitarías hacer rebase de tu rama para admitir el valor both

l10n_es_dua_sii/models/account_invoice.py Outdated Show resolved Hide resolved
@rafaelbn
Copy link
Member

Hola @angelmoya por favor puede echar un ojo a este último comentario? ¡Gracias!

not invoice.sii_dua_invoice:
invoice.sii_enabled = False
else:
super(AccountInvoice, invoice)._compute_sii_enabled()

@api.multi
def _compute_dua_invoice(self):
Copy link
Member

Choose a reason for hiding this comment

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

Esto necesita el @api.depends para recalcularse correctamente cuando cambie algún condicionante.

<field name="model">0</field>
</record>

<record id="aeat_dua_sii_map_line" model="l10n.es.aeat.map.tax.line">
Copy link
Member

Choose a reason for hiding this comment

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

Yo me refería a una línea aeat.sii.map.lines (que por cierto, lleva una s incorrectamente), y utilizar los métodos del módulo principal ya utilizados en el resto del SII para determinar los impuestos derivados.

Copy link
Member

Choose a reason for hiding this comment

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

De todas formas, por qué no debería bastar con la posición fiscal?

Copy link
Member Author

Choose a reason for hiding this comment

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

Funcionalmente no lo tengo claro, es como estaba en versiones anteriores.

Copy link
Member

Choose a reason for hiding this comment

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

Ya, pues es ver si puede haber una factura con esa posición fiscal que no deba mandarse al SII con lo extra del DUA.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, voy a cambiar el mapeo, pero no tengo claro si la nueva línea iría dentro del mapeo l10n_es_aeat_sii.aeat_sii_map o en un mapeo nuevo con una línea, no se si ese mapeo tiene alguna otra implicación.

Copy link
Member

Choose a reason for hiding this comment

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

Puedes ponerlo dentro del mapeo actual, y utiliza los métodos existentes para obtenerlo. Eso sirve para:

  1. Si hubiera distintos mapeos según agencia tributaria.
  2. Hubiera cambios con el tiempo.

Pero sigue vigente la pregunta de antes.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, hecho

taxes = map_line.tax_ids
for invoice in self:
dua_fiscal_position = self._get_dua_fiscal_position(
self.company_id)
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 invoice.company_id sino falla con Expected singleton

super(AccountInvoice, self)._compute_sii_enabled()
for invoice in self.filtered('sii_enabled'):
dua_fiscal_position = self._get_dua_fiscal_position(
self.company_id)
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 invoice.company_id sino falla con Expected singleton

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, hecho

self.company_id)
invoice.sii_dua_invoice = \
invoice.fiscal_position_id == dua_fiscal_position and \
invoice.tax_line_ids.filtered(lambda x: x.tax_id in taxes)
Copy link
Contributor

Choose a reason for hiding this comment

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

Se está comparado account.tax contra account.tax.template

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, ahora tira del mapeo del SII y comprobado que hace la comprobación correcta.

@angelmoya angelmoya force-pushed the 11.0-mig-l10n_es_dua_sii branch 2 times, most recently from f95d142 to 6c2f965 Compare May 27, 2019 08:46
@pedroortega-punt
Copy link

Este módulo lo tengo en un cliente funcionando desde primeros de año y funciona correctamente, cuando se va a realizar el merge? Gracias.

taxes = self._get_sii_taxes_map(['DUA'])
for invoice in self:
dua_fiscal_position = self._get_dua_fiscal_position(
self.company_id)
Copy link
Contributor

Choose a reason for hiding this comment

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

Aquí sigue mal, sería invoice.company_id


@api.multi
def _compute_dua_invoice(self):
taxes = self._get_sii_taxes_map(['DUA'])
Copy link
Contributor

Choose a reason for hiding this comment

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

Esta línea debe estar dentro del for porque _get_sii_taxes_map espera una única factura:

Suggested change
taxes = self._get_sii_taxes_map(['DUA'])
for invoice in self:
taxes = invoice._get_sii_taxes_map(['DUA'])

@Tardo
Copy link
Member

Tardo commented Aug 13, 2019

@angelmoya If you can't finish this i can superseed it... any problem to do that?

@pedrobaeza
Copy link
Member

Reemplazado por #1235

@pedrobaeza pedrobaeza closed this Dec 16, 2019
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.