# Review S4 runda 1 (PAGE3 "Articole factura") - ancorare coloane + calitate cod

Review de cod, fara nicio modificare aplicata. Obiect: diff aplicat (sters)
(`COMUN\clase\omodificari.vc2` clasa `frm_modific2024`, `COMUN\programe\ofacturare_editare.prg`).
Context citit: `docs\cercetare\rec_s4_runda1.md`, `docs\progres.md` (sectiunea "#6, S4 runda 1" si
decizia/nota despre pozitionarea in `actactan`), `COMUN\docs\reguli_lucru.md`,
`COMUN\docs\capcana_grid_controlsource.md`.

## 1. Riscul cel mai important - NU e despre coloane, e o pozitionare oarba in `tact` (corectitudine)

`omodificari.vc2:14171`, in `Show()`:

```
IF Reccount('tact') > 0
    Go Top In tact
    IncarcaVanzareNota(tact.cod, tact.nract, tact.serie_act, tact.dataact)
    ...
```

`tact` poate avea MAI MULTE randuri pentru aceeasi nota (o factura + o incasare, etc.), ordonate
dupa `id_act` (`ofacturare_editare.prg:54`, `order by id_act`). Exact acest tipar - "Go Top orb pe
`actactan`/`tact`" - e documentat ca riscant in `docs\cercetare\rec_pozitionare_actactan.md` (§2):
pe **39 de facturi in schema de dev**, primul rand dupa `id_act` e `INCASARE`/`INCASARE NUMERAR`,
cu `id_fact` = facturii minus 1, nu al facturii insesi. Acolo era vorba de `id_fact`; aici codul
nou citeste `tact.nract`/`tact.serie_act`/`tact.dataact` de pe randul gasit de `Go Top` - daca
INCASAREA are propriile ei `nract`/`serie_act`/`dataact` (referinta la chitanta, nu la factura),
filtrul compus din `IncarcaVanzareNota` (`cod + nract + serie_act + dataact`) cauta valorile
GRESITE in `VANZARI` -> 0 randuri gasite -> `lAreArticoleVanzari = .F.` **silentios**, desi factura
chiar are vanzare asociata. Nu inseamna eroare vizibila, ci pagina PAGE3 lipsind pe documente care
ar trebui sa o aiba.

Nu e o certitudine (n-am verificat direct daca `nract`/`serie_act`/`dataact` difera intre randul
INCASARE si randul facturii pe cele 39 de documente), dar riscul e concret si documentat de proiect
insusi pentru exact acelasi tipar de pozitionare, pe alt camp din acelasi cursor. Testele existente
(`test_page3_articole.prg:70` si `:167`) **repeta acelasi `Go Top In tact`**, deci nu-l acopera -
"zero cazuri in date" nu e dovada ca nu exista (`COMUN\docs\reguli_lucru.md` pct. 6).

**Recomandare**: inainte de a inchide runda, verifica direct pe unul din cele ~39 de documente din
`rec_pozitionare_actactan.md` (sau cauta altele cu `INCASARE` ca prim rand si rand de vanzare in
`VANZARI`) daca `Go Top` da alt `nract`/`serie_act`/`dataact` decat cel corect. Daca da, nu exista
azi in cod un anchor gata de refolosit la momentul `Show()` (`ales` e flag de UI, populat de
utilizator din grid, gol la deschidere - nu ajuta aici); ar trebui gasit un criteriu de pozitionare
mai bun, posibil analog cu ce se recomanda in `rec_pozitionare_actactan.md` §5 pentru `id_fact`.
Efort: mic ca sa verifici (o interogare + 1-2 randuri de test), nedeterminat ca sa remediezi -
depinde ce arata verificarea. **Prioritate maxima inainte de commit**, e mai important decat
subiectul de ancorare de coloane cerut initial.

## 2. Cerinta principala: ancorarea codului de structura tabelelor

### Ce e hardcodat azi, si ce se rupe

Structura e prezenta in **3-4 locuri separate**, toate manuale:

1. Lista de coloane din `SELECT` (`ofacturare_editare.prg:201-208`, `IncarcaArticoleFactura`) -
   20 coloane explicite (`vd.id_vanzare_det ... nv.nume_val`).
2. `CREATE CURSOR tvd` din fallback-ul de eroare Oracle, **in aceeasi functie**
   (`ofacturare_editare.prg:214-216`) - acelasi 20 de campuri, aceleasi tipuri, scrise separat.
