Skip to content

Upgrade to SilverStripe 6#44

Merged
jsirish merged 6 commits into
mainfrom
3.0/ss6-upgrade-clean
Nov 18, 2025
Merged

Upgrade to SilverStripe 6#44
jsirish merged 6 commits into
mainfrom
3.0/ss6-upgrade-clean

Conversation

@jsirish
Copy link
Copy Markdown
Member

@jsirish jsirish commented Nov 18, 2025

Upgrades the module to SilverStripe 6 compatibility.

Changes

  • Updated PHP requirement to ^8.1
  • Updated all core dependencies to SS6-compatible versions:
    • silverstripe/linkfield: ^4 → ^5
    • silverstripe/recipe-cms: ^5 → ^6
    • silverstripe/recipe-testing: ^3 → ^4
    • dnadesign/silverstripe-elemental: ^5 → ^6
    • symbiote/silverstripe-gridfieldextensions: ^4 → ^5
  • Replaced deprecated SilverStripe\ORM\DataExtension with SilverStripe\Core\Extension
  • Set branch-alias to dev-master: 3.0.x-dev

Testing

  • ✅ PHPCS: Clean

This prepares the module for the 3.0.0 release.

- Update PHP requirement to ^8.1
- Update core dependencies to SS6 versions:
  - silverstripe/linkfield: ^4 → ^5
  - silverstripe/recipe-cms: ^5 → ^6
  - silverstripe/recipe-testing: ^3 → ^4
  - dnadesign/silverstripe-elemental: ^5 → ^6
  - symbiote/silverstripe-gridfieldextensions: ^4 → ^5
- Replace deprecated SilverStripe\ORM\DataExtension with SilverStripe\Core\Extension
- Set branch-alias to dev-master: 3.0.x-dev
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR upgrades the module to SilverStripe 6 compatibility, targeting a 3.0.0 release. The main changes include updating PHP requirements, upgrading all SilverStripe dependencies to v6-compatible versions, and replacing deprecated extension classes.

  • Updated minimum PHP version from unspecified to ^8.1
  • Upgraded core SilverStripe dependencies to v6 (recipe-cms, linkfield, elemental, gridfieldextensions, recipe-testing)
  • Replaced deprecated DataExtension with Core\Extension in CarouselPageExtension

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/Extension/CarouselPageExtension.php Updated base class from deprecated DataExtension to Core\Extension
composer.json Updated PHP requirement to ^8.1 and upgraded all SilverStripe dependencies to v6-compatible versions; updated branch-alias to 3.0.x-dev

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

* @method \SilverStripe\ORM\ManyManyThroughList|Slide[] Slides()
*/
class CarouselPageExtension extends DataExtension
class CarouselPageExtension extends Extension
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

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

Incorrect base class for ORM extension. CarouselPageExtension should extend SilverStripe\ORM\DataExtension, not SilverStripe\Core\Extension. The class uses ORM-specific features like $db, $many_many, and updateCMSFields, which require DataExtension. In SilverStripe 6, DataExtension is not deprecated - only its import path remains the same at SilverStripe\ORM\DataExtension.

