Forms 3.1 a varování při getValues()
- David Grudl
- Nette Core | 8218
V Nette Forms 3.1 vyhodí metoda $form->getValues()
v případě nevalidního formuláře warning. A také respektuje omezení
pomocí setValidationScope() a vrací jen vybrané prvky. (Pokud
setValidationScope nepoužijete, nic se nemění.) S tím souvisí i parametr
$values
v handlerech. Do handleru A všem
handlerům včetně onSuccess a onClick se ve $values předávají jen
validované prvky, tedy podle omezení setValidationScope.onValidate
se
jako $values předá null, pokud je formulář nevalidní.
Tyto změny mají zajistit, že parameter $values
v handlerech
nebo vrácená hodnota z getValues()
nebude obsahovat žádná
nevalidní data.
Věřím, že úprava dává smysl. Pro mnohé může být naopak překvapení, že se to může chovat jinak.
Zároveň je to i nutnost od příchodu typehintů u properties v PHP
7.4 společně s mapováním.
Protože validní a nevalidní data mají často jiný typ. Nevyplněný prvek
může být null, vyplněný ale nesprávně může být string, vyplněný
správně je třeba int. Pokud property ve třídě má typ int
,
tak pokus zapsat tam nevalidní údaj způsobí Type Error.
Metoda $form->getValues()
volaná v Nette Forms
3.1 v případě nevalidního formuláře zatím vyhazuje jen varování, aby
bylo možné vychytat všechny situace, kdy vaše aplikace nebo komponenty tuto
metodu v takových situacích používají. Ve verzi 4 by pak vyhodila
výjimku.
- Polki
- Člen | 553
Bere se jako nevalidní hodnota jen to, co je definováno pomocí
addRule
nebo i to, že prvku nastaven error až v
onValidate
metodě?
Viděl jsem totiž hodně lidí, kteří buď nechtěli, nebo neuměli
používat addRule
a tak formulář tvořili tak, že vytvořili
továrnu na formulář, kde formulář vytvořili a přidali mu prvky, které
má mít a k tomu vytvořili nějaký ‚handler‘ který obsahoval události
pro daný formulář tedy metodu onValidate
a metodu
onSuccess
.
Otázka zní, jestli když to bude tímto způsobem uděláno, tak jestli
když budou například přidány dvě onValidate
metody a
v první se zjistí, že například uživatel s daným ID neexistuje
v databázi a provede se tedy
$form['user']->addError('Neznámý uživatel.');
, tak v druhé
onValidate
metodě již získání values vyhodí varování a
hodnota $values->user
bude nedefinovaná?
S tím se pojí taky druhá otázka:
Jak se bude od verze 4 řešit stejný případ jako výše, ale když má
programátor například nastavená nějaká validační pravidla na straně
formuláře a nějaká až v handleru onValidate
? V zásadě to
nyní funguje tak, že onValidate
handler se volá vždy. Tedy
pokud mám v definici formuláře toto:
$form->addSelect('user', 'Vyberte uživatele', $this->userRepo->getUsersToSelect())
->setRequired();
...
A onValidate
vypadá takto:
public function onValidate(Form $form, stdClass $values): void
{
$user = $this->userRepo->getById($values->user);
if (!$user) {
$form['user']->addError('Uživatel nenalezen. Zkuste vybrat jiného.');
}
...
}
Budu muset tedy odebrat $values z argumentu a hodit do něj ještě
try/catch? Případně zjišťovat, jestli uživatel něco vybral a jestli není
nedefinovaná hodnota user? Jak se to bude chovat v případě, kdy budu mít
více inputů a jeden z nich bude nevalidní, ale nebudu ho ověřovat
v onValidate, ale onValidate potřebuji provést abych věděl, jestli je
validní vybraný user? Musím i tak ověřovat v try/catch vyhozenou
výjimku, nebo abych nemusel řešit try/catch, tak budu muset ověřovat
hodnoty po jedné pomocí metod hasErrors
a getValue
přímo na konkrétním inputu?
- David Grudl
- Nette Core | 8218
Pro ten usecase, který popisuješ, by asi bylo lepší chování
$values
v onValidate změnit tak, že se parametr předával
vždy, ale nevalidní prvky tam budou chybět. Co myslíš?
- Polki
- Člen | 553
Tak podle mě je v pohodě i to, že by se místo prvku
$values
předalo null
, akorát se nějaké starší
projekty budou muset upravit.
Samozřejmě pokud by se předalo stdClass
, kde by akorát byly
nenastavené ty prvky, které jsou nevalidní, popřípadě v konkrétní
třídě by byly nastaveny na null
, tak je to méně práce pro
úpravu starších projektů.
Dokonce jsem hodně viděl projekty, které počítají s tím, že i když
si necháš předat $values
, tak jsou všechny property nastaveny
nehledě na datový typ $values
a tedy se s nimi podle toho
pracuje (například neověřuje se pomocí isset
jelikož se
počítá, že jsou nastaveny) a pak se to posílá třeba tak jak jsem psal
tedy $this->userRepo->getById($values->user)
takže má
developer jistotu, že je property user nastavená, ale to jestli je null nebo
existující ID si pořeší už UserManager.
Tedy z tohohle hlediska by možná bylo dobré zahovat chování i u
stdClass
, polí atd, kde by se property nastavila ale na
null
?
Případně jen takový nápad, ale co třeba udělat předávání values
pomocí parametrů funkce? Jsou případy, kdy programátoři nechtějí
všechny hodnoty, ale jen některé a stejně volají
->getValues()
s příchodem pojmenovaných argumentů by to
šlo udělat ne? Že by si člověk nadefinoval v parametrech ty hodnoty,
které potřebuje k validaci a ty potom v onValues
využil. No a
ty co by neprošli už první validací jako třeba email, tak by měly hodnotu
null.
Tedy:
$form->addText('name', 'Jméno');
$form->addText('surname', 'Přijmení');
$form->addEmail('email', 'E-mail');
$form->addText('phone', 'Mobil');
public function onValidate(Form $form, string $email = null, string $phone = null): void
{
// TODO: validace telefonu a emailu
}
Editoval Polki (24. 1. 2021 21:04)
- David Grudl
- Nette Core | 8218
Mně jde hlavně o to upozornit na situace, kdy se pracuje s nevalidní
hodnotou, takže je lepší ji tam nedávat, což při přístupu vygeneruje
warning. Když ji opravdu budu chtít použít, napíšu třeba
$values->user ?? null
.
- Polki
- Člen | 553
Jasně rozumím.
Jen nevím, jestli to nebude generovat zbytečné podmiňování, ať už půjde
o klasický if, nebo o ternár.
Rozumím tomu třeba u datumu. Pokud počítáš s tím, že prvek je
$form->addText('date', 'Datum narození')
->addRule(Form::PATTERN, 'Nejedná se o datum', "/([0-1]{1})([0-9]{1})-([0-3]{1})([0-9]{1})-([2]{1})([0-1]{1})([0-9]{2})/");
Tak pak když by se ještě nějak datum validovalo v onValue
metodě a bylo tam ten nevalidní řetězec, tak by se musela stejně dávat
podmínka ->hasErrors()
a to taktéž, kdyby ve value bylo null,
protože při nepovinném datumu není poznat, jestli je null správná hodnota,
nebo tam má být prázdný řetězec, nebo jestli je řetězec validní.
Nedefinovaná hodnota datumu je tedy takový workaround, aby se nemuselo
používat ->hasErrors()
a zároveň to, že to vygeneruje
warning/error je pro programátora dobré, protože ví, že si to musí nějak
ošetřit.
Co ale lidi, kteří preferují přístup pomocí návrhového vzoru null object, nebo s NULL
pracují jako s nedefinovaným objektem?
Rozumíš, jakože ti, co dělají návrh aplikace tak, aby když je hodnota
nevalidní, tak aby to nevadilo a pracují s tím stejně, jako když je
hodnota validní, čímž se vyhnou ifování.
Pro ně toto zavedení bude znamenat to ifování přidat ne?
EDIT: 1
Já osobně používám jiné metody než onSuccess
velice
zřídka, protože všechno píšu výchozími Nette validačními pravidly.
Takže pro mě to nemá význam, jestli se to udělá tak, nebo tak. Jen se chci
na to podívat i očima jiných lidí, kterým koukám do kódu.
Editoval Polki (25. 1. 2021 18:43)
- mkoula
- Backer | 57
Tak mě tohle úplně rozsekalo aplikaci a budu muset downgradnout na 3.0 a zjistit jak to vůbec přepsat. Formulář sám o sobě nevrací konkrétní chybu, jen že má chyby a hlášku, ale ne dané prvky.
Na onValidate jsme měli navěšený vlastní validátor, který projel form, našel chyby, dogeneroval k nim snippety s přeloženýma chybovýma hláškama…
Jenže se tam počítalo se \stdClass $values v parametru → teď to budu muset nějak rozdělit, pokud bude chyba, byť jen nezaškrtnutého checkboxu, tak mi to nepředá values. Ok vyhodil jsem je z parametru, ale nevrátí je ani getValues(), takže to teď musím nějak rozdělit na dvě části – chyba ve formuláři a pak naše validace, třeba i dynamických dat…
Změnu chápu, ale pro mě dosti zásadní break :)
- David Grudl
- Nette Core | 8218
Takže bylo by lepší řešení, aby se $values, jen bez předalo bez nevalidních položek?
- mkoula
- Backer | 57
@DavidGrudl
Na to je těžké odpovědět. Pro mě to je BC break, protože pro mě
onValidate[] znamenalo možnost poupravení validace, jelikož nepoužíváme
validaci přes netteForms.js, ale formulář se posílal ajaxem na server a
validoval na backendu a zpátky se posílala data pro zobrazení chyb /
případně success data.
Vzhledem k tomu, že třeba v našem případě se hledají letenky v rámci formuláře a posílají se nějaká data, která nejsou součástí formuláře, tak jsme je validovali právě přes metodu v onValidate(), kde jsme si je donačetli do values a dále vše zpracovali.
Bohužel všude byla druhý parameter \stdClass $values – nenulová :-D Takže mi to vyhodí výjimku už při zavolání té metody. A to je pro mě ten BC break. Samozřejmě, že to dokážu upravit v parametru na ?\stdClass, ale pak s tím v té metodě pracuju, takže mi vyhazuje výjimku dále…
Za sebe, já to dokážu nějak přepsat, jen to máme takhle víceméně ve všech formulářích… Takže to není úplně jednoduché. Nedokážu říct, kolika projektů se to dotkne, ale jestli to zabíjí i datagrid :-D, tak to je to na diskuzi…
Editoval mkoula (3. 2. 2021 23:32)
- David Grudl
- Nette Core | 8218
Tu diskuzi se právě snažím vést a dojít k nějakému řešení.
Primárně jde o to, že se snažím řešit potenciální slabé místo, které by mohlo případně vést k nějaké zranitelnosti. Myslím si, že je zkrátka dobré, aby se programátor dozvěděl, že sahá na nevalidní data. Třeba formou warningu. Samozřejmě jde o BC break, ale žádoucí. Teď jde o to, jak tohle provést. Ptal jsem se Polkiho, ptám se tebe, jestli je lepší řešení, aby onValidate() dostával $values jako předtím, jen bez nevalidních položek?
- mkoula
- Backer | 57
Za mě asi ano, protože onSuccess neprojde, chyby si dokážeš dohledat, ale máš ty values, na kterých si pak validuješ i třeba ty další dynamická data.
Já mám checkbox na umožnění hledání letenek – ten pokud je zaškrtnutý, tak se mi ajaxem otevře možnost je hledat a vybrat si (detaily letenek jsou tudíž dynamická data a nejdou přes validaci formuláře), pak mám checkbox na podmínky – must have. Když někdo nezaškrtne podmínky a klikne, nedostanu values a nejsem tudíž schopný si validovat ty svoje věci mimo formulář – jenže podmínky jsou až dole a letenky nad nimi.
Validuju to právě jen ajaxem na backendu a getErrors() vrátí jen chybové hlášky, kdežto já si přes onValidate projedu formulář a do payloadu si vrátím id všech elementů, kde je chyba, takže přes Javascript měním třeba barvy u elementů, plus mám okolo nich i snipetty, které generují chybové hlášky (které jsou jak z formulářové, tak mé navázané validace).
Pro mě prostě onValidate byla metoda, kde jsem si já nezávisle na frameworku mohl dále funkcionlitu validace rozšířit, a absence values (i nezvalidovaných) mě to úplně neumožňuje… Ale třeba to jen já chápu špatně no :-D
Editoval mkoula (4. 2. 2021 2:44)
- David Grudl
- Nette Core | 8218
Tak to udělám tak, že chování onValidate vratím do původní podoby a řešení této věci nechám do další verze.
- account23
- Člen | 36
@DavidGrudl
a čo tak to skúsiť skĺbiť? Stačí aby hodnota $value
a
rovnako aj getValues()
vracal rovnako ArrayHash ako doteraz,
akurát hodnota toho kľúča ktorý neprešiel validátorom by bol null.
Ale celý čas som myslel že to tak v nette funguje, nepotrebuje mi nejaký
warning oznamovať že robím s nevalidnimi dátami, keď už budú po prvej
validácii :)
Rovnako ako ostatný čo tu komentovali aj ja v onValidate
očakávam že príde ten ArrayHash object kde si jeho hodnoty porovnávam, keď
mi tam príde iba null tak som v prdeli :) no keď mi príde null v samotnej
property tak som v cajku, napr:
['email' => null, 'name' => null, 'surname' => 'Musk']
Editoval account23 (5. 2. 2021 2:53)
- mkoula
- Backer | 57
Na úpravě je problém, když máš druhou proměnnou u onValidate() metody $values. I když jí nastavím jako null, tak mi stejně vyskakoval warning ala že injected $values…
Já osobně jsem to už začal přepisovat na onValidate() kde řeším jen formulář a v něm v podmínce pokud není error si stáhnu $values a v jiné metodě validuju všechny hodnoty tam…
Snad to dnes všude dopřepíšu :-D
- Darkling
- Člen | 35
@DavidGrudl objevil jsem ještě jeden problém s tímto novým
chováním. Pokud je na jeden formulář navěšeno více callbacků
onValidate[]
, a například v prvním callbacku se přidá
$form->addError()
, tak k zavolání ostatních callbacků
vůbec nedojde a skončí to vyhozením
User Warning: invoked but the form is not valid
(https://github.com/…ontainer.php#L199).
Sice chápu čeho chce tahle úprava dosáhnout, ale nenechat formulář
sáhnout na svoje values mi minimálně v metodě onValidate
nedává smysl. Pokud tedy příjdou values, kde budou vynechány nevalidní
data, tak je to asi ok, ale vyhozovat warning mi nepřijde ok.
Editoval Darkling (10. 2. 2021 15:22)
- David Grudl
- Nette Core | 8218
Doplnil jsem metodu getUntrustedValues()
, která na rozdíl od
getValues()
nekontroluje validnost data a tedy vyhazuje žádné
varování.
- Polki
- Člen | 553
David Grudl napsal(a):
Doplnil jsem metodu
getUntrustedValues()
, která na rozdíl odgetValues()
nekontroluje validnost data a tedy vyhazuje žádné varování.
Dobře vyřešeno.
Akorát se musím zeptat.
Druhý parametr callbacku se plní daty pomocí metody getValues()
,
nebo getUntrustedValues()
?
Jde o to, že když si v projektech konvertovaných na nové Nette budu
nechávat předávat Values jako druhý parametr, tak jestli budu muset všude
parametr odebrat a nahradit ho až uvnitř metody, aby mi to neplnilo tracy log
hromadou warningů, nebo jestli to normálně projde jako doposud?
- David Grudl
- Nette Core | 8218
onValidate dostává data z getUntrustedValues(), takže warningy negeneruje.
- radas
- Člen | 224
Ahoj, možná ještě jeden pohled na věc a co tento BC break rozbíjí u mě. Občas mám formulář s položkami, které mají nastavenou povinnost vyplnění, ale zároveň dvě tlačítka. Jedno pro uložení konceptu a druhé pro odeslání dat. Přičemž při uložení konceptu jsem pomocí setValidationScope() měl nastaveno, aby položky prošly i bez vyplnění (u konceptu nejsou povinné). BC break způsobuje, že data odeslaná tímto tlačítkem ve výsledku nejsou. Jako workaround jsem nechal položky nepovinné a validaci přesunul do onValidate, ale zase se tím přijde o JS validaci.
Původní kód:
$form->addTextArea('description', 'Popis')
->setRequired();
$form->addMultiUpload('files', 'Přílohy')
->addRule($form::MIME_TYPE, ...);
$form->addSubmit('save', 'Uložit')
->setValidationScope([
$form['files'],
...
]);
$form->addSubmit('finish', 'Ukončit')
->setValidationScope([$form['description']]);