3. `CREATE CURSOR tvd` placeholder din `Load()` (`omodificari.vc2`, ~14076, `If !Used('tvd') ...`)
   - a treia copie, byte-cu-byte aceeasi structura ca (2), intr-un alt fisier.
4. Coloanele gridului `grdArticoleFactura` (`omodificari.vc2`, ADD OBJECT ~12258-12454) - 13
   `ControlSource`/`Header.Caption`/`Width`/`InputMask`, cate un bloc pe coloana.

**Coloana ADAUGATA in `VANZARI_DETALII`** (sau in `nom_articole`/`nom_gestiuni`/`nom_valute`): NU
rupe nimic. `SELECT`-ul explicit o ignora, cursorul `tvd` nu o capata, gridul (13 coloane fixe) nu o
cere. Zero impact pana cineva decide s-o afiseze - caz in care tot trebuie atinse (1) si (4) oricum,
indiferent de strategia de ancorare aleasa.

**Coloana STEARSA sau REDENUMITA** dintre cele 20 folosite: `SELECT`-ul explicit din (1) pica pe
Oracle -> `lnSucces < 0` -> se intra pe fallback-ul (2), cursorul `tvd` gol, `IncarcaArticoleFactura`
returneaza `.T.` **fara niciun mesaj vizibil** (vezi punctul 3 mai jos). Practic: pagina PAGE3 arata
goala, silentios, fara semnal ca ceva s-a stricat structural. Acesta e cazul real de reparat cand se
schimba schema - nu adaugarea de coloane.

**Cat de des se intampla la ROA**: dupa `docs\progres.md` (decizia 2, sursa DDL e schema de
dezvoltare `MARIUSM_AUTO`, aplicata prin scripturi de migrare versionate) - stergerea/redenumirea de
coloane pe tabele active ca `VANZARI_DETALII` nu pare o practica frecventa (schimbarile de schema
documentate in sesiune sunt adaugari de coloane/view-uri, nu redenumiri). Deci riscul real e rar, dar
cand se intampla azi e **silentios**, nu zgomotos - asta conteaza mai mult decat frecventa.

### Variante de ancorare, cu ce pierde fiecare

- **A. Grid construit dinamic din `AFIELDS()` + dictionar de etichete.**
  Elimina nevoia sa atingi (4) cand se schimba coloanele afisate implicit, dar tot trebuie sa
  intretii un dictionar {camp -> caption/width/format} undeva - muti hardcodarea din `.vcx` intr-un
  `.prg`, n-o elimini. Cost mare: e o **schimbare de tipar fara precedent** pe acest formular -
  gridurile surori `grdRulaje`/`grdRulajeObinv` de pe PAGE1/PAGE2 (`omodificari.vc2:8694`,
  `:10517`, 63 si 61 de coloane) sunt 100% declarative in `.vcx`, cu `ColumnOrder`/`DynamicForeColor`/
  `InputMask` per coloana - cautabile cu `vfp_symbols.ps1`/grep. Un grid dinamic ar fi unicat in tot
  formularul (si, dupa cat am vazut, in restul clasei) - o datorie de intretinut de unul singur, nu
  un castig, exact contrariul principiului "consistenta cu codul din jur" din brief. Nu recomand.

