Definition of Ready y Definition of Done - Los guardianes de la calidad en desarrollo ágil

Ticket: Arreglar rangos de edad

PLAT-2768 | Corregir rangos de edad superpuestos en pricing de turnos

**Contexto:**
Los grupos de edad 18-65 y 65-100 se superponen. Un usuario de 65 años
aparece en ambos grupos, causando precios duplicados en facturación.

**Impacto:**
- 150 usuarios afectados en último mes
- Descuadre en reportes financieros
- Confusión en clientes (+10 tickets de soporte)

**Módulo:** Pricing / Reservations

**Criterios de Aceptación:**

Happy Path:
✅ El rango 18-65 debe cambiar a 18-64
✅ El rango 65-100 permanece sin cambios
✅ Un usuario de 64 años debe tener precio del grupo 18-64
✅ Un usuario de 65 años debe tener SOLO precio del grupo 65-100

Edge Cases:
✅ Usuario sin edad definida → usar precio default
✅ Edad = 0 o negativa → error de validación
✅ Edad > 150 → error de validación

Validaciones:
✅ No puede haber rangos superpuestos
✅ Rangos deben cubrir todas las edades posibles (1-100+)
✅ Precio debe ser mayor a 0

**Alcance Técnico:**

Backend (Ruby on Rails):
- Models: PricingRule, ShiftPrice, AgeGroup
- Validations: AgeGroupValidator (actualizar lógica de rangos)
- Services: PricingCalculator (recalcular para edades límite)
- Migrations: UpdateOverlappingAgeRanges

APIs afectadas:
- GraphQL: shiftPricing query
- REST: /api/v1/pricing/calculate (deprecado pero aún en uso)

Frontend:
- React components: PricingForm, AgeRangeSelector
- Redux: pricing slice

Migraciones:
- Actualizar ~500 registros de pricing_rules existentes
- Recalcular precios históricos afectados (opcional)

Packs/Packages:
- Core app (no packs externos)

**Multi-tenancy:**
- Scope: Por facility (cada instalación tiene sus rangos)
- Timezone: No aplica (edad no depende de timezone)
- Datos existentes: 45 facilities con rangos a actualizar
- Configuración: Migración automática + opción manual post-deploy

**Backward Compatibility:**
- Mobile app v2.3+ soporta nuevos rangos
- Mobile app < v2.3 seguirá usando API legacy (sin cambios)

**Seguridad:**
- Datos sensibles: No (solo rangos de edad, no PII)
- Autorización: Solo admins pueden editar pricing rules (CanCanCan :manage, PricingRule)
- Auditoría: Loggear cambios en pricing_rules_audit_log
- Compliance: No aplica PCI/GDPR

**Permisos requeridos:**
- Desarrollo: :facility_admin o :super_admin
- QA: :facility_admin en staging

**Testing Strategy:**

Unit Tests:
- AgeGroupValidator#validate_no_overlaps
- PricingCalculator#price_for_age(64) → grupo 18-64
- PricingCalculator#price_for_age(65) → grupo 65-100

Integration Tests:
- Crear reserva con usuario edad 64 → precio correcto
- Crear reserva con usuario edad 65 → precio correcto
- Actualizar pricing rule → validaciones funcionan

System Tests (Playwright/Cypress):
- Admin crea nuevo pricing rule con rangos válidos
- Admin intenta crear rangos superpuestos → error visible

Testing Manual:
- Verificar migración en staging antes de producción
- Probar con 3-5 facilities reales
- Validar cálculos históricos no afectados

**Dependencias:**
- PLAT-2750: Refactor de PricingCalculator (debe estar en develop)
- Staging con datos recientes (snapshot de prod < 7 días)

**Blockers:**
- Ninguno identificado

**Riesgos:**
- Migración podría tomar >10min en prod (500k registros)
  Mitigación: Ejecutar en horario de bajo tráfico

**Checklist:**
✅ Funcionalidad completa según criterios de aceptación
✅ Código sigue convenciones del proyecto (RuboCop, ESLint, Prettier)
✅ No hay código comentado sin justificación
✅ No hay console.log o binding.pry olvidados
✅ Variables tienen nombres descriptivos
✅ Métodos tienen responsabilidad única (SRP)

**Validación:**
bundle exec rubocop -A                    # Ruby
npm run lint                              # JavaScript
bundle exec rails runner 'system("packwerk check")'  # Pack boundaries

**Unit Tests:**
✅ Toda lógica de negocio tiene tests
✅ Edge cases cubiertos
✅ Mocks/stubs usados apropiadamente
✅ Coverage no disminuye (mínimo mantener %)

**Integration Tests:**
✅ Flujos completos end-to-end
✅ Integraciones con servicios externos (Stripe, AWS, etc.)
✅ Background jobs probados

**System Tests (si aplica):**
✅ UX crítica probada (checkout, login, pagos)
✅ Tests no son flaky (usar Timecop, stubs consistentes)

