539 lines
17 KiB
Markdown
539 lines
17 KiB
Markdown
# ANÁLISIS: Problema Crítico de Duplicación de Valores por Defecto
|
|
|
|
**Fecha:** 2025-01-13
|
|
**Severidad:** 🔴 ALTA - Problema de diseño arquitectónico
|
|
**Tipo:** Violación del principio DRY (Don't Repeat Yourself)
|
|
|
|
---
|
|
|
|
## 🔍 PROBLEMA IDENTIFICADO
|
|
|
|
El texto `"Accede a más de 200,000 Análisis de Precios Unitarios actualizados para 2025."` está **duplicado en 7 ubicaciones diferentes**, lo que genera:
|
|
|
|
- ❌ **Difícil mantenimiento** - Cambiar requiere editar 7 archivos
|
|
- ❌ **Alto riesgo de errores** - Fácil olvidar actualizar un archivo
|
|
- ❌ **Inconsistencias** - Valores pueden desincronizarse
|
|
- ❌ **No hay fuente única de verdad** - Múltiples definiciones de defaults
|
|
|
|
---
|
|
|
|
## 📍 UBICACIONES DE LA DUPLICACIÓN
|
|
|
|
### 1. **admin/assets/js/admin-app.js** (línea 357)
|
|
```javascript
|
|
document.getElementById('topBarMessageText').value = topBar.message_text || 'Accede a más de 200,000 Análisis de Precios Unitarios actualizados para 2025.';
|
|
```
|
|
**Propósito:** Fallback en JavaScript al renderizar el formulario
|
|
**Problema:** Duplica el default que ya está en PHP
|
|
|
|
---
|
|
|
|
### 2. **admin/includes/sanitizers/class-topbar-sanitizer.php** (línea 37)
|
|
```php
|
|
public function get_defaults() {
|
|
return array(
|
|
// ...
|
|
'message_text' => 'Accede a más de 200,000 Análisis de Precios Unitarios actualizados para 2025.',
|
|
// ...
|
|
);
|
|
}
|
|
```
|
|
**Propósito:** Define defaults del sanitizer
|
|
**Problema:** ¿Por qué el sanitizer define defaults? Debería solo sanitizar.
|
|
|
|
---
|
|
|
|
### 3. **admin/includes/class-settings-manager.php** (línea 84)
|
|
```php
|
|
public function get_defaults() {
|
|
return array(
|
|
// ...
|
|
'components' => array(
|
|
'top_bar' => array(
|
|
'message_text' => 'Accede a más de 200,000 Análisis de Precios Unitarios actualizados para 2025.',
|
|
// ...
|
|
)
|
|
)
|
|
);
|
|
}
|
|
```
|
|
**Propósito:** Define defaults centralizados del Settings Manager
|
|
**Problema:** ⚠️ **DUPLICA lo que ya tiene el Sanitizer**
|
|
|
|
---
|
|
|
|
### 4. **admin/pages/main.php** (líneas 243-244, 495)
|
|
|
|
**Línea 243-244:**
|
|
```html
|
|
<textarea id="topBarMessageText"
|
|
placeholder="Ej: Accede a más de 200,000 Análisis de Precios Unitarios actualizados para 2025."
|
|
required>Accede a más de 200,000 Análisis de Precios Unitarios actualizados para 2025.</textarea>
|
|
```
|
|
|
|
**Línea 495 (preview):**
|
|
```html
|
|
<span>Accede a más de 200,000 Análisis de Precios Unitarios actualizados para 2025.</span>
|
|
```
|
|
|
|
**Propósito:**
|
|
- Placeholder del textarea
|
|
- Valor inicial del textarea
|
|
- Texto de preview
|
|
|
|
**Problema:** ❌ **TRIPLE duplicación en un solo archivo**
|
|
|
|
---
|
|
|
|
### 5. **admin/components/component-top-bar.php** (línea 190, aparece 2 veces)
|
|
```html
|
|
<textarea id="topBarMessageText"
|
|
placeholder="Ej: Accede a más de 200,000 Análisis de Precios Unitarios actualizados para 2025."
|
|
required="">Accede a más de 200,000 Análisis de Precios Unitarios actualizados para 2025.</textarea>
|
|
```
|
|
**Propósito:** Similar a main.php (placeholder + valor)
|
|
**Problema:** ¿Por qué existe este archivo si main.php ya tiene el formulario?
|
|
|
|
---
|
|
|
|
### 6. **header.php** (línea 34)
|
|
```php
|
|
'message_text' => 'Accede a más de 200,000 Análisis de Precios Unitarios actualizados para 2025.',
|
|
```
|
|
**Propósito:** Fallback en el front-end del tema
|
|
**Problema:** El front-end NO debería definir defaults, debería leerlos del Settings Manager
|
|
|
|
---
|
|
|
|
## 🏗️ ANÁLISIS ARQUITECTÓNICO
|
|
|
|
### Arquitectura ACTUAL (Problemática)
|
|
|
|
```
|
|
┌─────────────────────────────────────────────────────────────────┐
|
|
│ CAPA 1: DEFAULTS DUPLICADOS (7 lugares) │
|
|
├─────────────────────────────────────────────────────────────────┤
|
|
│ ❌ TopBar Sanitizer::get_defaults() │
|
|
│ ❌ Settings Manager::get_defaults() │
|
|
│ ❌ admin-app.js (fallbacks en render) │
|
|
│ ❌ main.php (placeholder + valor inicial + preview) │
|
|
│ ❌ component-top-bar.php (placeholder + valor) │
|
|
│ ❌ header.php (fallback front-end) │
|
|
└─────────────────────────────────────────────────────────────────┘
|
|
↓
|
|
🔴 PROBLEMA: No hay fuente única de verdad
|
|
```
|
|
|
|
### Arquitectura CORRECTA (Propuesta)
|
|
|
|
```
|
|
┌─────────────────────────────────────────────────────────────────┐
|
|
│ ÚNICA FUENTE DE VERDAD │
|
|
├─────────────────────────────────────────────────────────────────┤
|
|
│ ✅ Settings Manager::get_defaults() SOLAMENTE │
|
|
│ - Define TODOS los defaults de TODOS los componentes │
|
|
│ - Usa constantes PHP para valores reutilizables │
|
|
└─────────────────────────────────────────────────────────────────┘
|
|
↓
|
|
┌─────────────────────────────────────────────────────────────────┐
|
|
│ CONSUMIDORES (leen de Settings Manager) │
|
|
├─────────────────────────────────────────────────────────────────┤
|
|
│ ✅ TopBar Sanitizer → Llama Settings Manager::get_defaults() │
|
|
│ ✅ admin-app.js → AJAX lee settings (ya con defaults merged) │
|
|
│ ✅ main.php → Usa PHP para obtener defaults dinámicamente │
|
|
│ ✅ header.php → Lee Settings Manager (NO define defaults) │
|
|
└─────────────────────────────────────────────────────────────────┘
|
|
```
|
|
|
|
---
|
|
|
|
## 🔬 RAZONES DE LA DUPLICACIÓN
|
|
|
|
### 1. **Sanitizer vs Settings Manager** (Confusión de responsabilidades)
|
|
|
|
**Problema:**
|
|
- `APUS_TopBar_Sanitizer::get_defaults()` define defaults
|
|
- `APUS_Settings_Manager::get_defaults()` TAMBIÉN define defaults
|
|
|
|
**Pregunta:** ¿Por qué el SANITIZER define defaults?
|
|
|
|
**Responsabilidades correctas:**
|
|
- ✅ **Sanitizer:** Solo SANITIZAR datos (validar, limpiar)
|
|
- ✅ **Settings Manager:** Definir defaults, leer DB, hacer merge
|
|
|
|
**Solución:**
|
|
- Eliminar `get_defaults()` del Sanitizer
|
|
- Mantener solo en Settings Manager
|
|
|
|
---
|
|
|
|
### 2. **JavaScript con Fallbacks Hardcodeados**
|
|
|
|
**Código actual (admin-app.js:357):**
|
|
```javascript
|
|
topBar.message_text || 'Accede a más de 200,000...'
|
|
```
|
|
|
|
**Problema:** JavaScript NO debería tener defaults hardcodeados.
|
|
|
|
**Solución:**
|
|
Cuando JavaScript llama a AJAX para cargar settings, el Settings Manager YA hace merge con defaults:
|
|
|
|
```php
|
|
// Settings Manager ya retorna datos con defaults merged
|
|
public function get_settings() {
|
|
$db_data = $this->db_manager->get_all_settings();
|
|
$defaults = $this->get_defaults();
|
|
return wp_parse_args($db_data, $defaults); // ← Merge automático
|
|
}
|
|
```
|
|
|
|
Por lo tanto, JavaScript NUNCA recibirá un `message_text` vacío. El fallback `|| 'Accede...'` es **innecesario**.
|
|
|
|
**Corrección:**
|
|
```javascript
|
|
// ANTES:
|
|
document.getElementById('topBarMessageText').value = topBar.message_text || 'Accede...';
|
|
|
|
// DESPUÉS:
|
|
document.getElementById('topBarMessageText').value = topBar.message_text;
|
|
// ↑ Settings Manager YA hizo merge con defaults
|
|
```
|
|
|
|
---
|
|
|
|
### 3. **HTML con Valores Hardcodeados** (main.php, component-top-bar.php)
|
|
|
|
**Código actual:**
|
|
```html
|
|
<textarea placeholder="Ej: Accede...">Accede...</textarea>
|
|
```
|
|
|
|
**Problemas:**
|
|
1. Placeholder hardcodeado
|
|
2. Valor inicial hardcodeado
|
|
3. Preview hardcodeado
|
|
|
|
**Solución:** Usar PHP para obtener defaults dinámicamente
|
|
|
|
```php
|
|
<?php
|
|
$settings_manager = new APUS_Settings_Manager();
|
|
$defaults = $settings_manager->get_defaults();
|
|
$default_message = $defaults['components']['top_bar']['message_text'];
|
|
?>
|
|
|
|
<textarea
|
|
id="topBarMessageText"
|
|
placeholder="Ej: <?php echo esc_attr($default_message); ?>"
|
|
><?php echo esc_html($default_message); ?></textarea>
|
|
```
|
|
|
|
**Preview:**
|
|
```html
|
|
<span id="topBarPreview"><?php echo esc_html($default_message); ?></span>
|
|
```
|
|
|
|
---
|
|
|
|
### 4. **component-top-bar.php vs main.php** (¿Duplicación de archivos?)
|
|
|
|
**Observación:**
|
|
- `admin/pages/main.php` contiene el formulario del Top Bar
|
|
- `admin/components/component-top-bar.php` TAMBIÉN contiene el formulario del Top Bar
|
|
|
|
**Pregunta:** ¿Por qué existen 2 archivos con el mismo formulario?
|
|
|
|
**Hipótesis:**
|
|
1. **component-top-bar.php** es un archivo PHP modular (componente)
|
|
2. **main.php** debería INCLUIR el componente, no duplicar el código
|
|
|
|
**Solución propuesta:**
|
|
```php
|
|
// main.php - Debería ser así:
|
|
<div id="topBarTab" class="tab-pane fade show active">
|
|
<?php require_once APUS_ADMIN_PANEL_PATH . 'components/component-top-bar.php'; ?>
|
|
</div>
|
|
```
|
|
|
|
Pero si component-top-bar.php es solo HTML sin lógica, entonces:
|
|
- Opción 1: Eliminar component-top-bar.php (usar solo main.php)
|
|
- Opción 2: Convertir component-top-bar.php en plantilla reutilizable
|
|
|
|
---
|
|
|
|
### 5. **header.php con Fallback** (Front-end no debería definir defaults)
|
|
|
|
**Código actual (header.php:34):**
|
|
```php
|
|
$top_bar_config = wp_parse_args($config, array(
|
|
'message_text' => 'Accede a más de 200,000...',
|
|
// ...
|
|
));
|
|
```
|
|
|
|
**Problema:** El front-end NO debería definir defaults.
|
|
|
|
**¿Por qué está esto aquí?**
|
|
Probablemente por si Settings Manager falla o no retorna datos.
|
|
|
|
**Solución correcta:**
|
|
```php
|
|
// ANTES:
|
|
$settings_manager = new APUS_Settings_Manager();
|
|
$settings = $settings_manager->get_settings();
|
|
$config = isset($settings['components']['top_bar']) ? $settings['components']['top_bar'] : array();
|
|
$top_bar_config = wp_parse_args($config, array( /* defaults hardcodeados */ ));
|
|
|
|
// DESPUÉS:
|
|
$settings_manager = new APUS_Settings_Manager();
|
|
$settings = $settings_manager->get_settings(); // ← Ya incluye defaults merged
|
|
$top_bar_config = $settings['components']['top_bar']; // ← Sin fallback necesario
|
|
```
|
|
|
|
**Razón:** `get_settings()` del Settings Manager YA hace merge con defaults.
|
|
|
|
---
|
|
|
|
## 💡 SOLUCIÓN PROPUESTA
|
|
|
|
### PASO 1: Única Fuente de Verdad (Settings Manager)
|
|
|
|
**Crear constantes para valores reutilizables:**
|
|
|
|
```php
|
|
// class-settings-manager.php
|
|
|
|
class APUS_Settings_Manager {
|
|
|
|
// Constantes de defaults
|
|
const DEFAULT_TOPBAR_MESSAGE = 'Accede a más de 200,000 Análisis de Precios Unitarios actualizados para 2025.';
|
|
const DEFAULT_TOPBAR_HIGHLIGHT = 'Nuevo:';
|
|
const DEFAULT_TOPBAR_LINK_TEXT = 'Ver Catálogo';
|
|
// ...
|
|
|
|
public function get_defaults() {
|
|
return array(
|
|
'version' => APUS_ADMIN_PANEL_VERSION,
|
|
'components' => array(
|
|
'top_bar' => array(
|
|
'enabled' => true,
|
|
'message_text' => self::DEFAULT_TOPBAR_MESSAGE,
|
|
'highlight_text' => self::DEFAULT_TOPBAR_HIGHLIGHT,
|
|
'link_text' => self::DEFAULT_TOPBAR_LINK_TEXT,
|
|
// ...
|
|
)
|
|
)
|
|
);
|
|
}
|
|
}
|
|
```
|
|
|
|
**Ventajas:**
|
|
- ✅ Constantes documentadas en un solo lugar
|
|
- ✅ Fácil de cambiar (1 línea en vez de 7 archivos)
|
|
- ✅ PHP autocomplete para IDEs
|
|
|
|
---
|
|
|
|
### PASO 2: Eliminar Duplicaciones
|
|
|
|
#### 2.1. Sanitizer NO debe tener `get_defaults()`
|
|
|
|
```php
|
|
// class-topbar-sanitizer.php
|
|
|
|
class APUS_TopBar_Sanitizer {
|
|
|
|
// ❌ ELIMINAR:
|
|
// public function get_defaults() { ... }
|
|
|
|
// ✅ MANTENER SOLO:
|
|
public function sanitize($data) {
|
|
// Lógica de sanitización
|
|
}
|
|
}
|
|
```
|
|
|
|
**Si el Sanitizer necesita defaults para validación:**
|
|
```php
|
|
class APUS_TopBar_Sanitizer {
|
|
|
|
private $settings_manager;
|
|
|
|
public function __construct() {
|
|
$this->settings_manager = new APUS_Settings_Manager();
|
|
}
|
|
|
|
public function sanitize($data) {
|
|
$defaults = $this->settings_manager->get_defaults()['components']['top_bar'];
|
|
// Usar $defaults si es necesario para validación
|
|
}
|
|
}
|
|
```
|
|
|
|
---
|
|
|
|
#### 2.2. JavaScript SIN Fallbacks Hardcodeados
|
|
|
|
```javascript
|
|
// admin-app.js
|
|
|
|
renderTopBar(topBar) {
|
|
// ANTES:
|
|
// document.getElementById('topBarMessageText').value = topBar.message_text || 'Accede...';
|
|
|
|
// DESPUÉS:
|
|
document.getElementById('topBarMessageText').value = topBar.message_text;
|
|
document.getElementById('topBarHighlightText').value = topBar.highlight_text;
|
|
document.getElementById('topBarLinkText').value = topBar.link_text;
|
|
// ↑ Settings Manager YA hizo merge con defaults
|
|
}
|
|
```
|
|
|
|
**Razón:** AJAX obtiene settings de `get_settings()` que ya incluye defaults.
|
|
|
|
---
|
|
|
|
#### 2.3. HTML Dinámico (usar PHP)
|
|
|
|
```php
|
|
<!-- main.php -->
|
|
|
|
<?php
|
|
$settings_manager = new APUS_Settings_Manager();
|
|
$defaults = $settings_manager->get_defaults()['components']['top_bar'];
|
|
?>
|
|
|
|
<!-- Mensaje de texto -->
|
|
<div class="mb-3">
|
|
<label class="form-label">
|
|
<i class="bi bi-chat-text me-2"></i>Mensaje Principal
|
|
</label>
|
|
<textarea
|
|
id="topBarMessageText"
|
|
class="form-control"
|
|
rows="2"
|
|
maxlength="250"
|
|
placeholder="Ej: <?php echo esc_attr($defaults['message_text']); ?>"
|
|
><?php echo esc_html($defaults['message_text']); ?></textarea>
|
|
</div>
|
|
|
|
<!-- Preview -->
|
|
<div id="topBarPreview" class="preview-top-bar">
|
|
<span><?php echo esc_html($defaults['message_text']); ?></span>
|
|
</div>
|
|
```
|
|
|
|
---
|
|
|
|
#### 2.4. Front-end SIN Fallbacks
|
|
|
|
```php
|
|
// header.php
|
|
|
|
<?php
|
|
$settings_manager = new APUS_Settings_Manager();
|
|
$settings = $settings_manager->get_settings(); // ← Ya incluye defaults merged
|
|
$top_bar_config = $settings['components']['top_bar'];
|
|
|
|
// NO hacer wp_parse_args con defaults hardcodeados
|
|
// ❌ $top_bar_config = wp_parse_args($config, array('message_text' => '...'));
|
|
?>
|
|
|
|
<!-- Renderizar Top Bar -->
|
|
<?php if ($top_bar_config['enabled']): ?>
|
|
<div class="top-notification-bar">
|
|
<span><?php echo esc_html($top_bar_config['message_text']); ?></span>
|
|
</div>
|
|
<?php endif; ?>
|
|
```
|
|
|
|
---
|
|
|
|
#### 2.5. Eliminar component-top-bar.php (¿Duplicado?)
|
|
|
|
**Investigar:**
|
|
1. ¿Se usa `component-top-bar.php` en algún lugar?
|
|
2. Si NO se usa, eliminarlo
|
|
3. Si SÍ se usa, refactorizar para que main.php lo incluya
|
|
|
|
---
|
|
|
|
## 📊 RESUMEN DE CAMBIOS
|
|
|
|
| Archivo | Acción | Razón |
|
|
|---------|--------|-------|
|
|
| `class-settings-manager.php` | ✅ **Usar constantes para defaults** | Única fuente de verdad |
|
|
| `class-topbar-sanitizer.php` | ❌ **Eliminar `get_defaults()`** | Sanitizer no debe definir defaults |
|
|
| `admin-app.js` | ❌ **Eliminar fallbacks hardcodeados** | AJAX ya retorna defaults merged |
|
|
| `main.php` | ✏️ **Usar PHP dinámico para defaults** | Leer de Settings Manager |
|
|
| `component-top-bar.php` | 🔍 **Investigar si es duplicado** | Posible eliminación |
|
|
| `header.php` | ❌ **Eliminar fallbacks hardcodeados** | get_settings() ya incluye defaults |
|
|
|
|
---
|
|
|
|
## 🎯 BENEFICIOS DE LA SOLUCIÓN
|
|
|
|
### Antes (Actual)
|
|
```
|
|
Cambiar "Accede a más de 200,000..." requiere:
|
|
├── ✏️ Editar admin-app.js
|
|
├── ✏️ Editar class-topbar-sanitizer.php
|
|
├── ✏️ Editar class-settings-manager.php
|
|
├── ✏️ Editar main.php (3 lugares)
|
|
├── ✏️ Editar component-top-bar.php (2 lugares)
|
|
└── ✏️ Editar header.php
|
|
|
|
Total: 7 archivos, ~10 líneas a cambiar
|
|
Riesgo: 🔴 ALTO (fácil olvidar un archivo)
|
|
```
|
|
|
|
### Después (Propuesto)
|
|
```
|
|
Cambiar "Accede a más de 200,000..." requiere:
|
|
└── ✏️ Editar class-settings-manager.php (1 constante)
|
|
|
|
Total: 1 archivo, 1 línea
|
|
Riesgo: 🟢 BAJO (cambio centralizado)
|
|
```
|
|
|
|
---
|
|
|
|
## 🚨 IMPACTO EN OTROS COMPONENTES
|
|
|
|
**⚠️ IMPORTANTE:** Este problema probablemente se repite en los otros 3 componentes:
|
|
|
|
1. **Navbar** - ¿Tiene duplicación similar?
|
|
2. **Let's Talk Button** - ¿Tiene duplicación similar?
|
|
3. **Hero Section** - ¿Tiene duplicación similar?
|
|
|
|
**Recomendación:** Aplicar la misma refactorización a TODOS los componentes.
|
|
|
|
---
|
|
|
|
## ✅ CHECKLIST DE IMPLEMENTACIÓN
|
|
|
|
- [ ] Crear constantes en Settings Manager
|
|
- [ ] Eliminar `get_defaults()` de TopBar Sanitizer
|
|
- [ ] Eliminar fallbacks de admin-app.js
|
|
- [ ] Convertir HTML de main.php a dinámico
|
|
- [ ] Investigar si component-top-bar.php es necesario
|
|
- [ ] Eliminar fallbacks de header.php
|
|
- [ ] Verificar que NO hay regresiones
|
|
- [ ] Aplicar solución a Navbar
|
|
- [ ] Aplicar solución a Let's Talk Button
|
|
- [ ] Aplicar solución a Hero Section
|
|
|
|
---
|
|
|
|
## 🔗 REFERENCIAS
|
|
|
|
- **Principio DRY:** Don't Repeat Yourself
|
|
- **Single Source of Truth:** Una única fuente de verdad para datos
|
|
- **Separation of Concerns:** Cada clase tiene una responsabilidad clara
|
|
|
|
---
|
|
|
|
**Última actualización:** 2025-01-13
|