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

[8.0] Cambiado hardcodeo nombres de posición fiscal #565

Merged
merged 7 commits into from
Jul 6, 2017
Merged

[8.0] Cambiado hardcodeo nombres de posición fiscal #565

merged 7 commits into from
Jul 6, 2017

Conversation

JuanjoA
Copy link
Contributor

@JuanjoA JuanjoA commented Jul 5, 2017

Ahora se puede seleccionar a nivel de posición fiscal el tipo de identificación que se espera para los clientes asociados. Esto nos da opción a cambiar los nombres de las posiciones fiscales según necesidad, y da libertad a quien no quiera hacerlo y usar el estándar.

Si es cierto que una vez aplicado, será necesario configurar las posiciones fiscales según necesidad, ya que por defecto se pondrá nacional.
El mínimo a configurar será (para estos casos):

  1. Régimen nacional (ya estará ok, National)
  2. Régimen Intracomunitario (Seleccionar Intracom)
  3. Régimen Extracomunitario / Canarias, Ceuta y Melilla (Seleccionar Export)

@ozono
Copy link
Contributor

ozono commented Jul 5, 2017

A mi me parece correcto. Lo único que veo es que las comprobaciones del tipo se hacían contra enteros y ahora son cadenas.

@omar7r
Copy link
Contributor

omar7r commented Jul 5, 2017

Yo veo necesario un script de migración ya que este cambio obligaría a todo el mundo a recorrerse las posiciones fiscales a mano para escribirles este nuevo campo, por lo que en el momento que se aplique este commit fallarían los envíos sino se hace esto a mano.

@JuanjoA
Copy link
Contributor Author

JuanjoA commented Jul 5, 2017

El tema del script lo veo bien, si aplico post_init_hook, ¿sería necesario reinstalar el módulo o también se aplicaría en un update?

@rlizana
Copy link
Contributor

rlizana commented Jul 5, 2017

@JuanjoA post_init_hook solo se lanza al instalar un modulo, no en las actualizaciones.

@omar7r
Copy link
Contributor

omar7r commented Jul 5, 2017

Sería un migrations al cambiar la versión no un post_init_hook, hay ejemplos en l10n_es

@pedrobaeza
Copy link
Member

Pero esto no es lo que he comentado en el issue... ahora más tarde os propongo una alternativa.

@JuanjoA
Copy link
Contributor Author

JuanjoA commented Jul 5, 2017

Oki, thx @omar7r y @rlizana , si alguién puede que lo pruebe please, sigo mañana, que estoy Ko.
Nota: el tema del selection, string e int, lo he cambiado. (thx @ozono )

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.

Como he dicho en el issue, "la parte hardcodeada" no se debe quitar. Simplemente, hacer un elif más para si se tiene algún dato en la posición fiscal.

@@ -4,6 +4,11 @@

from openerp import fields, models, api

