[9.0][MIG] l10n_es_partner #410

pedrobaeza · 2016-12-10T01:39:45Z

rafaelbn · 2016-12-10T05:23:24Z

l10n_es_partner/README.rst

- * Permite validar las cuentas bancarias españolas. Para ello, se añade un
-   campo de país a las cuentas bancarias de las empresas y se realizan
-   validaciones cuando el país es España.
+* Añade el campo *Nombre comercial* a las empresas y permite buscar por él.


¿Qué significa permite buscar por él? Es obvio que si añades un campo puedes buscar por él. Sin embargo he probado a buscar por el nombre comercial usando el buscado rápido y no lo permite.

Se permite buscar desde los campos many2one, por ejemplo, buscando un cliente para poner en el pedido de venta.

yajo · 2016-12-13T11:42:25Z

l10n_es_partner/README.rst

+Configuración > Técnico > Parámetros > Parámetros del sistema
+Seleccionar la clave l10n_es_partner.name_pattern
+Definir el patron utilizando las etiquetas *%(name)s* para nombre y
+*%(comercial_name)s* para nombre comercial.


¿Esto no choca con https://github.com/OCA/partner-contact/tree/9.0/partner_firstname?

No debería chocar, ya que se añade a lo que devuelva el super.

yajo · 2016-12-13T11:43:10Z

l10n_es_partner/data/l10n_es_partner_data.xml

+<!-- © 2016 Carlos Dauden <carlos.dauden@tecnativa.com>
+     License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl-3). -->
+<odoo>
+    <data noupdate="1">


Podría ser <odoo noupdate="1">, aunque no es bloqueante.

yajo · 2016-12-13T11:45:45Z

l10n_es_partner/gen_src/gen_data_banks.py

        return
    # Preparar el archivo resultante
    output = codecs.open(dest_path, mode='w', encoding='utf-8')
    output.write("<?xml version='1.0' encoding='UTF-8'?>\n")
-    output.write("<openerp>\n")
-    output.write("    <data>\n")
+    output.write("<odoo>\n")


Me sentiría más confiado de usar lxml o csv para esto, por el tema de escapar datos y tal, aparte de que el código quedaría bastante más limpio. Supongo que sería una buena idea para el roadmap si no lo hacemos ahora.

Con CSV tendríamos que escapar más datos todavía, y lxml es muy poco pythonista y apuesto a que sería incluso más código, así que prefiero seguir haciéndolo así.

¿El CSV no lo escapa automáticamente LibreOffice? Hasta donde he probado es así... 🤔

Pero es que aquí no hay LibreOffice entre medias. Se procesa directamente por Odoo.

Ah cierto, de todas formas está el módulo csv, que te hace todo él solito.

Sí, pero ya te digo, en esto no merece la pena volverlo a cambiar. Lo dejamos así.

yajo · 2016-12-13T11:46:39Z

l10n_es_partner/gen_src/gen_data_banks.py

@@ -68,56 +71,67 @@ def gen_bank_data_xml(src_path, dest_path):
    try:
        reader = XlsDictReader(src_path)
    except IOError:
-        _logger.info("Archivo REGBANESP_CONESTAB_A.XLS no encontrado.")
+        _logger.error("Archivo '%s' no encontrado." % src_path)


Creo que es bueno poner los logs en inglés, más que nada por si algún extranjero tiene que instalar este módulo en la base de datos de un cliente, para que pueda darle mantenimiento.

yajo · 2016-12-13T11:49:39Z

l10n_es_partner/gen_src/gen_data_banks.py

+             escape(unicode(int(row['CODPOSTAL']))[:-3].zfill(2)) or False))
+        output.write(indent * 2 + '<field name="country" ref="base.es"/>\n')
+        output.write(indent + '</record>\n')
+    output.write("</odoo>\n")
    output.close()
    _logger.info("data_banks.xml generado correctamente.")


 if __name__ == "__main__":


