Conversation
…de by zero protection
…k, TextBlock, and ContentBlock
There was a problem hiding this comment.
Pull request overview
This PR introduces a broad set of new SilverStripe models, controllers, Elemental blocks, configuration, and styling assets (including a CSS file for moved inline styles), plus additional documentation and test scaffolding.
Changes:
- Added multiple new DataObject models and CMS/admin tooling (translations, reviews, products, pricing history).
- Added new controllers/services for search, voting, locale/language switching, and page base utilities.
- Added Elemental blocks + extensive YAML configuration, CSS/JS assets, and new/updated test suites/fixtures.
Reviewed changes
Copilot reviewed 121 out of 211 changed files in this pull request and generated 27 comments.
Show a summary per file
| File | Description |
|---|---|
| app/src/models/ReviewCriteria.php | Adds a new DataObject for review criteria and CMS fields/validation helpers |
| app/src/models/ProductPricingHistory.php | Adds pricing history model with CMS fields + validation + formatting helpers |
| app/src/models/ProductImage.php | Adds product image model with CMS fields + image helpers |
| app/src/models/ProductCategory.php | Adds category model with CMS fields + slug validation + hierarchy helpers |
| app/src/models/BaseTranslatablePage.php | Adds a translatable base page with Fluent integration/locale helpers |
| app/src/forms/SearchForm.php | Adds GET-based search form integration and redirect behavior |
| app/src/docs/ProjectTimelineSystem.md | Adds documentation for a project timeline/milestones system |
| app/src/docs/ClientPortalNotificationSystem.md | Adds documentation for client portal notifications |
| app/src/docs/ActivityLoggingSystem.md | Adds documentation for activity logging and download history |
| app/src/controllers/SearchController.php | Adds search results controller/actions and pagination helpers |
| app/src/controllers/ReviewVoteController.php | Adds JSON vote endpoint for reviews (like/dislike) |
| app/src/controllers/LanguageSwitcherController.php | Adds controller endpoint to switch locale + list locales |
| app/src/controllers/BasePageController.php | Adds shared controller helpers (rate limiting, sanitization, CSRF check) |
| app/src/admin/TranslationAdmin.php | Adds a ModelAdmin to manage and import/export translations |
| app/src/TextBlock.php | Adds an Elemental TextBlock with styling options stored in DB |
| app/src/Test/TestimonialTestSuite.php | Adds a SilverStripe TestSuite aggregator for testimonial tests |
| app/src/Test/TestimonialTest.yml | Adds testimonial fixtures |
| app/src/Test/TestimonialTest.php | Adds unit tests for Testimonial model behavior |
| app/src/Test/TestimonialPageTest.yml | Adds fixtures for TestimonialPage |
| app/src/Test/TestimonialPageTest.php | Adds unit tests for TestimonialPage behavior |
| app/src/Test/ProjectTimelineTest.yml | Adds empty YAML config stub for project timeline tests |
| app/src/Test/MathServiceTest.php | Adds unit tests for MathService edge cases |
| app/src/Test/I18nTestSuite.php | Adds a TestSuite aggregator for i18n-related tests |
| app/src/Test/I18nTestHarness.yml | Adds fixtures for i18n test harness |
| app/src/Test/ElementalRenderingTest.yml | Adds fixtures for elemental rendering tests |
| app/src/Test/ElementalRenderingTest.php | Adds tests around ElementContent rendering and elemental behavior |
| app/src/Test/ElementalIntegrationTest.yml | Adds fixtures for elemental integration tests |
| app/src/Test/ElementalBlockTest.yml | Adds fixtures for elemental block tests |
| app/src/Test/ElementalBlockTest.php | Adds tests for elemental block creation/rendering/permissions |
| app/src/Test/ElementalAdminTest.yml | Adds fixtures for elemental admin tests |
| app/src/Test/ElementalAdminTest.php | Adds tests for elemental admin/CMS field behavior |
| app/src/Test/ClientPortalTestSuite.yml | Adds empty YAML config stub for client portal test suite |
| app/src/Test/ClientPortalNotificationTest.yml | Adds empty YAML config stub for portal notification tests |
| app/src/Test/ClientPortalDownloadTest.yml | Adds empty YAML config stub for download tests |
| app/src/Test/ClientPortalAuthTest.yml | Adds empty YAML config stub for auth tests |
| app/src/Test/ClientPortalAccessControlTest.yml | Adds empty YAML config stub for access control tests |
| app/src/Test/ActivityLoggingTest.yml | Adds empty YAML config stub for activity logging tests |
| app/src/SpacerBlock.php | Adds a spacer Elemental block for vertical spacing |
| app/src/QuoteBlock.php | Adds a quote Elemental block with styling options |
| app/src/MathService.php | Adds MathService helper for safe division/percentage calculation |
| app/src/LanguageSwitcher.php | Adds a ViewableData-based language switcher component |
| app/src/LandingPageController.php | Adds controller helper methods for landing pages w/ Elemental |
| app/src/LandingPage.php | Adds LandingPage page type with Elemental + configurable sections |
| app/src/ImageBlock.php | Adds an image Elemental block with sizing/link options |
| app/src/HeroBlock.php | Adds a hero Elemental block with images + styling options |
| app/src/HTMLBlock.php | Adds an HTML Elemental block with optional “raw HTML” toggle |
| app/src/GalleryBlock.php | Adds a gallery Elemental block with relation-managed images |
| app/src/FileDownloadBlock.php | Adds a file download Elemental block with formatted metadata helpers |
| app/src/Extensions/TestimonialModerationExtension.php | Adds moderation helper extension and email notifications |
| app/src/Extensions/LocaleDetectionExtension.php | Adds locale detection extension that applies detected locale per request |
| app/src/Extensions/LanguageSwitcherExtension.php | Adds template helpers for language switching on pages |
| app/src/Extensions/ElementalAreaExtension.php | Adds allowed-elements filtering for elemental areas |
| app/src/ElementalPageController.php | Adds an elemental page controller shell |
| app/src/ElementalPage.php | Adds a page type with Elemental extension and allowed blocks |
| app/src/ElementalConfig.php | Adds a central registry for allowed elemental blocks per page type |
| app/src/DividerBlock.php | Adds a divider Elemental block with styling options |
| app/src/CtaBlock.php | Adds a CTA Elemental block with button/styling options |
| app/src/ContentBlock.php | Adds a content Elemental block with styling and content-type switch |
| app/src/ContactPageController.php | Adds contact form handling with anti-spam and rate limiting checks |
| app/src/ContactPage.php | Adds ContactPage page type with Elemental + contact details fields |
| app/src/CardBlock.php | Adds a card Elemental block with image + style selection |
| app/src/AccordionBlock.php | Adds an accordion Elemental block and nested AccordionItem DataObject |
| app/src/AboutPageController.php | Adds an about page controller shell |
| app/src/AboutPage.php | Adds AboutPage page type with Elemental + section toggles |
| app/javascript/testimonial-admin.js | Adds CMS gridfield UX behavior for testimonial moderation |
| app/css/testimonial-admin.css | Adds CSS for testimonial admin UI elements |
| app/css/search.css | Adds CSS for search UI (form, suggestions, facets, responsive) |
| app/css/notifications.css | Adds CSS for client portal notifications UI |
| app/css/inline-styles-moved.css | Adds CSS custom properties-based replacements for previous inline styles |
| app/css/download-history.css | Adds CSS for download history UI |
| app/css/documents.css | Adds CSS for documents UI |
| app/css/contact.css | Adds CSS for contact page layout/form |
| app/css/client-portal.css | Adds CSS for overall client portal layout/components |
| app/css/activity-history.css | Adds CSS for activity history UI |
| app/_config/translation_admin.yml | Defines TRANSLATE_MANAGE permission and grants it to default admins |
| app/_config/testimonial_admin.yml | Adds CMS JS/CSS requirements for testimonial admin |
| app/_config/reviews.yml | Adds routes for review voting and review-related controllers |
| app/_config/locale_detection.yml | Applies locale detection extension and defines supported locales |
| app/_config/language_switcher.yml | Adds routes for language switching and applies switcher extension |
| app/_config/i18n_templates.yml | Configures template/theme priorities for localized templates |
| app/_config/google_reviews.yml | Adds config and fields for Google reviews sync integration on testimonials |
| app/_config/fluent.yml | Adds Fluent configuration for pages/elements + locales + URL options |
| app/_config/elemental_pages.yml | Adds Elemental extension configuration for ElementalPage and LandingPage |
| app/_config/elemental_config.yml | Adds Elemental settings + allowed block lists per page type |
| app/_config/elemental_blocks.yml | Registers custom elemental block metadata + attempts to register types |
| app/_config/elemental-tests.yml | Adds test-time elemental injector/DB table options configuration |
| app/_config/contact_routes.yml | Adds routing for ContactPageController |
| app/_config/client_portal_routes.yml | Adds routing for ClientPortalPageController |
| app/_config/about_routes.yml | Adds routing for AboutPageController |
| MathService.md | Adds documentation for MathService usage and behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,238 @@ | |||
| <?php | |||
There was a problem hiding this comment.
The PR title/description indicates a limited refactor (“Move inline styles to CSS custom properties”), but this change set introduces many new models/controllers/configs/tests/docs across unrelated domains (search, translations, portal systems, elemental blocks, etc.). Please either (mandatory) split this into focused PRs (e.g., CSS custom properties refactor vs. new portal/multilingual/search features), or update the PR title/description to accurately reflect the expanded scope.
| * Represents specific criteria for a review | ||
| * Used to break down ratings into specific categories | ||
| */ | ||
| class ReviewCriteria extends DataObject |
There was a problem hiding this comment.
The PR title/description indicates a limited refactor (“Move inline styles to CSS custom properties”), but this change set introduces many new models/controllers/configs/tests/docs across unrelated domains (search, translations, portal systems, elemental blocks, etc.). Please either (mandatory) split this into focused PRs (e.g., CSS custom properties refactor vs. new portal/multilingual/search features), or update the PR title/description to accurately reflect the expanded scope.
| private static $allowed_actions = [ | ||
| 'index', | ||
| 'results', | ||
| 'SearchForm' |
There was a problem hiding this comment.
The controller defines a suggestions() action but it isn’t listed in $allowed_actions, so the endpoint will not be callable. Add suggestions to $allowed_actions (optionally restricted to an appropriate permission/condition).
| 'SearchForm' | |
| 'SearchForm', | |
| 'suggestions', |
| public function suggestions(HTTPRequest $request) | ||
| { |
There was a problem hiding this comment.
The controller defines a suggestions() action but it isn’t listed in $allowed_actions, so the endpoint will not be callable. Add suggestions to $allowed_actions (optionally restricted to an appropriate permission/condition).
| return [ | ||
| 'Title' => $hasQuery ? "Search Results for '{$query}'" : 'Search', | ||
| 'Query' => $query, | ||
| 'SearchResults' => $searchResults, | ||
| 'SearchStats' => $searchStats, | ||
| 'CurrentPage' => $page, | ||
| 'PerPage' => $limit, | ||
| 'HasQuery' => $hasQuery, | ||
| 'HasResults' => !empty($searchResults), | ||
| 'SearchForm' => $this->SearchForm() | ||
| ]; |
There was a problem hiding this comment.
The action results() returns an array, but the later helper methods (PaginatedResults(), getTotalResults(), getTotalPages(), HasMorePages(), etc.) reference controller properties like $this->SearchResults, $this->SearchStats, $this->PerPage, and $this->CurrentPage which are never assigned in this controller. As a result, pagination helpers will behave incorrectly (likely always empty/0). Fix by either (a) storing these values into controller properties before returning, or (b) computing them from the request (or a cached search call) inside the helper methods.
| public function testElementContentTemplateExists() | ||
| { | ||
| $element = new ElementContent(); |
There was a problem hiding this comment.
This test references several classes without importing or fully qualifying their namespaces (e.g., ElementContent, ElementController, ElementalArea, Page). In SilverStripe/Elemental, these are typically namespaced (e.g., DNADesign\\Elemental\\Models\\ElementContent). As written, this can cause class-not-found fatals. Add the correct use statements or prefix with fully-qualified class names.
| public function testElementControllerInitialization() | ||
| { |
There was a problem hiding this comment.
This test references several classes without importing or fully qualifying their namespaces (e.g., ElementContent, ElementController, ElementalArea, Page). In SilverStripe/Elemental, these are typically namespaced (e.g., DNADesign\\Elemental\\Models\\ElementContent). As written, this can cause class-not-found fatals. Add the correct use statements or prefix with fully-qualified class names.
| $controller = $element->getController(); | ||
| $this->assertNotNull($controller); | ||
| $this->assertInstanceOf(ElementController::class, $controller); |
There was a problem hiding this comment.
This test references several classes without importing or fully qualifying their namespaces (e.g., ElementContent, ElementController, ElementalArea, Page). In SilverStripe/Elemental, these are typically namespaced (e.g., DNADesign\\Elemental\\Models\\ElementContent). As written, this can cause class-not-found fatals. Add the correct use statements or prefix with fully-qualified class names.
| @@ -0,0 +1,161 @@ | |||
| <?php | |||
|
|
|||
There was a problem hiding this comment.
This test references several classes without importing or fully qualifying their namespaces (e.g., ElementContent, ElementController, ElementalArea, Page). In SilverStripe/Elemental, these are typically namespaced (e.g., DNADesign\\Elemental\\Models\\ElementContent). As written, this can cause class-not-found fatals. Add the correct use statements or prefix with fully-qualified class names.
| SilverStripe\Core\Injector\SilverStripe\Elemental\Models\ElementalArea: | ||
| properties: | ||
| elemental_objects: | ||
| - 'ContentBlock' | ||
| - 'TextBlock' | ||
| - 'HTMLBlock' | ||
| - 'AccordionBlock' | ||
| - 'CardBlock' | ||
| - 'HeroBlock' | ||
| - 'CtaBlock' | ||
| - 'QuoteBlock' | ||
| - 'DividerBlock' | ||
| - 'SpacerBlock' No newline at end of file |
There was a problem hiding this comment.
This YAML key looks incorrect: SilverStripe\\Core\\Injector\\SilverStripe\\Elemental\\Models\\ElementalArea is not a valid class, so this config won’t be applied. If the intent is Injector configuration, it should be under SilverStripe\\Core\\Injector\\Injector: with the actual ElementalArea class as a service/class key (and note ElementalArea is typically DNADesign\\Elemental\\Models\\ElementalArea).
| SilverStripe\Core\Injector\SilverStripe\Elemental\Models\ElementalArea: | |
| properties: | |
| elemental_objects: | |
| - 'ContentBlock' | |
| - 'TextBlock' | |
| - 'HTMLBlock' | |
| - 'AccordionBlock' | |
| - 'CardBlock' | |
| - 'HeroBlock' | |
| - 'CtaBlock' | |
| - 'QuoteBlock' | |
| - 'DividerBlock' | |
| - 'SpacerBlock' | |
| SilverStripe\Core\Injector\Injector: | |
| DNADesign\Elemental\Models\ElementalArea: | |
| properties: | |
| elemental_objects: | |
| - 'ContentBlock' | |
| - 'TextBlock' | |
| - 'HTMLBlock' | |
| - 'AccordionBlock' | |
| - 'CardBlock' | |
| - 'HeroBlock' | |
| - 'CtaBlock' | |
| - 'QuoteBlock' | |
| - 'DividerBlock' | |
| - 'SpacerBlock' |
Converted inline styles to CSS custom properties in DividerBlock, HTMLBlock, TextBlock, and ContentBlock templates for better maintainability and separation of concerns.