itca_api_sap_wms/docs/PORTE_FUNCIONALIDAD_NUEVA.md

# Porte de funcionalidad nueva desde la reescritura Python

Este documento (y los dos que lo acompañan, [`SOLICITUD_TRANSFERENCIA.md`](./SOLICITUD_TRANSFERENCIA.md)
y [`PICKING.md`](./PICKING.md)) son la guía para agregar a **este proyecto
`.NET Core 3.1`** la funcionalidad de negocio nueva que se diseñó e
implementó primero en la reescritura Python
(`/Users/george/Documents/Desarrollo/ITCG/itca_api_sap_wms_python`), porque
este `.NET` va a seguir corriendo en producción (IIS, Windows) más tiempo
del previsto originalmente.

**La lógica de negocio ya está validada en Python — acá es 100% traducción
de lenguaje, no rediseño.** El contrato HTTP (rutas, shapes de request/response,
status codes) debe quedar idéntico al de la API Python, porque ambas corren
sobre el mismo SAP Business One y el mismo WMS las va a consumir.

> Alcance confirmado: **solo la funcionalidad nueva** (sin equivalente en
> este `.NET` hoy). No se portan acá las correcciones de bugs de contrato que
> sí se hicieron en la reescritura Python (ej. `201` real en los `POST` de
> creación, `400` explícito en Inventario/Recibo en vez de éxito falso) —
> quedan fuera de este trabajo salvo que se decida lo contrario más adelante.

## Qué hay que agregar

1. **Módulo SolicitudTransferencia completo** (no existe ningún controller
   ni repositorio hoy) — ver [`SOLICITUD_TRANSFERENCIA.md`](./SOLICITUD_TRANSFERENCIA.md).
2. **Picking diurno/nocturno** — campo `Notificacion` en el `POST /api/Picking`
   existente + nuevo `POST /api/Picking/HojaRuta/{docEntry}/Cerrar` — ver
   [`PICKING.md`](./PICKING.md).

## Dónde vive cada pieza hoy (mapa de archivos .NET)

| Concepto | Archivo |
|---|---|
| Controllers existentes | `Net.Business.Services/Controllers/*.cs` |
| Repositorios de negocio (WMS) | `Net.Data/WMS/*/`  |
| Repositorios de Service Layer (SAP) | `Net.Data/ServicesLayer/*/` |
| Cliente HTTP de Service Layer | `Net.Connection.ServiceLayer/ConnectionServiceLayer.cs` |
| DTOs (shape del request HTTP) | `Net.Business.DTO/**/Dto_*.cs` |
| Entidades de negocio | `Net.Business.Entities/**/BE_*.cs` |
| Wrapper de inyección de dependencias | `Net.Data/DependencyInjection/{IRepositoryWrapper,RepositoryWrapper}.cs` |
| Wrapper de resultado (`BE_ResultadoTransaccion<T>`) | `Net.Business.Entities/ResultadoTransaccion/BE_ResultadoTransaccion.cs` + `Net.CrossCotting/ResultadoTransaccion.cs` |
| Logging de auditoría | `Net.Business.Services/Log/B_Log.cs` (se llama desde cada controller, después del repositorio) |
| Handler global de excepciones | `Net.Business.Services/Extensions/ExceptionMiddlewareExtensions.cs` |

Patrón a seguir para cada pieza nueva (igual que `TransferenciaController`/
`TransferenciaRepository`/`StockTransferRepository`): DTO → `RetornaModelo()`
→ Entidad → Repositorio de negocio (usa `ConnectionServiceLayer` + helpers de
HANA) → Controller (valida, llama al repo, llama a `B_Log.Add`, decide el
status code).

## Gaps de infraestructura compartida a tener en cuenta

Estos dos módulos nuevos necesitan cosas que `ConnectionServiceLayer.cs` hoy
**no soporta**, y hay que agregarlas:

1. **Headers extra en PATCH.** `SolicitudTransferencia` necesita mandar
   `B1S-ReplaceCollectionsOnPatch: true` en el PATCH a SAP. Ningún método de
   `ConnectionServiceLayer` (`PostAsync`, `PatchAsyncSBA`, etc.) acepta headers
   adicionales hoy — todos hardcodean `Accept`/`Content-Type`/`Cookie`
   únicamente. Hay que agregar un parámetro opcional (ej.
   `Dictionary<string, string> extraHeaders = null`) a `PatchAsyncSBA<T>`.
2. **GET de un solo recurso (no colección).** `SolicitudTransferencia`
   necesita `GET InventoryTransferRequests({docEntry})` (un objeto, no una
   lista `.value`). `ConnectionServiceLayer.GetAsync<T>` ya tiene un
   parámetro `dinamico` que deserializa `T` directo sin esperar el wrapper
   `{ "value": [...] }` (`ConnectionServiceLayer.cs:363-373`) — **reusar esa
   ruta** (`dinamico: true`) en vez de escribir un método nuevo.
