roafacturare/docs/cercetare/rec_review_ancorare_s4.md

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.