Igual estoy preguntando una perogrullada, pero ¿por qué íbamos a querer ejecutar un script directamente? ¿No hay un wizard en Odoo ya que lo hace? ¿Y no debería guardarse en un ir.attachment en el filestore en lugar de donde el código del módulo? las carpetas de código pueden no ser escribibles por el usuario odoo.

Esto está puesto así para poder ejecutar el script directamente para obtener el xml archivado que se incluye como salvaguarda en caso de que no funcione la conexión a Internet.

yajo · 2016-12-13T12:01:14Z

l10n_es_partner/models/res_partner.py

+
+    @api.multi
+    @api.depends('name', 'comercial')
+    def name_get(self):


¿No es más sencillo hacer del display_name un campo calculado (que, de hecho, puede estar almacenado)? Con esto puede que incluso nos ahorrásemos sobrecargar name_search...

yajo · 2016-12-13T12:02:39Z

l10n_es_partner/models/res_bank.py

+
+
+class ResBank(models.Model):
+    _inherit = 'res.bank'


¿No es res.partner.bank?

No, estos campos se añaden al banco, no a la cuenta bancaria del cliente.

yajo · 2016-12-13T12:04:38Z

l10n_es_partner/models/res_partner_bank.py

+        return "%1d%1d" % (dc1, dc2)
+
+    @api.model
+    def check_bank_account(self, account):


¿Esto no debería ser un constraint?

No, se activa en un onchange. El nombre es herencia, pero bueno, tampoco veo uno mejor.

yajo · 2016-12-13T12:04:57Z

l10n_es_partner/models/res_partner_bank.py

+        for i in account:
+            if i.isdigit():
+                number += i
+        if len(number) != 20:


Este chequeo debería omitirse si la cuenta no es española.

Esa comprobación ya está en el método que lo llama (el onchange).

yajo · 2016-12-13T12:06:29Z

l10n_es_partner/models/res_partner_bank.py

+        while iban_str:
+            res.append(iban_str[:4])
+            iban_str = iban_str[4:]
+        return ' '.join(res)


¿No sirve la función que incluye Odoo?

pedrobaeza · 2016-12-30T19:47:52Z

@yajo listo para revisar otra vez, con los comentarios correspondientes

yajo

Un último esfuerzo 😉

yajo · 2017-01-02T08:26:58Z

l10n_es_partner/models/res_partner_bank.py

@@ -14,23 +15,24 @@ class ResPartnerBank(models.Model):
    acc_country_id = fields.Many2one(
        comodel_name='res.country', related='partner_id.country_id',
        string='Bank country', readonly=True,
-        help='If the country of the bank is Spain, it validates the bank '
+        help='If the country ofº the bank is Spain, it validates the bank '


Se te ha colado un º

yajo · 2017-01-02T08:27:25Z

l10n_es_partner/models/account_journal.py

@@ -24,17 +24,23 @@ def create(self, vals):
            # Write a more complete version of the journal name for Spanish
            # bank and cash journals
            bank = self.env['res.bank'].browse(vals['bank_id'])
-            vals['name'] = "{} {}".format(bank.name, vals['bank_acc_number'])
+            vals['name'] = u"{} {}".format(bank.name, vals['bank_acc_number'])
        return super(AccountJournal, self).create(vals)

    @api.multi
    @api.onchange('bank_acc_country_id', 'bank_acc_number')
    def onchange_bank_acc_number_l10n_es_partner(self):


En api nueva los onchange deben ser privados.

yajo · 2017-01-02T08:27:43Z

l10n_es_partner/models/res_partner_bank.py

        factor = (1, 2, 4, 8, 5, 10, 9, 7, 3, 6)
        # Cálculo CRC
        nCRC = 0
        for n in range(10):
            nCRC += int(texto[n]) * factor[n]
-        # Reducción del CRC a un dígi9to
+        # Reducción del CRC a un dígiito


yajo · 2017-01-02T08:29:00Z

l10n_es_partner/models/account_journal.py

+
+    @api.multi
+    @api.onchange('bank_acc_country_id', 'bank_acc_number')
+    def onchange_bank_acc_number_l10n_es_partner(self):


Este método debería ser privado también.

yajo · 2017-01-02T08:29:13Z

l10n_es_partner/models/res_partner_bank.py

+
+    @api.multi
+    @api.onchange('acc_country_id', 'acc_number')
+    def onchange_acc_number_l10n_es_partner(self):


En api nueva, los onchange deberían ser privados.

No tiene por qué. De hecho, yo casi prefiero que no lo sean para evitar los avisos de los IDEs que estás invocando a un método privado y demás.

La api nueva llama al método onchange(), que se encarga de llamar a los otros, por eso se suelen quedar privados, aparte de que son las guidelines. No sé qué IDE gastas, pero el mío lleva flake8 integrado y nunca le he visto quejarse por eso 😆 . Bueno, de todas formas supongo que tampoco es obligatorio 😉

El aviso la da PyCharm como un warning, no como un error, y es debido a como no se utiliza herencia Python, si no el _inherit del ORM. Haciendo una búsqueda en el código de Odoo, hay ocurrencias de ambas formas, pero en las guidelines sí que habla de _onchange, así que lo cambio.

yajo · 2017-01-02T08:30:04Z

l10n_es_partner/models/res_partner_bank.py

+        nCRC = 0
+        for n in range(10):
+            nCRC += int(texto[n]) * factor[n]
+        # Reducción del CRC a un dígiito


yajo · 2017-01-02T08:31:27Z

l10n_es_partner/models/res_partner_bank.py

+    acc_country_id = fields.Many2one(
+        comodel_name='res.country', related='partner_id.country_id',
+        string='Bank country', readonly=True,
+        help='If the country ofº the bank is Spain, it validates the bank '


Se te ha colado un º

…onfigurable pattern

pedrobaeza · 2017-01-02T10:55:52Z

@yajo, últimos cambios hechos

* Better search on partner * Review fixes * More tests

pedrobaeza mentioned this pull request Dec 10, 2016

Migration to version 9.0 #225

Closed

31 tasks

rafaelbn approved these changes Dec 10, 2016

View reviewed changes

yajo suggested changes Dec 13, 2016

View reviewed changes

[MIG] l10n_es_partner

b5ed9d8

pedrobaeza force-pushed the 9.0-l10n_es_partner branch 2 times, most recently from b1dc166 to 5782757 Compare December 30, 2016 19:22

yajo suggested changes Jan 2, 2017

View reviewed changes

[9.0][IMP] l10n_es_partner: Return commercial name in name_get with c…

f8b033f

…onfigurable pattern

pedrobaeza force-pushed the 9.0-l10n_es_partner branch from 5782757 to 03ea949 Compare January 2, 2017 10:54

yajo approved these changes Jan 2, 2017

View reviewed changes

pedrobaeza force-pushed the 9.0-l10n_es_partner branch from 03ea949 to ba2b3b5 Compare January 3, 2017 08:38

[FIX+IMP] l10n_es_partner

988a906

* Better search on partner * Review fixes * More tests

pedrobaeza force-pushed the 9.0-l10n_es_partner branch from ba2b3b5 to 988a906 Compare January 3, 2017 09:30

[IMP] l10n_es_partner: Migration script

632f9be

pedrobaeza merged commit 19b0692 into OCA:9.0 Jan 3, 2017

pedrobaeza deleted the 9.0-l10n_es_partner branch January 3, 2017 10:32

[9.0][MIG] l10n_es_partner #410

[9.0][MIG] l10n_es_partner #410

Conversation

pedrobaeza commented Dec 10, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pedrobaeza commented Dec 30, 2016

yajo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pedrobaeza commented Jan 2, 2017

pedrobaeza commented Dec 10, 2016 •

edited

Loading