Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Form define $fields and $actions as optional but then it throws an error if you don't define them #8459

Closed
1 task done
maxime-rainville opened this issue Oct 7, 2018 · 3 comments

Comments

@maxime-rainville
Copy link
Contributor

maxime-rainville commented Oct 7, 2018

Form constructor says it's OK to not specify a $fields and $actions and have them default to null.

However, the first thing it does is try to call setForm on our potentially null values. That's not consistent. Also, setFields() and setActions(), do not do the setForm call.

public function __construct(
RequestHandler $controller = null,
$name = self::DEFAULT_NAME,
FieldList $fields = null,
FieldList $actions = null,
Validator $validator = null
) {
parent::__construct();
$fields->setForm($this);
$actions->setForm($this);

Pull Request

@robbieaverill
Copy link
Contributor

Yeah needs a cheeky if in there really

@phptek
Copy link

phptek commented Dec 18, 2018

Cool. Just stumbled into this one too.

phptek pushed a commit to phptek/silverstripe-framework that referenced this issue Dec 18, 2018
phptek pushed a commit to phptek/silverstripe-framework that referenced this issue Dec 18, 2018
- Missing conditionals for optional constructor args
- Missing guards when form instantiated prior to calling setFields()/setActions()
@phptek
Copy link

phptek commented Dec 18, 2018

See PR at #8683

[EDIT] PR now at #8684 (Fixes missing calls to setForm() as @maxime-rainville pointed out.)

phptek pushed a commit to phptek/silverstripe-framework that referenced this issue Dec 18, 2018
phptek pushed a commit to phptek/silverstripe-framework that referenced this issue Dec 18, 2018
- Missing conditionals for optional constructor args
- Missing calls to FieldList::setForm()
- Missing guards around naive calls to Form::Fields()->foo()
phptek pushed a commit to phptek/silverstripe-framework that referenced this issue Dec 18, 2018
- Missing conditionals for optional constructor args
- Missing calls to FieldList::setForm()
- Missing guards around naive calls to Form::Fields()->foo()
phptek pushed a commit to phptek/silverstripe-framework that referenced this issue Dec 19, 2018
- Missing conditionals for optional constructor args
- Missing calls to FieldList::setForm()
- Missing guards around naive calls to Form::Fields()->foo()
maxime-rainville added a commit that referenced this issue Dec 19, 2018
FIX: Fixes #8459 Missing conditionals for optional constructor args
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants