Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRemoves the legacy Analytics backend module and all associated schema, navigation, locale, JS hooks, settings defaults, and tests, while also slightly adjusting the locale XML import logic to be compatible without the module’s locale definitions. Sequence diagram for window resize without Analytics chart initializationsequenceDiagram
actor Admin as AdminUser
participant BrowserWindow
participant jsBackend
participant Navigation as jsBackend_navigation
Admin->>BrowserWindow: Resize window
BrowserWindow->>jsBackend: window.resize event
jsBackend->>jsBackend: resizeFunctions.calculate
jsBackend->>Navigation: navigation.resize()
Note over jsBackend: Analytics chart initialization call removed
Class diagram for removed Analytics backend moduleclassDiagram
direction LR
class AnalyticsConfig {
}
class AnalyticsInstaller {
}
class AnalyticsExtension {
}
class DateRange {
}
class SettingsType {
}
class SettingsStepTypeInterface {
}
class SettingsStepAccountTypeInterface {
}
class SettingsStepAuthConfigFileTypeInterface {
}
class SettingsStepProfileTypeInterface {
}
class SettingsStepWebPropertyTypeInterface {
}
class ClientFactory {
}
class Connector {
}
class AnalyticsActionsIndex {
}
class AnalyticsActionsReset {
}
class AnalyticsActionsSettings {
}
class RecentVisits {
}
class TraficSources {
}
class AnalyticsWidgetRecentVisits {
}
class AnalyticsWidgetTraficSources {
}
class AnalyticsModuleInfo {
}
AnalyticsInstaller --> AnalyticsConfig
AnalyticsInstaller --> AnalyticsExtension
AnalyticsInstaller --> SettingsType
AnalyticsInstaller --> DateRange
SettingsType ..> SettingsStepTypeInterface
SettingsType ..> SettingsStepAccountTypeInterface
SettingsType ..> SettingsStepAuthConfigFileTypeInterface
SettingsType ..> SettingsStepProfileTypeInterface
SettingsType ..> SettingsStepWebPropertyTypeInterface
ClientFactory --> Connector
Connector --> DateRange
AnalyticsActionsIndex --> SettingsType
AnalyticsActionsIndex --> DateRange
AnalyticsActionsSettings --> SettingsType
AnalyticsActionsReset --> SettingsType
RecentVisits --> Connector
TraficSources --> Connector
AnalyticsWidgetRecentVisits <|-- RecentVisits
AnalyticsWidgetTraficSources <|-- TraficSources
AnalyticsModuleInfo --> AnalyticsConfig
AnalyticsModuleInfo --> AnalyticsInstaller
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The changes in
Backend/Modules/Locale/Engine/Model.phpfrom$item->attributes()/$translation->attributes()tocurrent($item->attributes())/current($translation->attributes())look unrelated to removing Analytics and may break attribute access (you’re now taking the first attribute value rather than the attribute collection); consider reverting or moving this into a separate, well‑justified change. - In
Backend/Modules/Settings/Actions/Index.phpyou removed the fallback to the oldAnalyticssetting forgoogle_tracking_google_analytics_tracking_id; if existing installations relied on that stored value, consider adding an explicit migration or a one‑time copy so users don’t silently lose their configured tracking ID.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The changes in `Backend/Modules/Locale/Engine/Model.php` from `$item->attributes()` / `$translation->attributes()` to `current($item->attributes())` / `current($translation->attributes())` look unrelated to removing Analytics and may break attribute access (you’re now taking the first attribute value rather than the attribute collection); consider reverting or moving this into a separate, well‑justified change.
- In `Backend/Modules/Settings/Actions/Index.php` you removed the fallback to the old `Analytics` setting for `google_tracking_google_analytics_tracking_id`; if existing installations relied on that stored value, consider adding an explicit migration or a one‑time copy so users don’t silently lose their configured tracking ID.
## Individual Comments
### Comment 1
<location> `src/Backend/Modules/Locale/Engine/Model.php:479-482` </location>
<code_context>
foreach ($items as $item) {
// attributes
- $attributes = $item->attributes();
+ $attributes = current($item->attributes());
$type = in_array($attributes['type'], $possibleTypes, true) ? $attributes['type'] : '';
$name = s($attributes['name'] ?? '')->title()->toString();
</code_context>
<issue_to_address>
**issue (bug_risk):** Using current() on SimpleXMLElement attributes likely breaks attribute access.
Wrapping `attributes()` in `current()` changes `$attributes` from the attribute collection to a single attribute node. On a single-attribute `SimpleXMLElement`, `$attributes['type']` then looks for a child element named `type` (which doesn’t exist) instead of the `type` attribute, so `$type`, `$name`, and `$language` will end up empty and imports will be skipped. You likely want to keep `$attributes = $item->attributes();` / `$translation->attributes();` or, if you need a normalized structure, cast to an array: `$attributes = (array) $item->attributes();`.
</issue_to_address>
### Comment 2
<location> `src/Backend/Modules/Settings/Actions/Index.php:70` </location>
<code_context>
'Core',
'google_tracking_google_analytics_tracking_id',
- $this->get('fork.settings')->get('Analytics', 'web_property_id', '')
+ ''
);
$this->form->addCheckbox(
</code_context>
<issue_to_address>
**question (bug_risk):** Dropping the fallback from the old Analytics setting may affect existing installations.
The old default pulled `google_tracking_google_analytics_tracking_id` from `Analytics:web_property_id`, which ensured a transparent migration. With the default now set to `''`, existing sites that only configured the old module will lose their tracking ID after upgrade unless a separate migration exists. If that’s undesirable, either retain the fallback for one release or add an explicit upgrade step to copy the old setting into the new key.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
tijsverkoyen
requested review from
absumo,
ilaria-orlando and
jonasdekeukelaere
jonasdekeukelaere
approved these changes
Feb 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Remove the deprecated Analytics backend module and its integration across the application.
Bug Fixes:
Enhancements:
Build:
Tests:
Chores: