[#629] Use fixture path for file fields

[ericgsmith]

Summary

This PR extends fixture-based file path expansion in MediaTrait so media fields of type 'file' are treated the same as 'image' fields when resolving fixture file names to absolute fixture paths.

Changes

• File: src/Drupal/MediaTrait.php

  • In mediaExpandEntityFieldsFixtures() the field-type check was changed from === 'image' to in_array(..., ['image', 'file'], TRUE), allowing both image and file media fields to have provided fixture filenames expanded to absolute fixture file paths. Existing array/single-value handling and assignment logic are unchanged.

• File: tests/behat/features/drupal_media.feature

  • Feature scenarios updated (lines modified) to exercise media creation for document/file media (e.g., creation of media with field_media_document => document.pdf) and continued coverage for image fields. Scenario structure and steps remain using the media-focused creation steps.

Impact

• Scope: Internal behavior change in a protected helper used by media-creation step definitions.
• Backward compatibility: Backward compatible — extends fixture resolution to 'file' fields in addition to 'image'.
• API/public interface: No exported/public API changes.

Tests / Documentation

• Existing Behat feature scenarios in tests/behat/features/drupal_media.feature cover media creation for document/file fields and will exercise the updated behavior.

CONTRIBUTING.md step-definition rules

• I reviewed the relevant steps-format rules in CONTRIBUTING.md. This PR changes only internal logic and test usage; it does not add or alter any step definition signatures.
• No new violations of the step-definition rules in CONTRIBUTING.md were introduced by this PR. (The feature file continues to use existing step text such as "Given the following ..." which conforms to the guidelines; pre-existing "Given I ..." lines in test scenarios are unchanged by this change and therefore not a new violation.)

[coderabbitai]

Walkthrough

The mediaExpandEntityFieldsFixtures() method in MediaTrait was changed to treat fields of type 'file' the same as 'image' when converting fixture values into absolute file paths; related Behat scenarios were simplified to remove separate managed-file seeding steps.

Changes

Cohort / File(s)	Summary
Media trait logic src/Drupal/MediaTrait.php	Expanded field-type check from only 'image' to include 'file', enabling fixture path expansion for both image and file media fields.
Behat feature tests tests/behat/features/drupal_media.feature	Removed explicit Given the following managed files blocks and updated scenarios to rely solely on media-related steps; dropped Drupal\FileTrait annotation where no longer needed.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

• Support fixture paths for file fields in mediaExpandEntityFieldsFixtures #629: Matches this change — requests treating 'file' fields like 'image' for fixture path expansion.

Possibly related PRs

• Fixed Drupal driver check. #532: Touches the same method (mediaExpandEntityFieldsFixtures) and broadens accepted field types — strong overlap.
• Increase coverage for Drupal traits. #488: Modifies the same method and its branches — related to fixture-path handling.
• Fixed coding standards. #430: Also changes src/Drupal/MediaTrait.php (different adjustment) — relevant to the same area.

Suggested labels

PR: Requires more work

Suggested reviewers

• AlexSkrypnyk

Poem

"I'm a rabbit in the code, with whiskers in the trees,
I hop from 'image' to 'file' on a gentle breeze.
Fixtures now find homes on paths that shine so bright,
Hooray for media fields — both day and night!" 🐇✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title accurately describes the main change: extending fixture path support from 'image' fields to both 'image' and 'file' fields in the media trait.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

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_media.feature (1)

260-269: ⚠️ Potential issue | 🟠 Major

Tag the generated nested scenario with api.

The inner scenario creates media via Drupal API setup. With plain scenario steps:, the generated scenario may run without the api tag and fail during setup instead of testing the intended negative assertion.

Proposed fix

   `@api` `@trait`:Drupal\MediaTrait
   Scenario: Assert that negative assertion for "Then the :media_type media with the name :name should not exist" fails with an error
     Given some behat configuration
-    And scenario steps:
+    And scenario steps tagged with "api":
       """
       Given the following media "image" exist:
         | name              | field_media_image |
         | Test media image  | image.png         |
       Then the "image" media with the name "Test media image" should not exist
       """

Based on learnings, in tests/behat/features/**/*.feature, nested content-creation steps in negative trait:* scenarios should be placed inside scenario steps tagged with "api" so DrupalContext/API setup is available.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/behat/features/drupal_media.feature` around lines 260 - 269, The nested
scenario's content-creation steps run without the Drupal API setup because they
lack the api tag; update the inner block by changing "And scenario steps:" to
"And scenario steps tagged with \"api\":" (i.e., use the scenario steps variant
that adds the api tag to the generated nested scenario) so the media creation
uses DrupalContext/API during setup for the negative assertion scenario.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@tests/behat/features/drupal_media.feature`:
- Around line 260-269: The nested scenario's content-creation steps run without
the Drupal API setup because they lack the api tag; update the inner block by
changing "And scenario steps:" to "And scenario steps tagged with \"api\":"
(i.e., use the scenario steps variant that adds the api tag to the generated
nested scenario) so the media creation uses DrupalContext/API during setup for
the negative assertion scenario.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c6b7e72e-c16f-4ab7-8e63-b99c7a79a59c

📥 Commits

Reviewing files that changed from the base of the PR and between 22e707b and f604ed3.

📒 Files selected for processing (1)

• tests/behat/features/drupal_media.feature

[ericgsmith]

Test failures appear inconsistent to what I'm seeing locally - end of day here and I don't work Friday so I'll pick this back up after the weekend

[AlexSkrypnyk]

@ericgsmith
Thank you for your contribution.

Please feel free to leave this with me. Could you please check that Allow edits from maintainers. option is selected in your fork as per https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request-from-a-fork.

Alternatively, I can spin another PR and work there. The credit will be passed to that PR.

[ericgsmith]

Thanks Alex, have checked the allow access option