[FIX] account_statement_import: Enhance file import handling for Excel and CSV formats#378
[FIX] account_statement_import: Enhance file import handling for Excel and CSV formats#378rov-adhoc wants to merge 1 commit intoingadhoc:18.0from
Conversation
…l and CSV formats
There was a problem hiding this comment.
Pull request overview
Este PR modifica split_base64_excel en el módulo account_statement_import_sheet_file_bg para que, además de archivos xlsx, soporte también formatos .xls (via xlrd) y CSV (con detección automática de encoding via chardet) al dividir estados de cuenta bancarios en partes para el procesamiento en background.
Changes:
- Agrega soporte de parsing para archivos
.xls(xlrd) y CSV dentro desplit_base64_excel, con fallback encadenado entre los tres formatos. - Incorpora imports nuevos:
xlrd,chardet,csv.reader,StringIOylogging. - Reemplaza la llamada a
_filter_rows_with_datepor una comprensión de lista inline, dejando el método original como código sin usar.
Comments suppressed due to low confidence (1)
account_statement_import_sheet_file_bg/models/account_statement_import.py:287
- El método
_filter_rows_with_datequedó como código muerto. Su lógica fue reemplazada por la comprensión de lista en la línea 243. El método nunca es llamado desde ningún lugar tras este cambio. Debería eliminarse para mantener el código limpio y evitar confusión sobre si hay una diferencia de comportamiento entre ambas implementaciones (la original usabarow[date_column_index].valuepara celdas openpyxl, mientras la nueva usar[date_column_index]directamente).
def _filter_rows_with_date(self, data_rows, date_column_index):
"""Filter data rows to only include rows where the date column is not empty.
If date_column_index is None, return all rows."""
if date_column_index is None:
return data_rows
filtered_rows = []
for row in data_rows:
# Check if the row has enough columns and the date column is not empty
if len(row) > date_column_index and row[date_column_index].value:
filtered_rows.append(row)
elif len(row) > date_column_index and not row[date_column_index].value:
# Stop processing when we find the first empty date
break
return filtered_rows
| file_contents=file_bytes, | ||
| encoding_override=(mapping.file_encoding if mapping.file_encoding else None), | ||
| ) | ||
| sheet = workbook.sheet_by_index(0) |
There was a problem hiding this comment.
En la rama de xlrd (xls), all_rows nunca se asigna, pero se evalúa en la condición if not all_rows en la línea 218. Esto lanzará un NameError en tiempo de ejecución cada vez que el archivo sea un .xls válido. Solo se asigna csv_or_xlsx en esa rama, por lo que la variable all_rows queda indefinida al salir del bloque except.
| sheet = workbook.sheet_by_index(0) | |
| sheet = workbook.sheet_by_index(0) | |
| all_rows = [sheet.row(i) for i in range(sheet.nrows)] |
| file_bytes = base64.b64decode(self.statement_file) | ||
| read_buffer = BytesIO(file_bytes) | ||
|
|
||
| # Try openpyxl (xlsx) | ||
| input_workbook = load_workbook(read_buffer) | ||
| input_worksheet = input_workbook.active | ||
| all_rows = list(input_worksheet.rows) | ||
| csv_or_xlsx = (input_workbook, input_worksheet) | ||
|
|
||
| except Exception: | ||
| return [self.statement_file] | ||
| try: | ||
| # Try xlrd (xls) | ||
| workbook = xlrd.open_workbook( | ||
| file_contents=file_bytes, |
There was a problem hiding this comment.
Si base64.b64decode(self.statement_file) lanza una excepción en la línea 176 (por ejemplo, si el contenido no es base64 válido), la variable file_bytes no llega a definirse. Sin embargo, los bloques except anidados en las líneas 189 y 205 la referencian directamente, lo que produciría un NameError en lugar del error real. Se debería mover file_bytes = base64.b64decode(self.statement_file) fuera del bloque try, o manejar explícitamente ese caso antes.
| except UnicodeDecodeError: | ||
| detected_encoding = chardet.detect(file_bytes).get("encoding", False) | ||
| if not detected_encoding: | ||
| raise UserError(self.env._("No valid encoding was found for the attached file")) from None | ||
| decoded = file_bytes.decode(detected_encoding) |
There was a problem hiding this comment.
Si chardet no está instalado, solo se registra un warning (línea 25), pero chardet no queda definida. Luego, en la línea 207, se usa chardet.detect(...) directamente sin verificar si la variable está disponible, lo que lanzaría un NameError en tiempo de ejecución cuando falle la decodificación UTF-8 de un CSV. Se debe verificar que chardet esté disponible antes de usarlo, o bien lanzar un UserError descriptivo si no está instalado.
| output_workbook = Workbook() | ||
| output_worksheet = output_workbook.active | ||
|
|
||
| for header_row in header_rows: | ||
| row_values = [cell.value for cell in header_row] | ||
| output_worksheet.append(row_values) | ||
| output_worksheet.append(header_row) | ||
|
|
||
| for data_row in rows_for_current_part: | ||
| row_values = [cell.value for cell in data_row] | ||
| output_worksheet.append(row_values) | ||
| output_worksheet.append(data_row) | ||
|
|
||
| write_buffer = BytesIO() | ||
| output_workbook.save(write_buffer) | ||
| output_bytes = write_buffer.getvalue() | ||
|
|
||
| base64_content = base64.b64encode(output_bytes).decode("utf-8") | ||
| output_base64_list.append(base64_content) |
There was a problem hiding this comment.
Para archivos CSV y XLS, el resultado siempre se serializa como xlsx (openpyxl Workbook). Esto rompe la compatibilidad con el código de import_file_button que inspecciona el tipo de salida para determinar si es CSV o XLS (líneas 60–67): el campo csv_or_xls del wizard siempre será None para los chunks generados desde CSV o XLS de entrada, porque split_base64_excel devuelve strings base64 de xlsx en todos los casos. El parser del módulo dependiente podría no ser capaz de leer como xlsx lo que originalmente era CSV o XLS.
| for header_row in header_rows: | ||
| row_values = [cell.value for cell in header_row] | ||
| output_worksheet.append(row_values) | ||
| output_worksheet.append(header_row) | ||
|
|
||
| for data_row in rows_for_current_part: | ||
| row_values = [cell.value for cell in data_row] | ||
| output_worksheet.append(row_values) | ||
| output_worksheet.append(data_row) |
There was a problem hiding this comment.
Las líneas 256 y 259 pasan filas directamente a output_worksheet.append(). Para archivos xlsx (rama openpyxl), all_rows contiene tuplas de objetos Cell. La versión anterior del código extraía explícitamente .value con [cell.value for cell in header_row]. Al pasar objetos Cell directamente a append(), openpyxl intentará serializar el objeto en lugar del valor escalar, produciendo datos corruptos o errores. Se debe restaurar la extracción de .value para la rama xlsx, o bien normalizar todas las filas a listas de valores desde el principio según el tipo de archivo.
|
|
||
| _logger = logging.getLogger(__name__) | ||
| try: | ||
| from csv import reader | ||
|
|
There was a problem hiding this comment.
csv.reader es parte de la biblioteca estándar de Python y nunca puede lanzar ImportError ni OSError. Agruparla en el mismo bloque try junto a xlrd hace que si xlrd no está instalado y el ImportError es capturado, reader tampoco quede vinculada (el import de csv nunca se ejecuta). Se debería importar reader fuera del bloque try, y dejar solo import xlrd dentro.
| _logger = logging.getLogger(__name__) | |
| try: | |
| from csv import reader | |
| from csv import reader | |
| _logger = logging.getLogger(__name__) | |
| try: |
| # Only parse header and rows for Excel files (when all_rows is not yet populated) | ||
| if not all_rows: | ||
| header = parser.parse_header(csv_or_xlsx, mapping) | ||
| columns = dict() | ||
| for column_name in parser._get_column_names(): | ||
| columns[column_name] = parser._get_column_indexes(header, column_name, mapping) | ||
| data = csv_or_xlsx, self.statement_file | ||
| all_rows = parser._parse_rows(mapping, currency_code, data, columns) |
There was a problem hiding this comment.
La condición if not all_rows en la línea 218 pretende diferenciar el caso xlrd del xlsx, pero en realidad lo que hace es entrar en la rama _parse_rows cuando all_rows está vacío. Para xlsx con filas vacías, el código entraría incorrectamente en esa rama. Más grave aún: _parse_rows devuelve registros de transacciones parseados (diccionarios), no listas de celdas crudas. Luego, esas "filas" parseadas se pasan a output_worksheet.append(data_row) en la línea 259, lo que provocará un error o producirá datos incorrectos. El propósito del método split_base64_excel es generar partes del archivo original con sus filas sin procesar, no con transacciones ya parseadas. Se debería usar una bandera explícita (p. ej. is_xls) para distinguir el caso xlrd, y para xlrd hay que leer las filas crudas directamente con sheet.row_values(i).
|
@roboadhoc r+ nobump |
…l and CSV formats closes #378 Signed-off-by: Filoquin adhoc <maq@adhoc.com.ar>

No description provided.