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

API Set extension hook implementation visibility to protected #1190

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/Extensions/ElementalAreaUsedOnTableExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class ElementalAreaUsedOnTableExtension extends DataExtension
*
* @var array $excludedClasses
*/
public function updateUsageExcludedClasses(array &$excludedClasses)
protected function updateUsageExcludedClasses(array &$excludedClasses)
{
$excludedClasses[] = ElementalArea::class;
}
Expand All @@ -31,7 +31,7 @@ public function updateUsageExcludedClasses(array &$excludedClasses)
* @param bool $excludeDataObject
* @param DataObject $dataObject|null
*/
public function updateUsageDataObject(?DataObject &$dataObject)
protected function updateUsageDataObject(?DataObject &$dataObject)
{
if (!($dataObject instanceof BaseElement)) {
return;
Expand All @@ -51,7 +51,7 @@ public function updateUsageDataObject(?DataObject &$dataObject)
* @param array $ancestorDataObjects
* @param DataObject $dataObject
*/
public function updateUsageAncestorDataObjects(array &$ancestorDataObjects, DataObject $dataObject)
protected function updateUsageAncestorDataObjects(array &$ancestorDataObjects, DataObject $dataObject)
{
if (!($dataObject instanceof BaseElement)) {
return;
Expand Down
4 changes: 2 additions & 2 deletions src/Extensions/ElementalAreasExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ public function getElementalRelations()
*
* @param FieldList
*/
public function updateCMSFields(FieldList $fields)
protected function updateCMSFields(FieldList $fields)
{
if (!$this->supportsElemental()) {
return;
Expand Down Expand Up @@ -218,7 +218,7 @@ public function updateCMSFields(FieldList $fields)
/**
* Make sure there is always an ElementalArea for adding Elements
*/
public function onBeforeWrite()
protected function onBeforeWrite()
{
parent::onBeforeWrite();

Expand Down
2 changes: 1 addition & 1 deletion src/Extensions/ElementalCMSMainExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class ElementalCMSMainExtension extends Extension
*
* @param Form $form
*/
public function updateSearchForm(Form $form)
protected function updateSearchForm(Form $form)
{
/** @var DropdownField $filterField */
$filterField = $form->Fields()->fieldByName('Search__FilterClass');
Expand Down
2 changes: 1 addition & 1 deletion src/Extensions/ElementalPageExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public function getContentFromElementsForCmsSearch(): string
/**
* @see SiteTree::getAnchorsOnPage()
*/
public function updateAnchorsOnPage(array &$anchors): void
protected function updateAnchorsOnPage(array &$anchors): void
{
if (!($this->owner instanceof SiteTree)) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class GridFieldAddNewMultiClassHandlerExtension extends Extension
/**
* @param Form $form
*/
public function updateItemEditForm(Form $form)
protected function updateItemEditForm(Form $form)
{
// NOTE: this extension is applied to new item edit form only

Expand Down
2 changes: 1 addition & 1 deletion src/Extensions/GridFieldDetailFormItemRequestExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
*/
class GridFieldDetailFormItemRequestExtension extends Extension
{
public function updateBreadcrumbs($crumbs)
protected function updateBreadcrumbs($crumbs)
{
$record = $this->owner->getRecord();

Expand Down
4 changes: 2 additions & 2 deletions src/GraphQL/Resolvers/Resolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@ public static function resolveAddElementToArea(
// Assign the parent ID directly rather than via HasManyList to prevent multiple writes.
// See BaseElement::$has_one for the "Parent" naming.
$newElement->ParentID = $elementalArea->ID;
// Ensure that a sort order is assigned - see BaseElement::onBeforeWrite()
$newElement->onBeforeWrite();
// Ensure that a sort order is assigned
$newElement->ensureSortSet();

if ($afterElementID !== null) {
/** @var ReorderElements $reorderer */
Expand Down
12 changes: 7 additions & 5 deletions src/Models/BaseElement.php
Original file line number Diff line number Diff line change
Expand Up @@ -319,16 +319,18 @@ public function write(
return parent::write($showDebug, $forceInsert, $forceWrite, $writeComponents, $skipValidation);
}

protected function onBeforeWrite()
{
parent::onBeforeWrite();
$this->ensureSortSet();
}

/**
* Increment the sort order if one hasn't been already defined. This
* ensures that new elements are created at the end of the list by default.
*
* {@inheritDoc}
*/
public function onBeforeWrite()
public function ensureSortSet()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method has been added so that Resolver.php can call it directly, previously it was calling onBeforeWrite() directly

{
parent::onBeforeWrite();

// If a Sort has already been set, then we can exit early
if ($this->Sort) {
return;
Expand Down
8 changes: 4 additions & 4 deletions src/TopPage/DataExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,23 +56,23 @@ class DataExtension extends BaseDataExtension
*
* @throws ValidationException
*/
public function onAfterWrite(): void
protected function onAfterWrite(): void
{
$this->setTopPage();
}

/**
* Extension point in @see DataObject::duplicate()
*/
public function onBeforeDuplicate(): void
protected function onBeforeDuplicate(): void
{
$this->clearTopPage();
}

/**
* Extension point in @see DataObject::duplicate()
*/
public function onAfterDuplicate(): void
protected function onAfterDuplicate(): void
{
$this->updateTopPage();
}
Expand Down Expand Up @@ -198,7 +198,7 @@ public function withFixedTopPage(int $topPageID, callable $callback)
}
}

public function updateCMSFields(FieldList $fields)
protected function updateCMSFields(FieldList $fields)
{
$fields->removeByName('TopPageID');
}
Expand Down
2 changes: 1 addition & 1 deletion src/TopPage/FluentExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class FluentExtension extends DataExtension
'TopPageLocale' => 'Varchar',
];

public function updateCMSFields(FieldList $fields)
protected function updateCMSFields(FieldList $fields)
{
$fields->removeByName('TopPageID');
$fields->removeByName('TopPageLocale');
Expand Down
6 changes: 3 additions & 3 deletions src/TopPage/SiteTreeExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class SiteTreeExtension extends BaseSiteTreeExtension
*
* @throws ValidationException
*/
public function onAfterWrite(): void
protected function onAfterWrite(): void
{
$this->setTopPageForElementalArea();
$this->processDuplicationFromOriginal();
Expand All @@ -48,7 +48,7 @@ public function onAfterWrite(): void
*
* @param SiteTree $original
*/
public function onBeforeDuplicate(SiteTree $original): void
protected function onBeforeDuplicate(SiteTree $original): void
{
$this->initDuplication($original);
}
Expand All @@ -60,7 +60,7 @@ public function onBeforeDuplicate(SiteTree $original): void
* @param bool $doWrite
* @throws ValidationException
*/
public function onAfterDuplicate(SiteTree $original, $doWrite): void
protected function onAfterDuplicate(SiteTree $original, $doWrite): void
{
$this->processDuplication($original, (bool) $doWrite);
}
Expand Down
2 changes: 1 addition & 1 deletion tests/Behat/Extension/UniqueHtmlEditorConfigExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
*/
class UniqueHtmlEditorConfigExtension extends Extension implements TestOnly
{
public function updateCMSFields(FieldList $fields)
protected function updateCMSFields(FieldList $fields)
{
$wysiwyg = $fields->dataFieldByName('HTML');
if ($wysiwyg instanceof HTMLEditorField) {
Expand Down
6 changes: 5 additions & 1 deletion tests/Extensions/ElementalCMSMainExtensionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use SilverStripe\Forms\DropdownField;
use SilverStripe\Forms\FieldList;
use SilverStripe\Forms\Form;
use \ReflectionMethod;

class ElementalCMSMainExtensionTest extends SapphireTest
{
Expand All @@ -19,7 +20,10 @@ public function testRemovesDefaultEmptyStringFromClassFilter()
$fields = new FieldList($field);
$form = new Form(null, null, $fields, new FieldList());

$extension->updateSearchForm($form);
// Call extension method ElementalCMSMainExtension::updateSearchForm($form) which is protected
$method = new ReflectionMethod(ElementalCMSMainExtension::class, 'updateSearchForm');
$method->setAccessible(true);
$method->invoke($extension, $form);

$this->assertEmpty($field->getEmptyString(), 'Empty string should be empty');
$this->assertFalse($field->getHasEmptyDefault(), 'Empty string should not have an empty default');
Expand Down
2 changes: 1 addition & 1 deletion tests/Src/TestContentForSearchIndexExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

class TestContentForSearchIndexExtension extends Extension implements TestOnly
{
public function updateContentForSearchIndex(&$content)
protected function updateContentForSearchIndex(&$content)
{
$content = 'This is the updated content.';
}
Expand Down
2 changes: 1 addition & 1 deletion tests/Src/TestElementContentExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class TestElementContentExtension extends DataExtension implements TestOnly
'MyEnum' => 'Enum("Sunny, Cloudy", "Sunny")'
];

public function updateContentForCmsSearch(array &$contents)
protected function updateContentForCmsSearch(array &$contents)
{
$contents[] = 'This content is from an extension hook';
}
Expand Down
Loading