**Validación:**
bundle exec rspec                         # Todos los tests
SIMPLECOV_REPORT=true bundle exec rspec   # Con coverage
bin/test_system system_specs/pricing/     # System tests

✅ Usar Time.current en lugar de Time.now
✅ Usar Time.zone.parse en lugar de Time.parse
✅ Respetar timezone de facility/user según contexto
✅ Tests con tiempo usan Timecop.freeze para evitar flakiness

**Ejemplo correcto:**
# ❌ Mal
created_at = Time.now
due_date = Date.today + 7.days

# ✅ Bien
created_at = Time.current
due_date = 7.days.from_now

# ✅ Mejor (con timezone específico)
created_at = Time.current.in_time_zone(facility.timezone)

✅ Usar transacciones de DB para operaciones financieras
✅ Jobs de pago son idempotentes
✅ Validar montos y precisión (cents vs dollars)
✅ Manejar correctamente gateway selection
✅ Loggear TODAS las operaciones financieras
✅ Nunca hacer cálculos con floats (usar Money gem o cents en integers)

**Ejemplo de idempotencia:**
def process_payment(payment_id)
  payment = Payment.find(payment_id)

  return if payment.processed? # Idempotencia

  Payment.transaction do
    result = gateway.charge(payment.amount_cents)
    payment.update!(
      status: 'processed',
      gateway_transaction_id: result.id
    )
    PaymentAuditLog.create!(payment: payment, action: 'processed')
  end
rescue Stripe::CardError => e
  payment.update!(status: 'failed', error: e.message)
  raise
end

✅ Jobs reciben argumentos como hash con symbolized keys
✅ Validar args.is_a?(Hash) antes de procesar
✅ Variables inicializadas antes de rescue blocks
✅ Jobs son idempotentes (pueden ejecutarse múltiples veces)
✅ Retry logic configurado apropiadamente
✅ Dead letter queue para failures críticos

**Ejemplo robusto:**
class ProcessRefundJob < ApplicationJob
  queue_as :critical

  retry_on Stripe::APIConnectionError, wait: :exponentially_longer, attempts: 5
  discard_on Stripe::InvalidRequestError

  def perform(args)
    raise ArgumentError, 'Hash expected' unless args.is_a?(Hash)

    args = args.deep_symbolize_keys
    payment_id = args.fetch(:payment_id)
    reason = args.fetch(:reason, 'customer_request')

    payment = Payment.find(payment_id)

    return if payment.refunded? # Idempotencia

    # ... lógica de refund
  rescue => e
    ErrorTracker.notify(e, payment_id: payment_id)
    raise
  end
end

✅ Endpoints tienen autorización apropiada (CanCanCan, Pundit)
✅ No se exponen IDs de DB en APIs (usar UUIDs o secure_ids)
✅ Datos sensibles están encriptados (attr_encrypted)
✅ SQL injection prevenido (usar placeholders, no string interpolation)
✅ XSS prevenido (sanitize inputs)
✅ CSRF tokens en formularios

**Ejemplo de autorización correcta:**
# app/controllers/api/v1/pricing_rules_controller.rb
class Api::V1::PricingRulesController < ApiController
  load_and_authorize_resource # CanCanCan

  def update
    # authorize! ya fue ejecutado por load_and_authorize_resource

    if @pricing_rule.update(pricing_rule_params)
      render json: @pricing_rule
    else
      render json: { errors: @pricing_rule.errors }, status: :unprocessable_entity
    end
  end

  private

  def pricing_rule_params
    params.require(:pricing_rule).permit(:age_min, :age_max, :price_cents)
  end
end

# app/models/ability.rb
class Ability
  include CanCan::Ability

  def initialize(user)
    return unless user.present?

    if user.admin?
      can :manage, PricingRule
    elsif user.facility_manager?
      can :manage, PricingRule, facility_id: user.facility_id
    else
      can :read, PricingRule
    end
  end
end

✅ RuboCop sin warnings
✅ Brakeman sin vulnerabilidades nuevas
✅ Tests en CI pasando (no solo local)
✅ Coverage no disminuyó
✅ Build artifacts generados correctamente
✅ Docker image builds (si aplica)

**Validación local antes de push:**
bundle exec rails ci                    # Ejecuta todos los checks
bundle exec brakeman -z                 # Security scan
bundle exec pronto run -c develop       # Code review automatizado

✅ Migraciones son reversibles (método down implementado)
✅ Migraciones de datos si es necesario (backfill)
✅ Probadas en staging antes de prod
✅ Indexes apropiados para queries nuevas
✅ Compatible con zero-downtime deployment
✅ Considera volumen de datos en producción

**Ejemplo de migración reversible:**
class UpdateAgeRangesBounds < ActiveRecord::Migration[6.1]
  def up
    # Actualizar rangos 18-65 a 18-64
    PricingRule.where(age_min: 18, age_max: 65).find_each do |rule|
      rule.update_columns(age_max: 64, updated_at: Time.current)
    end
  end

  def down
    # Revertir cambios
    PricingRule.where(age_min: 18, age_max: 64).find_each do |rule|
      rule.update_columns(age_max: 65, updated_at: Time.current)
    end
  end