SII_GEN_TYPE = [
Copy link
Member

Choose a reason for hiding this comment

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

Esto no tiene ninguna ventaja ponerlo aquí arriba con la nueva función de selection_add. Mejor que esté en la propia definición del campo por legibilidad

sii_partner_identification_type = fields.Selection(
selection=SII_GEN_TYPE,
string="SII partner Identification Type",
default=1,
Copy link
Member

Choose a reason for hiding this comment

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

No tengo claro que los números se vayan a almacenar así en la BD y no como VARCHAR, así que lo mejor sería hacer un int() en el método que lo obtiene.

…a en caso de no tener indicado el tipo de identificador del partner.
@JuanjoA
Copy link
Contributor Author

JuanjoA commented Jul 6, 2017

He cambiado lo mencionado por @pedrobaeza, no me acaba de cuadrar del todo, pero ahorra el script de migración, y aporta la flexibilidad requerida.

Edito: le he quitado el valor por defecto, por lo que hay que configurarlo manualmente si se requiere.

elif self.fiscal_position.name == \
u'Régimen Extracomunitario / Canarias, Ceuta y Melilla':
return 3
partner_ident = self.fiscal_position.sii_partner_identification_type
Copy link
Member

@pedrobaeza pedrobaeza Jul 6, 2017

Choose a reason for hiding this comment

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

Sigue sin ser lo que quería, aunque bueno, llevas parte de razón en que debe tener prioridad lo puesto en la posición fiscal, pero no puedes quitar el 1 como valor por defecto en else. Pon este código:

partner_ident = self.fiscal_position.sii_partner_identification_type
if partner_ident:
    res = int(partner_ident)
elif self.fiscal_position.name == u'Régimen Intracomunitario':
    res = 2
elif (self.fiscal_position.name ==
        u'Régimen Extracomunitario / Canarias, Ceuta y Melilla'):
    res = 3
else:
    res = 1
return res

Copy link
Member

Choose a reason for hiding this comment

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

Y recuerda siempre tener la complejidad ciclomática en mente (los if y bifurcaciones que haces).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Está al comienzo, res=1

Copy link
Member

Choose a reason for hiding this comment

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

Cierto, se me pasó, pero mejor el código que yo propongo por legibilidad y complejidad menor.

Copy link
Member

Choose a reason for hiding this comment

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

Otro consejo es nunca utilizar la \ para múltiples líneas. Mejor paréntesis (ya he editado mi propuesta de código), porque con eso a veces pueden salir errores con líneas complejas.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Listo @pedrobaeza .

@pedrobaeza pedrobaeza merged commit 057ceb8 into OCA:8.0 Jul 6, 2017
pedrobaeza pushed a commit that referenced this pull request Jul 9, 2017
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
pedrobaeza pushed a commit that referenced this pull request Aug 10, 2017
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
javierjcf pushed a commit to Comunitea/l10n-spain that referenced this pull request Nov 13, 2017
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
javierjcf pushed a commit to Comunitea/l10n-spain that referenced this pull request Nov 14, 2017
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
javierjcf pushed a commit to Comunitea/l10n-spain that referenced this pull request Nov 15, 2017
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
jabibi pushed a commit to nodoo/l10n-spain that referenced this pull request Dec 11, 2017
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
pedrobaeza pushed a commit to Comunitea/l10n-spain that referenced this pull request Dec 19, 2017
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
pedrobaeza pushed a commit to Comunitea/l10n-spain that referenced this pull request Jan 3, 2018
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
misern2 pushed a commit to QubiQ/l10n-spain that referenced this pull request Feb 6, 2019
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
misern2 pushed a commit to QubiQ/l10n-spain that referenced this pull request Feb 7, 2019
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
misern2 pushed a commit to QubiQ/l10n-spain that referenced this pull request Feb 7, 2019
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
misern2 pushed a commit to QubiQ/l10n-spain that referenced this pull request Feb 7, 2019
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
misern2 pushed a commit to QubiQ/l10n-spain that referenced this pull request Feb 7, 2019
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
misern2 pushed a commit to QubiQ/l10n-spain that referenced this pull request Feb 10, 2019
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
pedrobaeza pushed a commit to QubiQ/l10n-spain that referenced this pull request Mar 5, 2019
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
pedrobaeza pushed a commit to Tecnativa/l10n-spain that referenced this pull request May 26, 2020
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
ValentinVinagre pushed a commit to sygel-technology/l10n-spain that referenced this pull request Jul 10, 2020
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
ValentinVinagre pushed a commit to sygel-technology/l10n-spain that referenced this pull request Jul 15, 2020
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
ValentinVinagre pushed a commit to sygel-technology/l10n-spain that referenced this pull request Jul 15, 2020
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
ValentinVinagre pushed a commit to sygel-technology/l10n-spain that referenced this pull request Jul 15, 2020
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
ValentinVinagre pushed a commit to sygel-technology/l10n-spain that referenced this pull request Aug 19, 2020
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
pedrobaeza pushed a commit to sygel-technology/l10n-spain that referenced this pull request Oct 1, 2020
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
pedrobaeza pushed a commit to sygel-technology/l10n-spain that referenced this pull request Oct 23, 2020
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
pedrobaeza pushed a commit to sygel-technology/l10n-spain that referenced this pull request Oct 23, 2020
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
joao-p-marques pushed a commit to Tecnativa/l10n-spain that referenced this pull request Feb 1, 2021
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
joao-p-marques pushed a commit to Tecnativa/l10n-spain that referenced this pull request Feb 8, 2021
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
joao-p-marques pushed a commit to Tecnativa/l10n-spain that referenced this pull request Feb 11, 2021
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
pedrobaeza pushed a commit to Tecnativa/l10n-spain that referenced this pull request Feb 19, 2021
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
ValentinVinagre pushed a commit to sygel-technology/l10n-spain that referenced this pull request Dec 23, 2021
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
ValentinVinagre pushed a commit to sygel-technology/l10n-spain that referenced this pull request Dec 28, 2021
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
ValentinVinagre pushed a commit to sygel-technology/l10n-spain that referenced this pull request Dec 28, 2021
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
zamberjo pushed a commit to aurestic/l10n-spain that referenced this pull request Feb 1, 2023
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
manuelregidor pushed a commit to sygel-technology/l10n-spain that referenced this pull request Mar 28, 2024
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
manuelregidor pushed a commit to sygel-technology/l10n-spain that referenced this pull request May 27, 2024
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
manuelregidor pushed a commit to sygel-technology/l10n-spain that referenced this pull request Sep 16, 2024
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
manuelregidor pushed a commit to sygel-technology/l10n-spain that referenced this pull request Sep 17, 2024
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
manuelregidor pushed a commit to sygel-technology/l10n-spain that referenced this pull request Sep 27, 2024
* Cambiado hardcodeado de nombres de posición fiscal a configurable por posición fiscal
* Traducciones
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.

5 participants