3. **Acción `/Close` sin body.** Ni `SolicitudTransferencia` ni
   `Picking/HojaRuta` tienen hoy un método para `POST {Recurso}({docEntry})/Close`
   con body vacío — el único análogo es `StockTransferRepository.StockTransfer`
   (`Net.Data/ServicesLayer/StockTransfer/StockTransferRepository.cs`), que sí
   manda body. Hay que agregar un método nuevo (ej. `CloseAsyncSBA`) o un
   overload de `PostAsync<T>` que acepte `obj: null`/`{}`.

## Bugs e inconsistencias encontrados en el código `.NET` existente

Auditado puntualmente el código que estos dos módulos nuevos van a tocar o
reusar (`ConnectionServiceLayer.cs`, `HelperProcesoPiciking.cs`,
`BE_ResultadoTransaccion.cs`). No es una auditoría completa del proyecto —
solo lo relevante para este trabajo.

### 1. `BE_ResultadoTransaccion<T>.NombreEstacion` — setter con recursión infinita (bug serio, no relacionado al porte pero lo tocan ambos módulos nuevos)

**Archivo**: `Net.Business.Entities/ResultadoTransaccion/BE_ResultadoTransaccion.cs`,
línea 36 (propiedad `NombreEstacion`).

```csharp
public string NombreEstacion { get => System.Environment.MachineName; set => NombreEstacion = System.Environment.MachineName; }
```

El `set` asigna `NombreEstacion = ...`, lo que vuelve a invocar al propio
setter — recursión infinita. Si alguna vez se invoca ese setter (por ejemplo,
si algo deserializa un `BE_ResultadoTransaccion<T>` con `JsonConvert.DeserializeObject`
y el JSON trae la propiedad `NombreEstacion`/`nombreEstacion`, o si cualquier
código hace `objeto.NombreEstacion = "algo"` directamente), el proceso
revienta con `StackOverflowException` — una excepción que **.NET no puede
atrapar con try/catch**, tumba el proceso entero (el worker de IIS).

Hoy no se dispara porque nada construye `BE_ResultadoTransaccion<T>` por
deserialización ni asigna esa propiedad directamente (todo pasa por
`ResultadoTransaccion<T>.GetInformacionResultado(...)`, que no la toca). Pero
al escribir el código nuevo para SolicitudTransferencia/HojaRuta, **nunca
hacer `.NombreEstacion = ...` a mano ni deserializar un `BE_ResultadoTransaccion<T>`
desde JSON** — usar siempre `GetInformacionResultado(...)`.

**Solución propuesta**: la propiedad solo necesita ser de lectura.

```csharp
public string NombreEstacion => System.Environment.MachineName;
```

(o, si hace falta que el setter exista por compatibilidad con algo que no
vimos en esta auditoría puntual, que el setter no haga nada: `set { }`).

### 2. `ConnectionServiceLayer.PatchAsyncSBA<T>` nunca marca éxito en un `204`

**Archivo**: `Net.Connection.ServiceLayer/ConnectionServiceLayer.cs`,
método `PatchAsyncSBA<T>`, líneas 739-856 (el caso puntual, líneas 840-848).

Con un `204 No Content` real de SAP, el código no entra en la rama de éxito
(la que solo cubre `200`/`201`, líneas 788-796) ni en ninguna rama de error
— el bloque `if (response_.StatusCode == NoContent) { ... }` (línea 840) está
con el cuerpo comentado, así que cae directo a `return default(T);` (línea
848), o sea `null`. Cualquier caller que acceda a una propiedad del
resultado (`.Exito`, `.DocEntry`, etc.) revienta con `NullReferenceException`.
Ya es un bug conocido y documentado en el proyecto Python (`CLAUDE.md`,
regla 4) — ahí se corrigió tratando un `204` real como éxito explícito.

**Importante**: confirmado que **nadie llama a este método hoy**
(`grep` de `PatchAsyncSBA` en todo el proyecto no da otro resultado que su
propia definición) — es seguro modificarlo directamente, no hace falta
duplicarlo para no romper nada en producción.

**Solución propuesta**: al construir el repositorio de SolicitudTransferencia,
modificar `PatchAsyncSBA<T>` para que un `204` (o `200`) se trate como éxito
real, sin intentar deserializar un body que puede venir vacío — por ejemplo,
devolviendo un `bool`/un objeto liviano de éxito en vez de `T` deserializado
a la fuerza, o agregando un chequeo explícito antes del `return default(T)`
final.

### 3. `ConnectionServiceLayer.PatchAsyncSBA<T>` — un `401` con `error.code` distinto de `"301"` también devuelve `null` silenciosamente

**Archivo**: `Net.Connection.ServiceLayer/ConnectionServiceLayer.cs`,
método `PatchAsyncSBA<T>`, líneas 797-822.

Dentro de la rama `401`, si `NumError < 2` y el código de error de SAP no es
`"301"` (el que dispara relogin + reintento), no hay ningún `goto band` ni
`return` — el flujo simplemente sale del `if/else if` (porque ya entró a la
rama `401`, la siguiente condición es `else if` y no se evalúa) y cae en el
`return default(T);` del final del método. Mismo síntoma que el bug anterior
(`null` silencioso, no un error explícito) pero por una causa distinta.

**Solución propuesta**: agregar un `else` final dentro de la rama `401` que
construya y devuelva un resultado de error explícito, en vez de dejar que el
flujo caiga al `return default(T)` genérico.

### 4. `ConnectionServiceLayer` — `throw` muerto después de `return` en 3 métodos

**Archivo**: `Net.Connection.ServiceLayer/ConnectionServiceLayer.cs`,
métodos `CancelAsyncSBA<T>` (líneas 621-625), `DeleteAsyncSBA<T>` (líneas
733-737) y `PatchAsyncSBA<T>` (líneas 851-855):

```csharp
catch (Exception ex)
{
    return default(T);
    throw new Exception(ex.Message.ToString());
}
```

El `throw` es código inalcanzable (el compilador de C# debería marcarlo con
el warning `CS0162`) — la excepción original queda silenciada sin loguear ni
propagar, y el caller recibe `null` sin ninguna pista de qué pasó.

**Solución propuesta**: sacar el `throw` muerto y decidir explícitamente el
comportamiento — si se quiere tragar el error y devolver `null`/un resultado
de error (dejar solo el `return`), o repropagar la excepción real (`throw;`,
sin el `return` antes). Para los métodos nuevos de SolicitudTransferencia, no
repetir este patrón: loguear o envolver el error en el `BE_ResultadoTransaccion`
de salida, igual que hace el resto del proyecto (ver `TransferenciaRepository.Create`,
que atrapa la excepción y la mete en `GetInformacionResultado(ex.Message, entidad)`).

### 5. `ConnectionServiceLayer.Login()` y `.LoginPost()` — código duplicado

**Archivo**: `Net.Connection.ServiceLayer/ConnectionServiceLayer.cs`,
`Login()` (líneas 44-130) y `LoginPost()` (líneas 131-218).

Son copias casi idénticas línea por línea — la única diferencia real es un
parámetro `Language` que está comentado en `LoginPost`. No es un bug
funcional, es una inconsistencia de mantenimiento (dos lugares que hay que
tocar a mano si cambia el login). No bloquea el porte de la funcionalidad
nueva — se deja solo documentado por si se decide refactorizar en el futuro.

**Solución propuesta**: unificar en un solo método (`LoginPost` ya tiene el
parámetro `Language` listo para habilitarse si hiciera falta).

### 6. `HelperProcesoPiciking.Set` — SQL injection real por interpolación de string

**Archivo**: `Net.Data/HelperData/ProcesoPiciking/HelperProcesoPiciking.cs`,
ambas sobrecargas de `Set` (líneas 14 y 28):

```csharp
cmd.CommandText = $"CALL\"B1_ARDI_PROD\".\"ITCA_WMS_SAP_SetProcesoPicking\"('{Proceso}', {IdEstado}, {DocEntry}, '', '', '{CodMotivo}', '{Code}')";
```

`Proceso` y `CodMotivo` vienen directo del body del request HTTP
(`entidad.Proceso`, `entidad.CodMotivo` en `PickingRepository.cs:96` y
`:374`) e, interpolados así, son un vector de SQL injection real — mismo
patrón de vulnerabilidad ya documentado y confirmado en el módulo Chatbot del
`.NET` (`CLAUDE.md` del proyecto Python, sección "Pendiente"). Este método es
justo el que hay que tocar para aislar la actualización de estado (ver
[`PICKING.md`](./PICKING.md)), así que es un buen momento para corregirlo de
una vez en vez de solo copiarlo.

**Solución propuesta**: parametrizar con `HanaParameter`/placeholders en vez
de interpolar el string, por ejemplo:

```csharp
cmd.CommandText = "CALL \"B1_ARDI_PROD\".\"ITCA_WMS_SAP_SetProcesoPicking\"(?, ?, ?, ?, ?, ?, ?)";
cmd.Parameters.Add(new HanaParameter("1", Proceso));
cmd.Parameters.Add(new HanaParameter("2", IdEstado));
cmd.Parameters.Add(new HanaParameter("3", DocEntry));
cmd.Parameters.Add(new HanaParameter("4", HoraInicio?.ToString() ?? ""));
cmd.Parameters.Add(new HanaParameter("5", HoraFin?.ToString() ?? ""));
cmd.Parameters.Add(new HanaParameter("6", CodMotivo));
cmd.Parameters.Add(new HanaParameter("7", Code));
```

(Ajustar la sintaxis exacta de binding de parámetros al driver `Sap.Data.Hana`
realmente instalado — confirmar la forma correcta de parámetros posicionales
de ese driver ADO.NET antes de aplicar, no asumida en esta auditoría.)