Form::setOptional() versus required

kucix napsal(a):

@DavidGrudl Co takhle addRule (v případě, že není setRequired) provádět jen v případě, že řetězec není prázdný.

• Pokud bude mít form jen addRule(Form::EMAIL), tak prvek nebude povinný a na typ e-mail se bude validovat jen v případě, že tam uživatel něco napíše
• Pokud někdo vyžaduje zadat prvek, musí říct setRequired()

IMHO je to transparentnější, ale asi by to spoustu webů rozbilo.
Možná nějaký přepínač chování s tím, aby to současné bylo defaultní?

kucix napsal(a):

@ali Já vím, taky to v podstatě píšu, že by to spoustu webů rozbilo

Proto by asi bylo dobré mít nějaký přepínač.

Dokážu si představit mít ve Formu přepínač $requiredDefault
Teď by byl na true a chovalo by se to stejně, jako teď.
Mohl bych si ho pro konkrétní formulář nastavit na false ($form->setRequiredDefault(false);) a chovalo by se to tak, jak popisuji.
A v Nette 3 by se tento přepínač dal ve výchozím stavu na false, takže u starých typů formulářů by stačilo přidat řádek $form->setRequiredDefault(true);

Teď by to tedy nebyla pro přechod žádná změna, jen by si mohl někdo psát už pro nové chování.
A ve trojce by zase byla možnost návratu.

Samozřejmě domyšlené do konce by bylo, že by se v config.neon dala přidat direktiva pro form, která by zapínala nové chování globálně nad všemi formy s tím, že by se pro konkrétní Form dalo chování přepnout.
Platí to, co jsem psal výše, v Nette 2.4 by bylo výchozí chování to současné a v Nette 3 to nové.
Přepínač v configu a setterem nad konkrétním formem.

David Grudl napsal(a):

@kucix chování „výchozí je optional“ možné není kvůli masivnímu BC breaku, ale vlastně bych ho ani nechtěl, v podstatě jde tak trochu proti logice Nette :) Totiž, ověřil jsem si na školeních, že spousta i úplně začínajících programátorů předpokládá prvek s jedním addRule pravidlem za povinný. Nette by v takovém případě mělo jít tou přísnější cestou a za povinný ho považovat taky (tedy alespoň efektivně povinný). Aby nevzniklo nepříjemné překvapení. Zkrátka nepovinný prvek by měl znamenat „více psaní“.

Také se mi zdá, že v praxi jsou mnohem častější prvky povinné, než nepovinné. Takže i v tomhle je efektnější, když více psaní představuje prvek nepovinný.

Teď, když se dalo setRequired(), tak jako parametr se předala hláška. Nyní by to mělo být tedy setOptional … to bude předpokládám bez parametru, protože by neměl smysl. A stejně budu muset někde hlášku, že je input povinný, nastavit. Na druhou stránku chápu, že pokud zadám Form::EMAIL, neměl by projít prázdný, ale pouze pokud je v něm zapsán validný email.

Opět jsou ale i obrácené situace, kdy se doopravdy validuje jen to, co je zapsáno. Čili prázdný email jako volitelná kolonka je ok, ale v případě že do ní něco napíšu, musí být správně.

Každopádně i z pohledu html kde se příznak required musí přidat, mi přijde přirozenější mít výchozí prvky volitelné.

Editoval GEpic (9. 6. 2016 9:50)

@kucix Stejně pokud chci použít podmíněnou povinnost tak narazím.
Co kdybych chtěl podmínit povinnost na nějaký sadě form. prvků? např. vyplnim tenhle input, druhej input bude mít hodnotu 3 atd.
Pokud by se tam dal třeba callback ->setOptional([$this, 'emailRequired']) (nevím jestli funguje i u addCondition) tak by se to třeba dalo…

David Grudl napsal(a):

@kucix Totiž, ověřil jsem si na školeních, že spousta i úplně začínajících programátorů předpokládá prvek s jedním addRule pravidlem za povinný. Nette by v takovém případě mělo jít tou přísnější cestou a za povinný ho považovat taky (tedy alespoň efektivně povinný). Aby nevzniklo nepříjemné překvapení. Zkrátka nepovinný prvek by měl znamenat „více psaní“.

Pokud ten začínající programátor nezná metodu setRequired(), umím si představit, že předpokládá, co píšeš. Jakmile ale znáš setRequired(), nemá takovéto chování absolutně žádnou logiku. Považovali jsme to dokonce za bugu a upravili si pravidla tak, aby se to takto zvláštně nechovalo. Hodně lidí v tom do té doby dělalo chyby, odstranili setRequired() a čekali, že pole nebude povinné.

Nikdy by me nenapadlo, ze

$form->addText('email')->addRule($form::EMAIL);

automaticky znamena i to, ze je pole povinne. Vzdy jsem dodaval setRequired(). Jsem ale zacatecnik, muj nazor nebude moc zajimavy.

jannek19 napsal(a):

@PetrHH ono tak úplně povinné není – tato definici jen říká, že do pole půjde zadat jenom platnou emailovou adresu – a prázdný řetězec (nevyplněné políčko) prostě validní emailová adresa není. Nejde tedy o povinné/nepovinné pole, jako spíš o validní/nevalidní hodnotu.

---

IMHO bych na současném chování nic neměnil, dělá přesně to, co se mu řekne a na tom nevidím nic špatného. Naopak si myslím, že zde navrhovaná řešení by celou problematiku formulářových pravidel akorát zkomplikovala a člověk by nad tím musel zbytečně přemýšlet.

Ono tak úplně povinné je. Už vidím, jak vysvětluješ uživateli, že do toho nepovinného pole e-mail při registraci musí zadat e-mail, i když to pole je nepovinné, protože je to implementované pomocí Nette a to na validaci platné e-mailové adresy používá regulární výraz, který prázdné pole vyhodnotí jako neplatnou e-mailovou adresu. Takže ten uživatel vlastně neměl to pole vyplňovat a on ho vyplnil prázdným řetězcem, takže je to vlastně jeho chyba :-D Sorry, ale vždyť je to padlé na hlavu :-)

Protiřečíš si v tom, že nad současným řešením člověk nemusí přemýšlet. Právě že musí, musí se zamyslet, jak je validace udělaná a pak mu dojde, že i když pole jako povinné nenastavil, tak povinné bude a musí přemýšlet, jak to udělat, aby se toho zbavil a možná při tom přemýšlení objeví metodu addCondition. V praxi pochybuju, že to někdo jen tím přemýšlením rozlouskne, spíš bude psát někde do nějakého fóra nebo googlit. Je to naprosto neintuitivní.

Nemyslím si, že máš pravdu. To není chyba uživatele, ani chyba Nette, jen chyba programátora, který špatně udělal svojí práci a nesprávně nastavil validační pravidla.

Jak říkám, validace dělá přesně to, co se jí řekne. Buď jí řeknu „hodnota musí být emailová adresa“ a pak v žádném případě nechci, aby podobnou validací prošla prázdná (nevalidní) hodnota, nebo jí řeknu „pokud je hodnota zadána, musí to být emailová adresa“.

Navíc fakt nevidím rozdíl mezi tím, jestli uvedu ->addCondition(FILLED)->addRule(EMAIL), nebo ->addRule(EMAIL)->setOptional(TRUE), nebo něco jiného, abych dosáhl toho, co už je teď stejně možné.

EDIT: Šaman byl rychlejší :)

Editoval jannek19 (9. 6. 2016 16:09)

A co vyřešit to tak, že při přidávání validačních pravidel, o kterých Nette předem ví, že budou odmítat prázdnou hodnotu (jako je Form::EMAIL), Nette zjistí, zda bylo setRequired voláno, a pokud ne, volat setRequired(TRUE) automaticky a zobrazit noticku o tom, že v kódu toto volání patrně chybí.

Šaman napsal(a):

Uživatel s tím nemá nic společného. Tohle je věc programátora. A ten by snad měl vědět, že když zapíše požadavek „toto pole musí obsahovat validní email“, tak nevyplněné neprojde.

To byla hlavně reakce na Jannkovu větu, že pole vlastně není povinné. Je to věc programátora. Otázka je, jestli by to měl vědět. Jestli to není spíš typická gotcha. Kromě lidí tady z komentářů neznám nikoho, kdo by to považoval za jasné a každý se to naučil obcházet až po praktické negativní zkušenosti z terénu.

Pokud by se implementovalo tvé řešení, tak když si vytvořím vlastní validaci, tak mi ji prázdné pole taky bypassne?

Pravda, to je blbé :) Tady BC break zůstává. Ale jako kompromisní řešení s nějakou informací v Nette 3 release notes by mi to přišlo v pohodě.

Aha, jasně, pro uživatele nastane problém, že pole není required podle HTML 5.

Ano a není u něj třeba hvězdička, takže uživatel netuší, že je povinné.

Těžko říct, co s tím, protože u některých vlastních pravidel může nastat stejný problém – a Nette netuší, jestli prázdná hodnota proje, či ne.
Asi bych skousl řešení povinného setRequired(TRUE|FALSE) u každého pole s nějakým rule.

Osobně jsem vždycky tohle bral jako fakt a nedokonalost Nette a naučili jsme se s tím žít. Až teď jsem na to reagoval kvůli tomu, že to sám @DavidGrudl zmínil jako problém, který se mu nedaří vyřešit. A když zvážíš, jak se kolem toho motáme a jaké emoce to vyvolává, tak to je jasná známka toho, že to API není zrovna ideální :-)

Pokud se to nezmění, tak s tím budu žít dál. Ale osobně se nemůžu smířit s postojem, že je to takhle vlastně naprosto v pořádku. To mi přijde jako „líný“ názor.

jannek19 napsal(a):

Nemyslím si, že máš pravdu…

Asi bych souhlasil, pokud by neexistovala metoda setRequired(), která programátorovi naznačuje, že chce-li aby pole bylo povinné, má ji použít. A pak existují nějaká rules, která říkají, jak se má validovat zadaný obsah a ta pravidla neřeší jestli pole je nebo není vyplněné, protože to už je vyřešené pomocí setRequired.

V podstatě osobně vůbec neřeším toho uživatele ani nějaký atribut v html.

Zařídili jsme se tak, aby to fungovalo, jak potřebujeme. Ale zkušenost je taková, že to API není intuitivní a dělá něco jiného, než jsme čekali a obecně tě pořád nutí přemýšlet a při změně jednoho požadavku (z povinného na nepovinné a naopak) to měnit na více místech. Což může způsobit krásné problémy při práci více lidí ve více větvích. Jeden má za úkol udělat z povinného pole nepovinné a jiný zase pro něj napsat validaci. Při mergování to bude na první pohled vypadat oukej, ale ono né :-)

Jako příklad uvedu tento „konzistentní“ zápis. Jak se vám líbí?

$f->addText("name");

$f->addText("nameRequired")
	->setRequired();

$f->addText("email")
	->addCondition(Form::FILLED)
	->addRule(Form::EMAIL);

$f->addText("emailRequired")
	->addRule(Form::EMAIL);

A nebo:

$f->addText("max10len")
	->addRule(Form::MAX_LENGTH, '', 10);

$f->addText("min10len")
	->addCondition(Form::FILLED)
	->addRule(Form::MIN_LENGTH, '', 10);

Aby to konzistentní bylo, tak by se to mělo upravit na:

$f->addText("name");

$f->addText("nameRequired")
	->setRequired();

$f->addText("email")
	->addCondition(Form::FILLED)
	->addRule(Form::EMAIL);

$f->addText("emailRequired")
	->setRequired()
	->addCondition(Form::FILLED)
	->addRule(Form::EMAIL);

A nebo:

$f->addText("max10len")
	->addCondition(Form::FILLED)
	->addRule(Form::MAX_LENGTH, '', 10);

$f->addText("min10len")
	->addCondition(Form::FILLED)
	->addRule(Form::MIN_LENGTH, '', 10);

Jinými slovy: Před KAŽDÉ pravidlo vždy a všude umístit ->addCondition(Form::FILLED). Jakmile toto před pravidlem není, máme bugu, možná skrytou, protože se maskuje s jinou, ale je tam a může vyplavat jako třeba teď vyplavala s HTML5. A proč, když to máme otrocky psát všude, to nezahrnout přímo do vlastností těch pravidel: „Pravidla se aplikují jen na nějaký obsah, ne když je pole nevyplněné.“

Problém s BC breakem samozřejmě zůstává.

Editoval josef.sabl (9. 6. 2016 16:43)

Aby nedošlo k mýlce, naprosto chápu proč to řešíte, ale nepociťuji to jako reálný problém, resp. nezastávám názor, že by se kvůli tomu musel přiohýbat systém validace.

Validace teď funguje přesně tak, jak programátor očekává. Pokud očekává něco jiného, měl by si doplnit znalosti a nenechávat se strhávat dojmy.

Samozřejmě, když chci skutečně povinné pole, volám setRequired(), to ale nesouvisí s validací, která teď funguje přísně logicky (dělá jen to co se jí řekne) a to je fajn.

BTW moc si nelíbí ani jedna z posledních změn, díky které validační pravidla ovlivňují input[type], to je podle mě už docela za čárou – tohle by validační pravidla ovlivňovat neměla, ta by měla jen validovat.

EDIT: další diskuse se bohužel nezúčastním, budu pár dní pryč.

Editoval jannek19 (9. 6. 2016 17:39)

Můj osobní názor je, že při ->addRule(Form::EMAIL); – či Form::NUMBER by se nemělo nastavovat type="email/number", protože v tomto je implementace HTML5 v prohlížečích natolik odlišná, že to je dle mého názoru nepoužitelné. Jak napsal @josef.sabl, to už by bylo lepší přidat addEmail, addNumber, atd…

