feat: add Silverstripe 5 compatibility by jsirish · Pull Request #250 · dynamic/silverstripe-locator

jsirish · 2026-02-02T16:29:07Z

No description provided.

Updated dependencies: - silverstripe/recipe-cms: ^5.0 - silverstripe/lumberjack: ^3.0 - symbiote/silverstripe-gridfieldextensions: ^4.0 - colymba/gridfield-bulk-editing-tools: ^4.0 || ^5.0 - dynamic/silverstripe-geocoder: ^3.0 - littlegiant/silverstripe-catalogmanager: ^6.0 - silverstripe/recipe-testing: ^3 (dev)

Replace external dependency with native Silverstripe ArrayList conversion. This removes a blocking dependency that had no SS5 compatibility.

- Revert Location namespace from Dynamic\Locator\Model\Location to Dynamic\Locator\Location - Remove new fields ShowResultsDefault and ShowFormDefault (move to separate PR) - Add squizlabs/php_codesniffer to require-dev - Simplify CI workflow to match elemental-card pattern

- Use getShowResultsDefault() and getShowFormDefault() instead of direct property access - Add URLSegment to controller test fixture for proper routing

jQuery is now bundled in silverstripe/admin webpack build and globally available. The thirdparty/jquery/jquery.js path no longer exists in SS5.

Copilot

Pull request overview

Updates the locator module to be compatible with SilverStripe 5 by aligning dependencies and internal class naming/configuration with SS5 conventions.

Changes:

Bumped Composer requirements to SilverStripe 5–compatible versions (and removed muskie9/data-to-arraylist usage).
Renamed the Location model class from Dynamic\Locator\Model\Location to Dynamic\Locator\Location and updated imports/fixtures accordingly.
Adjusted LocatorController behavior and test fixtures (e.g., added URLSegment where required).

Reviewed changes

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

Show a summary per file

File	Description
`composer.json`	Updates dependency constraints for SilverStripe 5 and adds PHPCS dev dependency.
`src/Model/Location.php`	Moves `Location` to the `Dynamic\Locator` namespace (class rename).
`src/Model/LocationCategory.php`	Updates `Location` import to new namespace.
`src/Page/LocatorController.php`	Removes dependency on `muskie9/data-to-arraylist` and introduces default getters for removed page fields.
`src/Page/Locator.php`	Removes `ShowResultsDefault`/`ShowFormDefault` DB fields + CMS controls.
`_config/config.yml`	Updates extension target from `Dynamic\Locator\Model\Location` to `Dynamic\Locator\Location`.
`_config/legacy.yml`	Removes legacy classname remaps (now only remaps `Locator`).
`.github/workflows/ci.yml`	Removes `phpcoverage: false` override in CI workflow call.
`tests/*/.php`	Updates imports to new `Location` namespace.
`tests/*/.yml`	Updates fixture class keys for the `Location` class and adds `URLSegment` in controller fixture.

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

Copilot · 2026-02-02T16:38:06Z

