Expanded fixture paths inside compound file/image field values.#638
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
WalkthroughEntity stub fixture handling now detects compound field values (raw strings or parsed structures), extracts a ChangesCompound fixture value handling
Dependency constraint update
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/behat/features/drupal_content.feature (1)
375-393: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAdd coverage for the parsed compound/media path.
These scenarios only exercise the raw node-cell branch (
target_id:"..."beforeparseEntityFields()runs).helperExpandEntityFieldsFixtures()now also mutates already-parsed compound arrays on the media route, and that new branch is still untested here. Please add one media file/image scenario so regressions in the parsed-shape handling are caught too.🤖 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/behat/features/drupal_content.feature` around lines 375 - 393, Add a new Behat scenario that covers the parsed-shape branch for media/compound fields so helperExpandEntityFieldsFixtures()’s mutation of already-parsed compound arrays is exercised. Specifically, add one scenario similar to the existing "Compound fixture file" or "Compound fixture image" cases but using the parsed/media-shaped fixture input that parseEntityFields() would produce (the shape your tests use for media routes), visit the corresponding content edit page and assert the expected filename/alt/description appears; this ensures the code paths in helperExpandEntityFieldsFixtures() and parseEntityFields() are both covered.
🤖 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.
Outside diff comments:
In `@tests/behat/features/drupal_content.feature`:
- Around line 375-393: Add a new Behat scenario that covers the parsed-shape
branch for media/compound fields so helperExpandEntityFieldsFixtures()’s
mutation of already-parsed compound arrays is exercised. Specifically, add one
scenario similar to the existing "Compound fixture file" or "Compound fixture
image" cases but using the parsed/media-shaped fixture input that
parseEntityFields() would produce (the shape your tests use for media routes),
visit the corresponding content edit page and assert the expected
filename/alt/description appears; this ensures the code paths in
helperExpandEntityFieldsFixtures() and parseEntityFields() are both covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: cbc49947-3ff1-4a82-b522-eaf40da8be3f
📒 Files selected for processing (2)
src/HelperTrait.phptests/behat/features/drupal_content.feature
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/HelperTrait.php`:
- Around line 260-302: The loop currently treats any array of scalars like
['a.jpg','b.jpg'] as a single compound record because $is_compound is only set
when the first element is itself an array; update the logic around
$is_compound/$records so that plain numeric-indexed lists of scalars are
detected and handled element-by-element (i.e. treat a list of scalars as
compound), iterate each scalar entry and replace its basename with $fixture_path
. $basename when appropriate (using helperManagedFileExists() and is_file()
checks), and finally call $stub->setValue($name, $is_compound ? $records :
$records[0]) as before; refer to $is_compound, $records, $value, $record,
helperManagedFileExists(), $fixture_path, and $stub->setValue() when making the
change.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2783de7d-4b9e-4d72-a054-737371bb8ae5
📒 Files selected for processing (2)
src/HelperTrait.phptests/phpunit/src/HelperTraitTest.php
… compound shape support.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@composer.json`:
- Line 23: The composer dependency "drupal/drupal-driver":
"dev-feature/normalise-public as 3.0" references a non-existent branch; locate
the correct branch or fork in the drupal-driver repo (or the intended fork) and
update the composer requirement to point to that existing branch, or replace the
dev-branch alias with a specific commit hash (e.g., dev-branch@<commit> or
"vX.Y.Z") or a forked repository reference if the branch lives elsewhere, and
add a brief comment in composer.json/README documenting the temporary workaround
and rationale for the chosen reference.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: accc341c-3f2d-4982-8c5d-ecea6a3f6874
📒 Files selected for processing (1)
composer.json
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #638 +/- ##
==========================================
- Coverage 96.94% 96.93% -0.01%
==========================================
Files 43 43
Lines 3040 3068 +28
==========================================
+ Hits 2947 2974 +27
- Misses 93 94 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
Summary
HelperTrait::helperExpandEntityFieldsFixtures()previously assumed field values were either plain scalars or simple arrays, missing the moderndrupal-extensioncompound parser shape where a file/image cell is written astarget_id:"foo.jpg", alt:"A". As a result, fixture path expansion was silently skipped for any compound cell, forcing callers to supply absolute paths. This change adds two new code paths - one for raw compound strings (the node route, where the cell arrives beforeparseEntityFields()runs) and one for the already-parsed associative-array shape (the media route) - so fixture basenames are expanded correctly in both cases.Changes
src/HelperTrait.phphelperLooksLikeCompoundCell()- detects raw compound cell strings matching thekey:"..."orkey:[...]pattern thatEntityFieldParseruses to enter compound mode.helperExpandCompoundCellFixtures()- rewrites thetarget_id:"basename"segment inside a raw compound string, leaving all other columns (e.g.alt,description) untouched.[['target_id' => 'foo.jpg', 'alt' => 'A']]), and the original plain scalar/simple array.tests/behat/features/drupal_content.featuretarget_id:"text.txt", description:"My document").target_id:"image.png", alt:"My image").Before / After
Summary
This PR extends fixture path expansion to support compound entity field values in drupal-extension compound parser syntax, ensuring file/image compound cell values (e.g., target_id:"foo.jpg", alt:"A") are expanded to absolute fixture paths so Drupal receives full file paths.
Changes Made
src/HelperTrait.php
tests/behat/features/drupal_content.feature
tests/phpunit/src/HelperTraitTest.php
composer.json
API / Public Surface
Step Definition Compliance (per CONTRIBUTING.md)
I reviewed CONTRIBUTING.md and the modified Behat feature file. The new scenarios follow the repository's step-definition formatting rules (no new step definitions introduced; tuple-format placeholders and phrasing preserved). No step-definition format violations were found. No Critical issues detected.