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

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?

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)

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)

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

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?

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

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

@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)

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

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