[14.0] [IMP] l10n_ar_afipws_fe: Mix of changes and improvements

nimarosa commented 1 year ago

Este PR introduce los siguientes cambios:

Solo se permite forzar el número de comprobante en facturas de venta, si el usuario está en modo debug y si hay una desincronización de secuencias. Esto es útil para reparar errores de secuencias directamente desde la UI.
Se agrega pyafipws al manifest y requirements para que la instalación de la librería sea automática.
Se añade mantenedores del módulo l10n_ar_bank
Se eliminan raise error en los mensajes de error XML. De esta forma, el mensaje de error se guarda en afip_message. El usuario puede entrar al registro y consultar el error. Esto es útil cuando se autorizan múltiples facturas.
FIX: Solo se muestran los decoradores de errores de secuencia en facturas de venta
FIX: Solo se muestra la pestaña AFIP si el comprobante es de venta. TODO: Luego al implementar la validación automática de comprobantes de venta contra AFIP se volverá a mostrar la tab.
No se muestra mas la tab de "EDI" ya que esta localización no usa la funcionalidad EDI de odoo.

Se probará si pyafipws instala correctamente en runbot de OCA o necesita agregar más dependencias al requirements.

En este PR se agregaran mas cambios menores de improvement.

OCA-git-bot commented 1 year ago

Hi @ibuioli, some modules you are maintaining are being modified, check this out!

nimarosa commented 1 year ago

@pedrobaeza Hola Pedro, ¿podria tener tu ayuda aca con los script de OCA que seguro vos conoces mejor que yo? Como verás el test "Detect unreleased dependencies" falla, porque justamente encuentra que se hace referencia a una URL en el requirements.txt.

Actualmente esta librería que se usa para la facturación electrónica de Argentina, solo se encuentra disponible tomándola desde Github, por lo cual en general usamos ese repositorio para poder usarla, ya que el desarrollador de pyafipws no publica actualizaciones de paquetes en pip.

¿Hay alguna forma de hacer que no falle el test? ¿O alguna otra forma de plasmar esta dependencia en el manifest? Como verás la dependencia instala correctamente ya que todos los test y runboat pasan, pero me queda este test fallando siempre por la URL. Si no hay otra forma, ¿de que forma puedo deshabilitar esta acción para este módulo en específico?

Espero puedas darme una ayuda aquí.

nimarosa commented 1 year ago

Sino, se que puedo deshabilitar la github action en todo el repositorio, pero sería bueno intentar conservarla y simplemente ignorar este módulo y/o lograr que acepte una excepción.

Podría modificar la acción manualmente pero no se si es una buena práctica aquí en OCA.

pedrobaeza commented 1 year ago

Acerca del rojo en el CI, creo que no se pueden establecer excepciones, pero no soy el experto justo en eso.

@sbidoul this line https://github.com/OCA/l10n-argentina/pull/69/files#diff-4d7c51b1efe9043e44439a949dfd92e5827321b34082903477fd04876edb7552R2 is giving red the unreleased dependencies. It seems the check is not very fine-tuned. Should it be fixed or is it a way to avoid it?

Luego, veo que hay un commit de l10n_ar_bank que toca también l10n_ar_afipws_fe. Cuidado con mezclar. Por último, la decisión de utilizar una librería Python externa es controvertida, porque así cualquier problema se tiene que solucionar en un sitio externo a OCA, y con una gobernanza también diferente, además de no tener control sobre la calidad de ello. Si la capa de código que os quita es poca, yo optaría por tenerla en el propio módulo Odoo.

nimarosa commented 1 year ago

Acerca del rojo en el CI, creo que no se pueden establecer excepciones, pero no soy el experto justo en eso.

Si creo que no se debe poder, amenos que desactive esa github action en todo el repositorio.

Luego, veo que hay un commit de l10n_ar_bank que toca también l10n_ar_afipws_fe. Cuidado con mezclar.

Si esto cuando haga un rebase lo arreglo, fue un error en el commit message.

Por último, la decisión de utilizar una librería Python externa es controvertida, porque así cualquier problema se tiene que solucionar en un sitio externo a OCA, y con una gobernanza también diferente, además de no tener control sobre la calidad de ello. Si la capa de código que os quita es poca, yo optaría por tenerla en el propio módulo Odoo.

Pasa lo siguiente, pyafipws en realidad es la librería más usada en Argentina para facturación electrónica, es estable y es usada hasta por sistemas de primera generación para conectar con AFIP (nuestra agencia tributaria).

La librería básicamente es un wrapper de todas las API y web services de AFIP, el cual contiene métodos que ya integran toda la funcionalidad y luego en odoo se utiliza una serie de mapeos de campos y datos para llamar a los métodos. Esta librería se utiliza desde prácticamente los inicios de cualquier localización Argentina y en general es muy fiable, no ha generado errores prácticamente nunca que yo recuerde.

La librería es muy grande como para ponerla en el mismo módulo, quizá podría hacerse con un git_submodule, pero creo que haría la instalación más compleja. Fuera de eso igualmente el módulo necesita usar Pysimplesoap y OpenSSL para la confección que igual también son librerías externas. En mi experiencia, pyafipws es fiable para ser usada como dependencia, ya que incluso es lo que se usa en Odoo Enterprise para la localización argentina (a través de una API privada de un partner pero finalmente esos endpoints terminan usando pyafipws).

Gracias por tu ayuda Pedro, esperare a que Stephane responda y sino veré cómo logró parchar este error de CI.

pedrobaeza commented 1 year ago

OK, es decisión vuestra lo de la librería y respetable. Los argumentos que comentas son razonables, pero en mi experiencia con otras librerías (de transportistas), al final, la capa de traducción de los campos a argumentos y luego de vuelta la interpretación de errores era casi tan grande como el comando de webservice directo.

En el SII español, que es el equivalente a vuestro AFIP WS, no utilizamos librería. El resultado son unas 1000 líneas de código, pero perfectamente integrado en cada proceso y sin el "debug-hell" que implican las librerías, por lo que es un trade-off más que adecuado.

nimarosa commented 1 year ago

OK, es decisión vuestra lo de la librería y respetable. Los argumentos que comentas son razonables, pero en mi experiencia con otras librerías (de transportistas), al final, la capa de traducción de los campos a argumentos y luego de vuelta la interpretación de errores era casi tan grande como el comando de webservice directo.

En el SII español, que es el equivalente a vuestro AFIP WS, no utilizamos librería. El resultado son unas 1000 líneas de código, pero perfectamente integrado en cada proceso y sin el "debug-hell" que implican las librerías, por lo que es un trade-off más que adecuado.

Si, en realidad tus argumentos también son muy válidos. De hecho el tema de tener que mapear los parámetros y el "debug-hell" fue algo que molesto bastante antes de subir estos módulos ya que prácticamente tuve que reescribirlos para hacer aunque sea un mapeo más ordenado usando dicts y funciones por webservice.

Quizá esto es algo que si agregaría al roadmap del módulo, para poder hacerlo en el futuro, ya que sería bastante útil que el soporte sea totalmente nativo. Como vos decís, en realidad la librería provoca un exceso de mapeo sobre mapeo haha.

Ahora nuestra prioridad numero 1 es reescribir/refactorear todos los módulos que pertenecen a la localización en v14 (ya que aún faltan muchos por subir) para poder de ahí hacer las migraciones directamente aquí en OCA con código nuevo y libre de código legacy que es una norma en las localizaciones argentinas... De esta forma podremos rápidamente tener la localización en este repo y que los usuarios comiencen a utilizarla y mejorarla.

Luego de eso, creería que armar la misma funcionalidad nativamente en el módulo sería una genial idea. De esa forma ya no dependeremos de nada externo.

Gracias por tu ayuda, Pedro, un gusto como siempre intercambiar opiniones.

pedrobaeza commented 1 year ago

A por ello. Me decís cualquier otra duda.

nimarosa commented 1 year ago

/ocabot rebase

OCA-git-bot commented 1 year ago

Congratulations, PR rebased to 14.0.

nimarosa commented 1 year ago

/ocabot merge patch

OCA-git-bot commented 1 year ago

Hey, thanks for contributing! Proceeding to merge this for you. Prepared branch 14.0-ocabot-merge-pr-69-by-nimarosa-bump-patch, awaiting test results.

OCA-git-bot commented 1 year ago

@nimarosa The merge process could not be finalized, because command twine upload --repository-url https://upload.pypi.org/legacy/ -u __token__ odoo14_addon_l10n_ar_afipws_fe-14.0.1.0.1-py3-none-any.whl failed with output:

Uploading distributions to https://upload.pypi.org/legacy/
Uploading odoo14_addon_l10n_ar_afipws_fe-14.0.1.0.1-py3-none-any.whl
[?25l
[2K[35m  0%[0m [90m━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━[0m [32m0.0/53.0 kB[0m • [36m--:--[0m • [31m?[0m
[2K[35m  0%[0m [90m━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━[0m [32m0.0/53.0 kB[0m • [36m--:--[0m • [31m?[0m
[2K[35m  0%[0m [90m━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━[0m [32m0.0/53.0 kB[0m • [36m--:--[0m • [31m?[0m
[2K[35m  0%[0m [90m━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━[0m [32m0.0/53.0 kB[0m • [36m--:--[0m • [31m?[0m
[2K[35m100%[0m [90m━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━[0m [32m53.0/53.0 kB[0m • [33m00:00[0m • [31m59.5 MB/s[0m
[2K[35m100%[0m [90m━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━[0m [32m53.0/53.0 kB[0m • [33m00:00[0m • [31m59.5 MB/s[0m
[2K[35m100%[0m [90m━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━[0m [32m53.0/53.0 kB[0m • [33m00:00[0m • [31m59.5 MB/s[0m
[2K[35m100%[0m [90m━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━[0m [32m53.0/53.0 kB[0m • [33m00:00[0m • [31m59.5 MB/s[0m
[?25h[33mWARNING [0m Error during upload. Retry with the --verbose option for more details. 
[31mERROR   [0m HTTPError: 400 Bad Request from https://upload.pypi.org/legacy/        
         Invalid value for requires_dist. Error: Can't have direct dependency:  
         'pyafipws @ https://github.com/reingart/pyafipws/archive/main.zip'

OCA / l10n-argentina

[14.0] [IMP] l10n_ar_afipws_fe: Mix of changes and improvements #69