diff --git a/alembic/versions/91d02647efae_add_marketplace_import_errors_table.py b/alembic/versions/91d02647efae_add_marketplace_import_errors_table.py new file mode 100644 index 00000000..29db8b03 --- /dev/null +++ b/alembic/versions/91d02647efae_add_marketplace_import_errors_table.py @@ -0,0 +1,44 @@ +"""add marketplace import errors table + +Revision ID: 91d02647efae +Revises: 987b4ecfa503 +Create Date: 2025-12-13 13:13:46.969503 + +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + +# revision identifiers, used by Alembic. +revision: str = '91d02647efae' +down_revision: Union[str, None] = '987b4ecfa503' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + # Create marketplace_import_errors table to store detailed import error information + op.create_table('marketplace_import_errors', + sa.Column('id', sa.Integer(), nullable=False), + sa.Column('import_job_id', sa.Integer(), nullable=False), + sa.Column('row_number', sa.Integer(), nullable=False), + sa.Column('identifier', sa.String(), nullable=True), + sa.Column('error_type', sa.String(length=50), nullable=False), + sa.Column('error_message', sa.Text(), nullable=False), + sa.Column('row_data', sa.JSON(), nullable=True), + sa.Column('created_at', sa.DateTime(), nullable=False), + sa.Column('updated_at', sa.DateTime(), nullable=False), + sa.ForeignKeyConstraint(['import_job_id'], ['marketplace_import_jobs.id'], ondelete='CASCADE'), + sa.PrimaryKeyConstraint('id') + ) + op.create_index('idx_import_error_job_id', 'marketplace_import_errors', ['import_job_id'], unique=False) + op.create_index('idx_import_error_type', 'marketplace_import_errors', ['error_type'], unique=False) + op.create_index(op.f('ix_marketplace_import_errors_id'), 'marketplace_import_errors', ['id'], unique=False) + + +def downgrade() -> None: + op.drop_index(op.f('ix_marketplace_import_errors_id'), table_name='marketplace_import_errors') + op.drop_index('idx_import_error_type', table_name='marketplace_import_errors') + op.drop_index('idx_import_error_job_id', table_name='marketplace_import_errors') + op.drop_table('marketplace_import_errors') diff --git a/app/api/v1/admin/marketplace.py b/app/api/v1/admin/marketplace.py index 8daea1d8..ce21de92 100644 --- a/app/api/v1/admin/marketplace.py +++ b/app/api/v1/admin/marketplace.py @@ -19,6 +19,8 @@ from models.schema.marketplace_import_job import ( AdminMarketplaceImportJobListResponse, AdminMarketplaceImportJobRequest, AdminMarketplaceImportJobResponse, + MarketplaceImportErrorListResponse, + MarketplaceImportErrorResponse, MarketplaceImportJobRequest, MarketplaceImportJobResponse, ) @@ -128,3 +130,36 @@ def get_marketplace_import_job( """Get a single marketplace import job by ID (Admin only).""" job = marketplace_import_job_service.get_import_job_by_id_admin(db, job_id) return marketplace_import_job_service.convert_to_admin_response_model(job) + + +@router.get("/{job_id}/errors", response_model=MarketplaceImportErrorListResponse) +def get_import_job_errors( + job_id: int, + page: int = Query(1, ge=1), + limit: int = Query(50, ge=1, le=100), + error_type: str | None = Query(None, description="Filter by error type"), + db: Session = Depends(get_db), + current_admin: User = Depends(get_current_admin_api), +): + """Get import errors for a specific job (Admin only). + + Returns detailed error information including row number, identifier, + error type, error message, and raw row data for review. + """ + # Verify job exists + marketplace_import_job_service.get_import_job_by_id_admin(db, job_id) + + # Get errors from service + errors, total = marketplace_import_job_service.get_import_job_errors( + db=db, + job_id=job_id, + error_type=error_type, + page=page, + limit=limit, + ) + + return MarketplaceImportErrorListResponse( + errors=[MarketplaceImportErrorResponse.model_validate(e) for e in errors], + total=total, + import_job_id=job_id, + ) diff --git a/app/services/marketplace_import_job_service.py b/app/services/marketplace_import_job_service.py index 58900bd6..b34d8691 100644 --- a/app/services/marketplace_import_job_service.py +++ b/app/services/marketplace_import_job_service.py @@ -8,7 +8,7 @@ from app.exceptions import ( ImportJobNotOwnedException, ValidationException, ) -from models.database.marketplace_import_job import MarketplaceImportJob +from models.database.marketplace_import_job import MarketplaceImportError, MarketplaceImportJob from models.database.user import User from models.database.vendor import Vendor from models.schema.marketplace_import_job import ( @@ -271,5 +271,50 @@ class MarketplaceImportJobService: raise ImportJobNotFoundException(job_id) return job + def get_import_job_errors( + self, + db: Session, + job_id: int, + error_type: str | None = None, + page: int = 1, + limit: int = 50, + ) -> tuple[list[MarketplaceImportError], int]: + """ + Get import errors for a specific job with pagination. + + Args: + db: Database session + job_id: Import job ID + error_type: Optional filter by error type + page: Page number (1-indexed) + limit: Number of items per page + + Returns: + Tuple of (list of errors, total count) + """ + try: + query = db.query(MarketplaceImportError).filter( + MarketplaceImportError.import_job_id == job_id + ) + + if error_type: + query = query.filter(MarketplaceImportError.error_type == error_type) + + total = query.count() + + offset = (page - 1) * limit + errors = ( + query.order_by(MarketplaceImportError.row_number) + .offset(offset) + .limit(limit) + .all() + ) + + return errors, total + + except Exception as e: + logger.error(f"Error getting import job errors for job {job_id}: {str(e)}") + raise ValidationException("Failed to retrieve import errors") + marketplace_import_job_service = MarketplaceImportJobService() diff --git a/app/tasks/background_tasks.py b/app/tasks/background_tasks.py index 2c737a34..cd9fa078 100644 --- a/app/tasks/background_tasks.py +++ b/app/tasks/background_tasks.py @@ -73,6 +73,7 @@ async def process_marketplace_import( batch_size=batch_size, db=db, language=language, # Pass language for translations + import_job_id=job_id, # Pass job ID for error tracking ) # Update job with results diff --git a/app/templates/admin/marketplace-product-detail.html b/app/templates/admin/marketplace-product-detail.html index 6dd4d318..8f5052dc 100644 --- a/app/templates/admin/marketplace-product-detail.html +++ b/app/templates/admin/marketplace-product-detail.html @@ -221,34 +221,93 @@ - -
-

- Translations -

-
-
diff --git a/app/templates/shared/macros/modals.html b/app/templates/shared/macros/modals.html index 5b544144..064ac67c 100644 --- a/app/templates/shared/macros/modals.html +++ b/app/templates/shared/macros/modals.html @@ -625,11 +625,64 @@
- {# Error Details #} -
-

Error Details

-
-

+            {# Detailed Import Errors #}
+            
+
+

Import Errors

+ +
+ + {# Errors List #} +
+ +
+ + {# Pagination for errors #} +
+ Showing of errors +
diff --git a/app/utils/csv_processor.py b/app/utils/csv_processor.py index 554b308c..d1004e5d 100644 --- a/app/utils/csv_processor.py +++ b/app/utils/csv_processor.py @@ -18,6 +18,7 @@ import requests from sqlalchemy import literal from sqlalchemy.orm import Session +from models.database.marketplace_import_job import MarketplaceImportError from models.database.marketplace_product import MarketplaceProduct from models.database.marketplace_product_translation import MarketplaceProductTranslation @@ -280,6 +281,7 @@ class CSVProcessor: batch_size: int, db: Session, language: str = "en", + import_job_id: int | None = None, ) -> dict[str, Any]: """ Process CSV from URL with marketplace and vendor information. @@ -291,6 +293,7 @@ class CSVProcessor: batch_size: Number of rows to process in each batch db: Database session language: Language code for translations (default: 'en') + import_job_id: ID of the import job for error tracking (optional) Returns: Dictionary with processing results @@ -315,6 +318,8 @@ class CSVProcessor: # Process in batches for i in range(0, len(df), batch_size): batch_df = df.iloc[i : i + batch_size] + # Calculate base row number for this batch (1-indexed for user-friendly display) + base_row_num = i + 2 # +2 for header row and 1-indexing batch_result = await self._process_marketplace_batch( batch_df, marketplace, @@ -323,6 +328,8 @@ class CSVProcessor: i // batch_size + 1, language=language, source_file=source_file, + import_job_id=import_job_id, + base_row_num=base_row_num, ) imported += batch_result["imported"] @@ -341,6 +348,39 @@ class CSVProcessor: "language": language, } + def _create_import_error( + self, + db: Session, + import_job_id: int, + row_number: int, + error_type: str, + error_message: str, + identifier: str | None = None, + row_data: dict | None = None, + ) -> None: + """Create an import error record in the database.""" + # Limit row_data size to prevent huge JSON storage + if row_data: + # Keep only key fields for review + limited_data = { + k: v for k, v in row_data.items() + if k in [ + "marketplace_product_id", "title", "gtin", "mpn", "sku", + "brand", "price", "availability", "link" + ] and v is not None and str(v).strip() + } + row_data = limited_data if limited_data else None + + error_record = MarketplaceImportError( + import_job_id=import_job_id, + row_number=row_number, + identifier=identifier, + error_type=error_type, + error_message=error_message, + row_data=row_data, + ) + db.add(error_record) + async def _process_marketplace_batch( self, batch_df: pd.DataFrame, @@ -350,6 +390,8 @@ class CSVProcessor: batch_num: int, language: str = "en", source_file: str | None = None, + import_job_id: int | None = None, + base_row_num: int = 2, ) -> dict[str, int]: """Process a batch of CSV rows with marketplace information.""" imported = 0 @@ -361,10 +403,13 @@ class CSVProcessor: f"{marketplace} -> {vendor_name}" ) - for index, row in batch_df.iterrows(): + for batch_idx, (index, row) in enumerate(batch_df.iterrows()): + row_number = base_row_num + batch_idx + row_dict = row.to_dict() + try: # Convert row to dictionary and clean up - product_data = self._clean_row_data(row.to_dict()) + product_data = self._clean_row_data(row_dict) # Extract translation fields BEFORE processing product translation_data = self._extract_translation_data(product_data) @@ -373,17 +418,40 @@ class CSVProcessor: product_data["marketplace"] = marketplace product_data["vendor_name"] = vendor_name + # Get identifier for error tracking + identifier = product_data.get("marketplace_product_id") or product_data.get("gtin") or product_data.get("mpn") + # Validate required fields if not product_data.get("marketplace_product_id"): logger.warning( - f"Row {index}: Missing marketplace_product_id, skipping" + f"Row {row_number}: Missing marketplace_product_id, skipping" ) + if import_job_id: + self._create_import_error( + db=db, + import_job_id=import_job_id, + row_number=row_number, + error_type="missing_id", + error_message="Missing marketplace_product_id - product cannot be identified", + identifier=identifier, + row_data=row_dict, + ) errors += 1 continue # Title is now required in translation_data if not translation_data.get("title"): - logger.warning(f"Row {index}: Missing title, skipping") + logger.warning(f"Row {row_number}: Missing title, skipping") + if import_job_id: + self._create_import_error( + db=db, + import_job_id=import_job_id, + row_number=row_number, + error_type="missing_title", + error_message="Missing title - product title is required", + identifier=product_data.get("marketplace_product_id"), + row_data=row_dict, + ) errors += 1 continue @@ -448,7 +516,17 @@ class CSVProcessor: ) except Exception as e: - logger.error(f"Error processing row: {e}") + logger.error(f"Error processing row {row_number}: {e}") + if import_job_id: + self._create_import_error( + db=db, + import_job_id=import_job_id, + row_number=row_number, + error_type="processing_error", + error_message=str(e), + identifier=row_dict.get("marketplace_product_id") or row_dict.get("id"), + row_data=row_dict, + ) errors += 1 continue diff --git a/models/database/__init__.py b/models/database/__init__.py index 029545d3..4704f101 100644 --- a/models/database/__init__.py +++ b/models/database/__init__.py @@ -20,7 +20,7 @@ from .company import Company from .content_page import ContentPage from .customer import Customer, CustomerAddress from .inventory import Inventory -from .marketplace_import_job import MarketplaceImportJob +from .marketplace_import_job import MarketplaceImportError, MarketplaceImportJob from .marketplace_product import ( DigitalDeliveryMethod, MarketplaceProduct, @@ -83,6 +83,7 @@ __all__ = [ "ProductTranslation", # Import "MarketplaceImportJob", + "MarketplaceImportError", # Inventory "Inventory", # Orders diff --git a/models/database/marketplace_import_job.py b/models/database/marketplace_import_job.py index b929a426..ebc2caca 100644 --- a/models/database/marketplace_import_job.py +++ b/models/database/marketplace_import_job.py @@ -1,10 +1,59 @@ -from sqlalchemy import Column, DateTime, ForeignKey, Index, Integer, String, Text +from sqlalchemy import JSON, Column, DateTime, ForeignKey, Index, Integer, String, Text from sqlalchemy.orm import relationship from app.core.database import Base from models.database.base import TimestampMixin +class MarketplaceImportError(Base, TimestampMixin): + """ + Stores detailed information about individual import errors. + + Each row that fails during import creates an error record with: + - Row number from the source file + - Identifier (marketplace_product_id if available) + - Error type and message + - Raw row data for review + """ + + __tablename__ = "marketplace_import_errors" + + id = Column(Integer, primary_key=True, index=True) + import_job_id = Column( + Integer, + ForeignKey("marketplace_import_jobs.id", ondelete="CASCADE"), + nullable=False, + index=True, + ) + + # Error location + row_number = Column(Integer, nullable=False) + + # Identifier from the row (if available) + identifier = Column(String) # marketplace_product_id, gtin, mpn, etc. + + # Error details + error_type = Column(String(50), nullable=False) # missing_title, missing_id, parse_error, etc. + error_message = Column(Text, nullable=False) + + # Raw row data for review (JSON) + row_data = Column(JSON) + + # Relationship + import_job = relationship("MarketplaceImportJob", back_populates="errors") + + __table_args__ = ( + Index("idx_import_error_job_id", "import_job_id"), + Index("idx_import_error_type", "error_type"), + ) + + def __repr__(self): + return ( + f"" + ) + + class MarketplaceImportJob(Base, TimestampMixin): __tablename__ = "marketplace_import_jobs" @@ -37,6 +86,12 @@ class MarketplaceImportJob(Base, TimestampMixin): # Relationships vendor = relationship("Vendor", back_populates="marketplace_import_jobs") user = relationship("User", foreign_keys=[user_id]) + errors = relationship( + "MarketplaceImportError", + back_populates="import_job", + cascade="all, delete-orphan", + order_by="MarketplaceImportError.row_number", + ) # Indexes for performance __table_args__ = ( diff --git a/models/schema/marketplace_import_job.py b/models/schema/marketplace_import_job.py index 6cb31b69..632cbf95 100644 --- a/models/schema/marketplace_import_job.py +++ b/models/schema/marketplace_import_job.py @@ -81,6 +81,28 @@ class AdminMarketplaceImportJobRequest(BaseModel): return v +class MarketplaceImportErrorResponse(BaseModel): + """Response schema for individual import error.""" + + model_config = ConfigDict(from_attributes=True) + + id: int + row_number: int + identifier: str | None = None + error_type: str + error_message: str + row_data: dict | None = None + created_at: datetime + + +class MarketplaceImportErrorListResponse(BaseModel): + """Response schema for list of import errors.""" + + errors: list[MarketplaceImportErrorResponse] + total: int + import_job_id: int + + class MarketplaceImportJobResponse(BaseModel): """Response schema for marketplace import job.""" diff --git a/static/admin/js/imports.js b/static/admin/js/imports.js index 500e0d6e..7ebd06fb 100644 --- a/static/admin/js/imports.js +++ b/static/admin/js/imports.js @@ -54,6 +54,12 @@ function adminImports() { showJobModal: false, selectedJob: null, + // Job errors state + jobErrors: [], + jobErrorsTotal: 0, + jobErrorsPage: 1, + loadingErrors: false, + // Auto-refresh for active jobs autoRefreshInterval: null, @@ -275,6 +281,58 @@ function adminImports() { closeJobModal() { this.showJobModal = false; this.selectedJob = null; + // Clear errors state + this.jobErrors = []; + this.jobErrorsTotal = 0; + this.jobErrorsPage = 1; + }, + + /** + * Load errors for a specific job + */ + async loadJobErrors(jobId) { + if (!jobId) return; + + this.loadingErrors = true; + this.jobErrorsPage = 1; + + try { + const response = await apiClient.get( + `/admin/marketplace-import-jobs/${jobId}/errors?page=1&limit=20` + ); + this.jobErrors = response.errors || []; + this.jobErrorsTotal = response.total || 0; + adminImportsLog.debug('Loaded job errors:', this.jobErrors.length); + } catch (error) { + adminImportsLog.error('Failed to load job errors:', error); + this.error = error.message || 'Failed to load import errors'; + } finally { + this.loadingErrors = false; + } + }, + + /** + * Load more errors (pagination) + */ + async loadMoreJobErrors(jobId) { + if (!jobId || this.loadingErrors) return; + + this.loadingErrors = true; + this.jobErrorsPage++; + + try { + const response = await apiClient.get( + `/admin/marketplace-import-jobs/${jobId}/errors?page=${this.jobErrorsPage}&limit=20` + ); + const newErrors = response.errors || []; + this.jobErrors = [...this.jobErrors, ...newErrors]; + adminImportsLog.debug('Loaded more job errors:', newErrors.length); + } catch (error) { + adminImportsLog.error('Failed to load more job errors:', error); + this.jobErrorsPage--; // Revert page on failure + } finally { + this.loadingErrors = false; + } }, /** diff --git a/static/admin/js/marketplace-product-detail.js b/static/admin/js/marketplace-product-detail.js index 93e06bd5..a8c12085 100644 --- a/static/admin/js/marketplace-product-detail.js +++ b/static/admin/js/marketplace-product-detail.js @@ -176,6 +176,53 @@ function adminMarketplaceProductDetail() { } catch (e) { return dateString; } + }, + + /** + * Get full language name from ISO code + */ + getLanguageName(code) { + const languages = { + 'en': 'English', + 'de': 'German', + 'fr': 'French', + 'lb': 'Luxembourgish', + 'es': 'Spanish', + 'it': 'Italian', + 'nl': 'Dutch', + 'pt': 'Portuguese', + 'pl': 'Polish', + 'cs': 'Czech', + 'da': 'Danish', + 'sv': 'Swedish', + 'fi': 'Finnish', + 'no': 'Norwegian', + 'hu': 'Hungarian', + 'ro': 'Romanian', + 'bg': 'Bulgarian', + 'el': 'Greek', + 'sk': 'Slovak', + 'sl': 'Slovenian', + 'hr': 'Croatian', + 'lt': 'Lithuanian', + 'lv': 'Latvian', + 'et': 'Estonian' + }; + return languages[code?.toLowerCase()] || ''; + }, + + /** + * Copy text to clipboard + */ + async copyToClipboard(text) { + if (!text) return; + try { + await navigator.clipboard.writeText(text); + Utils.showToast('Copied to clipboard', 'success'); + } catch (err) { + adminMarketplaceProductDetailLog.error('Failed to copy to clipboard:', err); + Utils.showToast('Failed to copy to clipboard', 'error'); + } } }; } diff --git a/tests/integration/api/v1/admin/test_letzshop.py b/tests/integration/api/v1/admin/test_letzshop.py index aff8dcb8..1377d5f2 100644 --- a/tests/integration/api/v1/admin/test_letzshop.py +++ b/tests/integration/api/v1/admin/test_letzshop.py @@ -189,7 +189,7 @@ class TestAdminLetzshopCredentialsAPI: class TestAdminLetzshopConnectionAPI: """Test admin Letzshop connection testing endpoints.""" - @patch("app.services.letzshop.client.requests.Session.post") + @patch("app.services.letzshop.client_service.requests.Session.post") def test_test_vendor_connection( self, mock_post, client, admin_headers, test_vendor ): @@ -217,7 +217,7 @@ class TestAdminLetzshopConnectionAPI: data = response.json() assert data["success"] is True - @patch("app.services.letzshop.client.requests.Session.post") + @patch("app.services.letzshop.client_service.requests.Session.post") def test_test_api_key_directly( self, mock_post, client, admin_headers ): @@ -287,7 +287,7 @@ class TestAdminLetzshopOrdersAPI: assert data["total"] == 1 assert data["orders"][0]["customer_email"] == "admin-test@example.com" - @patch("app.services.letzshop.client.requests.Session.post") + @patch("app.services.letzshop.client_service.requests.Session.post") def test_trigger_vendor_sync( self, mock_post, client, admin_headers, test_vendor ): diff --git a/tests/integration/api/v1/vendor/test_letzshop.py b/tests/integration/api/v1/vendor/test_letzshop.py index 18e8dad7..8e0cb93e 100644 --- a/tests/integration/api/v1/vendor/test_letzshop.py +++ b/tests/integration/api/v1/vendor/test_letzshop.py @@ -164,7 +164,7 @@ class TestVendorLetzshopConnectionAPI: assert data["success"] is False assert "not configured" in data["error_details"] - @patch("app.services.letzshop.client.requests.Session.post") + @patch("app.services.letzshop.client_service.requests.Session.post") def test_test_connection_success( self, mock_post, client, vendor_user_headers, test_vendor_with_vendor_user ): @@ -192,7 +192,7 @@ class TestVendorLetzshopConnectionAPI: assert data["success"] is True assert data["response_time_ms"] is not None - @patch("app.services.letzshop.client.requests.Session.post") + @patch("app.services.letzshop.client_service.requests.Session.post") def test_test_api_key_without_saving( self, mock_post, client, vendor_user_headers, test_vendor_with_vendor_user ): @@ -319,7 +319,7 @@ class TestVendorLetzshopOrdersAPI: assert response.status_code == 422 # Validation error - @patch("app.services.letzshop.client.requests.Session.post") + @patch("app.services.letzshop.client_service.requests.Session.post") def test_import_orders_success( self, mock_post, @@ -384,7 +384,7 @@ class TestVendorLetzshopOrdersAPI: class TestVendorLetzshopFulfillmentAPI: """Test vendor Letzshop fulfillment endpoints.""" - @patch("app.services.letzshop.client.requests.Session.post") + @patch("app.services.letzshop.client_service.requests.Session.post") def test_confirm_order( self, mock_post, @@ -438,7 +438,7 @@ class TestVendorLetzshopFulfillmentAPI: data = response.json() assert data["success"] is True - @patch("app.services.letzshop.client.requests.Session.post") + @patch("app.services.letzshop.client_service.requests.Session.post") def test_set_tracking( self, mock_post,