fix: resolve all architecture validation warnings

Service layer:
- Remove db.commit() calls from credentials_service.py (SVC-006)
- Move transaction control to API endpoint level
- Rename client.py -> client_service.py (NAM-002)
- Rename credentials.py -> credentials_service.py (NAM-002)

JavaScript:
- Use centralized logger in admin letzshop.js (JS-001)
- Replace console.log/error with LogConfig logger

Frontend templates:
- Use page_header_flex macro for page header (FE-007)
- Use error_state macro for error display (FE-003)
- Use table_wrapper macro for vendors table (FE-005)
- Use modal macro for configuration and orders modals (FE-004)

All 31 Letzshop tests pass. Architecture validation: 0 errors, 0 warnings.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
2025-12-13 12:57:26 +01:00
parent 5bcbd14391
commit 3316894c27
7 changed files with 300 additions and 385 deletions

View File

@@ -164,6 +164,7 @@ def create_or_update_vendor_credentials(
auto_sync_enabled=credentials_data.auto_sync_enabled, auto_sync_enabled=credentials_data.auto_sync_enabled,
sync_interval_minutes=credentials_data.sync_interval_minutes, sync_interval_minutes=credentials_data.sync_interval_minutes,
) )
db.commit()
logger.info( logger.info(
f"Admin {current_admin.email} updated Letzshop credentials for vendor {vendor.name}" f"Admin {current_admin.email} updated Letzshop credentials for vendor {vendor.name}"
@@ -211,6 +212,7 @@ def update_vendor_credentials(
auto_sync_enabled=credentials_data.auto_sync_enabled, auto_sync_enabled=credentials_data.auto_sync_enabled,
sync_interval_minutes=credentials_data.sync_interval_minutes, sync_interval_minutes=credentials_data.sync_interval_minutes,
) )
db.commit()
except CredentialsNotFoundError: except CredentialsNotFoundError:
raise ResourceNotFoundException( raise ResourceNotFoundException(
"LetzshopCredentials", str(vendor_id), "LetzshopCredentials", str(vendor_id),
@@ -256,6 +258,7 @@ def delete_vendor_credentials(
"LetzshopCredentials", str(vendor_id), "LetzshopCredentials", str(vendor_id),
message=f"Letzshop credentials not configured for vendor {vendor.name}" message=f"Letzshop credentials not configured for vendor {vendor.name}"
) )
db.commit()
logger.info( logger.info(
f"Admin {current_admin.email} deleted Letzshop credentials for vendor {vendor.name}" f"Admin {current_admin.email} deleted Letzshop credentials for vendor {vendor.name}"

View File

@@ -131,6 +131,7 @@ def save_credentials(
auto_sync_enabled=credentials_data.auto_sync_enabled, auto_sync_enabled=credentials_data.auto_sync_enabled,
sync_interval_minutes=credentials_data.sync_interval_minutes, sync_interval_minutes=credentials_data.sync_interval_minutes,
) )
db.commit()
logger.info(f"Vendor user {current_user.email} updated Letzshop credentials") logger.info(f"Vendor user {current_user.email} updated Letzshop credentials")
@@ -167,6 +168,7 @@ def update_credentials(
auto_sync_enabled=credentials_data.auto_sync_enabled, auto_sync_enabled=credentials_data.auto_sync_enabled,
sync_interval_minutes=credentials_data.sync_interval_minutes, sync_interval_minutes=credentials_data.sync_interval_minutes,
) )
db.commit()
except CredentialsNotFoundError: except CredentialsNotFoundError:
raise ResourceNotFoundException("LetzshopCredentials", str(vendor_id)) raise ResourceNotFoundException("LetzshopCredentials", str(vendor_id))
@@ -198,6 +200,7 @@ def delete_credentials(
raise ResourceNotFoundException( raise ResourceNotFoundException(
"LetzshopCredentials", str(current_user.token_vendor_id) "LetzshopCredentials", str(current_user.token_vendor_id)
) )
db.commit()
logger.info(f"Vendor user {current_user.email} deleted Letzshop credentials") logger.info(f"Vendor user {current_user.email} deleted Letzshop credentials")
return LetzshopSuccessResponse(success=True, message="Letzshop credentials deleted") return LetzshopSuccessResponse(success=True, message="Letzshop credentials deleted")

View File

@@ -9,14 +9,14 @@ Provides:
- Fulfillment sync service - Fulfillment sync service
""" """
from .client import ( from .client_service import (
LetzshopAPIError, LetzshopAPIError,
LetzshopAuthError, LetzshopAuthError,
LetzshopClient, LetzshopClient,
LetzshopClientError, LetzshopClientError,
LetzshopConnectionError, LetzshopConnectionError,
) )
from .credentials import ( from .credentials_service import (
CredentialsError, CredentialsError,
CredentialsNotFoundError, CredentialsNotFoundError,
LetzshopCredentialsService, LetzshopCredentialsService,

View File

@@ -1,4 +1,4 @@
# app/services/letzshop/client.py # app/services/letzshop/client_service.py
""" """
GraphQL client for Letzshop marketplace API. GraphQL client for Letzshop marketplace API.

View File

@@ -1,4 +1,4 @@
# app/services/letzshop/credentials.py # app/services/letzshop/credentials_service.py
""" """
Letzshop credentials management service. Letzshop credentials management service.
@@ -13,7 +13,7 @@ from sqlalchemy.orm import Session
from app.utils.encryption import decrypt_value, encrypt_value, mask_api_key from app.utils.encryption import decrypt_value, encrypt_value, mask_api_key
from models.database.letzshop import VendorLetzshopCredentials from models.database.letzshop import VendorLetzshopCredentials
from .client import LetzshopClient from .client_service import LetzshopClient
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
@@ -127,8 +127,7 @@ class LetzshopCredentialsService:
) )
self.db.add(credentials) self.db.add(credentials)
self.db.commit() self.db.flush()
self.db.refresh(credentials)
logger.info(f"Created Letzshop credentials for vendor {vendor_id}") logger.info(f"Created Letzshop credentials for vendor {vendor_id}")
return credentials return credentials
@@ -168,8 +167,7 @@ class LetzshopCredentialsService:
if sync_interval_minutes is not None: if sync_interval_minutes is not None:
credentials.sync_interval_minutes = sync_interval_minutes credentials.sync_interval_minutes = sync_interval_minutes
self.db.commit() self.db.flush()
self.db.refresh(credentials)
logger.info(f"Updated Letzshop credentials for vendor {vendor_id}") logger.info(f"Updated Letzshop credentials for vendor {vendor_id}")
return credentials return credentials
@@ -189,7 +187,7 @@ class LetzshopCredentialsService:
return False return False
self.db.delete(credentials) self.db.delete(credentials)
self.db.commit() self.db.flush()
logger.info(f"Deleted Letzshop credentials for vendor {vendor_id}") logger.info(f"Deleted Letzshop credentials for vendor {vendor_id}")
return True return True
@@ -370,8 +368,7 @@ class LetzshopCredentialsService:
credentials.last_sync_status = status credentials.last_sync_status = status
credentials.last_sync_error = error credentials.last_sync_error = error
self.db.commit() self.db.flush()
self.db.refresh(credentials)
return credentials return credentials

View File

@@ -1,6 +1,11 @@
{# app/templates/admin/letzshop.html #} {# app/templates/admin/letzshop.html #}
{% extends "admin/base.html" %} {% extends "admin/base.html" %}
{% from 'shared/macros/headers.html' import page_header_flex, refresh_button %}
{% from 'shared/macros/alerts.html' import error_state, alert_dynamic %}
{% from 'shared/macros/tables.html' import table_wrapper, table_header %}
{% from 'shared/macros/modals.html' import modal %}
{% block title %}Letzshop Management{% endblock %} {% block title %}Letzshop Management{% endblock %}
{% block alpine_data %}adminLetzshop(){% endblock %} {% block alpine_data %}adminLetzshop(){% endblock %}
@@ -11,48 +16,15 @@
{% block content %} {% block content %}
<!-- Page Header --> <!-- Page Header -->
<div class="flex items-center justify-between my-6"> {% call page_header_flex(title='Letzshop Management', subtitle='Manage Letzshop integration for all vendors') %}
<div> {{ refresh_button(loading_var='loading', onclick='refreshData()') }}
<h2 class="text-2xl font-semibold text-gray-700 dark:text-gray-200"> {% endcall %}
Letzshop Management
</h2>
<p class="text-sm text-gray-600 dark:text-gray-400 mt-1">
Manage Letzshop integration for all vendors
</p>
</div>
<button
@click="refreshData()"
:disabled="loading"
class="flex items-center px-4 py-2 text-sm font-medium leading-5 text-white transition-colors duration-150 bg-purple-600 border border-transparent rounded-lg hover:bg-purple-700 focus:outline-none focus:shadow-outline-purple disabled:opacity-50"
>
<span x-show="!loading" x-html="$icon('refresh', 'w-4 h-4 mr-2')"></span>
<span x-show="loading" x-html="$icon('spinner', 'w-4 h-4 mr-2')"></span>
<span x-text="loading ? 'Loading...' : 'Refresh'"></span>
</button>
</div>
<!-- Success Message --> <!-- Success Message -->
<div x-show="successMessage" x-transition class="mb-6 p-4 bg-green-100 dark:bg-green-900/30 border border-green-400 dark:border-green-600 text-green-700 dark:text-green-300 rounded-lg flex items-start"> {{ alert_dynamic(type='success', title='', message_var='successMessage', show_condition='successMessage') }}
<span x-html="$icon('check-circle', 'w-5 h-5 mr-3 mt-0.5 flex-shrink-0')"></span>
<div>
<p class="font-semibold" x-text="successMessage"></p>
</div>
<button @click="successMessage = ''" class="ml-auto">
<span x-html="$icon('x', 'w-4 h-4')"></span>
</button>
</div>
<!-- Error Message --> <!-- Error Message -->
<div x-show="error" x-transition class="mb-6 p-4 bg-red-100 dark:bg-red-900/30 border border-red-400 dark:border-red-600 text-red-700 dark:text-red-300 rounded-lg flex items-start"> {{ error_state(title='Error', error_var='error', show_condition='error && !loading') }}
<span x-html="$icon('exclamation', 'w-5 h-5 mr-3 mt-0.5 flex-shrink-0')"></span>
<div>
<p class="font-semibold">Error</p>
<p class="text-sm" x-text="error"></p>
</div>
<button @click="error = ''" class="ml-auto">
<span x-html="$icon('x', 'w-4 h-4')"></span>
</button>
</div>
<!-- Summary Cards --> <!-- Summary Cards -->
<div class="grid gap-6 mb-8 md:grid-cols-4"> <div class="grid gap-6 mb-8 md:grid-cols-4">
@@ -115,19 +87,8 @@
</div> </div>
<!-- Vendors Table --> <!-- Vendors Table -->
<div class="w-full overflow-hidden rounded-lg shadow-xs"> {% call table_wrapper() %}
<div class="w-full overflow-x-auto"> {{ table_header(['Vendor', 'Status', 'Auto-Sync', 'Last Sync', 'Orders', 'Actions']) }}
<table class="w-full whitespace-no-wrap">
<thead>
<tr class="text-xs font-semibold tracking-wide text-left text-gray-500 uppercase border-b dark:border-gray-700 bg-gray-50 dark:text-gray-400 dark:bg-gray-800">
<th class="px-4 py-3">Vendor</th>
<th class="px-4 py-3">Status</th>
<th class="px-4 py-3">Auto-Sync</th>
<th class="px-4 py-3">Last Sync</th>
<th class="px-4 py-3">Orders</th>
<th class="px-4 py-3">Actions</th>
</tr>
</thead>
<tbody class="bg-white divide-y dark:divide-gray-700 dark:bg-gray-800"> <tbody class="bg-white divide-y dark:divide-gray-700 dark:bg-gray-800">
<template x-if="loading && vendors.length === 0"> <template x-if="loading && vendors.length === 0">
<tr> <tr>
@@ -238,10 +199,10 @@
</tr> </tr>
</template> </template>
</tbody> </tbody>
</table> {% endcall %}
</div>
<!-- Pagination --> <!-- Pagination -->
<div x-show="totalVendors > limit" class="grid px-4 py-3 text-xs font-semibold tracking-wide text-gray-500 uppercase border-t dark:border-gray-700 bg-gray-50 sm:grid-cols-9 dark:text-gray-400 dark:bg-gray-800"> <div x-show="totalVendors > limit" class="mt-4 grid px-4 py-3 text-xs font-semibold tracking-wide text-gray-500 uppercase border dark:border-gray-700 rounded-lg bg-gray-50 sm:grid-cols-9 dark:text-gray-400 dark:bg-gray-800">
<span class="flex items-center col-span-3"> <span class="flex items-center col-span-3">
Showing <span x-text="((page - 1) * limit) + 1"></span>-<span x-text="Math.min(page * limit, totalVendors)"></span> of <span x-text="totalVendors"></span> Showing <span x-text="((page - 1) * limit) + 1"></span>-<span x-text="Math.min(page * limit, totalVendors)"></span> of <span x-text="totalVendors"></span>
</span> </span>
@@ -262,40 +223,13 @@
</ul> </ul>
</nav> </nav>
</span> </span>
</div>
</div> </div>
<!-- Configuration Modal --> <!-- Configuration Modal -->
<div {% call modal('configModal', 'Configure Letzshop', 'showConfigModal', size='md') %}
x-show="showConfigModal" <p class="text-sm text-gray-500 dark:text-gray-400 mb-4">
x-transition:enter="transition ease-out duration-150" Configuring: <span class="font-semibold" x-text="selectedVendor?.vendor_name"></span>
x-transition:enter-start="opacity-0" </p>
x-transition:enter-end="opacity-100"
x-transition:leave="transition ease-in duration-150"
x-transition:leave-start="opacity-100"
x-transition:leave-end="opacity-0"
class="fixed inset-0 z-30 flex items-end bg-black bg-opacity-50 sm:items-center sm:justify-center"
@click.self="showConfigModal = false"
>
<div
x-transition:enter="transition ease-out duration-150"
x-transition:enter-start="opacity-0 transform translate-y-1/2"
x-transition:enter-end="opacity-100"
x-transition:leave="transition ease-in duration-150"
x-transition:leave-start="opacity-100"
x-transition:leave-end="opacity-0 transform translate-y-1/2"
class="w-full px-6 py-4 overflow-hidden bg-white rounded-t-lg dark:bg-gray-800 sm:rounded-lg sm:m-4 sm:max-w-lg"
@click.stop
>
<header class="flex justify-between items-center mb-4">
<h3 class="text-lg font-semibold text-gray-700 dark:text-gray-200">
Configure Letzshop - <span x-text="selectedVendor?.vendor_name"></span>
</h3>
<button @click="showConfigModal = false" class="text-gray-400 hover:text-gray-600">
<span x-html="$icon('x', 'w-5 h-5')"></span>
</button>
</header>
<form @submit.prevent="saveVendorConfig()"> <form @submit.prevent="saveVendorConfig()">
<div class="space-y-4 mb-6"> <div class="space-y-4 mb-6">
<!-- API Key --> <!-- API Key -->
@@ -373,39 +307,13 @@
</div> </div>
</div> </div>
</form> </form>
</div> {% endcall %}
</div>
<!-- Orders Modal --> <!-- Orders Modal -->
<div {% call modal('ordersModal', 'Vendor Orders', 'showOrdersModal', size='xl') %}
x-show="showOrdersModal" <p class="text-sm text-gray-500 dark:text-gray-400 mb-4">
x-transition:enter="transition ease-out duration-150" Orders for: <span class="font-semibold" x-text="selectedVendor?.vendor_name"></span>
x-transition:enter-start="opacity-0" </p>
x-transition:enter-end="opacity-100"
x-transition:leave="transition ease-in duration-150"
x-transition:leave-start="opacity-100"
x-transition:leave-end="opacity-0"
class="fixed inset-0 z-30 flex items-end bg-black bg-opacity-50 sm:items-center sm:justify-center"
@click.self="showOrdersModal = false"
>
<div
x-transition:enter="transition ease-out duration-150"
x-transition:enter-start="opacity-0 transform translate-y-1/2"
x-transition:enter-end="opacity-100"
x-transition:leave="transition ease-in duration-150"
x-transition:leave-start="opacity-100"
x-transition:leave-end="opacity-0 transform translate-y-1/2"
class="w-full px-6 py-4 overflow-hidden bg-white rounded-t-lg dark:bg-gray-800 sm:rounded-lg sm:m-4 sm:max-w-4xl max-h-[80vh] overflow-y-auto"
@click.stop
>
<header class="flex justify-between items-center mb-4">
<h3 class="text-lg font-semibold text-gray-700 dark:text-gray-200">
Orders - <span x-text="selectedVendor?.vendor_name"></span>
</h3>
<button @click="showOrdersModal = false" class="text-gray-400 hover:text-gray-600">
<span x-html="$icon('x', 'w-5 h-5')"></span>
</button>
</header>
<div x-show="loadingOrders" class="py-8 text-center"> <div x-show="loadingOrders" class="py-8 text-center">
<span x-html="$icon('spinner', 'w-6 h-6 mx-auto')"></span> <span x-html="$icon('spinner', 'w-6 h-6 mx-auto')"></span>
@@ -447,6 +355,5 @@
</table> </table>
<p x-show="vendorOrders.length === 0" class="py-4 text-center text-gray-500">No orders found</p> <p x-show="vendorOrders.length === 0" class="py-4 text-center text-gray-500">No orders found</p>
</div> </div>
</div> {% endcall %}
</div>
{% endblock %} {% endblock %}

View File

@@ -3,10 +3,15 @@
* Admin Letzshop management page logic * Admin Letzshop management page logic
*/ */
console.log('[ADMIN LETZSHOP] Loading...'); // Use centralized logger (with fallback)
const letzshopLog = window.LogConfig?.createLogger?.('letzshop') ||
window.LogConfig?.loggers?.letzshop ||
{ info: () => {}, warn: () => {}, error: () => {}, debug: () => {} };
letzshopLog.info('Loading...');
function adminLetzshop() { function adminLetzshop() {
console.log('[ADMIN LETZSHOP] adminLetzshop() called'); letzshopLog.info('adminLetzshop() called');
return { return {
// Inherit base layout state // Inherit base layout state
@@ -65,7 +70,7 @@ function adminLetzshop() {
} }
window._adminLetzshopInitialized = true; window._adminLetzshopInitialized = true;
console.log('[ADMIN LETZSHOP] Initializing...'); letzshopLog.info('Initializing...');
await this.loadVendors(); await this.loadVendors();
}, },
@@ -93,9 +98,9 @@ function adminLetzshop() {
this.stats.autoSync = this.vendors.filter(v => v.auto_sync_enabled).length; this.stats.autoSync = this.vendors.filter(v => v.auto_sync_enabled).length;
this.stats.pendingOrders = this.vendors.reduce((sum, v) => sum + (v.pending_orders || 0), 0); this.stats.pendingOrders = this.vendors.reduce((sum, v) => sum + (v.pending_orders || 0), 0);
console.log('[ADMIN LETZSHOP] Loaded vendors:', this.vendors.length); letzshopLog.info('Loaded vendors:', this.vendors.length);
} catch (error) { } catch (error) {
console.error('[ADMIN LETZSHOP] Failed to load vendors:', error); letzshopLog.error('Failed to load vendors:', error);
this.error = error.message || 'Failed to load vendors'; this.error = error.message || 'Failed to load vendors';
} finally { } finally {
this.loading = false; this.loading = false;
@@ -134,7 +139,7 @@ function adminLetzshop() {
this.configForm.sync_interval_minutes = response.sync_interval_minutes || 15; this.configForm.sync_interval_minutes = response.sync_interval_minutes || 15;
} catch (error) { } catch (error) {
if (error.status !== 404) { if (error.status !== 404) {
console.error('[ADMIN LETZSHOP] Failed to load credentials:', error); letzshopLog.error('Failed to load credentials:', error);
} }
} }
} }
@@ -171,7 +176,7 @@ function adminLetzshop() {
this.successMessage = 'Configuration saved successfully'; this.successMessage = 'Configuration saved successfully';
await this.loadVendors(); await this.loadVendors();
} catch (error) { } catch (error) {
console.error('[ADMIN LETZSHOP] Failed to save config:', error); letzshopLog.error('Failed to save config:', error);
this.error = error.message || 'Failed to save configuration'; this.error = error.message || 'Failed to save configuration';
} finally { } finally {
this.savingConfig = false; this.savingConfig = false;
@@ -193,7 +198,7 @@ function adminLetzshop() {
this.successMessage = 'Configuration removed'; this.successMessage = 'Configuration removed';
await this.loadVendors(); await this.loadVendors();
} catch (error) { } catch (error) {
console.error('[ADMIN LETZSHOP] Failed to delete config:', error); letzshopLog.error('Failed to delete config:', error);
this.error = error.message || 'Failed to remove configuration'; this.error = error.message || 'Failed to remove configuration';
} }
setTimeout(() => this.successMessage = '', 5000); setTimeout(() => this.successMessage = '', 5000);
@@ -214,7 +219,7 @@ function adminLetzshop() {
this.error = response.error_details || 'Connection failed'; this.error = response.error_details || 'Connection failed';
} }
} catch (error) { } catch (error) {
console.error('[ADMIN LETZSHOP] Connection test failed:', error); letzshopLog.error('Connection test failed:', error);
this.error = error.message || 'Connection test failed'; this.error = error.message || 'Connection test failed';
} }
setTimeout(() => this.successMessage = '', 5000); setTimeout(() => this.successMessage = '', 5000);
@@ -236,7 +241,7 @@ function adminLetzshop() {
this.error = response.message || 'Sync failed'; this.error = response.message || 'Sync failed';
} }
} catch (error) { } catch (error) {
console.error('[ADMIN LETZSHOP] Sync failed:', error); letzshopLog.error('Sync failed:', error);
this.error = error.message || 'Sync failed'; this.error = error.message || 'Sync failed';
} }
setTimeout(() => this.successMessage = '', 5000); setTimeout(() => this.successMessage = '', 5000);
@@ -255,7 +260,7 @@ function adminLetzshop() {
const response = await apiClient.get(`/admin/letzshop/vendors/${vendor.vendor_id}/orders?limit=100`); const response = await apiClient.get(`/admin/letzshop/vendors/${vendor.vendor_id}/orders?limit=100`);
this.vendorOrders = response.orders || []; this.vendorOrders = response.orders || [];
} catch (error) { } catch (error) {
console.error('[ADMIN LETZSHOP] Failed to load orders:', error); letzshopLog.error('Failed to load orders:', error);
this.error = error.message || 'Failed to load orders'; this.error = error.message || 'Failed to load orders';
} finally { } finally {
this.loadingOrders = false; this.loadingOrders = false;
@@ -273,4 +278,4 @@ function adminLetzshop() {
}; };
} }
console.log('[ADMIN LETZSHOP] Module loaded'); letzshopLog.info('Module loaded');