- **B. `SELECT *` in loc de lista explicita de coloane.**
  Pentru coloana ADAUGATA nu aduce niciun beneficiu fata de azi (gridul tot leaga doar 13 coloane
  numite, indiferent cate vin din `SELECT`). Pentru coloana STEARSA/REDENUMITA e **mai rau**: azi
  eroarea e prinsa curat la nivel de SQL (`lnSucces < 0`, punct de control unic); cu `SELECT *` pe un
  join direct pe 4 tabele, interogarea SQL reuseste oricum (nu refera explicit campul lipsa), iar
  eroarea apare abia la binding-ul gridului pe un `ControlSource` inexistent - exact tipul de
  capcana (dialog nativ VFP) pe care runda asta a trebuit sa-l ocoleasca separat pentru `tvd`
  (`rec_s4_runda1.md`, blocajul #3). Tiparul corect pentru `SELECT *` folosit deja de gridurile
  surori (`trul`, `trul_obinv`, `tact`) nu e pe join brut, ci pe un **view Oracle dedicat**
  (`vrul_tot`, `vact_tot`, `vrul_obinv_tot` - vezi `ofacturare_editare.prg:54,69,100`) care izoleaza
  exact coloanele si numele expuse catre VFP. Replicarea corecta a tiparului ar insemna un view nou
  `vvanzari_articole`/similar pentru `VANZARI_DETALII` - fezabil, dar e o **migrare de schema Oracle**
  (`scripturi-migrare-db.md`), nu o editare VFP; cost si coordonare mai mari decat editarea `.vc2`.
  Merita luat in calcul DACA schema chiar incepe sa se miste des pe zona asta, nu acum pentru o
  runda "doar afisare".

- **C. Coloane declarate (ca azi), plus garda care semnaleaza divergenta la rulare.**
  Nu schimba nimic structural - pastreaza controlul total pe ordine/latime/format, consistent 1:1
  cu `grdRulaje`/`grdRulajeObinv`. Cere doar sa nu mai fie inghitita silentios eroarea Oracle: azi
  `IncarcaVanzareNota`/`IncarcaArticoleFactura` (`ofacturare_editare.prg:174-177`, `:213-217`)
  returneaza `.T.` cu cursor gol pe `lnSucces < 0`, spre deosebire de funcita sora
  `IncarcaCursoareModificareNota` din ACELASI FISIER (`ofacturare_editare.prg:59-62`), care afiseaza
  `AMESSAGEBOX(goExecutor.cEroare,...)`. Adaugarea aceluiasi `AMESSAGEBOX` (sau macar un log) pe cele
  doua functii noi transforma o coloana stearsa/redenumita dintr-un gol tacut intr-un semnal vizibil
  - fara sa schimbe deloc modul in care se intretine gridul. Cost: cateva linii, minim.

- **D. Lasat asa cum e.**
  Argument real: e runda 1, "doar afisare" (`rec_s4_runda1.md`), iar tiparul (SQL explicit + grid
  declarat) e **identic** cu ce exista deja de ani pe acelasi formular pentru `trul`/`trul_obinv` in
  partea de campuri neprovenite direct din view (vezi Column3-Column15 la `grdRulaje`,
  `omodificari.vc2:8721-8829`, multe cu `ControlSource` pe nume simplu de camp). Nu e o liabilitate
  noua introdusa de diff, e consistenta cu practica existenta. Singurul gol real fata de sora ei e
  lipsa mesajului de eroare (punctul C), nu structura declarativa insasi.

### Recomandare

**C**, nu A sau B: adauga `AMESSAGEBOX` (dupa modelul `IncarcaCursoareModificareNota`) pe cele doua
`lnSucces < 0` din `IncarcaVanzareNota`/`IncarcaArticoleFactura`. E schimbarea cu cel mai bun raport
cost/beneficiu - cateva linii, zero impact pe tipar, transforma exact riscul real (coloana
stearsa/redenumita) dintr-un gol silentios intr-un semnal vizibil. Grid dinamic (A) sau `SELECT *`
pe join brut (B) NU merita azi - ambele fie muta hardcodarea in alta parte fara sa reduca
intretinerea, fie inrautatesc raspunsul la exact riscul pe care vor sa-l elimine. Daca la un moment
dat `VANZARI_DETALII` incepe sa-si schimbe structura des, varianta corecta e B **cu view Oracle
dedicat** (ca la `trul`/`tact`), nu grid dinamic.

## 3. Duplicare de cod - structura cursorului `tvd`/`tvanz` scrisa manual de mai multe ori

- `CREATE CURSOR tvanz (...)` (6 campuri) apare **de doua ori in aceeasi functie**,
  `ofacturare_editare.prg:161` si `:175` (`IncarcaVanzareNota`), byte-cu-byte identic. Fix simplu:
  un singur `CREATE CURSOR` la inceputul functiei / dupa cele doua conditii de iesire timpurie, in
  loc de doua copii separate la 14 linii distanta. Efort: mic, cateva minute.
- `CREATE CURSOR tvd (...)` (20 campuri) apare **in doua fisiere diferite**: fallback-ul din
  `IncarcaArticoleFactura` (`ofacturare_editare.prg:214-216`) si placeholder-ul din `Load()`
  (`omodificari.vc2`, ~14076). Identice ca structura. O functie comuna in `ofacturare_editare.prg`
  (ex. `CreeazaCursorTvdGol`) apelata din ambele locuri ar elimina a treia copie manuala si ar
  garanta ca raman sincronizate cand se adauga/scoate un camp. Efort: mic-mediu (o functie noua +
  doua puncte de apel, testat deja indirect de suita existenta).

## 4. Alte observatii de calitate

- **Pozitiv**: toate `ControlSource`-urile noului grid `grdArticoleFactura` sunt calificate cu
  `tvd.` (`omodificari.vc2`, Column1-Column13, ex. `"tvd.denumire"`, `"tvd.codmat"`) - exact regula
  din `COMUN\docs\capcana_grid_controlsource.md` pentru formulare cu 2+ grid-uri (formularul are
  acum trei: `grdRulaje`, `grdRulajeObinv`, `grdArticoleFactura`). De comparat cu gridurile surori
  `grdRulaje`/`grdRulajeObinv`, unde o parte din coloane au `ControlSource` NECALIFICAT (ex.
  `"dataact"`, `"codmat"`, `"denumire"`, `"pret"`, `"cant"` la `omodificari.vc2:8728-8785`) - expuse
  in teorie la exact capcana descrisa in document daca alt cursor ajunge sa fie workarea curenta.
  E o expunere preexistenta, nu introdusa de acest diff, si gridul respectiv nu pare sa fi avut
  probleme raportate - semnalez doar ca informatie, nu ca ceva de reparat acum.
- **Comentariu usor peste norma**: header-ul `IncarcaVanzareNota` (`ofacturare_editare.prg:144-147`)
  are 4 linii; regula permite 2-3 pentru contract nebanal (`reguli_lucru.md` pct. 2). Continutul e
  util (parametri, capcana cod-neunic, cursor lasat deschis) - as comprima usor, nu as sterge
  informatie. Nu blocant.
- **`GO`/`Recno()`**: singura pozitionare noua e `Go Top In tact` (discutata la punctul 1) - nu e
  cazul "GO pe un Recno() capturat/primit ca parametru" din `conventie_go_recno.md`, deci acea
  conventie specifica nu se aplica direct, dar tot e o pozitionare pe un cursor cu mai multe randuri
  posibile, deci riscul de fond e inrudit.
- **`ALTER TABLE` pe cursor din `goExecutor.oExecute()`**: nu se foloseste in diff, nu se aplica.
- Nu am gasit cod mort introdus, nici nume inconsistente - `lAreArticoleVanzari`/`nIdVanzare`/
  `nTipVanzare` respecta exact conventia Hungarian deja folosita pe restul clasei
  (`lavertizatexigibilizare`, `nid_set` etc.).
- Nimic de refolosit ratat: n-am gasit o functie comuna existenta pentru "gaseste randul din
  VANZARI pentru o nota" sau "incarca liniile unei vanzari" inainte de acest diff - functiile noi
  chiar completeaza un gol, nu dubleaza ceva ce exista deja (conform si cu `rec_s4_runda1.md`).

## Ce NU merita schimbat

- Tiparul declarativ al gridului (`ColumnN.ControlSource`/`Header.Caption`/`Width` scrise manual in
  `.vcx`) - e identic cu tiparul din PAGE1/PAGE2, cautabil cu `vfp_symbols.ps1`, si schimbarea lui
  ar fi o inconsistenta noua, nu o simplificare reala (vezi Variantele A/B mai sus).
  `ReadOnly = .T.` pe grid si pe fiecare `Text1` e corect si suficient pentru o runda "doar
  afisare" - nu trebuie dus mai departe acum.
  `PageCount` comutat intre 2 si 3 in `Show()` e simplu si testat, nu are nevoie de alta
  arhitectura.
- Placeholder-ul `CREATE CURSOR tvd` in `Load()` ca sa evite dialogul nativ "Open" - solutia corecta
  pentru capcana documentata deja in `rec_s4_runda1.md`; singura problema e ca structura lui e
  duplicata (punctul 3), nu ca exista.
- Filtrul compus `cod + nract + serie_act + dataact` din `IncarcaVanzareNota` - justificat solid de
  `docs\progres.md` (`VANZARI.COD` nedovedit unic, coliziune verificata pe `cod=1139934`), corect
  implementat si testat pe cazul de coliziune. Nu-l simplifica inapoi la `cod` singur.

## Recomandare finala

Inainte de commit, in ordinea asta:
1. Verifica riscul de la punctul 1 (`Go Top In tact`) pe un caz real cu `INCASARE` ca prim rand -
   e singurul lucru care poate face pagina PAGE3 sa lipseasca gresit pe facturi reale.
2. Adauga `AMESSAGEBOX` pe erorile Oracle din `IncarcaVanzareNota`/`IncarcaArticoleFactura` (punctul
   2, varianta C) - raspunsul corect si ieftin la cerinta de ancorare a lui Marius.
3. Opional, daca ramane timp: elimina cele doua duplicari de `CREATE CURSOR` (punctul 3).
Restul (structura declarativa a gridului, filtrul compus, placeholder-ul din `Load()`) e in regula
asa cum e si nu merita atins.