Další věcí je, že povinné pole by mělo být pouze v případě, že nastavím ->setRequired();. Při addRule by se rozhodně nemělo stávat povinné pole (validovat pouze když je vyplněno).

Kód pak bude jasný a zřetelný.

Co přidat něco jako

$form->addText('email', 'Email:')
	->ifFilled()   // jako addCondition($form::FILLED)
		->addRule($form::EMAIL);

whenFilled

Mě se líbí ifFilled, protože napíšu iff prásknu do enteru a bum, je to tam :-)

jannek19 napsal(a):

IMHO bych na současném chování nic neměnil, dělá přesně to, co se mu řekne a na tom nevidím nic špatného. Naopak si myslím, že zde navrhovaná řešení by celou problematiku formulářových pravidel akorát zkomplikovala a člověk by nad tím musel zbytečně přemýšlet.

Souhlasím, ale je fakt, že ifFilled() usnadní život :)

Editoval kleinpetr (15. 6. 2016 22:11)

Podle délky diskuse je vidět, že je to skutečně složité téma.

Nejprve je nutné si stanovit, že žádný BC break s obrovským dopadem nepřipadá v úvahu. Tedy například možnost, že by prvek, který nemá nastaveno setRequired(), se stal nepovinným. Tohle si nemůžeme v žádném případě dovolit a tím pádem není o tom potřeba vůbec diskutovat.

Doplnění: chtěl jsem říct, že není možné se dopustit obřího BC breaku, ale samozřejmě lze začít třeba emitovat varování a postupně s dlouhodobým výhledem chování měnit pro příští verze.

Taky je potřeba připustit, že kvůli kompatibilitě nemůžou existovat jen dva stavy, povinný / nepovinný, ale i třetí mezi stav, současný default, kdy se prostě vyhodnotí validační pravidla i u nevyplněného prvku.

Otázkou tak zůstává jen to, jak mezi těmito stavy přepínat ve verzi 2.4.

Pro nepovinnost buď 1a) zavést novou metodu (setOptional, ifFilled, apod, žádný BC break), nebo 1b) setRequired(FALSE), což možný BC break je. Lze jít cestou 1c), kdy by ve verzi 2.4 setRequired(FALSE) vyhodilo varování, že význam se mění, takže funkčnost by byla dostupná až v příští verzi.

A obráceně pro povinnost buď 2a) ponechat současný stav, nebo 2b) při existenci pravidla (mimo podmínku) vyhodit varování, že chybí setRequired, nebo 2c) jej v takovém případě automaticky nastavit.

josef.sabl napsal(a):

Pokud u políčka bude použité pravidlo, které způsobí, že pole je efektivně povinné (např. Form::EMAIL ano, ale Form::MAX_LENGTH ne), nebude aplikované až po addCondition(Form::FILLED) a pole zároveň nebude setRequired, tak se vyhodí notice/deprecated podobně jako u jiných bc break změn. Programátor se doví, že má buďto přidat required nebo bude v příští verzi jeho políčko nepovinné.

Upozornit, že chybí setRequired(), mi připadá rozumné. Když budou mít všechny prvky explicitně nastavené, zda jsou povinné či nikoliv, je případně možné v budoucnu změnit výchozí chování.

Dělit validační pravidla na „efektivně povinné“ a nepovinné je docela problematické (obzvlášť kvůli vlastním pravidlům), takže bych to spíš vztáhl na všechny pravidla bez nějaké magie.

A tahle (dle mého skromného názoru) chybka v návrhu se opraví s pozdější verzí. setOptional do toho dle mého vnese další zmatek a chování bude mezi těmi jednotlivými metodami tak propojené, že znemožní např. algoritmické generování formů.

Nějaká možnost označení prvku jako nepovinného existovat musí. To, že se třeba v příští verzi stane nadbytečnou, je jiná věc. Existence metody setOptional společně se setRequired naznačuje, že prvek ve 2.4 není defaultně ani optional, ani required, což tak skutečně je, takže bych se tomu nebránil.

Za mě bych připravil programátory na tento přesun tím, že ve verzi 2.4, bych dal trigger error, když se u addEmail neobjeví setRequired. Ve verzi 3.0 nebo klidně později (někteří uživatelé překročí 2.4), která by byla od php 7.0, bych udělal BC break (předpokládám, že těch BC breaků bude více). ifFilled se mi nelíbí, ještě mě napadla možnost filledCondition, ale je to moc dlouhé pro lenivce.

Editoval Antik (20. 6. 2016 19:39)

Protože nejde o prázdný řetězec, prázdná hodnota může u jakéhokoliv prvku vypadat jinak, třeba UploadControl vrací objekt FileUpload na stavením error na UPLOAD_ERR_NO_FILE apod.