build(ddev): update multi-version add-on to 0.3.3 and add test fixtures - #49
Conversation
Update konradmichalik/ddev-typo3-multi-version-extension to 0.3.3 (adds TYPO3 14 support, .setup scripts and the Tests/Acceptance/Fixtures import mechanism). Limit installed instances to TYPO3 13 + 14 via TYPO3_VERSIONS and trim additional_hostnames accordingly. Add Tests/Acceptance/Fixtures/internal-news.sql with sample data (pinned welcome news, a news item with a notifying date, and a group-targeted news plus the matching be_group) so the toolbar item and dashboard widget show real content after `ddev install`.
WalkthroughThe PR adds shared DDEV setup utilities, rewires TYPO3 web install/command scripts and DDEV configuration to use Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
… to news fixtures Grant the widget via be_groups.availableWidgets so non-admin users may render it, resolve the admin dashboard owner through a username lookup (the _cli_ user can occupy uid 1), and drop the redundant idempotency guards (the add-on drops the database on every install).
Coverage Report for CI Build 28225297849Coverage remained the same at 78.378%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.ddev/.setup/scripts/utils.sh:
- Around line 306-309: The fixture symlink setup in
create_symlinks_additional_extensions() should not assume
Tests/Acceptance/Fixtures/packages/*/ always expands to real directories. Update
the loop to safely skip the literal glob and any non-directory entries before
calling ln -sr, so an absent or empty fixtures folder does not trigger set -e
during install. Use create_symlinks_additional_extensions and the
BASE_PATH/packages link creation as the location to apply the guard.
- Around line 256-258: The install_start function has an unguarded recursive
delete that duplicates cleanup already handled elsewhere and can become
dangerous if VERSION is unset or malformed. Remove the rm -rf call inside
install_start and rely on setup_environment’s existing cleanup path instead.
Keep the change localized to install_start in utils.sh and preserve the rest of
the setup flow unchanged.
- Around line 196-206: The pre_setup flow currently accepts any caller-provided
VERSION, which lets wrapper scripts bypass the TYPO3_VERSIONS contract. Add a
validation gate in pre_setup before proceeding that checks the requested version
against TYPO3_VERSIONS and aborts early for unsupported versions, so .install-11
and .install-12 cannot start installs outside the allowed set. Keep the check
centralized in pre_setup and use its existing VERSION handling to ensure all
wrappers share the same enforcement.
- Around line 290-299: The create_symlinks_main_extension helper is hard-coding
its own exclusion list instead of honoring the configured SYMLINK_EXCLUSIONS
values. Update the exclusion handling in create_symlinks_main_extension to read
and use the same exclusions defined by the Docker config, so directories like
vendor and public are skipped consistently when generating package symlinks.
In @.ddev/.setup/templates/index.php:
- Line 23: The DDEV landing page is rendering dynamic values raw, so escape all
HTML-facing variables in the template before output. Update the rendering in the
template that uses $extensionKey and also the other dynamic fields mentioned in
the review ($description, $version, $usage, $example, $typo3AdminUser, and
$typo3AdminPassword) by wrapping them with the existing h() helper, and use
rawurlencode() anywhere values are inserted into URL path or query parts.
- Around line 59-65: The DirectoryIterator setup in the template should not
assume every command directory exists, because missing `.ddev/commands/host`
will break the landing page. Update the loop around `$directories` and `new
DirectoryIterator($directory)` to check that each directory exists before
iterating, so the template still renders when only the web commands directory is
present.
- Line 49: The URL generated in the template output is exposing the backend
admin password as a query parameter. Update the link generation in the index.php
template so the Backend link points only to the TYPO3 login path and remove the
p=... parameter from the anchor href; keep the TYPO3 username/password only in
the credentials section or other non-URL display, and use the existing version,
extensionKey, typo3AdminUser, and typo3AdminPassword symbols to locate the
affected echo statement.
In @.ddev/commands/web/install:
- Around line 10-14: The argument parsing in the install command currently
accepts any unknown token and silently ignores extra positional arguments after
the first version value. Update the loop in the command script to explicitly
recognize only the supported flags and a single TYPO3 version argument, and
reject any unknown option or second positional value with an error. Keep the
logic centered around the existing VERBOSE handling and TYPO3 assignment so
typos like unsupported flags are reported correctly.
In `@Configuration/page.tsconfig`:
- Around line 1-9: The backend layout definition is using the wrong TSconfig key
casing and is missing the required grid config, so update the
PageTsBackendLayoutDataProvider-compatible structure in
Configuration/page.tsconfig by renaming backendLayouts to BackendLayouts and
adding a config.backend_layout block with colCount, rowCount, and rows for the
default entry. Remove the jsFile property from this backend layout definition,
since it is ignored here, and if JavaScript must be loaded, do it via
mod.SHARED.javascriptFile or another supported registration path.
In `@Tests/Acceptance/Fixtures/internal-news.sql`:
- Around line 45-64: The internal-news fixture has a hard-coded date in the
maintenance entry that makes the demo content expire and look stale. Update the
fixture around the maintenance news/date records so the timestamp is generated
at setup time or the copy in the related title, body, and notification text is
date-neutral. Use the existing fixture entries in internal-news.sql, especially
the maintenance news item and the tx_ximatypo3internalnews_domain_model_date
insert, to keep the scenario evergreen.
In `@Tests/Acceptance/Fixtures/packages/sitepackage/composer.json`:
- Around line 2-4: The Composer metadata for the fixture package still has a
copied, stale description that does not match the sitepackage/internal-news demo
content. Update the description field in the sitepackage composer.json to a
package-specific description that reflects this fixture, keeping the name and
other metadata unchanged.
In `@Tests/Acceptance/Fixtures/packages/sitepackage/README.md`:
- Around line 1-2: The README header currently renders the generator marker as
an H1, which triggers markdownlint; update the fixture README so the
`#ddev-generated` marker is not treated as a heading by changing it to a
non-rendered form such as an HTML comment or plain text, while keeping the
visible `# Sitepackage` heading intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c41b78ff-c1ee-4377-b969-94f3ab768b07
📒 Files selected for processing (22)
.ddev/.setup/scripts/utils.sh.ddev/.setup/templates/index.php.ddev/addon-metadata/typo3-multi-version-extension/manifest.yaml.ddev/commands/web/.install-11.ddev/commands/web/.install-12.ddev/commands/web/.install-13.ddev/commands/web/.install-14.ddev/commands/web/11.ddev/commands/web/12.ddev/commands/web/13.ddev/commands/web/14.ddev/commands/web/all.ddev/commands/web/install.ddev/config.typo3-setup.yaml.ddev/docker-compose.typo3-setup.yamlConfiguration/page.tsconfigResources/Public/Stylesheets/Backend.cssTests/Acceptance/Fixtures/internal-news.sqlTests/Acceptance/Fixtures/packages/sitepackage/Configuration/Services.yamlTests/Acceptance/Fixtures/packages/sitepackage/README.mdTests/Acceptance/Fixtures/packages/sitepackage/composer.jsonext_tables.php
💤 Files with no reviewable changes (1)
- ext_tables.php
| function pre_setup() { | ||
| export VERSION=$1 | ||
| message magenta "Install TYPO3 $VERSION" | ||
| _progress " ├─ Prepare environment" | ||
| export BASE_PATH="/var/www/html/.Build/$VERSION" | ||
| intro_typo3 | ||
| message blue "Pre Setup for TYPO3 $VERSION" | ||
| _done | ||
| install_start | ||
| install_composer_packages | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Enforce TYPO3_VERSIONS before setup starts.
Line 197 exports any caller-provided version, so .install-11/.install-12 can still bypass the new TYPO3_VERSIONS=13 14 contract and start unsupported installs. Validate once in pre_setup so all wrappers share the same gate.
Proposed fix
function pre_setup() {
- export VERSION=$1
+ check_typo3_version "$1"
+ export VERSION="$1"
message magenta "Install TYPO3 $VERSION"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function pre_setup() { | |
| export VERSION=$1 | |
| message magenta "Install TYPO3 $VERSION" | |
| _progress " ├─ Prepare environment" | |
| export BASE_PATH="/var/www/html/.Build/$VERSION" | |
| intro_typo3 | |
| message blue "Pre Setup for TYPO3 $VERSION" | |
| _done | |
| install_start | |
| install_composer_packages | |
| } | |
| function pre_setup() { | |
| check_typo3_version "$1" | |
| export VERSION="$1" | |
| message magenta "Install TYPO3 $VERSION" | |
| _progress " ├─ Prepare environment" | |
| export BASE_PATH="/var/www/html/.Build/$VERSION" | |
| intro_typo3 | |
| message blue "Pre Setup for TYPO3 $VERSION" | |
| _done | |
| install_start | |
| install_composer_packages | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.ddev/.setup/scripts/utils.sh around lines 196 - 206, The pre_setup flow
currently accepts any caller-provided VERSION, which lets wrapper scripts bypass
the TYPO3_VERSIONS contract. Add a validation gate in pre_setup before
proceeding that checks the requested version against TYPO3_VERSIONS and aborts
early for unsupported versions, so .install-11 and .install-12 cannot start
installs outside the allowed set. Keep the check centralized in pre_setup and
use its existing VERSION handling to ensure all wrappers share the same
enforcement.
| function install_start() { | ||
| rm -rf /var/www/html/.Build/$VERSION/* | ||
| _progress " ├─ Setup environment" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Remove the unguarded recursive delete.
Line 257 expands $VERSION unquoted in an rm -rf target, and setup_environment already removes $BASE_PATH immediately after. Dropping this duplicate delete avoids accidental broad deletion if the function is ever called with an unset or malformed version.
Proposed fix
function install_start() {
- rm -rf /var/www/html/.Build/$VERSION/*
_progress " ├─ Setup environment"
setup_environment📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function install_start() { | |
| rm -rf /var/www/html/.Build/$VERSION/* | |
| _progress " ├─ Setup environment" | |
| function install_start() { | |
| _progress " ├─ Setup environment" |
🧰 Tools
🪛 ast-grep (0.44.0)
[error] 256-256: Recursive rm is given an unquoted variable expansion as a target. If the variable is empty, contains whitespace, or holds glob characters, word splitting and pathname expansion can make rm delete unintended paths (e.g. rm -rf $dir with dir="" or dir="/ tmp"). Always double-quote the expansion and guard against empty values: rm -rf -- "${dir:?dir is unset}".
Context: $VERSION
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(rm-rf-unquoted-variable-bash)
🪛 Shellcheck (0.11.0)
[info] 257-257: Double quote to prevent globbing and word splitting.
(SC2086)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.ddev/.setup/scripts/utils.sh around lines 256 - 258, The install_start
function has an unguarded recursive delete that duplicates cleanup already
handled elsewhere and can become dangerous if VERSION is unset or malformed.
Remove the rm -rf call inside install_start and rely on setup_environment’s
existing cleanup path instead. Keep the change localized to install_start in
utils.sh and preserve the rest of the setup flow unchanged.
Source: Linters/SAST tools
| function create_symlinks_main_extension() { | ||
| local exclusions=(".*" "Documentation" "Documentation-GENERATED-temp" "var") | ||
| for item in ./*; do | ||
| local base_name=$(basename "$item") | ||
| for exclusion in "${exclusions[@]}"; do | ||
| if [[ $base_name == "$exclusion" ]]; then | ||
| continue 2 | ||
| fi | ||
| done | ||
| ln -sr "$item" "$BASE_PATH/packages/$EXTENSION_KEY/$base_name" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Honor SYMLINK_EXCLUSIONS when building the package symlinks.
The Docker config defines SYMLINK_EXCLUSIONS=.* Documentation Documentation-GENERATED-temp var vendor public, but Line 291 hard-codes a shorter list. This can symlink local vendor/public directories into the Composer path package and pollute the install.
Proposed fix
function create_symlinks_main_extension() {
- local exclusions=(".*" "Documentation" "Documentation-GENERATED-temp" "var")
+ local exclusions=()
+ IFS=' ' read -r -a exclusions <<< "${SYMLINK_EXCLUSIONS:-.* Documentation Documentation-GENERATED-temp var}"
for item in ./*; do📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function create_symlinks_main_extension() { | |
| local exclusions=(".*" "Documentation" "Documentation-GENERATED-temp" "var") | |
| for item in ./*; do | |
| local base_name=$(basename "$item") | |
| for exclusion in "${exclusions[@]}"; do | |
| if [[ $base_name == "$exclusion" ]]; then | |
| continue 2 | |
| fi | |
| done | |
| ln -sr "$item" "$BASE_PATH/packages/$EXTENSION_KEY/$base_name" | |
| function create_symlinks_main_extension() { | |
| local exclusions=() | |
| IFS=' ' read -r -a exclusions <<< "${SYMLINK_EXCLUSIONS:-.* Documentation-GENERATED-temp var}" | |
| for item in ./*; do | |
| local base_name=$(basename "$item") | |
| for exclusion in "${exclusions[@]}"; do | |
| if [[ $base_name == "$exclusion" ]]; then | |
| continue 2 | |
| fi | |
| done | |
| ln -sr "$item" "$BASE_PATH/packages/$EXTENSION_KEY/$base_name" |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 293-293: Declare and assign separately to avoid masking return values.
(SC2155)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.ddev/.setup/scripts/utils.sh around lines 290 - 299, The
create_symlinks_main_extension helper is hard-coding its own exclusion list
instead of honoring the configured SYMLINK_EXCLUSIONS values. Update the
exclusion handling in create_symlinks_main_extension to read and use the same
exclusions defined by the Docker config, so directories like vendor and public
are skipped consistently when generating package symlinks.
| function create_symlinks_additional_extensions() { | ||
| for dir in Tests/Acceptance/Fixtures/packages/*/; do | ||
| ln -sr "$dir" "$BASE_PATH/packages/$(basename "$dir")" | ||
| done |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Guard the optional fixture package glob.
If Tests/Acceptance/Fixtures/packages/ is absent or empty, the glob is passed literally to ln, and set -e aborts the install. Skip non-directories before linking.
Proposed fix
function create_symlinks_additional_extensions() {
for dir in Tests/Acceptance/Fixtures/packages/*/; do
+ [ -d "$dir" ] || continue
ln -sr "$dir" "$BASE_PATH/packages/$(basename "$dir")"
done
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function create_symlinks_additional_extensions() { | |
| for dir in Tests/Acceptance/Fixtures/packages/*/; do | |
| ln -sr "$dir" "$BASE_PATH/packages/$(basename "$dir")" | |
| done | |
| function create_symlinks_additional_extensions() { | |
| for dir in Tests/Acceptance/Fixtures/packages/*/; do | |
| [ -d "$dir" ] || continue | |
| ln -sr "$dir" "$BASE_PATH/packages/$(basename "$dir")" | |
| done | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.ddev/.setup/scripts/utils.sh around lines 306 - 309, The fixture symlink
setup in create_symlinks_additional_extensions() should not assume
Tests/Acceptance/Fixtures/packages/*/ always expands to real directories. Update
the loop to safely skip the literal glob and any non-directory entries before
calling ln -sr, so an absent or empty fixtures folder does not trigger set -e
during install. Use create_symlinks_additional_extensions and the
BASE_PATH/packages link creation as the location to apply the guard.
| <html lang="en"> | ||
| <head> | ||
| <meta charset="UTF-8"> | ||
| <title><?php echo $extensionKey; ?></title> |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Escape dynamic values before rendering HTML.
Environment values, Composer metadata, versions, and command descriptions are rendered raw. A quote or markup character can break the page or inject HTML into the DDEV landing page.
Proposed pattern
$supportedVersions = explode(' ', getenv('TYPO3_VERSIONS'));
+
+function h(mixed $value): string
+{
+ return htmlspecialchars((string)$value, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8');
+}- <title><?php echo $extensionKey; ?></title>
+ <title><?php echo h($extensionKey); ?></title>Apply the same h() wrapper to $description, $version, $usage, $example, $typo3AdminUser, and $typo3AdminPassword; use rawurlencode() for URL path/query components.
Also applies to: 42-42, 49-51, 89-89, 97-98
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.ddev/.setup/templates/index.php at line 23, The DDEV landing page is
rendering dynamic values raw, so escape all HTML-facing variables in the
template before output. Update the rendering in the template that uses
$extensionKey and also the other dynamic fields mentioned in the review
($description, $version, $usage, $example, $typo3AdminUser, and
$typo3AdminPassword) by wrapping them with the existing h() helper, and use
rawurlencode() anywhere values are inserted into URL path or query parts.
| for arg in "$@"; do | ||
| case "$arg" in | ||
| -v|--verbose) export VERBOSE=1 ;; | ||
| *) [ -z "$TYPO3" ] && TYPO3="$arg" ;; | ||
| esac |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject unknown flags and extra positional args.
This loop keeps only the first non-verbose token. ddev install 14 foo silently drops foo, and a typo like ddev install --verbsoe 14 gets reported as an unsupported TYPO3 version instead of an unknown option.
Suggested guard
TYPO3=""
for arg in "$@"; do
case "$arg" in
-v|--verbose) export VERBOSE=1 ;;
- *) [ -z "$TYPO3" ] && TYPO3="$arg" ;;
+ -*)
+ message red "Unknown option: $arg"
+ exit 1
+ ;;
+ *)
+ if [ -z "$TYPO3" ]; then
+ TYPO3="$arg"
+ else
+ message red "Unexpected extra argument: $arg"
+ exit 1
+ fi
+ ;;
esac
done📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for arg in "$@"; do | |
| case "$arg" in | |
| -v|--verbose) export VERBOSE=1 ;; | |
| *) [ -z "$TYPO3" ] && TYPO3="$arg" ;; | |
| esac | |
| TYPO3="" | |
| for arg in "$@"; do | |
| case "$arg" in | |
| -v|--verbose) export VERBOSE=1 ;; | |
| -*) | |
| message red "Unknown option: $arg" | |
| exit 1 | |
| ;; | |
| *) | |
| if [ -z "$TYPO3" ]; then | |
| TYPO3="$arg" | |
| else | |
| message red "Unexpected extra argument: $arg" | |
| exit 1 | |
| fi | |
| ;; | |
| esac | |
| done |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.ddev/commands/web/install around lines 10 - 14, The argument parsing in the
install command currently accepts any unknown token and silently ignores extra
positional arguments after the first version value. Update the loop in the
command script to explicitly recognize only the supported flags and a single
TYPO3 version argument, and reject any unknown option or second positional value
with an error. Keep the logic centered around the existing VERBOSE handling and
TYPO3 assignment so typos like unsupported flags are reported correctly.
| mod { | ||
| web_layout { | ||
| backendLayouts { | ||
| default { | ||
| jsFile = EXT:xima_typo3_internal_news/Resources/Public/JavaScript/backend.js | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify how TYPO3 13/14 parses page.tsconfig backendLayouts key casing
# and whether the generated backend layout is registered.
# Check for custom BackendLayoutDataProvider or jsFile usage
rg -n "jsFile" --type-add 'typo3:*.{php,tsconfig}' -t typo3
# Check for extension code that might read this PageTSConfig path
rg -n "BackendLayouts|backendLayouts" --type-add 'typo3:*.{php,tsconfig}' -t typo3
# Check if there's a custom data provider
rg -n "PageTsBackendLayoutDataProvider|BackendLayoutDataProvider" --type-add 'typo3:*.php' -t typo3Repository: xima-media/xima-typo3-internal-news
Length of output: 173
Fix missing backend layout grid definition and correct TSconfig key casing
The current Configuration/page.tsconfig snippet will not register a functional backend layout in TYPO3 13/14 because it lacks the required config.backend_layout grid definition and uses incorrect key casing. The core PageTsBackendLayoutDataProvider expects mod.web_layout.BackendLayouts (capital B and L) and requires each entry to contain a config.backend_layout block with colCount, rowCount, and rows definitions. Additionally, jsFile is not a valid property for backend layout definitions at this level and is ignored.
Update the file to match the standard structure:
- Change
backendLayoutstoBackendLayouts. - Add a
config.backend_layoutblock with the grid definition. - Remove
jsFile(load custom JavaScript viamod.SHARED.javascriptFileor service registration instead).
Corrected structure example
mod {
web_layout {
BackendLayouts {
default {
title = Default Layout
config {
backend_layout {
colCount = 2
rowCount = 1
rows {
1 {
columns {
1 {
name = Main
colPos = 0
}
2 {
name = Sidebar
colPos = 1
}
}
}
}
}
}
}
}
}
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Configuration/page.tsconfig` around lines 1 - 9, The backend layout
definition is using the wrong TSconfig key casing and is missing the required
grid config, so update the PageTsBackendLayoutDataProvider-compatible structure
in Configuration/page.tsconfig by renaming backendLayouts to BackendLayouts and
adding a config.backend_layout block with colCount, rowCount, and rows for the
default entry. Remove the jsFile property from this backend layout definition,
since it is ignored here, and if JavaScript must be loaded, do it via
mod.SHARED.javascriptFile or another supported registration path.
| (2, 0, 1700001000, 1700001000, 0, 0, | ||
| 'Scheduled maintenance this weekend', | ||
| '<p>The system will be unavailable during the maintenance window. See the attached date for details.</p>', | ||
| 0, 0, 1, ''), | ||
| (3, 0, 1700002000, 1700002000, 0, 0, | ||
| 'New editorial workflow (Editors only)', | ||
| '<p>This news item is only visible to members of the "Internal News Editors" backend group.</p>', | ||
| 0, 0, 0, '90'), | ||
| (4, 0, 1700003000, 1700003000, 0, 0, | ||
| 'Spring campaign kickoff (Marketing only)', | ||
| '<p>This news item is only visible to members of the "Marketing" backend group.</p>', | ||
| 0, 0, 0, '91'); | ||
|
|
||
| -- Date attached to news #2 (single date, with a notification enabled) ------- | ||
| INSERT INTO tx_ximatypo3internalnews_domain_model_date | ||
| (uid, pid, tstamp, crdate, deleted, hidden, title, type, single_date, recurrence, notify, notify_type, notify_message, news) | ||
| VALUES | ||
| (1, 0, 1700001000, 1700001000, 0, 0, | ||
| 'Maintenance window', 'single_date', 1735693200, '', 1, 'info', | ||
| 'Reminder: scheduled maintenance is coming up.', 2); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Make the maintenance fixture evergreen.
Line 63 hard-codes 1735693200 (January 1, 2025 UTC), while Lines 46-47 and 64 still say “this weekend” and “coming up.” That means fresh installs now show stale demo content immediately. Either generate this timestamp during setup or make the title/description/notification copy date-neutral.
Proposed text-only fix
- 'Scheduled maintenance this weekend',
- '<p>The system will be unavailable during the maintenance window. See the attached date for details.</p>',
+ 'Example maintenance window',
+ '<p>Example internal-news entry with an attached date for fixture purposes.</p>',
@@
- 'Reminder: scheduled maintenance is coming up.', 2);
+ 'Example reminder for the attached maintenance date.', 2);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| (2, 0, 1700001000, 1700001000, 0, 0, | |
| 'Scheduled maintenance this weekend', | |
| '<p>The system will be unavailable during the maintenance window. See the attached date for details.</p>', | |
| 0, 0, 1, ''), | |
| (3, 0, 1700002000, 1700002000, 0, 0, | |
| 'New editorial workflow (Editors only)', | |
| '<p>This news item is only visible to members of the "Internal News Editors" backend group.</p>', | |
| 0, 0, 0, '90'), | |
| (4, 0, 1700003000, 1700003000, 0, 0, | |
| 'Spring campaign kickoff (Marketing only)', | |
| '<p>This news item is only visible to members of the "Marketing" backend group.</p>', | |
| 0, 0, 0, '91'); | |
| -- Date attached to news #2 (single date, with a notification enabled) ------- | |
| INSERT INTO tx_ximatypo3internalnews_domain_model_date | |
| (uid, pid, tstamp, crdate, deleted, hidden, title, type, single_date, recurrence, notify, notify_type, notify_message, news) | |
| VALUES | |
| (1, 0, 1700001000, 1700001000, 0, 0, | |
| 'Maintenance window', 'single_date', 1735693200, '', 1, 'info', | |
| 'Reminder: scheduled maintenance is coming up.', 2); | |
| (2, 0, 1700001000, 1700001000, 0, 0, | |
| 'Example maintenance window', | |
| '<p>Example internal-news entry with an attached date for fixture purposes.</p>', | |
| 0, 0, 1, ''), | |
| (3, 0, 1700002000, 1700002000, 0, 0, | |
| 'New editorial workflow (Editors only)', | |
| '<p>This news item is only visible to members of the "Internal News Editors" backend group.</p>', | |
| 0, 0, 0, '90'), | |
| (4, 0, 1700003000, 1700003000, 0, 0, | |
| 'Spring campaign kickoff (Marketing only)', | |
| '<p>This news item is only visible to members of the "Marketing" backend group.</p>', | |
| 0, 0, 0, '91'); | |
| -- Date attached to news `#2` (single date, with a notification enabled) ------- | |
| INSERT INTO tx_ximatypo3internalnews_domain_model_date | |
| (uid, pid, tstamp, crdate, deleted, hidden, title, type, single_date, recurrence, notify, notify_type, notify_message, news) | |
| VALUES | |
| (1, 0, 1700001000, 1700001000, 0, 0, | |
| 'Maintenance window', 'single_date', 1735693200, '', 1, 'info', | |
| 'Example reminder for the attached maintenance date.', 2); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Tests/Acceptance/Fixtures/internal-news.sql` around lines 45 - 64, The
internal-news fixture has a hard-coded date in the maintenance entry that makes
the demo content expire and look stale. Update the fixture around the
maintenance news/date records so the timestamp is generated at setup time or the
copy in the related title, body, and notification text is date-neutral. Use the
existing fixture entries in internal-news.sql, especially the maintenance news
item and the tx_ximatypo3internalnews_domain_model_date insert, to keep the
scenario evergreen.
| "name": "test/sitepackage", | ||
| "description": "typo3-natural-language-query testing", | ||
| "license": "GPL-2.0-or-later", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the copied package description.
Line 3 still says "typo3-natural-language-query testing", which does not match this fixture sitepackage or the internal-news demo content added in this PR. That stale metadata will show up in Composer output and makes the fixture look copied from another package.
Proposed fix
- "description": "typo3-natural-language-query testing",
+ "description": "Demo sitepackage for internal-news fixture testing",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "name": "test/sitepackage", | |
| "description": "typo3-natural-language-query testing", | |
| "license": "GPL-2.0-or-later", | |
| "name": "test/sitepackage", | |
| "description": "Demo sitepackage for internal-news fixture testing", | |
| "license": "GPL-2.0-or-later", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Tests/Acceptance/Fixtures/packages/sitepackage/composer.json` around lines 2
- 4, The Composer metadata for the fixture package still has a copied, stale
description that does not match the sitepackage/internal-news demo content.
Update the description field in the sitepackage composer.json to a
package-specific description that reflects this fixture, keeping the name and
other metadata unchanged.
| #ddev-generated | ||
| # Sitepackage |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Avoid rendering the generator marker as a heading.
#ddev-generated is parsed as an H1 and triggers the markdownlint warning here. If the marker is only informational, switch it to an HTML comment or plain text so the README renders cleanly and lint stays green.
Proposed fix
-#ddev-generated
+<!-- ddev-generated -->
# Sitepackage📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #ddev-generated | |
| # Sitepackage | |
| <!-- ddev-generated --> | |
| # Sitepackage |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 1-1: No space after hash on atx style heading
(MD018, no-missing-space-atx)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Tests/Acceptance/Fixtures/packages/sitepackage/README.md` around lines 1 - 2,
The README header currently renders the generator marker as an H1, which
triggers markdownlint; update the fixture README so the `#ddev-generated` marker
is not treated as a heading by changing it to a non-rendered form such as an
HTML comment or plain text, while keeping the visible `# Sitepackage` heading
intact.
Source: Linters/SAST tools
2 participants
Summary
konradmichalik/ddev-typo3-multi-version-extensionDDEV add-on to the latest release 0.3.3 (adds TYPO3 14 support,.setuphelper scripts and theTests/Acceptance/Fixturesimport mechanism)TYPO3_VERSIONSand trimadditional_hostnamesaccordinglyddev installChanges
.ddev/- Add-on 0.3.3 files (.setup/scripts,.install-14/14commands, updatedinstall/all/version commands and manifest).ddev/docker-compose.typo3-setup.yaml-TYPO3_VERSIONS=13 14.ddev/config.typo3-setup.yaml- hostnames reduced to13/14Tests/Acceptance/Fixtures/internal-news.sql- sample data: pinned welcome news, a news item with a notifying date, a group-targeted news plus the matchingbe_group(records atpid=0, idempotent)Tests/Acceptance/Fixtures/packages/sitepackage/- demo sitepackage scaffolded by the add-onTest plan
ddev restart(registers13/14hostnames, needs sudo)ddev install allinstalls TYPO3 13 + 14 and imports the fixturesddev launch 13 /typo3→ admin sees the sample news in the toolbar dropdown and dashboard widgetddev launch 14 /typo3→ same on TYPO3 14Summary by CodeRabbit
New Features
Bug Fixes
Tests