end

✅ Comentarios en código complejo explicando el "por qué"
✅ Métodos públicos documentados (YARD, JSDoc)
✅ GraphQL schema tiene descriptions
✅ README actualizado si hay nuevos setup steps
✅ Variables de entorno documentadas (.env.example)
✅ API docs actualizados (Swagger/OpenAPI)
✅ Cambios arquitectónicos documentados en /docs

**Ejemplo de documentación clara:**
# app/services/pricing_calculator.rb

# Calcula el precio de una reserva considerando:
# - Edad del usuario (aplica descuentos por grupo etario)
# - Horario del turno (peak vs off-peak)
# - Membresía activa (descuentos adicionales)
# - Promociones vigentes
#
# @param user [User] Usuario que hace la reserva
# @param shift [Shift] Turno a reservar
# @return [Money] Precio final en cents
# @raise [PricingError] Si no se puede calcular precio
#
# @example
#   calculator = PricingCalculator.new(user, shift)
#   price = calculator.calculate
#   # => #<Money cents: 1500 currency: USD>
def calculate
  # ...
end

✅ PR tiene descripción clara con contexto
✅ Screenshots para cambios de UI
✅ Al menos 1 reviewer aprobó (2 para features críticos)
✅ Todos los comentarios resueltos
✅ No hay merge conflicts
✅ Branch está actualizado con develop/main

**Template de PR bien hecho:**

## 🎯 Objetivo
PLAT-2768: Corregir rangos de edad superpuestos en pricing

## 📝 Cambios
- Actualizar AgeGroupValidator para prevenir overlaps
- Cambiar rango 18-65 a 18-64 en migración
- Agregar tests para edge cases

## 🧪 Testing
- ✅ Unit tests: 12 nuevos, todos pasan
- ✅ Integration tests: Flujo completo de pricing
- ✅ Manual: Probado en staging con 5 facilities

## 📸 Screenshots
[imagen del form con validación de overlaps]

## ⚠️ Consideraciones
- Migración toma ~5min en prod (estimado)
- Ejecutar en horario de bajo tráfico
- Rollback disponible vía down migration

✅ No hay N+1 queries (usar includes/preload)
✅ Queries complejas optimizadas
✅ Caching donde es apropiado (Redis, Rails.cache)
✅ Background jobs para operaciones pesadas (>5 segundos)
✅ Pagination en listados grandes

**Ejemplo de prevención N+1:**
# ❌ Mal (N+1)
facilities.each do |facility|
  puts facility.owner.name  # Query por cada facility
end

# ✅ Bien
facilities.includes(:owner).each do |facility|
  puts facility.owner.name  # Una sola query
end

**Validación en Rails console:**
ActiveRecord::Base.logger = Logger.new(STDOUT)
# Ejecutar código y contar queries

1. Product/PM crea ticket
   ↓
2. Equipo revisa contra DoR en refinement
   ├─ ✅ Cumple DoR → Pasa a backlog "Ready"
   └─ ❌ No cumple → Regresa a PM para completar
   ↓
3. Sprint Planning: Desarrollador toma ticket de backlog "Ready"
   ↓
4. Desarrollo e implementación
   ↓
5. Desarrollador auto-revisa DoD antes de crear PR
   ↓
6. Code review verifica DoD
   ├─ ✅ Cumple DoD → Aprobado para merge
   └─ ❌ No cumple → Request changes
   ↓
7. Merge a develop → Auto-deploy a staging
   ↓
8. QA verifica en staging
   ↓
9. Deploy a producción
   ↓
10. Monitoring post-deploy (24-48h)

✅ Contexto: Rangos 18-65 y 65-100 causan precios duplicados
✅ Impacto: 150 usuarios/mes, descuadre en reportes
✅ Criterios aceptación: Cambiar a 18-64, validar no-overlap
✅ Alcance técnico:
   - Backend: PricingRule model, AgeGroupValidator
   - Migración: UpdateAgeRangeBounds
   - Tests: Unit + integration
✅ Multi-tenancy: Por facility, 45 facilities afectadas
✅ Seguridad: Solo admins, auditoría en logs
✅ Testing plan: Definido (ver arriba)
✅ Documentación: Actualizar validator docs
✅ Dependencias: Ninguna
✅ Estimación: 5 story points (1-2 días)

✅ Código: Validaciones implementadas, migración creada
✅ Tests: 15 specs nuevos, coverage +2%, todos pasan
✅ RuboCop: 0 warnings
✅ Brakeman: 0 issues nuevos
✅ Migraciones: Reversible, probada en staging
✅ Performance: Sin queries adicionales, usa índices existentes
✅ Documentación: Comentarios en validator, migración documentada
✅ PR: Descripción completa, screenshots de admin UI
✅ CI: Todos los checks en verde
✅ QA: Probado en staging con 5 facilities reales