@@ -52,8 +50,6 @@ class Locator extends \Page
     */
    private static $db = [
        'Unit' => 'Enum("m,km","m")',


The ShowResultsDefault / ShowFormDefault page fields (and their defaults + CMS controls) were removed from the Locator schema, but LocatorController still reads these values and now falls back to hard-coded defaults. This is a breaking behavior change for existing sites (saved values will be ignored). Consider keeping these as $db fields (and re-adding CMS fields), or replacing them with explicit config options/migration so current installations retain their configured behavior.

Suggested change

'Unit' => 'Enum("m,km","m")',

'Unit' => 'Enum("m,km","m")',

'ShowResultsDefault' => 'Boolean',

'ShowFormDefault' => 'Boolean',

Copilot · 2026-02-02T16:38:06Z

+namespace Dynamic\Locator;



Dynamic\Locator\Location is declared in src/Model/Location.php, which no longer matches the PSR-4 directory structure used elsewhere in this module (e.g., src/Model/LocationCategory.php => Dynamic\Locator\Model\LocationCategory). This can confuse tooling and may break non-SilverStripe PSR-4 autoloading expectations. Either move this class to src/Location.php (matching Dynamic\Locator\Location) or keep the namespace as Dynamic\Locator\Model and update references accordingly.

Copilot · 2026-02-02T16:38:06Z

 SilverStripe\ORM\DatabaseAdmin:
  classname_value_remapping:
-    Dynamic\Locator\Location: Dynamic\Locator\Model\Location
    Dynamic\Locator\Locator: Dynamic\Locator\Page\Locator


classname_value_remapping no longer includes a mapping for the Location class rename (Dynamic\Locator\Model\Location -> Dynamic\Locator\Location). Without this, existing databases with stored ClassName values for the old class may not be remapped during dev/build, which can break upgrades. Add an explicit remap entry for the Location class rename (direction should match SilverStripe's expected old=>new mapping).

jQuery is bundled in admin webpack for CMS only. Frontend pages need jQuery loaded separately for the store locator plugin.

Copilot

Pull request overview

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

Comments suppressed due to low confidence (1)

_config/legacy.yml:7

classname_value_remapping entries for Location/LocationCategory were removed, but this PR changes the Location class’ fully-qualified name. Without a remapping (e.g. Dynamic\\Locator\\Model\\Location -> Dynamic\\Locator\\Location), existing records with the old ClassName value may not hydrate correctly after an upgrade/build. Recommend adding the appropriate remap entries back (updated to reflect the new namespace), rather than removing them.

SilverStripe\ORM\DatabaseAdmin:
  classname_value_remapping:
    Dynamic\Locator\Locator: Dynamic\Locator\Page\Locator

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

Copilot · 2026-02-02T16:52:04Z

@@ -1,4 +1,5 @@
 <% require css('dynamic/silverstripe-locator: css/map.css') %>
+<% require javascript('https://code.jquery.com/jquery-3.7.1.min.js') %>


Loading jQuery from a public CDN in a template introduces external runtime dependencies (CSP/SRI compliance, offline/dev environments, supply-chain risk) and can make builds non-deterministic. If the goal is to avoid relying on silverstripe/admin assets in SS5, consider vendoring a known jQuery version within this module (e.g. under thirdparty/ and exposing it) or including it via your normal frontend build pipeline, rather than pulling from code.jquery.com at runtime.

Suggested change

<% require javascript('https://code.jquery.com/jquery-3.7.1.min.js') %>

<% require javascript('dynamic/silverstripe-locator: thirdparty/jquery/jquery-3.7.1.min.js') %>

Copilot · 2026-02-02T16:52:04Z

    private static $db = [
        'Unit' => 'Enum("m,km","m")',
-        'ShowResultsDefault' => 'Boolean',
-        'ShowFormDefault' => 'Boolean',
    ];


ShowResultsDefault / ShowFormDefault were removed from $db (and also from defaults/CMS fields later in this file), but LocatorController still branches on these values. As-is, they can no longer be configured in the CMS and will always fall back to hardcoded defaults. If these options are still supported, reintroduce the DB fields + CMS fields (and keep defaults); otherwise remove the controller branching and document the breaking change.

Existing database records may have ClassName values with the old namespace. This ensures records migrate correctly during upgrades.

Copilot

Pull request overview

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

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

Copilot · 2026-02-02T17:00:38Z

        $class = $this->data()->ClassName;
        $locations = $class::get_locations($filter, $filterAny, $exclude);
-        $locations = DataToArrayListHelper::to_array_list($locations);
+        // Convert DataList to ArrayList for additional manipulation
+        $locations = ArrayList::create($locations->toArray());



setLocations() converts the DataList to an ArrayList via toArray() before sorting/limiting. This forces all matching locations to be loaded into memory and prevents the database from applying sort()/limit() efficiently. Prefer applying sort/limit on the DataList first (when possible), and only convert to ArrayList at the last point where in-memory manipulation is required.

Copilot · 2026-02-02T17:00:38Z

  classname_value_remapping:
-    Dynamic\Locator\Location: Dynamic\Locator\Model\Location
+    Dynamic\Locator\Model\Location: Dynamic\Locator\Location
+    Dynamic\Locator\Model\LocationCategory: Dynamic\Locator\Model\LocationCategory


classname_value_remapping now includes a no-op mapping for Dynamic\\Locator\\Model\\LocationCategory and removes the previous remap for Dynamic\\Locator\\LocationCategory. If older installations stored Dynamic\\Locator\\LocationCategory in the ClassName column, dropping that remap will break upgrades. Consider restoring the Dynamic\\Locator\\LocationCategory -> Dynamic\\Locator\\Model\\LocationCategory entry and removing the no-op line.

Suggested change

Dynamic\Locator\Model\LocationCategory: Dynamic\Locator\Model\LocationCategory

Dynamic\Locator\LocationCategory: Dynamic\Locator\Model\LocationCategory

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

jsirish added 4 commits February 2, 2026 10:28

refactor: remove data-to-arraylist dependency

bb6f7df

Replace external dependency with native Silverstripe ArrayList conversion. This removes a blocking dependency that had no SS5 compatibility.

fix: use getter methods for removed fields, fix test fixture

419f9ab

- Use getShowResultsDefault() and getShowFormDefault() instead of direct property access - Add URLSegment to controller test fixture for proper routing

jsirish requested a review from Copilot February 2, 2026 16:29

Copilot started reviewing on behalf of jsirish February 2, 2026 16:29 View session

fix: remove obsolete jQuery requirement for SS5 compatibility

0f68e1c

jQuery is now bundled in silverstripe/admin webpack build and globally available. The thirdparty/jquery/jquery.js path no longer exists in SS5.

Copilot AI reviewed Feb 2, 2026

View reviewed changes

fix: add jQuery CDN for frontend use in SS5

8c3738e

jQuery is bundled in admin webpack for CMS only. Frontend pages need jQuery loaded separately for the store locator plugin.

jsirish requested a review from Copilot February 2, 2026 16:42

Copilot started reviewing on behalf of jsirish February 2, 2026 16:43 View session

Copilot AI reviewed Feb 2, 2026

View reviewed changes

fix: add classname remapping for backward compatibility

15105ac

Existing database records may have ClassName values with the old namespace. This ensures records migrate correctly during upgrades.

jsirish requested a review from Copilot February 2, 2026 16:54

Copilot started reviewing on behalf of jsirish February 2, 2026 16:54 View session

Copilot AI reviewed Feb 2, 2026

View reviewed changes

jsirish closed this Feb 2, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: add Silverstripe 5 compatibility#250

feat: add Silverstripe 5 compatibility#250
jsirish wants to merge 7 commits into
masterfrom
feature/ss5-upgrade

jsirish commented Feb 2, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -1,4 +1,5 @@
		<% require css('dynamic/silverstripe-locator: css/map.css') %>
		<% require javascript('https://code.jquery.com/jquery-3.7.1.min.js') %>

	<% require javascript('https://code.jquery.com/jquery-3.7.1.min.js') %>
	<% require javascript('dynamic/silverstripe-locator: thirdparty/jquery/jquery-3.7.1.min.js') %>

	Dynamic\Locator\Model\LocationCategory: Dynamic\Locator\Model\LocationCategory
	Dynamic\Locator\LocationCategory: Dynamic\Locator\Model\LocationCategory

Uh oh!

Conversation

jsirish commented Feb 2, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants