docs: planurile, proiectarile si rapoartele de lucru intra in versionare
Folderul docs\ era pana acum in afara oricarui control de versiuni - nici git, nici SVN - desi contine planurile pe puncte, proiectarile si rapoartele de cercetare pe care se sprijina modificarile din cod. O stergere acolo era definitiva. Fisierele intermediare (handoff-uri intre sesiuni, diff-uri deja aplicate) au fost sterse inainte, nu versionate: ce era durabil in ele a intrat in antetele fisierelor de test la care se refereau. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SN8snvkk94KuhWwoXUUey3
This commit is contained in:
202
docs/cercetare/rec_review_ancorare_s4.md
Normal file
202
docs/cercetare/rec_review_ancorare_s4.md
Normal file
@@ -0,0 +1,202 @@
|
||||
# Review S4 runda 1 (PAGE3 "Articole factura") - ancorare coloane + calitate cod
|
||||
|
||||
Review de cod, fara nicio modificare aplicata. Obiect: `docs\diff_s4_runda1_page3.patch`
|
||||
(`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.
|
||||
Reference in New Issue
Block a user