Forms 3.1 a varování při getValues()

David Grudl
Nette Core | 7445
+
+1
-

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 onValidate se jako $values předá null, pokud je formulář nevalidní. A všem handlerům včetně onSuccess a onClick se ve $values předávají jen validované prvky, tedy podle omezení setValidationScope.

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 | 303
+
0
-

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 | 7445
+
0
-

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 | 303
+
0
-

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. 21:04)

David Grudl
Nette Core | 7445
+
0
-

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 | 303
+
0
-

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. 18:43)

mkoula
Generous Backer | 43
+
+1
-

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 | 7445
+
+1
-

Takže bylo by lepší řešení, aby se $values, jen bez předalo bez nevalidních položek?

mkoula
Generous Backer | 43
+
+1
-

@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. 23:32)

David Grudl
Nette Core | 7445
+
0
-

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
Generous Backer | 43
+
0
-

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. 2:44)

David Grudl
Nette Core | 7445
+
+11
-

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
+
0
-

@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. 2:53)

David Grudl
Nette Core | 7445
+
0
-

@account23 teď úplně nevím jak se to liší od toho, co jsem tu psal?

account23
Člen | 36
+
0
-

@DavidGrudl no tak možno som niečo v tejto diskusii nepochopil, ale z toho čo som skúšal, tak po odoslaní formuláru mi $values prišiel ako null a teda napr. $values->name bol nedefinovaný. Asi je to to čo myslíš v komente #p214169 ..

mkoula
Generous Backer | 43
+
0
-

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

David Grudl
Nette Core | 7445
+
0
-

Nerozumím, co je to „warning ala že injected $values“?

Darkling
Člen | 35
+
0
-

@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. 15:22)

David Grudl
Nette Core | 7445
+
0
-

Ve verzi 3.1.2 je to opravené.

David Grudl
Nette Core | 7445
+
+1
-

Doplnil jsem metodu getUnsafeValues(), která na rozdíl od getValues() nekontroluje validnost data a tedy vyhazuje žádné varování.

Polki
Člen | 303
+
0
-

David Grudl napsal(a):

Doplnil jsem metodu getUnsafeValues(), která na rozdíl od getValues() 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 getUnsafeValues()?
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 | 7445
+
+2
-

onValidate dostává data z getUnsafeValues(), takže warningy negeneruje.

radas
Člen | 217
+
+2
-

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']]);