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

12.0 bug l10n es account bank statement import n43 #1228

Closed
wants to merge 4 commits into from
Closed

12.0 bug l10n es account bank statement import n43 #1228

wants to merge 4 commits into from

Conversation

acysos
Copy link
Member

@acysos acysos commented Nov 22, 2019

Hola,

Añade un array con líneas que deben ser ignoradas. Se realiza así por ahora sabemos que algunos bancos mandan una línea con todos ceros, pero en un futuro puede ser otra línea diferente.

Saludos

@OCA-git-bot
Copy link
Contributor

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

@pedrobaeza
Copy link
Member

Gracias por el PR. Te hago varios comentarios:

  • El PR está sucio con tres commits de merge.
  • En lugar de colocar la línea completa a ignorarla, ¿no se podría desactivar el check en el propio parseo?
  • Comentaban que si se quitaba así tal cual, luego daba error en el nº de líneas esperado en algunos bancos.

@acysos
Copy link
Member Author

acysos commented Nov 23, 2019

@pedrobaeza Son merge para actualizar mi repositorio a la ultima versión pero esta claro que no he hecho algo bien porque se han quedado los restos, alguna pista para poder quitarlos?
De hecho he probado con rebase pero ha metido más commit, que ha generado un post de OCA-Clabot. Lo he dejado al original, a la espera de que me des algún consejo.

@OCA OCA deleted a comment from oca-clabot Nov 23, 2019
@acysos
Copy link
Member Author

acysos commented Nov 23, 2019

@pedrobaeza Respecto a quitar la línea, si se quita en el archivo de texto directamente da error, es correcto, se debe a la última línea del archivo de texto que marca cuantas hay, pero al quitarla a este nivel realmente no modificas el archivo, odoo no la procesa. Esta probado ya con un cliente en producción.

Copy link
Sponsor Member

@cubells cubells left a comment

Choose a reason for hiding this comment

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

Listo para aprobar.

Funciona correcto.

@@ -169,6 +174,8 @@ def _parse(self, data_file):
for raw_line in data_file.split("\n"):
if not raw_line.strip():
continue
if raw_line in ignore_lines:
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Mejor hacer la comparación arriba:

if not raw_line.strip() or raw_line in ignore_lines:

@@ -36,6 +36,11 @@
'99': '5720%00',
}

ignore_lines = [
'22 0000000000000000000000000000000000000000000000000000000000 '
' '
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Añade coma al final por si en un futuro se añaden más líneas a ignorar.

@pedrobaeza
Copy link
Member

@acysos puedes por favor limpiar el PR de commits de merge para poder así fusionarlo?

@pedrobaeza
Copy link
Member

También atiende por favor a los comentarios de @cubells

@acysos
Copy link
Member Author

acysos commented Jun 15, 2020

Continua en #1397

@acysos acysos closed this Jun 15, 2020
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.

None yet

4 participants