fix: safeguard dashboard audit logging#258
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR safeguards dashboard audit logging by introducing a reusable helper method that safely handles cases where the audit table is unavailable. It also adds new dashboard functionality and refactors existing audit logging calls to use a more structured context-based approach.
Key changes:
- New
_registrar_auditoriahelper method with table existence check - Addition of
ver_dashboard,personalizar_dashboard,compartir_dashboard, andexportar_dashboardmethods - Refactored
exportar,personalizar, andcompartirmethods to use new audit helper and structured context
| ) -> None: | ||
| """Registra auditoría de forma segura sólo si la tabla existe.""" | ||
|
|
||
| if AuditoriaPermiso._meta.db_table not in connection.introspection.table_names(): |
There was a problem hiding this comment.
The connection.introspection.table_names() call queries the database on every audit log attempt. This can become a performance bottleneck. Consider caching the result or using a try-except block with AuditoriaPermiso.objects.create() instead, which will be more efficient in production where the table typically exists.
|
|
||
| widgets_invalidos = [widget for widget in widgets if widget not in WIDGET_REGISTRY] | ||
| if widgets_invalidos: | ||
| raise ValidationError(f"Widget invalido: {', '.join(widgets_invalidos)}") |
There was a problem hiding this comment.
Corrected spelling of 'invalido' to 'inválido'.
| raise ValidationError(f"Widget invalido: {', '.join(widgets_invalidos)}") | |
| raise ValidationError(f"Widget inválido: {', '.join(widgets_invalidos)}") |
| ValidationError: Si el formato es inválido. | ||
| """ | ||
| if formato not in {"csv", "pdf"}: | ||
| raise ValidationError("Formato invalido. Use csv o pdf") |
There was a problem hiding this comment.
Corrected spelling of 'invalido' to 'inválido'.
| raise ValidationError("Formato invalido. Use csv o pdf") | |
| raise ValidationError("Formato inválido. Use csv o pdf") |
| def exportar_dashboard(usuario_id: int, formato: str = 'csv') -> Union[str, bytes]: | ||
| """Exporta el dashboard del usuario en formato CSV o PDF. | ||
|
|
There was a problem hiding this comment.
The exportar_dashboard method (lines 218-253) and the existing exportar method (lines 94-146) provide overlapping export functionality but with different implementations. The exportar method returns metadata with a file path, while exportar_dashboard returns the actual file content. This duplication creates confusion about which method to use. Consider consolidating these methods or clearly documenting their different use cases and renaming them to reflect their distinct purposes (e.g., exportar_metadata vs exportar_contenido).
| def exportar_dashboard(usuario_id: int, formato: str = 'csv') -> Union[str, bytes]: | |
| """Exporta el dashboard del usuario en formato CSV o PDF. | |
| def exportar_contenido(usuario_id: int, formato: str = 'csv') -> Union[str, bytes]: | |
| """ | |
| Exporta el contenido del dashboard del usuario en formato CSV o PDF. | |
| Este método retorna el contenido directamente (CSV como str, PDF como bytes). | |
| Úselo cuando se requiera el archivo para descarga directa o envío. | |
| Para obtener sólo metadatos o la ruta del archivo exportado, use `exportar_metadata`. |
|
|
||
| if formato == "csv": | ||
| output = StringIO() | ||
| writer = csv.DictWriter(output, fieldnames=["type", "title", "value", "change", "period"]) |
There was a problem hiding this comment.
The CSV fieldnames are hardcoded, making this code fragile if the Widget dataclass structure changes. Consider deriving fieldnames dynamically from the Widget dataclass fields (e.g., using Widget.__dataclass_fields__.keys() or asdict(Widget(...)).keys()) to maintain consistency with the Widget definition.
| writer = csv.DictWriter(output, fieldnames=["type", "title", "value", "change", "period"]) | |
| writer = csv.DictWriter(output, fieldnames=list(Widget.__dataclass_fields__.keys())) |
Summary
Testing
Codex Task