search

Matoo125 / Vegapo

0 stars 2 forks source link

Editovatelna lokalizacia #22

Closed mrkovec closed 7 years ago

mrkovec commented 7 years ago

Issue #16. Pridanie dvoch novych modulov Locale a Edits.

Localizacne texty boli presunute do samostatnych json fajlov, ktorych editacia je spristupnena cez admin vetvu. Uprava textov prebieha v dvoch krokoch, v prvom sa lokalizacne udaje uzamknu (vytvorenim noveho "otvoreneho" edits zaznamu) aby sa zabranilo vzajomnemu prepisovaniu udajov subeznou editaciou viacerymi uzivatelmi. V druhom sa zmeny ulozia do suborov a "uzavrie" sa edits zaznam - cim sa upravy spristupnia dalsiemu uzivatelovi. Zaroven sa ulozi info o zmenach v datach a toto info sa zobrazuju tiez v admin vetve. Funkcia logovania je zatial pracovna, ale ak sa do buducna rata s moznostou editovania udajov produktov atd., predpokladam ze bude treba zabezpecit konzistentnost pri meneni udajov a tiez aj historiu zmien. Momentalne by nebol problem zaviest logovanie kdo a kedy schvalil/zamietol sugesstions.

Matoo125 commented 7 years ago

Takže pozrel som si to a idea je dobrá.

sdiff-error

Takýto error mi hádže sdiff, obišiel som to s if ($tabcount >= 0) {). Aj by som to opravil ale nerozumiem uplne tomu súboru.

wierd-behaviour

Problém je, že sa to správa veľmi zvláštne, ak vymažem iba jedno slovo, takýto nejaký mám output.

Ak pridám iba jedno slovo a nevymažem nič, je to ok. Ak upravím existujúci string, tiež to je v pohode, problém nastane iba ak vymažem string úplne. Môže to byť samozrejme spôsobené mojim fixom tvojej knižnice.

Problém 2, čo ak niekto klikne edit, otvorí si lock a zabudne na to

Možno JSON diff by bol jednoduchší, ako text diff na takýto projekt. Ale hlavne aby to fungovalo 👍

Matoo125 commented 7 years ago

Tiež som úplne nepochopil existenciu 2 tabuliek namiesto jednej. Prečo nemôžu object_typea object_idbyť súčasťou tabuľky edits?

Matoo125 commented 7 years ago

Rovno aj navrhnem riešenie problému 2

Extra riešenie

mrkovec commented 7 years ago

Pozriem ten diff, ocividne v tom html format kode je nejaky bug. Ten chaos vo vystupe bude sposobeny tym, ze som mu nechal nastavenu prilisnu "citlivost". Opravim. Snazil som sa najst uz nejaku diff kniznicu ktora by vyhovovala, ale minimalne som nenarazil na ziadnu, ktora by nezobrazovala aj nemenene udaje - kvoli prehladnosti v zalogovanych udajoch.

Pri tom zabudnutom locku - uzivatel s aktivnym lockom, bude mat data pristupne stale na editovanie - teda nebude znova stlacat button edit, ale rovno sa mu formular otvori v editovacom mode - co by mu malo napovedat, ze by to mal dokoncit. Asi by bolo dobre mu tam hodit nejake upozornenie, aby nezabudol na "save" pred odchodom z frm. Tak isto bude v prehlade editov vidiet, ze uzivatel ten a ten ma otvorenu (a locknutu) lokalizaciu a bude sa mu ten lock dat aj jednoducho zrusit (napr updateom stavu v edits tabulke). Ten wokflow bude treba este doriesit.

Pri tej "podvesenej" edit_changes tabulke som mal predstavu, ze by sluzila pre eventualne logovanie viacerych zmien pre jeden edit. Napr Produkt A bol updatnuty na produkt B na zaklade suggestion C (schvalenej uzivatelom D atd) by bolo realizobane 3 riadkami v edit_changes. Ale mozno je to overkill a bolo by to lepsie spravit vsetko v jednej tabulke. To som chcel doriesit v takejto diskusii :)

Matoo125 commented 7 years ago

Všetko by malo byť foolproof, teda minimálne to surové uzavretie editu je potrebné. Situácia, že niekto si otvorí edit, zazvoní mu mobil a už sa k tomu nevráti pár dní nieje taká nemožná.

(edit_changes) nápad dobrý. user_id sa tam už loguje (v edits tabuľke). Ale možno trochu overkill.

Ak máme produkt A a suggestion B, ktorú schváli admin C:

teda by som navrhol aby sme to skúsili iba s jednou tabuľkou

Jedine, že by si chcel rušiť suggestions tabuľku a návrhy by sa riešili vytvorením kópie produktu, kde by už edit_changesvyužiť dala.

Produkt A dostane návrh na zmenu a vznikne neviditelný Produkt B, zároveň sa vytvorí otvorený edit záznam 

type = suggestion
state = open
user_id = person who created suggestion
comment = (some unusual action i.e this product is meat, remove it!)
diff = (not sure what would be here)

edit_changes

object_type = product
object_id = A
----------
object_type = product
object_id = B
----------
object_type = user
object_id = C (person who resolved edit)

hmm už sa strácam, ja by som asi išiel podľa pôvodného (prvého) návrhu a keďtak pozmenil suggestions tabuľku aby spájala, produkt id návrhu na zmenu a skutočného produktu.

Ak by si mal problém sfunkčniť ten diff, čo keby iba porovnáš prvky na úrovni array, buď a === b alebo nie.

Chýba mi tam ešte informácia v ktorom súbore bola vykonaná zmena (cz alebo sk)

mrkovec commented 7 years ago

Upravil som tu nestastnu diff funkcionalitu. Chybu pohladam neskor, ale ak by si mal typ na jednoduchu a funkcnu diff kniznicu budem len rad ;) Je potrebny aj composer update kedze som menil SDiff. Neviem composer donutit zdvihnut verziu nejakej kniznice inac ako zmazanim composer.lock co mi pride cudne, ale kedze composer vobec neovladam asi nieco nerobim spravne.

Okej suhlasim so zjednodusenim tej zmeny logujucej funkcionality, to je vzdy lepsie ako opacny pripad. Navrhujem presunut stlpce object_typea object_id do edits tabulky a edit_changes dropnut. Log zmeny bude potom:

suggestions.product_id = produkt ku ktoremu je suggestion na zmenu suggestions.user_id = user schvalujuci/zamietajuci suggestion - toto by bola zmena voci terajsej fnc suggestions./type/state = by zostalo bez zmeny


- pre  ziadosti

edits.type = new edits.state = opened/closed - close aktivuje novy produkt edits.user_id = user zadavajuci novy produkt edits.object_type = suggestion edits.object_id = suggestion_id

suggestions.product_id = novy produk suggestions.user_id = user schvalujuci/zamietajuci novy produk suggestions./type/state = ako pri kalsickych suggestions


- do buducna pre pre priamu upravu dat produktu uzivatelom

edits.type = update edits.state = opened/closed - close prepne visibility medzi povodnym a starym produktom edits.user_id = user editujuci produkt edits.object_type = suggestion edits.object_id = suggestion_id

suggestions.product_id = novy produk suggestions.user_id = user schvalujuci/zamietajuci novy produk suggestions./type/state = ako pri kalsickych suggestions suggestions.original_product_id (?) = novy stlpec s odkazom na meneny produkt



Edit:
Este ma napadlo, ze asi by podla tohto navrhu bola potrebna aj migracia doteraz nahranych suggestions  co asi nieje najlepsie riesenie (kedze napr pri schvalenych/zamietnutych aj tak nie je zaznam ktory admin to spravil). Mozno tento udaj ani nie je potrebne logovat - potom by sa tabulky dali este viac zjednodusit.
Matoo125 commented 7 years ago

Dobré riešenie

suggestions.user_id = user schvalujuci/zamietajuci suggestion - toto by bola zmena voci terajsej fnc

toto je podľa mňa zbytočné. Suggestion by mal mať id autora a v edits by sme iba logovali, kto to schváli.

edits.user_id = user zadavajuci novy produkt

zase to isté, veď si to prečítaj - EDITs.USER_id - to by mal byť ten čo schvaluje, teda vytvára edit a nie ten čo pridal produkt.

Keď user pridá produkt vytvorí sa plnohodnotný produkt riadok s viditelnosťou iba pre adminou. Tam sa ukladá aj user_id. Ak to admin schváli vytvorí sa edit riadok. Tento riadok by mal obsahovať informáciu, že admin A schválil / zamietol produkt B. Žiadna suggestion sa nevytvára pri pridaní nového produktu.

Rozumiem, čo si sa tu snažil spraviť ale mám pocit, že iba pridávaš miestami extra level zložitosti.

Matoo125 commented 7 years ago

https://packagist.org/search/?q=diff Tu sú knižnice ale ako som pozeral niesu moc dobre dokumentované.

Ja som skôr myslel, že by sme porovnávali obsah arrays a nie text. Niečo ako toto

Matoo125 commented 7 years ago

teraz som pozrel tento druhý commit. Funguje to dobre, nemám námietky. Ked to upravíš na jednu tabuľku a pridáš surový close, spravím merge.

mrkovec commented 7 years ago

Jasne, mas pravdu. Upravil som to, pls pozri najma tu javascript cast pre vynutene uzavretie lakalizacneho editu pri odchode zo stranky (onbeforeunload event vo app\view\admin\locale\index.twig). Bol to boj.

Matoo125 commented 7 years ago

Je to super ... som ti to trochu preformátoval

skús si to pozrieť teraz z hladiska čitatelnosti to vyzerá ako úplne iný kód. Neskôr spíšem contributor guide. Zatial aspoň takto neoficiálne.

mrkovec commented 7 years ago

Sorry, pri mojom editore so vsetkymi tymi indent guidami a soft wrapom, zabudam ako ten kod vyzera v skutocnosti :) polepsim sa.