ENH Various changes to support SiteTree form field scaffolding #11327
Conversation
GuySartorelli
force-pushed
the
pulls/6/scaffold-sitetree
branch
2 times, most recently
from
9ee0b8c to
7394956
Compare
| */ | ||
| public $includeRelations = false; | ||
| public bool|array $includeRelations = false; |
There was a problem hiding this comment.
Looking at the code that uses this, you could use it to explicitly only allow has_many relations but not many_many relations (or vice versa) - in which case it needs to be an associative array. Hence bool|array.
There was a problem hiding this comment.
Added strict typing for the properties as a matter of course - it's unrelated refactoring.
| if ($this->restrictFields && !in_array($fieldName, $this->restrictFields ?? [])) { | ||
| if (!empty($this->restrictFields) && !in_array($fieldName, $this->restrictFields)) { |
There was a problem hiding this comment.
Must be an array, so we can tidy this a little now. Applies to similar changes below as well.
GuySartorelli
force-pushed
the
pulls/6/scaffold-sitetree
branch
2 times, most recently
from
05d5b9f to
0f6c387
Compare
src/Forms/Tab.php
Outdated
| public function setName($name) | ||
| { | ||
| if ($this->ID() === Convert::raw2htmlid($this->getName())) { | ||
| $this->setID(Convert::raw2htmlid($name)); | ||
| } | ||
| return parent::setName($name); | ||
| } |
There was a problem hiding this comment.
This ensures the id attribute in the HTML markup correctly reflects the name, even if the name is set after the tab was created. This is needed by https://github.com/silverstripe/silverstripe-userforms/pull/1315/files#diff-d659fc65ecb263c36d5428815131e80ef3c1750bf18e18860f1fd9d79d4df3e0R195 because the relation name doesn't match the name we want for the tab.
It's also just a sensible change to make.
Note that we don't set the ID if the ID was manually set to something other than the name.
GuySartorelli
force-pushed
the
pulls/6/scaffold-sitetree
branch
from
0f6c387 to
7591b6f
Compare
GuySartorelli
changed the title
src/ORM/DataObject.php
Outdated
There was a problem hiding this comment.
Changes here are to support the new strict types, and removing the deprecated ajaxSafe
GuySartorelli
force-pushed
the
pulls/6/scaffold-sitetree
branch
from
7591b6f to
787b4cc
Compare
GuySartorelli
force-pushed
the
pulls/6/scaffold-sitetree
branch
from
787b4cc to
ae8088a
Compare
| public function testNameAndID(): void | ||
| { | ||
| // ID is set on instantiation based on the name | ||
| $tab = new Tab('MyName _-()!@#$'); | ||
| $this->assertSame('MyName _-()!@#$', $tab->getName()); | ||
| $this->assertSame('MyName_-', $tab->ID()); | ||
|
|
||
| // Changing the name changes the ID | ||
| $tab->setName('NewName'); | ||
| $this->assertSame('NewName', $tab->getName()); | ||
| $this->assertSame('NewName', $tab->ID()); | ||
|
|
||
| // If ID is explicitly set, changing the name doesn't override it | ||
| $tab->setID('Custom-ID'); | ||
| $tab->setName('AnotherName'); | ||
| $this->assertSame('AnotherName', $tab->getName()); | ||
| $this->assertSame('Custom-ID', $tab->ID()); | ||
| } |
There was a problem hiding this comment.
Can't use a data provider for this - the scenarios need to be run sequentially.
GuySartorelli
force-pushed
the
pulls/6/scaffold-sitetree
branch
from
ae8088a to
535cb46
Compare
Issue