Copilot uses AI. Check for mistakes.
Comment thread composer.json Outdated
"extra": {
"branch-alias": {
"dev-main": "2.x-dev"
"dev-master": "3.0.x-dev"
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

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

Incorrect branch name in branch-alias. The branch should be 'dev-main' (as indicated by the diff showing it was previously 'dev-main'), not 'dev-master'. Most repositories use 'main' as the default branch name.

Suggested change
"dev-master": "3.0.x-dev"
"dev-main": "3.0.x-dev"

Copilot uses AI. Check for mistakes.
- Change branch-alias from dev-master to dev-main (repo uses main as default branch)
- Update unclecheese/display-logic from ^3.0 to ^4.0 for SS6 compatibility
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

use Dynamic\Carousel\Model\Slide;
use SilverStripe\Forms\FieldList;
use SilverStripe\ORM\DataExtension;
use SilverStripe\Core\Extension;
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

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

This change from DataExtension to Extension is incorrect. CarouselPageExtension defines ORM properties ($db, $many_many, $many_many_extraFields, $owns, $cascade_duplicates, $cascade_deletes) and uses ORM methods like getOwner()->exists(), getOwner()->dbObject(), and onBeforeWrite().

These features are provided by SilverStripe\ORM\DataExtension, not the base SilverStripe\Core\Extension class. DataExtension is not deprecated in SilverStripe 6 - it's the correct base class for extensions that add database functionality to DataObjects.

This should be reverted to:

use SilverStripe\ORM\DataExtension;

Copilot uses AI. Check for mistakes.
- nathancox/embedfield ^3.0 requires framework ^4.11|^5 (SS5 only)
- fromholdio/silverstripe-embedfield ^5.1 requires framework ^6.0 (SS6)
- fromholdio is an actively maintained fork with same functionality
- Latest release 5.1.0 from Nov 17, 2025
- jonom/focuspoint ^5 requires asset-admin ^2 (SS5)
- jonom/focuspoint ^6 requires asset-admin ^3 (SS6)
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

use Dynamic\Carousel\Model\Slide;
use SilverStripe\Forms\FieldList;
use SilverStripe\ORM\DataExtension;
use SilverStripe\Core\Extension;
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

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

This import should be use SilverStripe\ORM\DataExtension; instead of use SilverStripe\Core\Extension;. See the related comment on line 32 - this extension needs to extend DataExtension to support ORM features.

Copilot uses AI. Check for mistakes.
Comment thread composer.json
"symbiote/silverstripe-gridfieldextensions": "^4",
"unclecheese/display-logic": "^3.0"
"php": "^8.1",
"fromholdio/silverstripe-embedfield": "^5.1",
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

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

This package change from nathancox/embedfield to fromholdio/silverstripe-embedfield will break existing code. The VideoSlide.php file still imports nathancox\EmbedField\Forms\EmbedField and nathancox\EmbedField\Model\EmbedObject, which need to be updated to use the new package's namespace. The namespace likely changed to Fromholdio\EmbedField or similar - please verify the correct namespace for version ^5.1 and update all imports accordingly.

Copilot uses AI. Check for mistakes.
- Use SilverStripe\Core\Extension instead of SilverStripe\ORM\DataExtension
  DataExtension class no longer exists in SS6
- Remove parent::onBeforeWrite() call (Extension doesn't have this method)
- Change onBeforeWrite() visibility to protected (SS6 extension hook standard)
- Update VideoSlide to use Fromholdio\EmbedField namespace
  Replaces nathancox\EmbedField which is not SS6 compatible

Fixes the 'Class SilverStripe\ORM\DataExtension not found' error.
Extensions in SS6 extend Core\Extension and lifecycle hooks are called
via the extension system using invokeExtension().
The class.notFound ignores are no longer needed since we properly
updated the namespace from nathancox\EmbedField to Fromholdio\EmbedField.
PHPStan can now resolve these classes correctly.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

{
if (!$this->getOwner()->Interval || $this->getOwner()->Interval < 0) {
$this->getOwner()->Interval = self::$defaults['Interval'];
}
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

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

The removal of parent::onBeforeWrite() is potentially problematic. While SilverStripe\Core\Extension doesn't have an implementation of onBeforeWrite(), removing the parent call breaks the chain if this extension is ever subclassed. In SilverStripe, it's a best practice to always call parent::onBeforeWrite() in extension hooks to ensure proper behavior in inheritance chains.

Consider keeping the parent call:

protected function onBeforeWrite()
{
    if (!$this->getOwner()->Interval || $this->getOwner()->Interval < 0) {
        $this->getOwner()->Interval = self::$defaults['Interval'];
    }
    parent::onBeforeWrite();
}
Suggested change
}
}
parent::onBeforeWrite();

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jsirish jsirish merged commit 9889ea4 into main Nov 18, 2025
22 checks passed
@jsirish jsirish deleted the 3.0/ss6-upgrade-clean branch November 18, 2025 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants